Several issues regarding initialization order
+ new tests re compile order (had issues requesting package-info.java and annotations too early)
+ fixed by using an UnresolvedReferenceBinding when initializing type/package annotations
+ also make initialization of type annotation and annotation of its package more independent
to better serve various call scenarios
+ perform more analysis even if a method has no parameters
Improved reporting and quickfixes:
+ avoid reporting issues inside dead code (would be imprecise anyway)
+ added a test re Bug 354554 - [null] conditional with redundant condition yields weak error message
+ new IProblem ParameterLackingNullableAnnotation (same parameterized message, though)
+ support quickfixes for a few more IProblems, but for ParameterLackingXYZ don't propose to change the super method
+ fixed which annotation to add (depending on modifyOverridden)
diff --git a/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/CompilerAdaptation.java b/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/CompilerAdaptation.java
index 6de1d3c..8755233 100644
--- a/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/CompilerAdaptation.java
+++ b/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/CompilerAdaptation.java
@@ -56,6 +56,7 @@
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.lookup.UnresolvedReferenceBinding;
import org.eclipse.jdt.internal.compiler.lookup.VariableBinding;
import org.eclipse.jdt.internal.compiler.problem.ProblemSeverities;
import org.eclipse.jdt.internal.compiler.util.HashtableOfInt;
@@ -114,6 +115,8 @@
abstract Expression getExpression();
// use custom hook from JDT/Core (https://bugs.eclipse.org/335093)
+ // TODO(SH): should calls to this method be guarded by "if (flowInfo.reachMode() == FlowInfo.REACHABLE)"?
+ // otherwise we'll proceed with incomplete information (normal null analysis avoids dead code).
checkAgainstNullAnnotation <- replace checkAgainstNullAnnotation;
/** Check assignment to local with null annotation. */
@@ -271,7 +274,8 @@
if ((this.expression.implicitConversion & org.eclipse.jdt.internal.compiler.lookup.TypeIds.UNBOXING) != 0) {
this.expression.checkNPE(blockScope, flowContext, flowInfo);
}
- checkAgainstNullAnnotation(currentScope, this.expression.nullStatus(flowInfo));
+ if (flowInfo.reachMode() == FlowInfo.REACHABLE)
+ checkAgainstNullAnnotation(currentScope, this.expression.nullStatus(flowInfo));
}
this.initStateIndex =
currentScope.methodScope().recordInitializationStates(flowInfo);
@@ -430,11 +434,6 @@
}
}
}
- TypeBinding annotationBinding = findDefaultNullness(binding.getDeclaringSourceType(), getScope().environment());
- if (annotationBinding != null) {
- long defaultNullness = Constants.getNullnessTagbit(annotationBinding);
- binding.fillInDefaultNullness(defaultNullness, annotationBinding);
- }
}
/** Feed null status from parameter annotation into the analysis of the method's body. */
@@ -594,10 +593,20 @@
? null : currentMethod.parameterNonNullness[i];
if (inheritedNonNullNess != null) { // super has a null annotation
if (currentNonNullNess == null) { // current parameter lacks null annotation
+ boolean needNonNull = false;
+ char[][] annotationName;
+ if (inheritedNonNullNess == Boolean.TRUE) {
+ needNonNull = true;
+ annotationName = environment.getNonNullAnnotationName();
+ } else {
+ annotationName = environment.getNullableAnnotationName();
+ }
+
getType().problemReporter().parameterLackingNonNullAnnotation(
currentArguments[i],
inheritedMethod.getDeclaringClass(),
- environment.getNullableAnnotationName());
+ needNonNull,
+ annotationName);
continue;
}
}
@@ -632,14 +641,14 @@
SourceTypeBinding currentType = type;
TypeBinding annotationBinding = null;
while (currentType != null) {
- annotationBinding = currentType.nullnessDefaultAnnotation;
+ annotationBinding = currentType.getNullnessDefaultAnnotation();
if (annotationBinding != null)
return annotationBinding;
currentType = currentType.enclosingType();
}
// package
- annotationBinding = type.getPackage().nullnessDefaultAnnotation;
+ annotationBinding = type.getPackage().getNullnessDefaultAnnotation();
if (annotationBinding != null)
return annotationBinding;
@@ -680,34 +689,55 @@
SourceTypeBinding enclosingType() -> ReferenceBinding enclosingType()
with { result <- (SourceTypeBinding)result }
- void computeAnnotations() -> long getAnnotationTagBits();
+ long computeTypeAnnotationTagBits() -> long getAnnotationTagBits();
- protected TypeBinding nullnessDefaultAnnotation;
+ private TypeBinding nullnessDefaultAnnotation;
+ private int nullnessDefaultInitialized = 0; // 0: nothing; 1: type; 2: package
/** initialize a normal type */
- evaluateNullAnnotations <- after getAnnotationTagBits;
-
+ void evaluateNullAnnotations(long tagBits) <- after long getAnnotationTagBits()
+ with { tagBits <- result }
+
/** initialize a package-info.java */
- evaluateNullAnnotations <- after initializeDeprecatedAnnotationTagBits
+ readAndEvaluateAnnotations <- after initializeDeprecatedAnnotationTagBits
base when (CharOperation.equals(base.sourceName, TypeConstants.PACKAGE_INFO_NAME));
- private void callBindArguments(MethodBinding method) {
- if (method.getParameters() != Binding.NO_PARAMETERS) {
- computeAnnotations();
- AbstractMethodDeclaration methodDecl = method.sourceMethod();
- if (methodDecl != null)
- methodDecl.bindArguments();
- }
- }
void callBindArguments(MethodBinding method) <- after MethodBinding resolveTypesFor(MethodBinding method);
+
+ private void callBindArguments(MethodBinding method) {
+ switch (this.nullnessDefaultInitialized) {
+ case 0:
+ computeTypeAnnotationTagBits();
+ //$FALL-THROUGH$
+ case 1:
+ getPackage().computeAnnotations();
+ this.nullnessDefaultInitialized = 2;
+ }
+ AbstractMethodDeclaration methodDecl = method.sourceMethod();
+ if (methodDecl != null) {
+ if (method.getParameters() != Binding.NO_PARAMETERS)
+ methodDecl.bindArguments();
+ TypeBinding annotationBinding = findDefaultNullness(method.getDeclaringSourceType(), methodDecl.getScope().environment());
+ if (annotationBinding != null) {
+ long defaultNullness = Constants.getNullnessTagbit(annotationBinding);
+ method.fillInDefaultNullness(defaultNullness, annotationBinding);
+ }
+ }
+ }
+ void readAndEvaluateAnnotations() {
+ computeTypeAnnotationTagBits();
+ }
@SuppressWarnings("inferredcallout")
- void evaluateNullAnnotations() {
+ void evaluateNullAnnotations(long tagBits) {
+ if (this.nullnessDefaultInitialized > 0)
+ return;
+ this.nullnessDefaultInitialized = 1;
// transfer nullness info from tagBits to this.nullnessDefaultAnnotation
- long tagBit = Constants.applyDefaultNullnessTagbit(getTagBits());
+ long tagBit = Constants.applyDefaultNullnessTagbit(tagBits);
if (tagBit == 0)
return;
- TypeBinding nullnessDefaultAnnotation = getPackage().getEnvironment().getNullAnnotationBinding(tagBit);
+ TypeBinding nullnessDefaultAnnotation = getPackage().getEnvironment().getUnresolvedNullAnnotationBinding(tagBit);
if (nullnessDefaultAnnotation != null) {
if (CharOperation.equals(this.sourceName, TypeConstants.PACKAGE_INFO_NAME)) {
getPackage().nullnessDefaultAnnotation = nullnessDefaultAnnotation;
@@ -716,6 +746,13 @@
}
}
}
+ public TypeBinding getNullnessDefaultAnnotation() {
+ if (this.nullnessDefaultAnnotation instanceof UnresolvedReferenceBinding)
+ return this.nullnessDefaultAnnotation =
+ org.eclipse.jdt.internal.compiler.lookup.BinaryTypeBinding.resolveType(this.nullnessDefaultAnnotation,
+ getPackage().getEnvironment(), false);
+ return this.nullnessDefaultAnnotation;
+ }
}
/** Retrieve null annotations from binary methods. */
@@ -804,12 +841,12 @@
char[][] typeName = CharOperation.splitOn('/', annotationTypeName, 1, annotationTypeName.length-1); // cut of leading 'L' and trailing ';'
if (CharOperation.equals(typeName, nonNullByDefaultAnnotationName)) {
annotationBit = TagBits.AnnotationNonNullByDefault;
- defaultNullness = getEnvironment().getNullAnnotationBinding(TagBits.AnnotationNonNull);
+ defaultNullness = getEnvironment().getUnresolvedNullAnnotationBinding(TagBits.AnnotationNonNull);
break;
}
if (CharOperation.equals(typeName, nullableByDefaultAnnotationName)) {
annotationBit = TagBits.AnnotationNullableByDefault;
- defaultNullness = getEnvironment().getNullAnnotationBinding(TagBits.AnnotationNullable);
+ defaultNullness = getEnvironment().getUnresolvedNullAnnotationBinding(TagBits.AnnotationNullable);
break;
}
}
@@ -829,6 +866,9 @@
/** Initiate setup of configured null annotation types. */
protected class LookupEnvironment playedBy LookupEnvironment {
+ @SuppressWarnings("decapsulation")
+ TypeBinding getTypeFromCompoundName(char[][] compoundName, boolean isParameterized, boolean wasMissingType)
+ -> ReferenceBinding getTypeFromCompoundName(char[][] compoundName, boolean isParameterized, boolean wasMissingType);
ReferenceBinding getType(char[][] compoundName) -> ReferenceBinding getType(char[][] compoundName);
PackageBinding createPackage(char[][] compoundName) -> PackageBinding createPackage(char[][] compoundName);
@@ -899,12 +939,20 @@
return getType(getNullableAnnotationName());
return null;
}
+ public TypeBinding getUnresolvedNullAnnotationBinding(long annotationTagBit) {
+ if (annotationTagBit == TagBits.AnnotationNonNull)
+ return getTypeFromCompoundName(getNonNullAnnotationName(), false, false);
+ if (annotationTagBit == TagBits.AnnotationNullable)
+ return getTypeFromCompoundName(getNullableAnnotationName(), false, false);
+ return null;
+ }
}
/** The package holding the configured null annotation types detects and marks these annotation types. */
protected class PackageBinding playedBy PackageBinding {
LookupEnvironment getEnvironment() -> get LookupEnvironment environment;
+ void computeAnnotations() -> boolean isViewedAsDeprecated();
protected char[] nullableName = null;
protected char[] nonNullName = null;
@@ -931,6 +979,13 @@
type.id = id; // ensure annotations of this type are detected as standard annotations.
}
+
+ public TypeBinding getNullnessDefaultAnnotation() {
+ if (this.nullnessDefaultAnnotation instanceof UnresolvedReferenceBinding)
+ return this.nullnessDefaultAnnotation =
+ org.eclipse.jdt.internal.compiler.lookup.BinaryTypeBinding.resolveType(this.nullnessDefaultAnnotation, getEnvironment(), false);
+ return this.nullnessDefaultAnnotation;
+ }
}
/** Intermediate role to provide access to environment and problemReporter as roles, too. */
@@ -1068,9 +1123,9 @@
argument.type.sourceEnd);
}
}
- public void parameterLackingNonNullAnnotation(Argument argument, ReferenceBinding declaringClass, char[][] inheritedAnnotationName) {
+ public void parameterLackingNonNullAnnotation(Argument argument, ReferenceBinding declaringClass, boolean needNonNull, char[][] inheritedAnnotationName) {
this.handle(
- IProblem.ParameterLackingNonNullAnnotation,
+ needNonNull ? IProblem.ParameterLackingNonNullAnnotation : IProblem.ParameterLackingNullableAnnotation,
new String[] { new String(argument.name), new String(declaringClass.readableName()), CharOperation.toString(inheritedAnnotationName)},
new String[] { new String(argument.name), new String(declaringClass.shortReadableName()), new String(inheritedAnnotationName[inheritedAnnotationName.length-1])},
argument.type.sourceStart,
diff --git a/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/Constants.java b/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/Constants.java
index 2f2fb23..f4fed45 100644
--- a/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/Constants.java
+++ b/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/Constants.java
@@ -63,9 +63,11 @@
/** @since 3.7 */
int ParameterLackingNonNullAnnotation = MethodRelated + 917;
/** @since 3.7 */
- int PotentialNullMessageSendReference = Internal + 918;
+ int ParameterLackingNullableAnnotation = MethodRelated + 918;
/** @since 3.7 */
- int RedundantNullCheckOnNonNullMessageSend = Internal + 919;
+ int PotentialNullMessageSendReference = Internal + 919;
+ /** @since 3.7 */
+ int RedundantNullCheckOnNonNullMessageSend = Internal + 920;
}
/** Translate from a nullness annotation to the corresponding tag bit or 0L. */
diff --git a/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/problem_messages.properties b/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/problem_messages.properties
index 83d86e2..4871e6c 100644
--- a/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/problem_messages.properties
+++ b/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/problem_messages.properties
@@ -19,5 +19,6 @@
915 = Illegal redefinition of parameter {0}, inherited method from {1} declares this parameter as @{2}
916 = Illegal redefinition of parameter {0}, inherited method from {1} does not constrain this parameter
917 = Missing null annotation: inherited method from {1} declares this parameter as @{2}
-918 = Potential null pointer access: The method {0} may return null
-919 = Redundant null check: The method {0} cannot return null
+918 = Missing null annotation: inherited method from {1} declares this parameter as @{2}
+919 = Potential null pointer access: The method {0} may return null
+920 = Redundant null check: The method {0} cannot return null
diff --git a/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/quickfix/NullAnnotationsCleanUp.java b/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/quickfix/NullAnnotationsCleanUp.java
index d20aaa4..cc044bf 100644
--- a/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/quickfix/NullAnnotationsCleanUp.java
+++ b/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/quickfix/NullAnnotationsCleanUp.java
@@ -101,6 +101,7 @@
case RequiredNonNullButProvidedNull:
case RequiredNonNullButProvidedPotentialNull:
case RequiredNonNullButProvidedUnknown:
+ case ParameterLackingNullableAnnotation:
result.add(FixMessages.NullAnnotationsCleanUp_add_nullable_annotation);
break;
case IllegalDefinitionToNonNullParameter:
diff --git a/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/quickfix/QuickFixes.java b/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/quickfix/QuickFixes.java
index 125359a..9010784 100644
--- a/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/quickfix/QuickFixes.java
+++ b/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/quickfix/QuickFixes.java
@@ -17,6 +17,7 @@
import static org.eclipse.objectteams.internal.jdt.nullity.Constants.IProblem.IllegalRedefinitionToNonNullParameter;
import static org.eclipse.objectteams.internal.jdt.nullity.Constants.IProblem.IllegalReturnNullityRedefinition;
import static org.eclipse.objectteams.internal.jdt.nullity.Constants.IProblem.ParameterLackingNonNullAnnotation;
+import static org.eclipse.objectteams.internal.jdt.nullity.Constants.IProblem.ParameterLackingNullableAnnotation;
import java.util.ArrayList;
import java.util.Collection;
@@ -29,13 +30,14 @@
import org.eclipse.core.runtime.CoreException;
import org.eclipse.jdt.core.ICompilationUnit;
import org.eclipse.jdt.core.IJavaElement;
-import org.eclipse.jdt.core.IJavaProject;
import org.eclipse.jdt.core.compiler.IProblem;
import org.eclipse.jdt.core.dom.ASTNode;
import org.eclipse.jdt.core.dom.CompilationUnit;
import org.eclipse.jdt.core.dom.IBinding;
import org.eclipse.jdt.core.dom.IVariableBinding;
import org.eclipse.jdt.core.dom.SimpleName;
+import org.eclipse.jdt.core.dom.VariableDeclaration;
+import org.eclipse.jdt.internal.corext.dom.ASTNodes;
import org.eclipse.jdt.internal.corext.fix.CompilationUnitRewriteOperationsFix;
import org.eclipse.jdt.internal.corext.fix.CompilationUnitRewriteOperationsFix.CompilationUnitRewriteOperation;
import org.eclipse.jdt.internal.corext.util.JavaModelUtil;
@@ -76,6 +78,7 @@
case IllegalRedefinitionToNonNullParameter:
case IllegalDefinitionToNonNullParameter:
case ParameterLackingNonNullAnnotation:
+ case ParameterLackingNullableAnnotation:
case IProblem.NonNullLocalVariableComparisonYieldsFalse:
case IProblem.RedundantNullCheckOnNonNullLocalVariable:
return true;
@@ -116,13 +119,14 @@
case IllegalReturnNullityRedefinition:
case IllegalDefinitionToNonNullParameter:
case IllegalRedefinitionToNonNullParameter:
- case ParameterLackingNonNullAnnotation:
addNullAnnotationInSignatureProposal(context, problem, proposals, false);
addNullAnnotationInSignatureProposal(context, problem, proposals, true);
break;
case RequiredNonNullButProvidedNull:
case RequiredNonNullButProvidedPotentialNull:
case RequiredNonNullButProvidedUnknown:
+ case ParameterLackingNonNullAnnotation:
+ case ParameterLackingNullableAnnotation:
case IProblem.NonNullLocalVariableComparisonYieldsFalse:
case IProblem.RedundantNullCheckOnNonNullLocalVariable:
if (isComplainingAboutArgument(selectedNode)
@@ -183,6 +187,11 @@
IBinding binding = nameNode.resolveBinding();
if (binding.getKind() == IBinding.VARIABLE && ((IVariableBinding) binding).isParameter())
return true;
+ VariableDeclaration argDecl = (VariableDeclaration) ASTNodes.getParent(selectedNode, VariableDeclaration.class);
+ if (argDecl != null)
+ binding = argDecl.resolveBinding();
+ if (binding.getKind() == IBinding.VARIABLE && ((IVariableBinding) binding).isParameter())
+ return true;
return false;
}
@@ -200,6 +209,12 @@
switch (problem.getProblemId()) {
case IllegalDefinitionToNonNullParameter:
case IllegalRedefinitionToNonNullParameter:
+ // case ParameterLackingNullableAnnotation: // never proposed with modifyOverridden
+ if (modifyOverridden) {
+ annotationToAdd = nonNullAnnotationName;
+ annotationToRemove = nullableAnnotationName;
+ }
+ break;
case ParameterLackingNonNullAnnotation:
case IllegalReturnNullityRedefinition:
if (!modifyOverridden) {
diff --git a/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/quickfix/RewriteOperations.java b/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/quickfix/RewriteOperations.java
index 031dbc7..b7d5247 100644
--- a/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/quickfix/RewriteOperations.java
+++ b/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/quickfix/RewriteOperations.java
@@ -297,7 +297,9 @@
switch (problem.getProblemId()) {
case ParameterLackingNonNullAnnotation:
+ case ParameterLackingNullableAnnotation:
case IllegalDefinitionToNonNullParameter:
+ case IllegalRedefinitionToNonNullParameter:
case IProblem.NonNullLocalVariableComparisonYieldsFalse:
case IProblem.RedundantNullCheckOnNonNullLocalVariable:
// statements suggest changing parameters: