From 3f5517e75d6599a27f77b55d4955b3d6154dc0b8 Mon Sep 17 00:00:00 2001 From: Srikanth Sankaran <131454720+srikanth-sankaran@users.noreply.github.com> Date: Mon, 21 Oct 2024 18:51:47 +0530 Subject: [PATCH] [Reconciler][Sealed types] Extra errors in editor that go away on file save (#3130) * Fixes https://github.com/eclipse-jdt/eclipse.jdt.core/issues/3122 --- .../internal/compiler/ast/TypeReference.java | 3 - .../internal/compiler/lookup/ClassScope.java | 2 +- .../compiler/lookup/SourceTypeBinding.java | 38 +++-- .../tests/model/SealedTypeModelTests.java | 133 +++++++++++++++++- .../codeassist/CompletionElementNotifier.java | 4 +- .../compiler/SourceElementNotifier.java | 53 +++++-- .../compiler/SourceElementParser.java | 84 +---------- 7 files changed, 199 insertions(+), 118 deletions(-) diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/TypeReference.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/TypeReference.java index c6639e7131f..954836a909c 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/TypeReference.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/TypeReference.java @@ -556,9 +556,6 @@ && isTypeUseDeprecated(type, scope)) { public boolean isTypeReference() { return true; } -public boolean isSynthetic() { - return false; -} public boolean isWildcard() { return false; } diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ClassScope.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ClassScope.java index 74ee28648a9..a650adb21d6 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ClassScope.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ClassScope.java @@ -1273,7 +1273,7 @@ else if (subType.module() != sealedTypeModule) void connectPermittedTypes() { SourceTypeBinding sourceType = this.referenceContext.binding; - if (this.referenceContext.permittedTypes != null && (this.referenceContext.permittedTypes.length == 0 || !this.referenceContext.permittedTypes[0].isSynthetic())) { + if (this.referenceContext.permittedTypes != null) { sourceType.setPermittedTypes(Binding.NO_PERMITTED_TYPES); try { sourceType.tagBits |= TagBits.SealingTypeHierarchy; diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/SourceTypeBinding.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/SourceTypeBinding.java index d436a36ba5c..881161fe30e 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/SourceTypeBinding.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/SourceTypeBinding.java @@ -3062,6 +3062,22 @@ public RecordComponentBinding[] setComponents(RecordComponentBinding[] comps) { return this.permittedTypes = permittedTypes; } +private void setImplicitPermittedType(SourceTypeBinding permittedType) { + ReferenceBinding[] typesPermitted = this.permittedTypes(); + int sz = typesPermitted == null ? 0 : typesPermitted.length; + if (this.scope.referenceCompilationUnit() == permittedType.scope.referenceCompilationUnit()) { + if (sz == 0) { + typesPermitted = new ReferenceBinding[] { permittedType }; + } else { + System.arraycopy(typesPermitted, 0, typesPermitted = new ReferenceBinding[sz + 1], 0, sz); + typesPermitted[sz] = permittedType; + } + this.setPermittedTypes(typesPermitted); + } else if (sz == 0) { + this.setPermittedTypes(Binding.NO_PERMITTED_TYPES); + } +} + // Propagate writes to all annotated variants so the clones evolve along. public ReferenceBinding setSuperClass(ReferenceBinding superClass) { @@ -3075,8 +3091,7 @@ public ReferenceBinding setSuperClass(ReferenceBinding superClass) { annotatedType.superclass = superClass; } } - if (superClass != null && superClass.actualType() instanceof SourceTypeBinding sourceSuperType && sourceSuperType.isSealed() - && (sourceSuperType.scope.referenceContext.permittedTypes == null || (sourceSuperType.scope.referenceContext.permittedTypes.length > 0 && sourceSuperType.scope.referenceContext.permittedTypes[0].isSynthetic()))) { + if (superClass != null && superClass.actualType() instanceof SourceTypeBinding sourceSuperType && sourceSuperType.isSealed() && sourceSuperType.scope.referenceContext.permittedTypes == null) { sourceSuperType.setImplicitPermittedType(this); if (this.isAnonymousType() && superClass.isEnum()) this.modifiers |= ClassFileConstants.AccFinal; @@ -3084,22 +3099,6 @@ public ReferenceBinding setSuperClass(ReferenceBinding superClass) { return this.superclass = superClass; } -private void setImplicitPermittedType(SourceTypeBinding permittedType) { - ReferenceBinding[] typesPermitted = this.permittedTypes(); - int sz = typesPermitted == null ? 0 : typesPermitted.length; - if (this.scope.referenceCompilationUnit() == permittedType.scope.referenceCompilationUnit()) { - if (sz == 0) { - typesPermitted = new ReferenceBinding[] { permittedType }; - } else { - System.arraycopy(typesPermitted, 0, typesPermitted = new ReferenceBinding[sz + 1], 0, sz); - typesPermitted[sz] = permittedType; - } - this.setPermittedTypes(typesPermitted); - } else if (sz == 0) { - this.setPermittedTypes(Binding.NO_PERMITTED_TYPES); - } -} - // Propagate writes to all annotated variants so the clones evolve along. public ReferenceBinding [] setSuperInterfaces(ReferenceBinding [] superInterfaces) { @@ -3115,8 +3114,7 @@ private void setImplicitPermittedType(SourceTypeBinding permittedType) { } for (int i = 0, length = superInterfaces == null ? 0 : superInterfaces.length; i < length; i++) { ReferenceBinding superInterface = superInterfaces[i]; - if (superInterface.actualType() instanceof SourceTypeBinding sourceSuperType && sourceSuperType.isSealed() - && (sourceSuperType.scope.referenceContext.permittedTypes == null || (sourceSuperType.scope.referenceContext.permittedTypes.length > 0 && sourceSuperType.scope.referenceContext.permittedTypes[0].isSynthetic()))) { + if (superInterface.actualType() instanceof SourceTypeBinding sourceSuperType && sourceSuperType.isSealed() && sourceSuperType.scope.referenceContext.permittedTypes == null) { sourceSuperType.setImplicitPermittedType(this); } } diff --git a/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/SealedTypeModelTests.java b/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/SealedTypeModelTests.java index e4382202806..d4066dc6122 100644 --- a/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/SealedTypeModelTests.java +++ b/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/SealedTypeModelTests.java @@ -144,7 +144,7 @@ public void test004() throws Exception { createFile( "/SealedTypes/src/X.java", fileContent); ICompilationUnit unit = getCompilationUnit("/SealedTypes/src/X.java"); IType[] types = unit.getTypes(); - assertEquals("Incorret no of types", 3, types.length); + assertEquals("Incorrect no of types", 3, types.length); for (IType iType : types) { if (iType.getElementName().equals("I")) { assertTrue("modifier should contain sealed", iType.isSealed()); @@ -160,6 +160,137 @@ public void test004() throws Exception { deleteProject("SealedTypes"); } } + // Test implicitly permitted sub types in Source Type + public void test004_2() throws Exception { + String[] permitted = new String[] {"Maybe.Maybe1", "Maybe.Maybe2"}; + try { + IJavaProject project = createJavaProject("SealedTypes"); + project.open(null); + String fileContent = + """ + interface SuperInt {} + + abstract sealed class Maybe { + final class Maybe1 extends Maybe {} + final class Maybe2 extends Maybe implements SuperInt {} + } + + class Test { + + void testMaybe(Maybe maybe) { + if (maybe == null) return; + } + } + """; + + createFile( "/SealedTypes/src/X.java", fileContent); + ICompilationUnit unit = getCompilationUnit("/SealedTypes/src/X.java"); + IType[] types = unit.getTypes(); + assertEquals("Incorrect no of types", 3, types.length); + for (IType iType : types) { + if (iType.getElementName().equals("Maybe")) { + assertTrue("modifier should contain sealed", iType.isSealed()); + String[] permittedSubtypeNames = iType.getPermittedSubtypeNames(); + assertEquals("incorrect permitted sub types", permitted.length, permittedSubtypeNames.length); + for (int i = 0; i < permitted.length; i++) { + assertEquals("incorrect permitted sub type", permitted[i], permittedSubtypeNames[i]); + } + } + } + } + finally { + deleteProject("SealedTypes"); + } + } + + // Test implicitly permitted sub types in Source Type + public void test004_3() throws Exception { + String[] permitted = new String[] {"Maybe.Maybe1", "Maybe.Maybe2"}; + try { + IJavaProject project = createJavaProject("SealedTypes"); + project.open(null); + String fileContent = + """ + interface SuperInt {} + + sealed interface Maybe { + final class Maybe1 implements Maybe {} + non-sealed interface Maybe2 extends Maybe {} + } + + class Test { + + void testMaybe(Maybe maybe) { + if (maybe == null) return; + } + } + """; + + createFile( "/SealedTypes/src/X.java", fileContent); + ICompilationUnit unit = getCompilationUnit("/SealedTypes/src/X.java"); + IType[] types = unit.getTypes(); + assertEquals("Incorrect no of types", 3, types.length); + for (IType iType : types) { + if (iType.getElementName().equals("Maybe")) { + assertTrue("modifier should contain sealed", iType.isSealed()); + String[] permittedSubtypeNames = iType.getPermittedSubtypeNames(); + assertEquals("incorrect permitted sub types", permitted.length, permittedSubtypeNames.length); + for (int i = 0; i < permitted.length; i++) { + assertEquals("incorrect permitted sub type", permitted[i], permittedSubtypeNames[i]); + } + } + } + } + finally { + deleteProject("SealedTypes"); + } + } + + // Test implicitly permitted sub types in Source Type + public void test004_4() throws Exception { + String[] permitted = new String[] {"Maybe.Maybe1", "Maybe.Maybe2"}; + try { + IJavaProject project = createJavaProject("SealedTypes"); + project.open(null); + String fileContent = + """ + interface SuperInt {} + + abstract sealed class Maybe { + final class Maybe1 extends Maybe {} + final class Maybe2 extends Maybe implements SuperInt {} + } + + class Test { + + void testMaybe(Maybe maybe) { + if (maybe == null) return; + zork(); + } + } + """; + + createFile( "/SealedTypes/src/X.java", fileContent); + ICompilationUnit unit = getCompilationUnit("/SealedTypes/src/X.java"); + IType[] types = unit.getTypes(); + assertEquals("Incorrect no of types", 3, types.length); + for (IType iType : types) { + if (iType.getElementName().equals("Maybe")) { + assertTrue("modifier should contain sealed", iType.isSealed()); + String[] permittedSubtypeNames = iType.getPermittedSubtypeNames(); + assertEquals("incorrect permitted sub types", permitted.length, permittedSubtypeNames.length); + for (int i = 0; i < permitted.length; i++) { + assertEquals("incorrect permitted sub type", permitted[i], permittedSubtypeNames[i]); + } + } + } + } + finally { + deleteProject("SealedTypes"); + } + } + + // Test explicitly permitted sub types in binary public void test005() throws Exception { String[] permitted = new String[] {"p.X", "p.Y"}; diff --git a/org.eclipse.jdt.core/codeassist/org/eclipse/jdt/internal/codeassist/CompletionElementNotifier.java b/org.eclipse.jdt.core/codeassist/org/eclipse/jdt/internal/codeassist/CompletionElementNotifier.java index 9f157f1aec6..f95a60088c4 100644 --- a/org.eclipse.jdt.core/codeassist/org/eclipse/jdt/internal/codeassist/CompletionElementNotifier.java +++ b/org.eclipse.jdt.core/codeassist/org/eclipse/jdt/internal/codeassist/CompletionElementNotifier.java @@ -201,8 +201,8 @@ protected void notifySourceElementRequestor(ImportReference importReference, boo } @Override - protected void notifySourceElementRequestor(TypeDeclaration typeDeclaration, boolean notifyTypePresence, TypeDeclaration declaringType, ImportReference currentPackage) { + protected void notifySourceElementRequestor(CompilationUnitDeclaration parsedUnit, TypeDeclaration typeDeclaration, boolean notifyTypePresence, TypeDeclaration declaringType, ImportReference currentPackage) { if (typeDeclaration instanceof CompletionOnAnnotationOfType) return; - super.notifySourceElementRequestor(typeDeclaration, notifyTypePresence, declaringType, currentPackage); + super.notifySourceElementRequestor(parsedUnit, typeDeclaration, notifyTypePresence, declaringType, currentPackage); } } diff --git a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/compiler/SourceElementNotifier.java b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/compiler/SourceElementNotifier.java index 68c5f243e25..fd4b6a4ff0c 100644 --- a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/compiler/SourceElementNotifier.java +++ b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/compiler/SourceElementNotifier.java @@ -14,6 +14,7 @@ package org.eclipse.jdt.internal.compiler; import java.util.ArrayList; +import java.util.List; import java.util.Map; import org.eclipse.jdt.core.compiler.CharOperation; import org.eclipse.jdt.internal.compiler.ISourceElementRequestor.ParameterInfo; @@ -52,12 +53,12 @@ public TypeDeclaration peekDeclaringType() { } @Override public boolean visit(TypeDeclaration typeDeclaration, BlockScope scope) { - notifySourceElementRequestor(typeDeclaration, true, peekDeclaringType(), this.currentPackage); + notifySourceElementRequestor(null, typeDeclaration, true, peekDeclaringType(), this.currentPackage); return false; // don't visit members as this was done during notifySourceElementRequestor(...) } @Override public boolean visit(TypeDeclaration typeDeclaration, ClassScope scope) { - notifySourceElementRequestor(typeDeclaration, true, peekDeclaringType(), this.currentPackage); + notifySourceElementRequestor(null, typeDeclaration, true, peekDeclaringType(), this.currentPackage); return false; // don't visit members as this was done during notifySourceElementRequestor(...) } } @@ -136,8 +137,44 @@ protected char[] getSuperclassName(TypeDeclaration typeDeclaration) { TypeReference superclass = typeDeclaration.superclass; return superclass != null ? CharOperation.concatWith(superclass.getParameterizedTypeName(), '.') : null; } -protected char[][] getPermittedSubTypes(TypeDeclaration typeDeclaration) { - return extractTypeReferences(typeDeclaration.permittedTypes); +private void gatherPermittedTypesOf(TypeDeclaration potentialSubtype, TypeDeclaration sealedType, List list, char [] prefix) { + if (potentialSubtype != sealedType) { + char[][] qName = potentialSubtype.superclass == null ? null : potentialSubtype.superclass.getTypeName(); + if (qName != null && CharOperation.equals(qName[qName.length - 1], sealedType.name)) { + char [] subTypeName = CharOperation.concat(prefix, potentialSubtype.name, '.'); + list.add(subTypeName); + } + if (potentialSubtype.superInterfaces != null) { + for (TypeReference ref : potentialSubtype.superInterfaces) { + qName = ref.getTypeName(); + if (CharOperation.equals(qName[qName.length - 1], sealedType.name)) { + char [] subTypeName = CharOperation.concat(prefix, potentialSubtype.name, '.'); + list.add(subTypeName); + break; + } + } + } + } + if (potentialSubtype.memberTypes != null) { + for (int i = 0, size = potentialSubtype.memberTypes.length; i < size; i++) { + char [] prefixNow = CharOperation.concat(prefix, potentialSubtype.name, '.'); + gatherPermittedTypesOf(potentialSubtype.memberTypes[i], sealedType, list, prefixNow); + } + } +} +protected char[][] getPermittedSubTypes(CompilationUnitDeclaration parsedUnit, TypeDeclaration sealedType) { + + if (sealedType.permittedTypes != null) + return extractTypeReferences(sealedType.permittedTypes); + + // compute implicit permitted types on the fly. + List list = new ArrayList(); + if (parsedUnit != null) { // == null for local types. + for (TypeDeclaration type : parsedUnit.types) { + gatherPermittedTypesOf(type, sealedType, list, CharOperation.NO_CHAR); + } + } + return list.toArray(new char[list.size()][]); } protected char[][] getThrownExceptions(AbstractMethodDeclaration methodDeclaration) { return extractTypeReferences(methodDeclaration.thrownExceptions); @@ -455,7 +492,7 @@ public void notifySourceElementRequestor( notifySourceElementRequestor(importRef, false); } } else if (node instanceof TypeDeclaration && !new String(parsedUnit.getFileName()).endsWith(TypeConstants.MODULE_INFO_FILE_NAME_STRING)) { - notifySourceElementRequestor((TypeDeclaration)node, true, null, currentPackage); + notifySourceElementRequestor(parsedUnit, (TypeDeclaration)node, true, null, currentPackage); } else if (node instanceof ModuleDeclaration) { notifySourceElementRequestor(parsedUnit.moduleDeclaration); } @@ -640,7 +677,7 @@ protected void notifySourceElementRequestor(ModuleDeclaration moduleDeclaration) // } // //} -protected void notifySourceElementRequestor(TypeDeclaration typeDeclaration, boolean notifyTypePresence, TypeDeclaration declaringType, ImportReference currentPackage) { +protected void notifySourceElementRequestor(CompilationUnitDeclaration parsedUnit, TypeDeclaration typeDeclaration, boolean notifyTypePresence, TypeDeclaration declaringType, ImportReference currentPackage) { if (CharOperation.equals(TypeConstants.PACKAGE_INFO_NAME, typeDeclaration.name)) return; @@ -704,7 +741,7 @@ protected void notifySourceElementRequestor(TypeDeclaration typeDeclaration, boo typeInfo.extraFlags = ExtraFlags.getExtraFlags(typeDeclaration); typeInfo.node = typeDeclaration; if ((currentModifiers & ExtraCompilerModifiers.AccSealed) != 0) { - typeInfo.permittedSubtypes = getPermittedSubTypes(typeDeclaration); + typeInfo.permittedSubtypes = getPermittedSubTypes(parsedUnit, typeDeclaration); } switch (kind) { case TypeDeclaration.CLASS_DECL : @@ -776,7 +813,7 @@ protected void notifySourceElementRequestor(TypeDeclaration typeDeclaration, boo break; case 2 : memberTypeIndex++; - notifySourceElementRequestor(nextMemberDeclaration, true, null, currentPackage); + notifySourceElementRequestor(parsedUnit, nextMemberDeclaration, true, null, currentPackage); break; } } diff --git a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/compiler/SourceElementParser.java b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/compiler/SourceElementParser.java index 34e348feed8..f20b91acc67 100644 --- a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/compiler/SourceElementParser.java +++ b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/compiler/SourceElementParser.java @@ -13,9 +13,7 @@ *******************************************************************************/ package org.eclipse.jdt.internal.compiler; -import java.util.ArrayList; import java.util.HashMap; -import java.util.List; import org.eclipse.core.runtime.IProgressMonitor; import org.eclipse.core.runtime.OperationCanceledException; import org.eclipse.jdt.core.compiler.CategorizedProblem; @@ -26,11 +24,6 @@ import org.eclipse.jdt.internal.compiler.impl.CompilerOptions; import org.eclipse.jdt.internal.compiler.impl.ReferenceContext; import org.eclipse.jdt.internal.compiler.lookup.Binding; -import org.eclipse.jdt.internal.compiler.lookup.BlockScope; -import org.eclipse.jdt.internal.compiler.lookup.ClassScope; -import org.eclipse.jdt.internal.compiler.lookup.ExtraCompilerModifiers; -import org.eclipse.jdt.internal.compiler.lookup.Scope; -import org.eclipse.jdt.internal.compiler.lookup.TypeBinding; import org.eclipse.jdt.internal.compiler.lookup.TypeConstants; import org.eclipse.jdt.internal.compiler.problem.AbortCompilation; import org.eclipse.jdt.internal.compiler.problem.ProblemReporter; @@ -975,77 +968,7 @@ protected QualifiedNameReference newQualifiedNameReference(char[][] tokens, long protected SingleNameReference newSingleNameReference(char[] source, long positions) { return new SingleNameReference(source, positions); } -private static class DummyTypeReference extends TypeReference { - char[] token; - DummyTypeReference(char[] name) { - this.token = name; - } - @Override - public TypeReference augmentTypeWithAdditionalDimensions(int additionalDimensions, - Annotation[][] additionalAnnotations, boolean isVarargs) { - // TODO Auto-generated method stub - return null; - } - @Override - public char[] getLastToken() { - return this.token; - } - @Override - protected TypeBinding getTypeBinding(Scope scope) { - return null; - } - @Override - public char[][] getTypeName() { - return new char[][] {this.token}; - } - @Override - public void traverse(ASTVisitor visitor, BlockScope scope) { - // TODO Auto-generated method stub - } - - @Override - public void traverse(ASTVisitor visitor, ClassScope scope) { - // TODO Auto-generated method stub - } - @Override - public StringBuilder printExpression(int indent, StringBuilder output) { - return output.append(this.token); - } - @Override - public String toString() { - return new String(this.token); - } - @Override - public boolean isSynthetic() { - return true; - } -} -private void processImplicitPermittedTypes(TypeDeclaration typeDecl, TypeDeclaration[] allTypes) { - if (typeDecl.permittedTypes == null && - (typeDecl.modifiers & ExtraCompilerModifiers.AccSealed) != 0) { - List list = new ArrayList(); - for (TypeDeclaration type : allTypes) { - if (type != typeDecl) { - char[][] qName = type.superclass == null ? null : type.superclass.getTypeName(); - if (qName != null && - CharOperation.equals(qName[qName.length -1], typeDecl.name)) { - list.add(new DummyTypeReference(type.name)); - } - if (type.superInterfaces != null) { - for (TypeReference ref : type.superInterfaces) { - qName = ref.getTypeName(); - if (CharOperation.equals(qName[qName.length -1], typeDecl.name)) { - list.add(new DummyTypeReference(type.name)); - break; - } - } - } - } - } - typeDecl.permittedTypes = list.toArray(new TypeReference[list.size()]); - } -} public CompilationUnitDeclaration parseCompilationUnit( ICompilationUnit unit, boolean fullParse, @@ -1060,12 +983,7 @@ public CompilationUnitDeclaration parseCompilationUnit( this.reportReferenceInfo = fullParse; CompilationResult compilationUnitResult = new CompilationResult(unit, 0, 0, this.options.maxProblemsPerUnit); parsedUnit = parse(unit, compilationUnitResult); - TypeDeclaration[] types = parsedUnit.types; - if (types != null) { - for (TypeDeclaration typeDecl : types) { - processImplicitPermittedTypes(typeDecl, types); - } - } + if (pm != null && pm.isCanceled()) throw new OperationCanceledException(Messages.operation_cancelled); if (this.scanner.recordLineSeparator) {