Skip to content

Commit

Permalink
Annotation-based analysis breaks when annotations are not on the build
Browse files Browse the repository at this point in the history
path

+ tag elements annotated with a missing type relevant for analysis
+ report the missing annotation at client sites when possibly relevant
+ also: treat Closeable lambda as fresh resource

Fixes https://github.com/eclipse-jdt/eclipse.jdt.core/
  • Loading branch information
stephan-herrmann committed Dec 2, 2024
1 parent 2528b4a commit 1424c75
Show file tree
Hide file tree
Showing 27 changed files with 849 additions and 11 deletions.
4 changes: 2 additions & 2 deletions JCL/jclMin23/src/java/io/InputStream.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
package java.io;

public class InputStream {

public class InputStream implements AutoCloseable {
public void close() {}
}
18 changes: 18 additions & 0 deletions JCL/jclMin23/src/java/lang/AssertionError.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package java.lang;

public class AssertionError extends Error {

static final long serialVersionUID = 1L;

public AssertionError(java.lang.String s) {
super(s);
}

public AssertionError(java.lang.String s, java.lang.Throwable cause) {
super(s, cause);
}

public AssertionError() {
}

}
4 changes: 4 additions & 0 deletions JCL/jclMin23/src/java/lang/AutoCloseable.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package java.lang;
public interface AutoCloseable {
void close() throws Exception;
}
2 changes: 2 additions & 0 deletions JCL/jclMin23/src/java/lang/IncompatibleClassChangeError.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
public
class IncompatibleClassChangeError extends LinkageError {

static final long serialVersionUID = 1L;

public IncompatibleClassChangeError () {
super();
}
Expand Down
4 changes: 3 additions & 1 deletion JCL/jclMin23/src/java/lang/LinkageError.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
public
class LinkageError extends Error {

public LinkageError() {
static final long serialVersionUID = 1L;

public LinkageError() {
super();
}

Expand Down
2 changes: 2 additions & 0 deletions JCL/jclMin23/src/java/lang/NoClassDefFoundError.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
public
class NoClassDefFoundError extends LinkageError {

static final long serialVersionUID = 1L;

public NoClassDefFoundError() {
super();
}
Expand Down
2 changes: 2 additions & 0 deletions JCL/jclMin23/src/java/lang/NoSuchFieldError.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
public
class NoSuchFieldError extends IncompatibleClassChangeError {

static final long serialVersionUID = 1L;

public NoSuchFieldError() {
super();
}
Expand Down
3 changes: 3 additions & 0 deletions JCL/jclMin23/src/java/lang/invoke/LambdaMetafactory.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package java.lang.invoke;
public class LambdaMetafactory {
}
2 changes: 1 addition & 1 deletion org.eclipse.jdt.core.compiler.batch/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Main-Class: org.eclipse.jdt.internal.compiler.batch.Main
Bundle-ManifestVersion: 2
Bundle-Name: Eclipse Compiler for Java(TM)
Bundle-SymbolicName: org.eclipse.jdt.core.compiler.batch
Bundle-Version: 3.40.100.qualifier
Bundle-Version: 3.41.0.qualifier
Bundle-ClassPath: .
Bundle-Vendor: Eclipse.org
Automatic-Module-Name: org.eclipse.jdt.core.compiler.batch
Expand Down
2 changes: 1 addition & 1 deletion org.eclipse.jdt.core.compiler.batch/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
<version>4.35.0-SNAPSHOT</version>
</parent>
<artifactId>org.eclipse.jdt.core.compiler.batch</artifactId>
<version>3.40.100-SNAPSHOT</version>
<version>3.41.0-SNAPSHOT</version>
<packaging>eclipse-plugin</packaging>

<properties>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1995,6 +1995,12 @@ public interface IProblem {
* @since 3.36
*/
int NullityUncheckedTypeAnnotation = Internal + 986;
/** @since 3.40 */
int MessageSendWithUnresolvedOwningAnnotation = Internal + 987;
/** @since 3.40 */
int ParameterWithUnresolvedOwningAnnotation = Internal + 988;
/** @since 3.40 */
int FieldWithUnresolvedOwningAnnotation = Internal + 989;


// Java 8 work
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1000,6 +1000,7 @@ public static void resolveAnnotations(BlockScope scope, Annotation[] sourceAnnot
if (annotations != null) {
annotations[i] = annotation.getCompilerAnnotation();
}
tagMissingAnalysisAnnotation(scope, recipient, annotation);
}
}

Expand Down Expand Up @@ -1075,6 +1076,32 @@ public static void resolveAnnotations(BlockScope scope, Annotation[] sourceAnnot
return annotations;
}

private static void tagMissingAnalysisAnnotation(Scope scope, Binding recipient, Annotation annotation) {
long extendedTagBits = scope.environment().checkForMissingAnalysisAnnotation(annotation.resolvedType);
if (extendedTagBits != 0) {
if (recipient instanceof MethodBinding method) {
method.extendedTagBits |= extendedTagBits;
} else if (recipient instanceof VariableBinding variable) {
if (extendedTagBits == ExtendedTagBits.HasMissingOwningAnnotation
&& scope instanceof MethodScope methodScope
&& variable.isParameter()
&& variable instanceof LocalVariableBinding local
&& local.declaration instanceof Argument arg)
{
Argument[] arguments = methodScope.referenceMethod().arguments;
for (int j=0;j < arguments.length;j++){
if (arguments[j] == arg) {
methodScope.referenceMethodBinding().original().markMissingOwningAnnotationOnParameter(j);
break;
}
}
} else if (variable instanceof FieldBinding field) {
field.extendedTagBits |= extendedTagBits;
}
}
}
}

/** Resolve JSR308 annotations on a type reference, array creation expression or a wildcard. Type parameters go directly to the method/ctor,
By construction the bindings associated with QTR, PQTR etc get resolved first and then annotations for different levels get resolved
and applied at one go. Likewise for multidimensional arrays.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,18 @@ public static FakedTrackingVariable preConnectTrackerAcrossAssignment(ASTNode lo
}
return closeTracker;
}
static void checkMethodForMissingAnnotation(MessageSend messageSend, Scope scope) {
if ((messageSend.binding.original().extendedTagBits & ExtendedTagBits.HasMissingOwningAnnotation) != 0)
scope.problemReporter().messageWithUnresolvedOwningAnnotation(messageSend, scope.environment());
}
static void checkParameterForMissingAnnotation(ASTNode location, MethodBinding method, int rank, Scope scope) {
if (method != null && method.parameterHasMissingOwningAnnotation(rank))
scope.problemReporter().parameterWithUnresolvedOwningAnnotation(location, method.original(), rank, scope.environment());
}
static void checkFieldForMissingAnnotation(ASTNode location, FieldBinding fieldBinding, Scope scope) {
if (fieldBinding != null && (fieldBinding.extendedTagBits & ExtendedTagBits.HasMissingOwningAnnotation) != 0)
scope.problemReporter().fieldWithUnresolvedOwningAnnotation(location, fieldBinding, scope.environment());
}

private static boolean containsAllocation(SwitchExpression location) {
for (Expression re : location.resultExpressions()) {
Expand Down Expand Up @@ -510,6 +522,7 @@ public static FlowInfo analyseCloseableAcquisition(BlockScope scope, FlowInfo fl
} else { // regular resource
FakedTrackingVariable tracker = acquisition.closeTracker;
if (scope.compilerOptions().isAnnotationBasedResourceAnalysisEnabled) {
checkMethodForMissingAnnotation(acquisition, scope);
long owningTagBits = acquisition.binding.tagBits & TagBits.AnnotationOwningMASK;
int initialNullStatus = (owningTagBits == TagBits.AnnotationNotOwning) ? FlowInfo.NON_NULL : FlowInfo.NULL;
if (tracker == null) {
Expand Down Expand Up @@ -839,6 +852,7 @@ public static void handleResourceFieldAssignment(BlockScope scope, FlowInfo flow
field = fieldReference.binding;
}
if (field != null&& (field.tagBits & TagBits.AnnotationNotOwning) == 0) { // assignment to @NotOwned has no meaning
checkFieldForMissingAnnotation(lhs, field, scope);
if ((field.tagBits & TagBits.AnnotationOwning) != 0) {
rhsTrackVar.markNullStatus(flowInfo, flowContext, FlowInfo.NON_NULL);
} else {
Expand Down Expand Up @@ -914,6 +928,7 @@ else if (expression instanceof CastExpression)
if (useAnnotations && (expression.bits & RestrictiveFlagMASK) == Binding.FIELD) {
// field read
FieldBinding fieldBinding = ((Reference) expression).lastFieldBinding();
checkFieldForMissingAnnotation(expression, fieldBinding, scope);
long owningBits = 0;
if (fieldBinding != null) {
owningBits = fieldBinding.getAnnotationTagBits() & TagBits.AnnotationOwningMASK;
Expand Down Expand Up @@ -946,7 +961,7 @@ else if (expression instanceof CastExpression)
if (isBlacklistedMethod(expression)) {
initialNullStatus = FlowInfo.NULL;
} else if (useAnnotations) {
initialNullStatus = getNullStatusFromMessageSend(expression);
initialNullStatus = getNullStatusFromMessageSend(expression, scope);
}
if (initialNullStatus != 0)
return new FakedTrackingVariable(local, location, flowInfo, flowContext, initialNullStatus, useAnnotations);
Expand All @@ -968,6 +983,9 @@ else if (expression instanceof CastExpression)
// leave state as UNKNOWN, the bit OWNED_BY_OUTSIDE will prevent spurious warnings
return tracker;
}
} else if (expression instanceof LambdaExpression) {
// treat fresh lambda like a fresh resource
return new FakedTrackingVariable(local, location, flowInfo, flowContext, FlowInfo.NULL, useAnnotations);
}

if (local.closeTracker != null)
Expand All @@ -992,8 +1010,9 @@ private static boolean isBlacklistedMethod(Expression expression) {
}

/* pre: usesOwningAnnotations. */
protected static int getNullStatusFromMessageSend(Expression expression) {
if (expression instanceof MessageSend) {
protected static int getNullStatusFromMessageSend(Expression expression, Scope scope) {
if (expression instanceof MessageSend message) {
checkMethodForMissingAnnotation(message, scope);
if ((((MessageSend) expression).binding.tagBits & TagBits.AnnotationNotOwning) != 0)
return FlowInfo.NON_NULL;
return FlowInfo.NULL; // per default assume responsibility to close
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,11 @@ default FlowInfo handleResourcePassedToInvocation(BlockScope currentScope, Metho
if (currentScope.compilerOptions().isAnnotationBasedResourceAnalysisEnabled) {
FakedTrackingVariable trackVar = FakedTrackingVariable.getCloseTrackingVariable(argument, flowInfo, flowContext, true);
if (trackVar != null) {
if (methodBinding.ownsParameter(rank)) {
int safeRank = Math.min(rank, methodBinding.parameters.length-1); // account for varargs
FakedTrackingVariable.checkParameterForMissingAnnotation(argument, methodBinding, safeRank, currentScope);
if (methodBinding.ownsParameter(safeRank)) {
trackVar.markOwnedByOutside(flowInfo, flowContext);
} else if (methodBinding.notownsParameter(rank)) {
} else if (methodBinding.notownsParameter(safeRank)) {
// ignore, no relevant change
} else {
trackVar.markAsShared();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,6 @@ public interface ExtendedTagBits {

// @Owning / closing
int IsClosingMethod = ASTNode.Bit1; // method

int HasMissingOwningAnnotation = ASTNode.Bit2; // method/ctor or field
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2020 IBM Corporation and others.
* Copyright (c) 2000, 2024 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand Down Expand Up @@ -38,6 +38,7 @@ public class FieldBinding extends VariableBinding {
public int compoundUseFlag = 0; // number or accesses via postIncrement or compoundAssignment

public FakedTrackingVariable closeTracker;
public long extendedTagBits;

protected FieldBinding() {
super(null, null, 0, null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1656,6 +1656,27 @@ int getAnalysisAnnotationBit(char[][] qualifiedTypeName) {
Integer typeBit = this.allAnalysisAnnotations.get(qualifiedTypeString);
return typeBit == null ? 0 : typeBit;
}
/**
* Check if the given type is a missing type that could be relevant for static analysis.
* @return A bit from {@link ExtendedTagBits} encoding the check result, or {@code 0}.
*/
public long checkForMissingAnalysisAnnotation(TypeBinding resolvedType) {
if (resolvedType instanceof MissingTypeBinding missing) {
if (this.globalOptions.isAnnotationBasedResourceAnalysisEnabled) {
if ((getAnalysisAnnotationBit(missing.compoundName) & TypeIds.BitAnyOwningAnnotation) != 0)
return ExtendedTagBits.HasMissingOwningAnnotation;
char[] simpleName = missing.compoundName[missing.compoundName.length-1];
if (matchesSimpleName(simpleName, this.globalOptions.owningAnnotationName)
|| matchesSimpleName(simpleName, this.globalOptions.notOwningAnnotationName))
return ExtendedTagBits.HasMissingOwningAnnotation;
}
}
return 0;
}
private boolean matchesSimpleName(char[] simpleName, char[][] qualifiedName) {
return CharOperation.equals(simpleName, qualifiedName[qualifiedName.length-1]);
}

public boolean isNullnessAnnotationPackage(PackageBinding pkg) {
return this.nonnullAnnotationPackage == pkg || this.nullableAnnotationPackage == pkg || this.nonnullByDefaultAnnotationPackage == pkg;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ public class MethodBinding extends Binding {
public static byte PARAM_NULLITY = (byte) (PARAM_NONNULL | PARAM_NULLABLE);
public static byte PARAM_OWNING = 4;
public static byte PARAM_NOTOWNING = 8;
public static byte PARAM_MISSING_OWNING_ANN = 16;

public static byte flowBitFromAnnotationTagBit(long tagBit) {
if (tagBit == TagBits.AnnotationNonNull)
Expand Down Expand Up @@ -1509,6 +1510,17 @@ public boolean notownsParameter(int i) {
return (this.parameterFlowBits[i] & PARAM_NOTOWNING) != 0;
return false;
}
public boolean parameterHasMissingOwningAnnotation(int rank) {
if (this.parameterFlowBits != null)
return (this.parameterFlowBits[rank] & PARAM_MISSING_OWNING_ANN) != 0;
return false;
}
public void markMissingOwningAnnotationOnParameter(int rank) {
if (this.parameterFlowBits == null)
this.parameterFlowBits = new byte[this.parameters.length];
this.parameterFlowBits[rank] |= PARAM_MISSING_OWNING_ANN;
}

/** @return TRUE means @NonNull declared, FALSE means @Nullable declared, null means nothing declared */
public Boolean getParameterNullness(int idx) {
if (this.parameterFlowBits != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,8 @@ public interface TypeIds {
/** Mark the type as notowning-annotation for resource analysis. */
final int BitNotOwningAnnotation = 4096;

final int BitAnyOwningAnnotation = BitOwningAnnotation | BitNotOwningAnnotation;

/**
* Set of type bits that should be inherited by any sub types.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6410,6 +6410,59 @@ public void nullAnnotationUnsupportedLocation(TypeReference type) {
handle(IProblem.NullAnnotationUnsupportedLocationAtType,
NoArgument, NoArgument, type.sourceStart, sourceEnd);
}
private char[][] missingAnalysisAnnotationName(AnnotationBinding[] annotations, LookupEnvironment environment) {
for (AnnotationBinding annotationBinding : annotations) {
if (annotationBinding.getAnnotationType() instanceof ReferenceBinding type
&& environment.checkForMissingAnalysisAnnotation(type) != 0)
return type.compoundName;
}
return null;
}
public void messageWithUnresolvedOwningAnnotation(MessageSend send, LookupEnvironment environment) {
char[][] compoundName = missingAnalysisAnnotationName(send.binding.original().getAnnotations(), environment);
if (compoundName == null)
return;
String selector = String.valueOf(send.selector);
handle(IProblem.MessageSendWithUnresolvedOwningAnnotation,
new String[] {selector, CharOperation.toString(compoundName)},
new String[] {selector, String.valueOf(compoundName[compoundName.length-1])},
ProblemSeverities.Warning,
send.sourceStart,
send.nameSourceEnd());
}
public void parameterWithUnresolvedOwningAnnotation(ASTNode location, MethodBinding method, int rank, LookupEnvironment environment) {
// locate the missing annotation on parameter #rank:
AnnotationBinding[][] parameterAnnotations = method.original().getParameterAnnotations();
char[][] compoundName = missingAnalysisAnnotationName(parameterAnnotations[rank], environment);
if (compoundName == null)
return;
String parameterName = method.parameterNames != Binding.NO_PARAMETER_NAMES
? String.valueOf(method.parameterNames[rank])
: "arg"+rank; //$NON-NLS-1$
String[] args = {
String.valueOf(parameterName),
String.valueOf(method.selector),
CharOperation.toString(compoundName)
};
handle(IProblem.ParameterWithUnresolvedOwningAnnotation,
args,
args,
ProblemSeverities.Warning,
location.sourceStart,
location.sourceEnd);
}
public void fieldWithUnresolvedOwningAnnotation(ASTNode location, FieldBinding fieldBinding, LookupEnvironment environment) {
char[][] compoundName = missingAnalysisAnnotationName(fieldBinding.getAnnotations(), environment);
if (compoundName == null)
return;
String selector = String.valueOf(fieldBinding.name);
handle(IProblem.FieldWithUnresolvedOwningAnnotation,
new String[] {selector, CharOperation.toString(compoundName)},
new String[] {selector, String.valueOf(compoundName[compoundName.length-1])},
ProblemSeverities.Warning,
location.sourceStart,
location.sourceEnd);
}
public void localVariableNullInstanceof(LocalVariableBinding local, ASTNode location) {
int severity = computeSeverity(IProblem.NullLocalVariableInstanceofYieldsFalse);
if (severity == ProblemSeverities.Ignore) return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -870,6 +870,9 @@
985 = This array dimension with declared element type {0} will be initialized with ''null'' entries
# same text as 955:
986 = Null type safety (type annotations): The expression of type ''{1}'' needs unchecked conversion to conform to ''{0}''
987 = Method ''{0}'' has an unresolved annotation ''{1}'' that could be relevant for static analysis
988 = Parameter ''{0}'' of method ''{1}'' has an unresolved annotation ''{2}'' that could be relevant for static analysis
989 = Field ''{0}'' has an unresolved annotation ''{1}'' that could be relevant for static analysis

# Java 8
1001 = Syntax error, modifiers and annotations are not allowed for the lambda parameter {0} as its type is elided
Expand Down
Loading

0 comments on commit 1424c75

Please sign in to comment.