Skip to content

Commit bf1a14e

Browse files
lahodajlahodaj
committed
8316470: Incorrect error location for "invalid permits clause" depending on file order
Reviewed-by: vromero
1 parent 5224e97 commit bf1a14e

File tree

2 files changed

+248
-101
lines changed

2 files changed

+248
-101
lines changed

‎src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java‎

Lines changed: 101 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -5404,125 +5404,125 @@ void attribClass(ClassSymbol c) throws CompletionFailure {
54045404
// Get environment current at the point of class definition.
54055405
Env<AttrContext> env = typeEnvs.get(c);
54065406

5407-
if (c.isSealed() &&
5408-
!c.isEnum() &&
5409-
!c.isPermittedExplicit &&
5410-
c.permitted.isEmpty()) {
5411-
log.error(TreeInfo.diagnosticPositionFor(c, env.tree), Errors.SealedClassMustHaveSubclasses);
5412-
}
5413-
5414-
if (c.isSealed()) {
5415-
Set<Symbol> permittedTypes = new HashSet<>();
5416-
boolean sealedInUnnamed = c.packge().modle == syms.unnamedModule || c.packge().modle == syms.noModule;
5417-
for (Symbol subTypeSym : c.permitted) {
5418-
boolean isTypeVar = false;
5419-
if (subTypeSym.type.getTag() == TYPEVAR) {
5420-
isTypeVar = true; //error recovery
5421-
log.error(TreeInfo.diagnosticPositionFor(subTypeSym, env.tree),
5422-
Errors.InvalidPermitsClause(Fragments.IsATypeVariable(subTypeSym.type)));
5423-
}
5424-
if (subTypeSym.isAnonymous() && !c.isEnum()) {
5425-
log.error(TreeInfo.diagnosticPositionFor(subTypeSym, env.tree), Errors.LocalClassesCantExtendSealed(Fragments.Anonymous));
5426-
}
5427-
if (permittedTypes.contains(subTypeSym)) {
5428-
DiagnosticPosition pos =
5429-
env.enclClass.permitting.stream()
5430-
.filter(permittedExpr -> TreeInfo.diagnosticPositionFor(subTypeSym, permittedExpr, true) != null)
5431-
.limit(2).collect(List.collector()).get(1);
5432-
log.error(pos, Errors.InvalidPermitsClause(Fragments.IsDuplicated(subTypeSym.type)));
5433-
} else {
5434-
permittedTypes.add(subTypeSym);
5435-
}
5436-
if (sealedInUnnamed) {
5437-
if (subTypeSym.packge() != c.packge()) {
5407+
// The info.lint field in the envs stored in typeEnvs is deliberately uninitialized,
5408+
// because the annotations were not available at the time the env was created. Therefore,
5409+
// we look up the environment chain for the first enclosing environment for which the
5410+
// lint value is set. Typically, this is the parent env, but might be further if there
5411+
// are any envs created as a result of TypeParameter nodes.
5412+
Env<AttrContext> lintEnv = env;
5413+
while (lintEnv.info.lint == null)
5414+
lintEnv = lintEnv.next;
5415+
5416+
// Having found the enclosing lint value, we can initialize the lint value for this class
5417+
env.info.lint = lintEnv.info.lint.augment(c);
5418+
5419+
Lint prevLint = chk.setLint(env.info.lint);
5420+
JavaFileObject prev = log.useSource(c.sourcefile);
5421+
ResultInfo prevReturnRes = env.info.returnResult;
5422+
5423+
try {
5424+
if (c.isSealed() &&
5425+
!c.isEnum() &&
5426+
!c.isPermittedExplicit &&
5427+
c.permitted.isEmpty()) {
5428+
log.error(TreeInfo.diagnosticPositionFor(c, env.tree), Errors.SealedClassMustHaveSubclasses);
5429+
}
5430+
5431+
if (c.isSealed()) {
5432+
Set<Symbol> permittedTypes = new HashSet<>();
5433+
boolean sealedInUnnamed = c.packge().modle == syms.unnamedModule || c.packge().modle == syms.noModule;
5434+
for (Symbol subTypeSym : c.permitted) {
5435+
boolean isTypeVar = false;
5436+
if (subTypeSym.type.getTag() == TYPEVAR) {
5437+
isTypeVar = true; //error recovery
54385438
log.error(TreeInfo.diagnosticPositionFor(subTypeSym, env.tree),
5439-
Errors.ClassInUnnamedModuleCantExtendSealedInDiffPackage(c)
5440-
);
5439+
Errors.InvalidPermitsClause(Fragments.IsATypeVariable(subTypeSym.type)));
54415440
}
5442-
} else if (subTypeSym.packge().modle != c.packge().modle) {
5443-
log.error(TreeInfo.diagnosticPositionFor(subTypeSym, env.tree),
5444-
Errors.ClassInModuleCantExtendSealedInDiffModule(c, c.packge().modle)
5445-
);
5446-
}
5447-
if (subTypeSym == c.type.tsym || types.isSuperType(subTypeSym.type, c.type)) {
5448-
log.error(TreeInfo.diagnosticPositionFor(subTypeSym, ((JCClassDecl)env.tree).permitting),
5449-
Errors.InvalidPermitsClause(
5450-
subTypeSym == c.type.tsym ?
5451-
Fragments.MustNotBeSameClass :
5452-
Fragments.MustNotBeSupertype(subTypeSym.type)
5453-
)
5454-
);
5455-
} else if (!isTypeVar) {
5456-
boolean thisIsASuper = types.directSupertypes(subTypeSym.type)
5457-
.stream()
5458-
.anyMatch(d -> d.tsym == c);
5459-
if (!thisIsASuper) {
5441+
if (subTypeSym.isAnonymous() && !c.isEnum()) {
5442+
log.error(TreeInfo.diagnosticPositionFor(subTypeSym, env.tree), Errors.LocalClassesCantExtendSealed(Fragments.Anonymous));
5443+
}
5444+
if (permittedTypes.contains(subTypeSym)) {
5445+
DiagnosticPosition pos =
5446+
env.enclClass.permitting.stream()
5447+
.filter(permittedExpr -> TreeInfo.diagnosticPositionFor(subTypeSym, permittedExpr, true) != null)
5448+
.limit(2).collect(List.collector()).get(1);
5449+
log.error(pos, Errors.InvalidPermitsClause(Fragments.IsDuplicated(subTypeSym.type)));
5450+
} else {
5451+
permittedTypes.add(subTypeSym);
5452+
}
5453+
if (sealedInUnnamed) {
5454+
if (subTypeSym.packge() != c.packge()) {
5455+
log.error(TreeInfo.diagnosticPositionFor(subTypeSym, env.tree),
5456+
Errors.ClassInUnnamedModuleCantExtendSealedInDiffPackage(c)
5457+
);
5458+
}
5459+
} else if (subTypeSym.packge().modle != c.packge().modle) {
54605460
log.error(TreeInfo.diagnosticPositionFor(subTypeSym, env.tree),
5461-
Errors.InvalidPermitsClause(Fragments.DoesntExtendSealed(subTypeSym.type)));
5461+
Errors.ClassInModuleCantExtendSealedInDiffModule(c, c.packge().modle)
5462+
);
5463+
}
5464+
if (subTypeSym == c.type.tsym || types.isSuperType(subTypeSym.type, c.type)) {
5465+
log.error(TreeInfo.diagnosticPositionFor(subTypeSym, ((JCClassDecl)env.tree).permitting),
5466+
Errors.InvalidPermitsClause(
5467+
subTypeSym == c.type.tsym ?
5468+
Fragments.MustNotBeSameClass :
5469+
Fragments.MustNotBeSupertype(subTypeSym.type)
5470+
)
5471+
);
5472+
} else if (!isTypeVar) {
5473+
boolean thisIsASuper = types.directSupertypes(subTypeSym.type)
5474+
.stream()
5475+
.anyMatch(d -> d.tsym == c);
5476+
if (!thisIsASuper) {
5477+
log.error(TreeInfo.diagnosticPositionFor(subTypeSym, env.tree),
5478+
Errors.InvalidPermitsClause(Fragments.DoesntExtendSealed(subTypeSym.type)));
5479+
}
54625480
}
54635481
}
54645482
}
5465-
}
54665483

5467-
List<ClassSymbol> sealedSupers = types.directSupertypes(c.type)
5468-
.stream()
5469-
.filter(s -> s.tsym.isSealed())
5470-
.map(s -> (ClassSymbol) s.tsym)
5471-
.collect(List.collector());
5484+
List<ClassSymbol> sealedSupers = types.directSupertypes(c.type)
5485+
.stream()
5486+
.filter(s -> s.tsym.isSealed())
5487+
.map(s -> (ClassSymbol) s.tsym)
5488+
.collect(List.collector());
54725489

5473-
if (sealedSupers.isEmpty()) {
5474-
if ((c.flags_field & Flags.NON_SEALED) != 0) {
5475-
boolean hasErrorSuper = false;
5490+
if (sealedSupers.isEmpty()) {
5491+
if ((c.flags_field & Flags.NON_SEALED) != 0) {
5492+
boolean hasErrorSuper = false;
54765493

5477-
hasErrorSuper |= types.directSupertypes(c.type)
5478-
.stream()
5479-
.anyMatch(s -> s.tsym.kind == Kind.ERR);
5494+
hasErrorSuper |= types.directSupertypes(c.type)
5495+
.stream()
5496+
.anyMatch(s -> s.tsym.kind == Kind.ERR);
54805497

5481-
ClassType ct = (ClassType) c.type;
5498+
ClassType ct = (ClassType) c.type;
54825499

5483-
hasErrorSuper |= !ct.isCompound() && ct.interfaces_field != ct.all_interfaces_field;
5500+
hasErrorSuper |= !ct.isCompound() && ct.interfaces_field != ct.all_interfaces_field;
54845501

5485-
if (!hasErrorSuper) {
5486-
log.error(TreeInfo.diagnosticPositionFor(c, env.tree), Errors.NonSealedWithNoSealedSupertype(c));
5502+
if (!hasErrorSuper) {
5503+
log.error(TreeInfo.diagnosticPositionFor(c, env.tree), Errors.NonSealedWithNoSealedSupertype(c));
5504+
}
5505+
}
5506+
} else {
5507+
if (c.isDirectlyOrIndirectlyLocal() && !c.isEnum()) {
5508+
log.error(TreeInfo.diagnosticPositionFor(c, env.tree), Errors.LocalClassesCantExtendSealed(c.isAnonymous() ? Fragments.Anonymous : Fragments.Local));
54875509
}
5488-
}
5489-
} else {
5490-
if (c.isDirectlyOrIndirectlyLocal() && !c.isEnum()) {
5491-
log.error(TreeInfo.diagnosticPositionFor(c, env.tree), Errors.LocalClassesCantExtendSealed(c.isAnonymous() ? Fragments.Anonymous : Fragments.Local));
5492-
}
54935510

5494-
if (!c.type.isCompound()) {
5495-
for (ClassSymbol supertypeSym : sealedSupers) {
5496-
if (!supertypeSym.permitted.contains(c.type.tsym)) {
5497-
log.error(TreeInfo.diagnosticPositionFor(c.type.tsym, env.tree), Errors.CantInheritFromSealed(supertypeSym));
5511+
if (!c.type.isCompound()) {
5512+
for (ClassSymbol supertypeSym : sealedSupers) {
5513+
if (!supertypeSym.permitted.contains(c.type.tsym)) {
5514+
log.error(TreeInfo.diagnosticPositionFor(c.type.tsym, env.tree), Errors.CantInheritFromSealed(supertypeSym));
5515+
}
5516+
}
5517+
if (!c.isNonSealed() && !c.isFinal() && !c.isSealed()) {
5518+
log.error(TreeInfo.diagnosticPositionFor(c, env.tree),
5519+
c.isInterface() ?
5520+
Errors.NonSealedOrSealedExpected :
5521+
Errors.NonSealedSealedOrFinalExpected);
54985522
}
5499-
}
5500-
if (!c.isNonSealed() && !c.isFinal() && !c.isSealed()) {
5501-
log.error(TreeInfo.diagnosticPositionFor(c, env.tree),
5502-
c.isInterface() ?
5503-
Errors.NonSealedOrSealedExpected :
5504-
Errors.NonSealedSealedOrFinalExpected);
55055523
}
55065524
}
5507-
}
55085525

5509-
// The info.lint field in the envs stored in typeEnvs is deliberately uninitialized,
5510-
// because the annotations were not available at the time the env was created. Therefore,
5511-
// we look up the environment chain for the first enclosing environment for which the
5512-
// lint value is set. Typically, this is the parent env, but might be further if there
5513-
// are any envs created as a result of TypeParameter nodes.
5514-
Env<AttrContext> lintEnv = env;
5515-
while (lintEnv.info.lint == null)
5516-
lintEnv = lintEnv.next;
5517-
5518-
// Having found the enclosing lint value, we can initialize the lint value for this class
5519-
env.info.lint = lintEnv.info.lint.augment(c);
5520-
5521-
Lint prevLint = chk.setLint(env.info.lint);
5522-
JavaFileObject prev = log.useSource(c.sourcefile);
5523-
ResultInfo prevReturnRes = env.info.returnResult;
5524-
5525-
try {
55265526
deferredLintHandler.flush(env.tree);
55275527
env.info.returnResult = null;
55285528
// java.lang.Enum may not be subclassed by a non-enum

‎test/langtools/tools/javac/sealed/SealedErrorPositions.java‎

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
/*
2+
* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
/**
25+
* @test
26+
* @bug 8316470
27+
* @summary Verify correct source file is set while reporting errors for sealing from Attr
28+
* @library /tools/lib
29+
* @modules jdk.compiler/com.sun.tools.javac.api
30+
* jdk.compiler/com.sun.tools.javac.main
31+
* jdk.compiler/com.sun.tools.javac.util
32+
* @build toolbox.ToolBox toolbox.JavacTask
33+
* @run main SealedErrorPositions
34+
*/
35+
36+
import java.io.IOException;
37+
import java.nio.file.Files;
38+
import java.nio.file.Path;
39+
import java.nio.file.Paths;
40+
import java.util.List;
41+
42+
import toolbox.TestRunner;
43+
import toolbox.JavacTask;
44+
import toolbox.Task;
45+
import toolbox.ToolBox;
46+
47+
public class SealedErrorPositions extends TestRunner {
48+
49+
ToolBox tb;
50+
51+
public static void main(String... args) throws Exception {
52+
new SealedErrorPositions().runTests();
53+
}
54+
55+
SealedErrorPositions() {
56+
super(System.err);
57+
tb = new ToolBox();
58+
}
59+
60+
public void runTests() throws Exception {
61+
runTests(m -> new Object[] { Paths.get(m.getName()) });
62+
}
63+
64+
@Test
65+
public void testDoesNotExtendErrorPosition(Path base) throws IOException {
66+
Path current = base.resolve(".");
67+
Path src = current.resolve("src");
68+
tb.writeJavaFiles(src,
69+
"""
70+
package test;
71+
sealed class C permits A, B { }
72+
""",
73+
"""
74+
package test;
75+
final class A extends C { }
76+
""",
77+
"""
78+
package test;
79+
final class B { }
80+
""");
81+
Path test = src.resolve("test");
82+
Path classes = current.resolve("classes");
83+
84+
Files.createDirectories(classes);
85+
86+
var log =
87+
new JavacTask(tb)
88+
.options("-XDrawDiagnostics",
89+
"-implicit:none",
90+
"-sourcepath", src.toString())
91+
.outdir(classes)
92+
.files(test.resolve("A.java"))
93+
.run(Task.Expect.FAIL)
94+
.writeAll()
95+
.getOutputLines(Task.OutputKind.DIRECT);
96+
97+
List<String> expectedErrors = List.of(
98+
"C.java:2:27: compiler.err.invalid.permits.clause: (compiler.misc.doesnt.extend.sealed: test.B)",
99+
"1 error");
100+
101+
if (!expectedErrors.equals(log)) {
102+
throw new AssertionError("Incorrect errors, expected: " + expectedErrors +
103+
", actual: " + log);
104+
}
105+
}
106+
107+
@Test
108+
public void testEmptyImplicitPermitsErrorPosition(Path base) throws IOException {
109+
Path current = base.resolve(".");
110+
Path src = current.resolve("src");
111+
tb.writeJavaFiles(src,
112+
"""
113+
package test;
114+
sealed class C { }
115+
""",
116+
"""
117+
package test;
118+
final class A extends C { }
119+
""");
120+
Path test = src.resolve("test");
121+
Path classes = current.resolve("classes");
122+
123+
Files.createDirectories(classes);
124+
125+
var log =
126+
new JavacTask(tb)
127+
.options("-XDrawDiagnostics",
128+
"-implicit:none",
129+
"-sourcepath", src.toString())
130+
.outdir(classes)
131+
.files(test.resolve("A.java"))
132+
.run(Task.Expect.FAIL)
133+
.writeAll()
134+
.getOutputLines(Task.OutputKind.DIRECT);
135+
136+
List<String> expectedErrors = List.of(
137+
"C.java:2:8: compiler.err.sealed.class.must.have.subclasses",
138+
"A.java:2:7: compiler.err.cant.inherit.from.sealed: test.C",
139+
"2 errors");
140+
141+
if (!expectedErrors.equals(log)) {
142+
throw new AssertionError("Incorrect errors, expected: " + expectedErrors +
143+
", actual: " + log);
144+
}
145+
}
146+
147+
}

0 commit comments

Comments
 (0)