8316470: Incorrect error location for "invalid permits clause" depending on file order

Reviewed-by: vromero
This commit is contained in:
Jan Lahoda 2023-10-24 12:18:33 +00:00
parent 5224e979a1
commit bf1a14e367
2 changed files with 249 additions and 102 deletions
src/jdk.compiler/share/classes/com/sun/tools/javac/comp
test/langtools/tools/javac/sealed

@ -5404,108 +5404,6 @@ public class Attr extends JCTree.Visitor {
// Get environment current at the point of class definition.
Env<AttrContext> env = typeEnvs.get(c);
if (c.isSealed() &&
!c.isEnum() &&
!c.isPermittedExplicit &&
c.permitted.isEmpty()) {
log.error(TreeInfo.diagnosticPositionFor(c, env.tree), Errors.SealedClassMustHaveSubclasses);
}
if (c.isSealed()) {
Set<Symbol> permittedTypes = new HashSet<>();
boolean sealedInUnnamed = c.packge().modle == syms.unnamedModule || c.packge().modle == syms.noModule;
for (Symbol subTypeSym : c.permitted) {
boolean isTypeVar = false;
if (subTypeSym.type.getTag() == TYPEVAR) {
isTypeVar = true; //error recovery
log.error(TreeInfo.diagnosticPositionFor(subTypeSym, env.tree),
Errors.InvalidPermitsClause(Fragments.IsATypeVariable(subTypeSym.type)));
}
if (subTypeSym.isAnonymous() && !c.isEnum()) {
log.error(TreeInfo.diagnosticPositionFor(subTypeSym, env.tree), Errors.LocalClassesCantExtendSealed(Fragments.Anonymous));
}
if (permittedTypes.contains(subTypeSym)) {
DiagnosticPosition pos =
env.enclClass.permitting.stream()
.filter(permittedExpr -> TreeInfo.diagnosticPositionFor(subTypeSym, permittedExpr, true) != null)
.limit(2).collect(List.collector()).get(1);
log.error(pos, Errors.InvalidPermitsClause(Fragments.IsDuplicated(subTypeSym.type)));
} else {
permittedTypes.add(subTypeSym);
}
if (sealedInUnnamed) {
if (subTypeSym.packge() != c.packge()) {
log.error(TreeInfo.diagnosticPositionFor(subTypeSym, env.tree),
Errors.ClassInUnnamedModuleCantExtendSealedInDiffPackage(c)
);
}
} else if (subTypeSym.packge().modle != c.packge().modle) {
log.error(TreeInfo.diagnosticPositionFor(subTypeSym, env.tree),
Errors.ClassInModuleCantExtendSealedInDiffModule(c, c.packge().modle)
);
}
if (subTypeSym == c.type.tsym || types.isSuperType(subTypeSym.type, c.type)) {
log.error(TreeInfo.diagnosticPositionFor(subTypeSym, ((JCClassDecl)env.tree).permitting),
Errors.InvalidPermitsClause(
subTypeSym == c.type.tsym ?
Fragments.MustNotBeSameClass :
Fragments.MustNotBeSupertype(subTypeSym.type)
)
);
} else if (!isTypeVar) {
boolean thisIsASuper = types.directSupertypes(subTypeSym.type)
.stream()
.anyMatch(d -> d.tsym == c);
if (!thisIsASuper) {
log.error(TreeInfo.diagnosticPositionFor(subTypeSym, env.tree),
Errors.InvalidPermitsClause(Fragments.DoesntExtendSealed(subTypeSym.type)));
}
}
}
}
List<ClassSymbol> sealedSupers = types.directSupertypes(c.type)
.stream()
.filter(s -> s.tsym.isSealed())
.map(s -> (ClassSymbol) s.tsym)
.collect(List.collector());
if (sealedSupers.isEmpty()) {
if ((c.flags_field & Flags.NON_SEALED) != 0) {
boolean hasErrorSuper = false;
hasErrorSuper |= types.directSupertypes(c.type)
.stream()
.anyMatch(s -> s.tsym.kind == Kind.ERR);
ClassType ct = (ClassType) c.type;
hasErrorSuper |= !ct.isCompound() && ct.interfaces_field != ct.all_interfaces_field;
if (!hasErrorSuper) {
log.error(TreeInfo.diagnosticPositionFor(c, env.tree), Errors.NonSealedWithNoSealedSupertype(c));
}
}
} else {
if (c.isDirectlyOrIndirectlyLocal() && !c.isEnum()) {
log.error(TreeInfo.diagnosticPositionFor(c, env.tree), Errors.LocalClassesCantExtendSealed(c.isAnonymous() ? Fragments.Anonymous : Fragments.Local));
}
if (!c.type.isCompound()) {
for (ClassSymbol supertypeSym : sealedSupers) {
if (!supertypeSym.permitted.contains(c.type.tsym)) {
log.error(TreeInfo.diagnosticPositionFor(c.type.tsym, env.tree), Errors.CantInheritFromSealed(supertypeSym));
}
}
if (!c.isNonSealed() && !c.isFinal() && !c.isSealed()) {
log.error(TreeInfo.diagnosticPositionFor(c, env.tree),
c.isInterface() ?
Errors.NonSealedOrSealedExpected :
Errors.NonSealedSealedOrFinalExpected);
}
}
}
// The info.lint field in the envs stored in typeEnvs is deliberately uninitialized,
// because the annotations were not available at the time the env was created. Therefore,
// we look up the environment chain for the first enclosing environment for which the
@ -5523,6 +5421,108 @@ public class Attr extends JCTree.Visitor {
ResultInfo prevReturnRes = env.info.returnResult;
try {
if (c.isSealed() &&
!c.isEnum() &&
!c.isPermittedExplicit &&
c.permitted.isEmpty()) {
log.error(TreeInfo.diagnosticPositionFor(c, env.tree), Errors.SealedClassMustHaveSubclasses);
}
if (c.isSealed()) {
Set<Symbol> permittedTypes = new HashSet<>();
boolean sealedInUnnamed = c.packge().modle == syms.unnamedModule || c.packge().modle == syms.noModule;
for (Symbol subTypeSym : c.permitted) {
boolean isTypeVar = false;
if (subTypeSym.type.getTag() == TYPEVAR) {
isTypeVar = true; //error recovery
log.error(TreeInfo.diagnosticPositionFor(subTypeSym, env.tree),
Errors.InvalidPermitsClause(Fragments.IsATypeVariable(subTypeSym.type)));
}
if (subTypeSym.isAnonymous() && !c.isEnum()) {
log.error(TreeInfo.diagnosticPositionFor(subTypeSym, env.tree), Errors.LocalClassesCantExtendSealed(Fragments.Anonymous));
}
if (permittedTypes.contains(subTypeSym)) {
DiagnosticPosition pos =
env.enclClass.permitting.stream()
.filter(permittedExpr -> TreeInfo.diagnosticPositionFor(subTypeSym, permittedExpr, true) != null)
.limit(2).collect(List.collector()).get(1);
log.error(pos, Errors.InvalidPermitsClause(Fragments.IsDuplicated(subTypeSym.type)));
} else {
permittedTypes.add(subTypeSym);
}
if (sealedInUnnamed) {
if (subTypeSym.packge() != c.packge()) {
log.error(TreeInfo.diagnosticPositionFor(subTypeSym, env.tree),
Errors.ClassInUnnamedModuleCantExtendSealedInDiffPackage(c)
);
}
} else if (subTypeSym.packge().modle != c.packge().modle) {
log.error(TreeInfo.diagnosticPositionFor(subTypeSym, env.tree),
Errors.ClassInModuleCantExtendSealedInDiffModule(c, c.packge().modle)
);
}
if (subTypeSym == c.type.tsym || types.isSuperType(subTypeSym.type, c.type)) {
log.error(TreeInfo.diagnosticPositionFor(subTypeSym, ((JCClassDecl)env.tree).permitting),
Errors.InvalidPermitsClause(
subTypeSym == c.type.tsym ?
Fragments.MustNotBeSameClass :
Fragments.MustNotBeSupertype(subTypeSym.type)
)
);
} else if (!isTypeVar) {
boolean thisIsASuper = types.directSupertypes(subTypeSym.type)
.stream()
.anyMatch(d -> d.tsym == c);
if (!thisIsASuper) {
log.error(TreeInfo.diagnosticPositionFor(subTypeSym, env.tree),
Errors.InvalidPermitsClause(Fragments.DoesntExtendSealed(subTypeSym.type)));
}
}
}
}
List<ClassSymbol> sealedSupers = types.directSupertypes(c.type)
.stream()
.filter(s -> s.tsym.isSealed())
.map(s -> (ClassSymbol) s.tsym)
.collect(List.collector());
if (sealedSupers.isEmpty()) {
if ((c.flags_field & Flags.NON_SEALED) != 0) {
boolean hasErrorSuper = false;
hasErrorSuper |= types.directSupertypes(c.type)
.stream()
.anyMatch(s -> s.tsym.kind == Kind.ERR);
ClassType ct = (ClassType) c.type;
hasErrorSuper |= !ct.isCompound() && ct.interfaces_field != ct.all_interfaces_field;
if (!hasErrorSuper) {
log.error(TreeInfo.diagnosticPositionFor(c, env.tree), Errors.NonSealedWithNoSealedSupertype(c));
}
}
} else {
if (c.isDirectlyOrIndirectlyLocal() && !c.isEnum()) {
log.error(TreeInfo.diagnosticPositionFor(c, env.tree), Errors.LocalClassesCantExtendSealed(c.isAnonymous() ? Fragments.Anonymous : Fragments.Local));
}
if (!c.type.isCompound()) {
for (ClassSymbol supertypeSym : sealedSupers) {
if (!supertypeSym.permitted.contains(c.type.tsym)) {
log.error(TreeInfo.diagnosticPositionFor(c.type.tsym, env.tree), Errors.CantInheritFromSealed(supertypeSym));
}
}
if (!c.isNonSealed() && !c.isFinal() && !c.isSealed()) {
log.error(TreeInfo.diagnosticPositionFor(c, env.tree),
c.isInterface() ?
Errors.NonSealedOrSealedExpected :
Errors.NonSealedSealedOrFinalExpected);
}
}
}
deferredLintHandler.flush(env.tree);
env.info.returnResult = null;
// java.lang.Enum may not be subclassed by a non-enum

@ -0,0 +1,147 @@
/*
* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
/**
* @test
* @bug 8316470
* @summary Verify correct source file is set while reporting errors for sealing from Attr
* @library /tools/lib
* @modules jdk.compiler/com.sun.tools.javac.api
* jdk.compiler/com.sun.tools.javac.main
* jdk.compiler/com.sun.tools.javac.util
* @build toolbox.ToolBox toolbox.JavacTask
* @run main SealedErrorPositions
*/
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.List;
import toolbox.TestRunner;
import toolbox.JavacTask;
import toolbox.Task;
import toolbox.ToolBox;
public class SealedErrorPositions extends TestRunner {
ToolBox tb;
public static void main(String... args) throws Exception {
new SealedErrorPositions().runTests();
}
SealedErrorPositions() {
super(System.err);
tb = new ToolBox();
}
public void runTests() throws Exception {
runTests(m -> new Object[] { Paths.get(m.getName()) });
}
@Test
public void testDoesNotExtendErrorPosition(Path base) throws IOException {
Path current = base.resolve(".");
Path src = current.resolve("src");
tb.writeJavaFiles(src,
"""
package test;
sealed class C permits A, B { }
""",
"""
package test;
final class A extends C { }
""",
"""
package test;
final class B { }
""");
Path test = src.resolve("test");
Path classes = current.resolve("classes");
Files.createDirectories(classes);
var log =
new JavacTask(tb)
.options("-XDrawDiagnostics",
"-implicit:none",
"-sourcepath", src.toString())
.outdir(classes)
.files(test.resolve("A.java"))
.run(Task.Expect.FAIL)
.writeAll()
.getOutputLines(Task.OutputKind.DIRECT);
List<String> expectedErrors = List.of(
"C.java:2:27: compiler.err.invalid.permits.clause: (compiler.misc.doesnt.extend.sealed: test.B)",
"1 error");
if (!expectedErrors.equals(log)) {
throw new AssertionError("Incorrect errors, expected: " + expectedErrors +
", actual: " + log);
}
}
@Test
public void testEmptyImplicitPermitsErrorPosition(Path base) throws IOException {
Path current = base.resolve(".");
Path src = current.resolve("src");
tb.writeJavaFiles(src,
"""
package test;
sealed class C { }
""",
"""
package test;
final class A extends C { }
""");
Path test = src.resolve("test");
Path classes = current.resolve("classes");
Files.createDirectories(classes);
var log =
new JavacTask(tb)
.options("-XDrawDiagnostics",
"-implicit:none",
"-sourcepath", src.toString())
.outdir(classes)
.files(test.resolve("A.java"))
.run(Task.Expect.FAIL)
.writeAll()
.getOutputLines(Task.OutputKind.DIRECT);
List<String> expectedErrors = List.of(
"C.java:2:8: compiler.err.sealed.class.must.have.subclasses",
"A.java:2:7: compiler.err.cant.inherit.from.sealed: test.C",
"2 errors");
if (!expectedErrors.equals(log)) {
throw new AssertionError("Incorrect errors, expected: " + expectedErrors +
", actual: " + log);
}
}
}