From aeefecb11ea2e589795a31355f32607e30eb34e4 Mon Sep 17 00:00:00 2001 From: Stephan Herrmann Date: Sun, 24 Mar 2024 13:41:03 +0100 Subject: [PATCH] Null-Analysis - The interface ... cannot be implemented more than once with different arguments ... (#2173) fixes #2158 + Propagate late annotations also into parameterized supertypes + Additional fix ensuring proper interning in TypeSystem --- .../compiler/ast/TypeDeclaration.java | 16 +++- .../compiler/lookup/SourceTypeBinding.java | 1 + .../internal/compiler/lookup/TypeSystem.java | 23 +++++- .../compiler/lookup/TypeVariableBinding.java | 7 ++ .../regression/NullTypeAnnotationTest.java | 74 +++++++++++++++++++ 5 files changed, 118 insertions(+), 3 deletions(-) diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/TypeDeclaration.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/TypeDeclaration.java index 5385d4b7be7..25469cb4350 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/TypeDeclaration.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/TypeDeclaration.java @@ -98,6 +98,9 @@ public class TypeDeclaration extends Statement implements ProblemSeverities, Ref // 15 Sealed Type preview support public TypeReference[] permittedTypes; + // TEST ONLY: disable one fix here to challenge another related fix (in TypeSystem): + public static boolean TESTING_GH_2158 = false; + static { disallowedComponentNames = new HashSet<>(6); disallowedComponentNames.add("clone"); //$NON-NLS-1$ @@ -1924,6 +1927,15 @@ public void updateSupertypesWithAnnotations(Map outerUpdates, Map updates) { + if (!TESTING_GH_2158 + && previousType instanceof ParameterizedTypeBinding previousPTB + && previousPTB.original() instanceof SourceTypeBinding previousOriginal + && previousOriginal.supertypeAnnotationsUpdated) { + // re-initialized parameterized type with updated annotations from the original: + typeRef.resolvedType = this.scope.environment().createParameterizedType(previousOriginal, // <- has been updated + previousPTB.arguments, previousType.enclosingType(), previousType.getAnnotations()); // <- no changes here + } + typeRef.updateWithAnnotations(this.scope, 0); ReferenceBinding updatedType = (ReferenceBinding) typeRef.resolvedType; if (updatedType instanceof ParameterizedTypeBinding) { @@ -1937,8 +1949,10 @@ protected ReferenceBinding updateWithAnnotations(TypeReference typeRef, Referenc if (previousType != null) { if (previousType.id == TypeIds.T_JavaLangObject && ((this.binding.tagBits & TagBits.HierarchyHasProblems) != 0)) return previousType; // keep this cycle breaker - if (previousType != updatedType) //$IDENTITY-COMPARISON$ + if (previousType != updatedType) { //$IDENTITY-COMPARISON$ updates.put(previousType, updatedType); + this.binding.supertypeAnnotationsUpdated = true; + } } return updatedType; } 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 32b7546653b..dd7b0d6f3e7 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 @@ -139,6 +139,7 @@ public class SourceTypeBinding extends ReferenceBinding { public boolean isVarArgs = false; // for record declaration private FieldBinding[] implicitComponentFields; // cache private MethodBinding[] recordComponentAccessors = null; // hash maybe an overkill + public boolean supertypeAnnotationsUpdated = false; // have any supertype annotations been updated during CompleteTypeBindingsSteps.INTEGRATE_ANNOTATIONS_IN_HIERARCHY? public SourceTypeBinding(char[][] compoundName, PackageBinding fPackage, ClassScope scope) { this.compoundName = compoundName; diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/TypeSystem.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/TypeSystem.java index d1f7c9ac0ef..df729cb7d29 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/TypeSystem.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/TypeSystem.java @@ -20,6 +20,7 @@ package org.eclipse.jdt.internal.compiler.lookup; import java.util.HashMap; +import java.util.function.Consumer; import org.eclipse.jdt.internal.compiler.ast.ASTNode; import org.eclipse.jdt.internal.compiler.util.SimpleLookupTable; @@ -82,11 +83,26 @@ public PTBKey(ReferenceBinding type, TypeBinding[] arguments, ReferenceBinding e if (type instanceof UnresolvedReferenceBinding) ((UnresolvedReferenceBinding) type).addWrapper(this, environment); if (arguments != null) { - for (TypeBinding argument : arguments) { + for (int i = 0; i < arguments.length; i++) { + TypeBinding argument = arguments[i]; if (argument instanceof UnresolvedReferenceBinding) ((UnresolvedReferenceBinding) argument).addWrapper(this, environment); if (argument.hasNullTypeAnnotations()) this.tagBits |= TagBits.HasNullTypeAnnotation; + if (argument.getClass() == TypeVariableBinding.class) { + final int idx = i; + TypeVariableBinding typeVariableBinding = (TypeVariableBinding) argument; + Consumer previousConsumer = typeVariableBinding.updateWhenSettingTypeAnnotations; + typeVariableBinding.updateWhenSettingTypeAnnotations = (newTvb) -> { + // update the TVB argument and simulate a re-hash: + ParameterizedTypeBinding[] value = HashedParameterizedTypes.this.hashedParameterizedTypes.get(this); + arguments[idx] = newTvb; + HashedParameterizedTypes.this.hashedParameterizedTypes.put(this, value); + // for the unlikely case of multiple PTBKeys referring to this TVB chain to the next consumer: + if (previousConsumer != null) + previousConsumer.accept(newTvb); + }; + } } } } @@ -253,13 +269,16 @@ public final TypeBinding getUnannotatedType(TypeBinding type) { * If it itself is already registered as the key unannotated type of its family, * create a clone to play that role from now on and swap types in the types cache. */ - public void forceRegisterAsDerived(TypeBinding derived) { + public void forceRegisterAsDerived(TypeVariableBinding derived) { int id = derived.id; if (id != TypeIds.NoId && this.types[id] != null) { TypeBinding unannotated = this.types[id][0]; if (unannotated == derived) { //$IDENTITY-COMPARISON$ // was previously registered as unannotated, replace by a fresh clone to remain unannotated: this.types[id][0] = unannotated = derived.clone(null); + if (derived.updateWhenSettingTypeAnnotations != null) { + derived.updateWhenSettingTypeAnnotations.accept((TypeVariableBinding) unannotated); + } } // proceed as normal: cacheDerivedType(unannotated, derived); diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/TypeVariableBinding.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/TypeVariableBinding.java index 2e5b6955bce..f7ef22e0f74 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/TypeVariableBinding.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/TypeVariableBinding.java @@ -44,6 +44,7 @@ import java.util.Arrays; import java.util.Set; +import java.util.function.Consumer; import org.eclipse.jdt.core.compiler.CharOperation; import org.eclipse.jdt.internal.compiler.ast.ASTNode; @@ -77,6 +78,12 @@ public class TypeVariableBinding extends ReferenceBinding { public char[] genericTypeSignature; LookupEnvironment environment; + /* + * In one particular situation a TVB will be cloned and the clone will be used as the 'naked' type + * within TypeSystem. This may require some updating inside TypeSystem's hash structure. + */ + Consumer updateWhenSettingTypeAnnotations; + public TypeVariableBinding(char[] sourceName, Binding declaringElement, int rank, LookupEnvironment environment) { this.sourceName = sourceName; this.declaringElement = declaringElement; diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullTypeAnnotationTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullTypeAnnotationTest.java index e2bb6f5fa1c..739123e7c50 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullTypeAnnotationTest.java +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullTypeAnnotationTest.java @@ -29,6 +29,7 @@ import org.eclipse.jdt.core.JavaCore; import org.eclipse.jdt.core.tests.compiler.regression.AbstractRegressionTest.JavacTestOptions.Excuse; +import org.eclipse.jdt.internal.compiler.ast.TypeDeclaration; import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants; import org.eclipse.jdt.internal.compiler.impl.CompilerOptions; @@ -19242,4 +19243,77 @@ public boolean hasNext() { "----------\n"; runner.runNegativeTest(); } + +public void testGH2158() { + Runner runner = new Runner(); + runner.testFiles = new String[] { + "abc/Connection.java", + """ + package abc; + public interface Connection<@org.eclipse.jdt.annotation.NonNull M> { } + """, + "abc/IncomingMessageData.java", + """ + package abc; + public interface IncomingMessageData<@org.eclipse.jdt.annotation.NonNull T> { } + """, + "abc/MessageHandlerRegistry.java", + """ + package abc; + import org.eclipse.jdt.annotation.*; + public interface MessageHandlerRegistry + <@NonNull C extends Connection, @NonNull T, @NonNull D extends IncomingMessageData> { } + """, + "abc/MessageHandlerRegistryImpl.java", + """ + package abc; + import org.eclipse.jdt.annotation.*; + public class MessageHandlerRegistryImpl + <@NonNull C extends Connection, @NonNull T, @NonNull D extends IncomingMessageData> + implements MessageHandlerRegistry { } + """, + "abc/d/DConnection.java", + """ + package abc.d; + import abc.*; + import org.eclipse.jdt.annotation.*; + public interface DConnection extends Connection<@NonNull CharSequence> { } + """, + "abc/d/DIncomingMessageData.java", + """ + package abc.d; + import org.eclipse.jdt.annotation.*; + import abc.*; + public interface DIncomingMessageData extends IncomingMessageData<@NonNull CharSequence> { } + """, + "abc/d/DMessageHandlerRegistry.java", + """ + package abc.d; + import org.eclipse.jdt.annotation.*; + import abc.*; + public interface DMessageHandlerRegistry<@NonNull C extends DConnection> + extends MessageHandlerRegistry { } + """, + "abc/d/DMessageHandlerRegistryImpl.java", + """ + package abc.d; + import org.eclipse.jdt.annotation.*; + import abc.*; + public class DMessageHandlerRegistryImpl<@NonNull C extends DConnection> + extends MessageHandlerRegistryImpl + implements DMessageHandlerRegistry { } + """ + }; + runner.customOptions = getCompilerOptions(); + runner.classLibraries = this.LIBS; + runner.runConformTest(); + + // challenge other part of the fix: + TypeDeclaration.TESTING_GH_2158 = true; + try { + runner.runConformTest(); + } finally { + TypeDeclaration.TESTING_GH_2158 = false; + } +} }