Skip to content

Commit

Permalink
fix "Potential null pointer access" markers (#1086)
Browse files Browse the repository at this point in the history 
* disabled warning in test and example code
* raised level to warning in production code
* refactored code to get rid of all warnings

Co-authored-by: Jörg Kubitz <jkubitz-eclipse@gmx.de>
  • Loading branch information
@jukzi @EcljpseB0T
jukzi and EcljpseB0T committed Jan 18, 2024
1 parent d367439 commit 3ce9943
Show file tree
Hide file tree
Showing 65 changed files with 236 additions and 221 deletions.
3 changes: 2 additions & 1 deletion org.eclipse.jdt.bcoview/src/org/eclipse/jdt/bcoview/asm/CommentedASMifierClassVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Objects;

import org.objectweb.asm.Attribute;
import org.objectweb.asm.Handle;
Expand Down Expand Up @@ -157,7 +158,7 @@ public ASMifier visitMethod(int access, String name1, String desc, String signat
break;
}
}
assert meth != null;
Objects.requireNonNull(meth);

currMethod = new DecompiledMethod(className, new HashMap<>(), meth, options, access);
ASMifier textifier = super.visitMethod(access, name1, desc, signature, exceptions);
Expand Down
3 changes: 2 additions & 1 deletion org.eclipse.jdt.bcoview/src/org/eclipse/jdt/bcoview/asm/CommentedClassVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Objects;

import org.objectweb.asm.Attribute;
import org.objectweb.asm.Handle;
Expand Down Expand Up @@ -158,7 +159,7 @@ public Textifier visitMethod(int access, String name, String desc, String signat
break;
}
}
assert meth != null;

This comment has been minimized.

Sign in to view

Copy link
@stephan-herrmann

stephan-herrmann Jan 18, 2024

Contributor

FYI: ecj can be configured to respect assert during null analysis. Code change was not necessary.
Objects.requireNonNull(meth);

currMethod = new DecompiledMethod(className, new HashMap<>(), meth, options, access);
Textifier textifier = super.visitMethod(access, name, desc, signature, exceptions);
Expand Down
7 changes: 6 additions & 1 deletion org.eclipse.jdt.core.manipulation/.settings/org.eclipse.jdt.core.prefs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,12 @@ org.eclipse.jdt.core.compiler.annotation.nonnull=org.eclipse.jdt.annotation.NonN
org.eclipse.jdt.core.compiler.annotation.nonnull.secondary=
org.eclipse.jdt.core.compiler.annotation.nonnullbydefault=org.eclipse.jdt.annotation.NonNullByDefault
org.eclipse.jdt.core.compiler.annotation.nonnullbydefault.secondary=
org.eclipse.jdt.core.compiler.annotation.notowning=org.eclipse.jdt.annotation.NotOwning
org.eclipse.jdt.core.compiler.annotation.nullable=org.eclipse.jdt.annotation.Nullable
org.eclipse.jdt.core.compiler.annotation.nullable.secondary=
org.eclipse.jdt.core.compiler.annotation.nullanalysis=disabled
org.eclipse.jdt.core.compiler.annotation.owning=org.eclipse.jdt.annotation.Owning
org.eclipse.jdt.core.compiler.annotation.resourceanalysis=disabled
org.eclipse.jdt.core.compiler.codegen.inlineJsrBytecode=enabled
org.eclipse.jdt.core.compiler.codegen.methodParameters=do not generate
org.eclipse.jdt.core.compiler.codegen.targetPlatform=17
Expand Down Expand Up @@ -62,8 +65,10 @@ org.eclipse.jdt.core.compiler.problem.forbiddenReference=error
org.eclipse.jdt.core.compiler.problem.hiddenCatchBlock=error
org.eclipse.jdt.core.compiler.problem.includeNullInfoFromAsserts=disabled
org.eclipse.jdt.core.compiler.problem.incompatibleNonInheritedInterfaceMethod=error
org.eclipse.jdt.core.compiler.problem.incompatibleOwningContract=warning
org.eclipse.jdt.core.compiler.problem.incompleteEnumSwitch=error
org.eclipse.jdt.core.compiler.problem.indirectStaticAccess=warning
org.eclipse.jdt.core.compiler.problem.insufficientResourceAnalysis=warning
org.eclipse.jdt.core.compiler.problem.invalidJavadoc=warning
org.eclipse.jdt.core.compiler.problem.invalidJavadocTags=enabled
org.eclipse.jdt.core.compiler.problem.invalidJavadocTagsDeprecatedRef=disabled
Expand Down Expand Up @@ -100,7 +105,7 @@ org.eclipse.jdt.core.compiler.problem.overridingPackageDefaultMethod=error
org.eclipse.jdt.core.compiler.problem.parameterAssignment=ignore
org.eclipse.jdt.core.compiler.problem.pessimisticNullAnalysisForFreeTypeVariables=warning
org.eclipse.jdt.core.compiler.problem.possibleAccidentalBooleanAssignment=error
org.eclipse.jdt.core.compiler.problem.potentialNullReference=info
org.eclipse.jdt.core.compiler.problem.potentialNullReference=warning
org.eclipse.jdt.core.compiler.problem.potentiallyUnclosedCloseable=warning
org.eclipse.jdt.core.compiler.problem.rawTypeReference=warning
org.eclipse.jdt.core.compiler.problem.redundantNullAnnotation=warning
Expand Down
2 changes: 1 addition & 1 deletion ...tion/common/org/eclipse/jdt/core/manipulation/internal/javadoc/CoreJavadocAccessImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1487,7 +1487,7 @@ protected void handleLink(List<? extends ASTNode> fragments) {
fBuf.append('.');
}
fBuf.append(refMemberName);
if (refMethodParamTypes != null) {
if (refMethodParamTypes != null && refMethodParamNames != null) {
fBuf.append('(');
for (int i= 0; i < refMethodParamTypes.length; i++) {
String pType= refMethodParamTypes[i];
Expand Down
2 changes: 1 addition & 1 deletion ...ation/common/org/eclipse/jdt/internal/core/manipulation/JavaElementLabelComposerCore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ public void appendMethodLabel(IMethod method, long flags) {

// constructor type arguments
if (getFlag(flags, JavaElementLabelsCore.T_TYPE_PARAMETERS) && method.exists() && method.isConstructor()) {
if (resolvedSig != null && resolvedKey.isParameterizedType()) {
if (resolvedKey != null && resolvedSig != null && resolvedKey.isParameterizedType()) {
BindingKey declaringType= resolvedKey.getDeclaringType();
if (declaringType != null) {
String[] declaringTypeArguments= declaringType.getTypeArguments();
Expand Down
2 changes: 0 additions & 2 deletions ...pulation/common/org/eclipse/jdt/internal/core/manipulation/JavaElementPropertyTester.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,6 @@ public class JavaElementPropertyTester extends PropertyTester {
*/
public static final String PROJECT_OPTION = "projectOption"; //$NON-NLS-1$


@SuppressWarnings("boxing")
@Override
public boolean test(Object receiver, String method, Object[] args, Object expectedValue) {
return JavaCore.callReadOnly(() -> testCached(receiver, method, args, expectedValue));
Expand Down
4 changes: 2 additions & 2 deletions ...ore.manipulation/common/org/eclipse/jdt/internal/ui/viewsupport/CoreJavaElementLinks.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,8 @@ public static IJavaElement parseURI(URI uri) {
refModuleName= refTypeName.substring(0, index);
refTypeName= refTypeName.substring(index+1);
}
if ((refTypeName== null || refTypeName.isEmpty()) &&
(refModuleName != null && !refModuleName.isEmpty())) {
if (refTypeName == null ||
(refTypeName.isEmpty() && (refModuleName != null && !refModuleName.isEmpty()))) {
return getModule(element, refModuleName);
}
if (refTypeName.indexOf('/') == -1) {
Expand Down
4 changes: 2 additions & 2 deletions ...ion/org/eclipse/jdt/internal/corext/codemanipulation/GenerateHashCodeEqualsOperation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -583,9 +583,9 @@ private Statement[] createAddSimpleHashCode(ITypeBinding type, IHashCodeAccessPr
// Double.doubleToIntBits(aDouble)
Expression comparison= createDoubleInvocation(provider.getThisAccess(name));

if (singleTemp)
if (singleTemp && fragment != null) {
fragment.setInitializer(comparison);
else {
} else {
Assignment ass= fAst.newAssignment();
ass.setLeftHandSide(fAst.newSimpleName(VARIABLE_NAME_DOUBLE_TEMPORARY));
ass.setRightHandSide(comparison);
Expand Down
2 changes: 2 additions & 0 deletions ...se/jdt/internal/corext/codemanipulation/tostringgeneration/AbstractToStringGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Objects;
import java.util.Set;

import org.eclipse.core.runtime.CoreException;
Expand Down Expand Up @@ -228,6 +229,7 @@ protected void createMethodComment() throws CoreException {
objectMethod= objm;
}
}
Objects.requireNonNull(objectMethod);
if (fContext.isCreateComments()) {
String docString= CodeGeneration.getMethodComment(fContext.getCompilationUnit(), fContext.getTypeBinding().getQualifiedName(), toStringMethod, objectMethod, StubUtility
.getLineDelimiterUsed(fContext.getCompilationUnit()));
Expand Down
3 changes: 2 additions & 1 deletion ...lipse/jdt/internal/corext/codemanipulation/tostringgeneration/CustomBuilderGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,8 @@ private MethodInvocation createAppendMethodForMember(Object member) {
}
}
while (ami == null) {
memberType= memberType.getSuperclass();
ITypeBinding oldMemberType= memberType;
memberType= oldMemberType.getSuperclass();
if (memberType != null)
memberTypeName= memberType.getQualifiedName();
else
Expand Down
2 changes: 1 addition & 1 deletion ...lipse/jdt/internal/corext/codemanipulation/tostringgeneration/ToStringTemplateParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ protected ArrayList<String> extractElements(String template, String[] wantedVari
foundVariable= wantedVariable;
}
}
if (variablePosition == template.length()) {
if (foundVariable == null) {
result.add(template);
break;
} else {
Expand Down
6 changes: 2 additions & 4 deletions ...lation/core extension/org/eclipse/jdt/internal/corext/dom/VariableDeclarationRewrite.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@
import java.util.List;
import java.util.Map;

import org.eclipse.core.runtime.Assert;

import org.eclipse.text.edits.TextEditGroup;

import org.eclipse.jdt.core.dom.AST;
Expand Down Expand Up @@ -187,15 +185,15 @@ public static void rewriteModifiers(final VariableDeclarationStatement declarati
List<VariableDeclarationFragment> fragments= declarationNode.fragments();
Iterator<VariableDeclarationFragment> iter= fragments.iterator();

ListRewrite blockRewrite= null;
ListRewrite blockRewrite;
ASTNode parentStatement= declarationNode.getParent();
if (parentStatement instanceof SwitchStatement) {
blockRewrite= rewrite.getListRewrite(parentStatement, SwitchStatement.STATEMENTS_PROPERTY);
} else if (parentStatement instanceof Block) {
blockRewrite= rewrite.getListRewrite(parentStatement, Block.STATEMENTS_PROPERTY);
} else {
// should not happen. VariableDeclaration's can not be in a control statement body
Assert.isTrue(false);
throw new IllegalStateException(parentStatement.getClass().getName());
}

VariableDeclarationFragment lastFragment= iter.next();
Expand Down
9 changes: 8 additions & 1 deletion ...ore extension/org/eclipse/jdt/internal/corext/fix/AddMissingMethodDeclarationFixCore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import java.util.List;

import org.eclipse.core.runtime.CoreException;

import org.eclipse.jdt.core.dom.AST;
import org.eclipse.jdt.core.dom.ASTNode;
import org.eclipse.jdt.core.dom.Assignment;
Expand All @@ -41,9 +42,11 @@
import org.eclipse.jdt.core.dom.rewrite.ASTRewrite;
import org.eclipse.jdt.core.dom.rewrite.ImportRewrite;
import org.eclipse.jdt.core.dom.rewrite.ListRewrite;

import org.eclipse.jdt.internal.corext.dom.ASTNodes;
import org.eclipse.jdt.internal.corext.dom.Bindings;
import org.eclipse.jdt.internal.corext.refactoring.structure.CompilationUnitRewrite;

import org.eclipse.jdt.internal.ui.text.correction.CorrectionMessages;
import org.eclipse.jdt.internal.ui.text.correction.QuickAssistProcessorUtil;

Expand Down Expand Up @@ -91,6 +94,7 @@ public static AddMissingMethodDeclarationFixCore createAddMissingMethodDeclarati
type= variableDeclarationStatement.getType();
returnType= getReturnType(type);
} else {
@SuppressWarnings("null") // variableAssignment != null
Expression leftHandSide= variableAssignment.getLeftHandSide();
ITypeBinding assignmentTypeBinding= leftHandSide.resolveTypeBinding();
if (assignmentTypeBinding == null) {
Expand All @@ -104,7 +108,10 @@ public static AddMissingMethodDeclarationFixCore createAddMissingMethodDeclarati
}
return new AddMissingMethodDeclarationFixCore(label, compilationUnit, new AddMissingMethodDeclarationProposalOperation(methodReferenceNode, returnType, null));
} else {
IMethodBinding methodBinding= methodInvocationNode == null ? null : methodInvocationNode.resolveMethodBinding();
if (methodInvocationNode == null) {
return null;
}
IMethodBinding methodBinding= methodInvocationNode.resolveMethodBinding();
if (methodBinding == null) {
return null;
}
Expand Down
3 changes: 1 addition & 2 deletions ...nsion/org/eclipse/jdt/internal/corext/refactoring/code/PromoteTempToFieldRefactoring.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -736,8 +736,7 @@ private void addLocalDeclarationSplit(ASTRewrite rewrite) {
listRewrite= rewrite.getListRewrite(parentStatement, Block.STATEMENTS_PROPERTY);
} else {
// should not happen. VariableDeclaration's can not be in a control statement body
listRewrite= null;
Assert.isTrue(false);
throw new IllegalStateException(parentStatement.getClass().getName());
}
int statementIndex= listRewrite.getOriginalList().indexOf(tempDeclarationStatement);
Assert.isTrue(statementIndex != -1);
Expand Down
23 changes: 7 additions & 16 deletions ...on/core extension/org/eclipse/jdt/internal/corext/refactoring/code/flow/FlowAnalyzer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@
import org.eclipse.jdt.core.dom.FieldDeclaration;
import org.eclipse.jdt.core.dom.ForStatement;
import org.eclipse.jdt.core.dom.IBinding;
import org.eclipse.jdt.core.dom.IMethodBinding;
import org.eclipse.jdt.core.dom.ITypeBinding;
import org.eclipse.jdt.core.dom.IVariableBinding;
import org.eclipse.jdt.core.dom.IfStatement;
Expand All @@ -70,7 +69,6 @@
import org.eclipse.jdt.core.dom.MemberValuePair;
import org.eclipse.jdt.core.dom.MethodDeclaration;
import org.eclipse.jdt.core.dom.MethodInvocation;
import org.eclipse.jdt.core.dom.Name;
import org.eclipse.jdt.core.dom.NameQualifiedType;
import org.eclipse.jdt.core.dom.NormalAnnotation;
import org.eclipse.jdt.core.dom.NullLiteral;
Expand Down Expand Up @@ -371,8 +369,10 @@ protected SwitchData createSwitchData(List<Statement> statements) {
start= statement.getStartPosition();
}
}
} else {
} else if (info != null) {
info.merge(getFlowInfo(statement), fFlowContext);
} else {
throw new IllegalStateException(statement.getClass().getName());
}
end= statement.getStartPosition() + statement.getLength() - 1;
}
Expand Down Expand Up @@ -751,7 +751,7 @@ public void endVisit(MethodDeclaration node) {

@Override
public void endVisit(MethodInvocation node) {
endVisitMethodInvocation(node, node.getExpression(), node.arguments(), getMethodBinding(node.getName()));
endVisitMethodInvocation(node, node.getExpression(), node.arguments());
}

@Override
Expand Down Expand Up @@ -906,7 +906,7 @@ public void endVisit(StringLiteral node) {

@Override
public void endVisit(SuperConstructorInvocation node) {
endVisitMethodInvocation(node, node.getExpression(), node.arguments(), node.resolveConstructorBinding());
endVisitMethodInvocation(node, node.getExpression(), node.arguments());
}

@Override
Expand All @@ -918,7 +918,7 @@ public void endVisit(SuperFieldAccess node) {

@Override
public void endVisit(SuperMethodInvocation node) {
endVisitMethodInvocation(node, node.getQualifier(), node.arguments(), getMethodBinding(node.getName()));
endVisitMethodInvocation(node, node.getQualifier(), node.arguments());
}

@Override
Expand Down Expand Up @@ -1065,7 +1065,7 @@ public void endVisit(WildcardType node) {
assignFlowInfo(node, node.getBound());
}

private void endVisitMethodInvocation(ASTNode node, ASTNode receiver, List<Expression> arguments, IMethodBinding binding) {
private void endVisitMethodInvocation(ASTNode node, ASTNode receiver, List<Expression> arguments) {
if (skipNode(node))
return;
MessageSendFlowInfo info= createMessageSendFlowInfo();
Expand Down Expand Up @@ -1096,13 +1096,4 @@ private void endVisitIncDecOperation(Expression node, Expression operand) {
setFlowInfo(node, info);
}
}

private IMethodBinding getMethodBinding(Name name) {
if (name == null)
return null;
IBinding binding= name.resolveBinding();
if (binding instanceof IMethodBinding)
return (IMethodBinding)binding;
return null;
}
}
4 changes: 2 additions & 2 deletions ...n/org/eclipse/jdt/internal/corext/refactoring/code/makestatic/FinalConditionsChecker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public void checkMethodIsNotDuplicate(MethodDeclaration methodDeclaration, IMeth
String methodName= methodDeclaration.getName().getIdentifier();
IMethodBinding methodBinding= methodDeclaration.resolveBinding();
if (methodBinding == null) {
fStatus.merge(RefactoringStatus.createFatalErrorStatus(RefactoringCoreMessages.MakeStaticRefactoring_unexpected_binding_error));
throw new NullPointerException(RefactoringCoreMessages.MakeStaticRefactoring_unexpected_binding_error + ':' + methodDeclaration.toString());
}
ITypeBinding typeBinding= methodBinding.getDeclaringClass();
IType type= (IType) typeBinding.getJavaElement();
Expand Down Expand Up @@ -190,7 +190,7 @@ public void checkIsNotRecursive(SimpleName node, MethodDeclaration methodDeclara
IMethodBinding nodeMethodBinding= (IMethodBinding) node.resolveBinding();
IMethodBinding outerMethodBinding= methodDeclaration.resolveBinding();
if (nodeMethodBinding == null || outerMethodBinding == null) {
fStatus.merge(RefactoringStatus.createFatalErrorStatus(RefactoringCoreMessages.MakeStaticRefactoring_unexpected_binding_error));
throw new NullPointerException(RefactoringCoreMessages.MakeStaticRefactoring_unexpected_binding_error + ' ' + methodDeclaration);
}

if (nodeMethodBinding.isEqualTo(outerMethodBinding)) {
Expand Down
6 changes: 2 additions & 4 deletions ...ion/org/eclipse/jdt/internal/corext/refactoring/generics/ParametricStructureComputer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -278,9 +278,8 @@ private boolean updateStructureOfIthParamFrom(ParametricStructure structure1, in
Assert.isTrue(structure1 != otherStructure, "updateStructureOfIthParamFrom(): attempt to unify ith param of a parametric type with itself!"); //$NON-NLS-1$

ParametricStructure param1= structure1.getParameters()[i];
boolean param1Unknown= (param1 == null);

if (param1Unknown) {
if (param1 == null) {
if (DEBUG_INITIALIZATION)
System.out.println(" setting param " + i + " of " + structure1 + " to " + otherStructure); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
structure1.getParameters()[i]= otherStructure;
Expand Down Expand Up @@ -426,10 +425,9 @@ private boolean updateStructureOfVar(ConstraintVariable2 v, ParametricStructure
return false ;

ParametricStructure vStructure= elemStructure(v);
boolean vStructureUnknown= (vStructure == null);
boolean type2Structured= type2 != ParametricStructure.NONE;

if (vStructureUnknown) {
if ((vStructure == null)) {
if (DEBUG_INITIALIZATION)
System.out.println(" setting structure of " + v + " to " + type2); //$NON-NLS-1$ //$NON-NLS-2$
setStructureAndPush(v, type2);
Expand Down
4 changes: 2 additions & 2 deletions ...nipulation/core extension/org/eclipse/jdt/internal/corext/refactoring/nls/NLSScanner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ else if (defaultCounter > 0)
case ITerminalSymbols.TokenNameStringLiteral:
if (insideAnnotation.isEmpty() && defaultCounter == 0) {
currentLineNr= scanner.getLineNumber(scanner.getCurrentTokenStartPosition());
if (currentLineNr != previousLineNr) {
if (currentLine== null ||currentLineNr != previousLineNr) {
currentLine= new NLSLine(currentLineNr - 1);
lines.add(currentLine);
previousLineNr= currentLineNr;
Expand All @@ -164,7 +164,7 @@ else if (defaultCounter > 0)
case ITerminalSymbols.TokenNameTextBlock:
if (insideAnnotation.isEmpty() && defaultCounter == 0) {
currentLineNr= scanner.getLineNumber(scanner.getCurrentTokenEndPosition());
if (currentLineNr != previousLineNr) {
if (currentLine== null || currentLineNr != previousLineNr) {
currentLine= new NLSLine(currentLineNr - 1);
lines.add(currentLine);
previousLineNr= currentLineNr;
Expand Down
2 changes: 1 addition & 1 deletion ...nsion/org/eclipse/jdt/internal/corext/refactoring/structure/ChangeSignatureProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1403,7 +1403,7 @@ private TextChangeManager createChangeManager(IProgressMonitor pm, RefactoringSt
occurrenceUpdate.updateNode();
}

if (isNoArgConstructor && namedSubclassMapping.containsKey(cu)){
if (namedSubclassMapping != null && namedSubclassMapping.containsKey(cu)) {
//only non-anonymous subclasses may have noArgConstructors to modify - see bug 43444
for (IType subtype : namedSubclassMapping.get(cu)) {
AbstractTypeDeclaration subtypeNode= ASTNodeSearchUtil.getAbstractTypeDeclarationNode(subtype, cuRewrite.getRoot());
Expand Down
Loading

0 comments on commit 3ce9943

Please sign in to comment.