Skip to content

Commit

Permalink
Null-Analysis - The interface ... cannot be implemented more than onc…
Browse files Browse the repository at this point in the history 
…e with different arguments ... (#2173)

fixes #2158

+ Propagate late annotations also into parameterized supertypes
+ Additional fix ensuring proper interning in TypeSystem
  • Loading branch information
@stephan-herrmann
stephan-herrmann authored Mar 24, 2024
1 parent cd3c1b5 commit aeefecb
Show file tree
Hide file tree
Showing 5 changed files with 118 additions and 3 deletions.
16 changes: 15 additions & 1 deletion ...se.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/TypeDeclaration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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$
Expand Down Expand Up @@ -1924,6 +1927,15 @@ public void updateSupertypesWithAnnotations(Map<ReferenceBinding,ReferenceBindin
protected ReferenceBinding updateWithAnnotations(TypeReference typeRef, ReferenceBinding previousType,
Map<ReferenceBinding, ReferenceBinding> outerUpdates, Map<ReferenceBinding, ReferenceBinding> 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) {
Expand All @@ -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;
}
Expand Down
1 change: 1 addition & 0 deletions ...t.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/SourceTypeBinding.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
23 changes: 21 additions & 2 deletions ...ipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/TypeSystem.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<TypeVariableBinding> 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);
};
}
}
}
}
Expand Down Expand Up @@ -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);
Expand Down
7 changes: 7 additions & 0 deletions ...core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/TypeVariableBinding.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<TypeVariableBinding> updateWhenSettingTypeAnnotations;

public TypeVariableBinding(char[] sourceName, Binding declaringElement, int rank, LookupEnvironment environment) {
this.sourceName = sourceName;
this.declaringElement = declaringElement;
Expand Down
74 changes: 74 additions & 0 deletions ...s.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullTypeAnnotationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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<T>> { }
""",
"abc/MessageHandlerRegistryImpl.java",
"""
package abc;
import org.eclipse.jdt.annotation.*;
public class MessageHandlerRegistryImpl
<@NonNull C extends Connection<?>, @NonNull T, @NonNull D extends IncomingMessageData<T>>
implements MessageHandlerRegistry<C, T, D> { }
""",
"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<C, @NonNull CharSequence, @NonNull DIncomingMessageData> { }
""",
"abc/d/DMessageHandlerRegistryImpl.java",
"""
package abc.d;
import org.eclipse.jdt.annotation.*;
import abc.*;
public class DMessageHandlerRegistryImpl<@NonNull C extends DConnection>
extends MessageHandlerRegistryImpl<C, @NonNull CharSequence, @NonNull DIncomingMessageData>
implements DMessageHandlerRegistry<C> { }
"""
};
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;
}
}
}

0 comments on commit aeefecb

Please sign in to comment.