diff --git a/checker/src/main/java/org/checkerframework/checker/nullness/NullnessNoInitVisitor.java b/checker/src/main/java/org/checkerframework/checker/nullness/NullnessNoInitVisitor.java index a11cc6173d6..897265c6b56 100644 --- a/checker/src/main/java/org/checkerframework/checker/nullness/NullnessNoInitVisitor.java +++ b/checker/src/main/java/org/checkerframework/checker/nullness/NullnessNoInitVisitor.java @@ -286,9 +286,9 @@ protected boolean commonAssignmentCheck( /** Case 1: Check for null dereferencing. */ @Override public Void visitMemberSelect(MemberSelectTree tree, Void p) { - // if (atypeFactory.isUnreachable(tree)) { - // return super.visitMemberSelect(tree, p); - // } + if (ignoreDeadCode && atypeFactory.isUnreachable(tree)) { + return super.visitMemberSelect(tree, p); + } Element e = TreeUtils.elementFromUse(tree); if (e.getKind() == ElementKind.CLASS) { if (atypeFactory.containsNullnessAnnotation(null, tree.getExpression())) { diff --git a/checker/src/test/java/org/checkerframework/checker/test/junit/NullnessIgnoreDeadCodeTest.java b/checker/src/test/java/org/checkerframework/checker/test/junit/NullnessIgnoreDeadCodeTest.java new file mode 100644 index 00000000000..1273e81062d --- /dev/null +++ b/checker/src/test/java/org/checkerframework/checker/test/junit/NullnessIgnoreDeadCodeTest.java @@ -0,0 +1,33 @@ +package org.checkerframework.checker.test.junit; + +import org.checkerframework.framework.test.CheckerFrameworkPerDirectoryTest; +import org.junit.runners.Parameterized; + +import java.io.File; +import java.util.List; + +/** JUnit tests for the Nullness Checker when the -AignoreDeadCode command-line argument is used. */ +public class NullnessIgnoreDeadCodeTest extends CheckerFrameworkPerDirectoryTest { + /** + * Create a NullnessIgnoreDeadCodeTest. + * + * @param testFiles the files containing test code, which will be type-checked + */ + public NullnessIgnoreDeadCodeTest(List testFiles) { + super( + testFiles, + org.checkerframework.checker.nullness.NullnessChecker.class, + "nullness-ignoredeadcode", + "-AignoreDeadCode"); + } + + /** + * Returns an array of test directory paths for parameterized testing. + * + * @return an array containing the test directory names + */ + @Parameterized.Parameters + public static String[] getTestDirs() { + return new String[] {"nullness-ignoredeadcode"}; + } +} diff --git a/checker/tests/nullness-ignoredeadcode/DeadBranchNPE.java b/checker/tests/nullness-ignoredeadcode/DeadBranchNPE.java new file mode 100644 index 00000000000..d061a41fdb6 --- /dev/null +++ b/checker/tests/nullness-ignoredeadcode/DeadBranchNPE.java @@ -0,0 +1,36 @@ +// @skip-test until we add dead branch analysis +class DeadBranchNPE { + void test1() { + Object obj = null; + if (true) { + // :: error: (dereference.of.nullable) + obj.toString(); + } else { + // :: error: (dereference.of.nullable) + obj.toString(); + } + } + + void test2() { + Object objOut = null; + Object objInner = null; + // :: error: (dereference.of.nullable) + objOut.toString(); + // The following loop is dead code because the loop condition is false. + for (int i = 0; i < 0; i++) { + // :: error: (dereference.of.nullable) + objInner.toString(); + } + } + + void test3() { + Object objOut = null; + Object objInner = null; + // :: error: (dereference.of.nullable) + objOut.toString(); + while (objOut != null) { + // :: error: (dereference.of.nullable) + objInner.toString(); + } + } +} diff --git a/checker/tests/nullness/DeadBranchNPE.java b/checker/tests/nullness/DeadBranchNPE.java new file mode 100644 index 00000000000..e51a9837226 --- /dev/null +++ b/checker/tests/nullness/DeadBranchNPE.java @@ -0,0 +1,36 @@ +class DeadBranchNPE { + void test1() { + Object obj = null; + if (true) { + // :: error: (dereference.of.nullable) + obj.toString(); + } else { + // :: error: (dereference.of.nullable) + obj.toString(); + } + } + + void test2() { + Object objOut = null; + Object objInner = null; + // :: error: (dereference.of.nullable) + objOut.toString(); + // The following loop is dead code because the loop condition is false. + for (int i = 0; i < 0; i++) { + // :: error: (dereference.of.nullable) + objInner.toString(); + } + } + + void test3() { + Object objOut = null; + Object objInner = null; + // :: error: (dereference.of.nullable) + objOut.toString(); + // :: warning: (nulltest.redundant) + while (objOut != null) { + // :: error: (dereference.of.nullable) + objInner.toString(); + } + } +} diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 66633190053..03ba80004d1 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -3,6 +3,8 @@ Version 3.42.0-eisop6 (January ??, 2025) **User-visible changes:** +New command-line option `-AignoreDeadCode` to ignore unreachable code when running checkers. + **Implementation details:** **Closed issues:** diff --git a/docs/manual/introduction.tex b/docs/manual/introduction.tex index 4d53afaceb1..30382796f23 100644 --- a/docs/manual/introduction.tex +++ b/docs/manual/introduction.tex @@ -599,6 +599,8 @@ \item \<-AignoreTargetLocations> Disables validating the target locations of qualifiers based on `@TargetLocations` meta-annotations. This option is not enabled by default. +\item \<-AignoreDeadCode> + Ignore unreachable code when running checkers. \end{itemize} \label{unsound-by-default} diff --git a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java index 6cb489b93f6..87fa6396237 100644 --- a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java +++ b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java @@ -290,6 +290,9 @@ public class BaseTypeVisitor getExpressionAndOffsetFromJavaExpressionStr return value != null ? value.getAnnotations().iterator().next() : null; } - /* + /** * Returns true if the {@code exprTree} is unreachable. This is a conservative estimate and may * return {@code false} even though the {@code exprTree} is unreachable. * * @param exprTree an expression tree * @return true if the {@code exprTree} is unreachable - * + */ public boolean isUnreachable(ExpressionTree exprTree) { if (!everUseFlow) { return false; @@ -1106,7 +1112,6 @@ public boolean isUnreachable(ExpressionTree exprTree) { // None of the corresponding nodes is reachable, so this tree is dead. return true; } - */ /** * Track the state of org.checkerframework.dataflow analysis scanning for each class tree in the @@ -1122,7 +1127,7 @@ protected enum ScanState { /** Map from ClassTree to their dataflow analysis state. */ protected final Map scannedClasses = new HashMap<>(); - /* + /** * A set of trees whose corresponding nodes are reachable. This is not an exhaustive set of * reachable trees. Use {@link #isUnreachable(ExpressionTree)} instead of this set directly. * @@ -1130,7 +1135,7 @@ protected enum ScanState { * same name but represent different uses of the variable. So instead of storing Nodes, it * stores the result of {@code Node#getTree}. */ - // private final Set reachableNodes = new HashSet<>(); + private final Set reachableNodes = new HashSet<>(); /** * The result of the flow analysis. Invariant: @@ -1609,15 +1614,16 @@ protected void analyze( @Nullable Store capturedStore) { ControlFlowGraph cfg = CFCFGBuilder.build(this.getRoot(), ast, checker, this, processingEnv); - /* - cfg.getAllNodes(this::isIgnoredExceptionType) - .forEach( - node -> { - if (node.getTree() != null) { - reachableNodes.add(node.getTree()); - } - }); - */ + if (ignoreDeadCode) { + cfg.getAllNodes(this::isIgnoredExceptionType) + .forEach( + node -> { + if (node.getTree() != null) { + reachableNodes.add(node.getTree()); + } + }); + } + if (isInitializationCode) { Store initStore = !isStatic ? initializationStore : initializationStaticStore; if (initStore != null) {