Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ignoreDeadCode option #633

Draft
wants to merge 22 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
7102456
initial commit for adding notcheckdeadcode option
Ao-senXiong Nov 25, 2023
bd2e5e7
Merge branch 'master' into add-not-check-dead-code-option
Ao-senXiong Nov 25, 2023
58f4773
change option name to ignoreCheckDeadCode
Ao-senXiong Nov 25, 2023
1d1c348
add comment for entry point of the option
Ao-senXiong Nov 25, 2023
2540cd3
add protect variable of checker option
Ao-senXiong Nov 25, 2023
02c69a8
add * make javadoc happy
Ao-senXiong Nov 25, 2023
a3e4013
Merge branch 'master' into add-not-check-dead-code-option
Ao-senXiong Nov 25, 2023
73de62f
add deadbranch test case and skip the test first
Ao-senXiong Nov 28, 2023
30638eb
Merge branch 'master' into add-not-check-dead-code-option
Ao-senXiong Dec 17, 2024
6392013
Address all comments
Ao-senXiong Dec 17, 2024
decd355
Rename Junit test file
Ao-senXiong Dec 17, 2024
f7b1bb5
Use getter and revert access modifier for root
Ao-senXiong Dec 17, 2024
a93748b
Comment first...
Ao-senXiong Dec 17, 2024
1576672
Javadoc
Ao-senXiong Dec 17, 2024
3571c39
Merge branch 'master' into add-not-check-dead-code-option
wmdietl Dec 31, 2024
fa183ec
Update CHANGELOG.md
wmdietl Dec 31, 2024
f678255
Wording
Ao-senXiong Dec 31, 2024
2117ffc
Skip the test util we add dead branch analysis
Ao-senXiong Jan 5, 2025
aff7485
Add test case for normal nullness checker
Ao-senXiong Jan 5, 2025
1508ca6
Merge branch 'master' into add-not-check-dead-code-option
Ao-senXiong Jan 5, 2025
bf1652c
Fixes the test cases
Ao-senXiong Jan 5, 2025
85d3a9a
Extra warning
Ao-senXiong Jan 5, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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())) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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<File> 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"};
}
}
36 changes: 36 additions & 0 deletions checker/tests/nullness-ignoredeadcode/DeadBranchNPE.java
Original file line number Diff line number Diff line change
@@ -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();
}
}
}
36 changes: 36 additions & 0 deletions checker/tests/nullness/DeadBranchNPE.java
Original file line number Diff line number Diff line change
@@ -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();
}
}
}
2 changes: 2 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:**
Expand Down
2 changes: 2 additions & 0 deletions docs/manual/introduction.tex
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,9 @@ public class BaseTypeVisitor<Factory extends GenericAnnotatedTypeFactory<?, ?, ?
/** True if "-AcheckEnclosingExpr" was passed on the command line. */
private final boolean checkEnclosingExpr;

/** True if "-AignoreDeadCode" was passed on the command line. */
protected final boolean ignoreDeadCode;

/** The tree of the enclosing method that is currently being visited. */
protected @Nullable MethodTree methodTree = null;

Expand Down Expand Up @@ -351,6 +354,7 @@ protected BaseTypeVisitor(BaseTypeChecker checker, @Nullable Factory typeFactory
checkCastElementType = checker.hasOption("checkCastElementType");
conservativeUninferredTypeArguments =
checker.hasOption("conservativeUninferredTypeArguments");
ignoreDeadCode = checker.hasOption("ignoreDeadCode");
}

/** An array containing just {@code BaseTypeChecker.class}. */
Expand Down Expand Up @@ -5198,9 +5202,9 @@ protected TypeValidator createTypeValidator() {
*/
protected final boolean shouldSkipUses(ExpressionTree exprTree) {
// System.out.printf("shouldSkipUses: %s: %s%n", exprTree.getClass(), exprTree);
// if (atypeFactory.isUnreachable(exprTree)) {
// return true;
// }
if (ignoreDeadCode && atypeFactory.isUnreachable(exprTree)) {
return true;
}
Element elm = TreeUtils.elementFromTree(exprTree);
return checker.shouldSkipUses(elm);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,10 @@
// org.checkerframework.framework.source.SourceChecker.report
"warns",

// Ignore dead code
// org.checkerframework.framework.common.basetype.BaseTypeVisitor.shouldSkipUses
"ignoreDeadCode",

///
/// More sound (strict checking): enable errors that are disabled by default
///
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,9 @@ public abstract class GenericAnnotatedTypeFactory<
*/
public final boolean hasOrIsSubchecker;

/** True if "-AignoreDeadCode" was passed on the command line. */
protected final boolean ignoreDeadCode;

/** An empty store. */
// Set in postInit only
protected Store emptyStore;
Expand Down Expand Up @@ -396,6 +399,7 @@ protected GenericAnnotatedTypeFactory(BaseTypeChecker checker, boolean useFlow)
hasOrIsSubchecker =
!this.getChecker().getSubcheckers().isEmpty()
|| this.getChecker().getParentChecker() != null;
ignoreDeadCode = checker.hasOption("ignoreDeadCode");

// Every subclass must call postInit, but it must be called after
// all other initialization is finished.
Expand Down Expand Up @@ -470,7 +474,9 @@ public void setRoot(@Nullable CompilationUnitTree root) {

super.setRoot(root);
this.scannedClasses.clear();
// this.reachableNodes.clear();
if (ignoreDeadCode) {
this.reachableNodes.clear();
}
this.flowResult = null;
this.regularExitStores.clear();
this.exceptionalExitStores.clear();
Expand Down Expand Up @@ -1081,13 +1087,13 @@ public IPair<JavaExpression, String> 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;
Expand All @@ -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
Expand All @@ -1122,15 +1127,15 @@ protected enum ScanState {
/** Map from ClassTree to their dataflow analysis state. */
protected final Map<ClassTree, ScanState> 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.
*
* <p>This cannot be a set of Nodes, because two LocalVariableNodes are equal if they have the
* 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<Tree> reachableNodes = new HashSet<>();
private final Set<Tree> reachableNodes = new HashSet<>();

/**
* The result of the flow analysis. Invariant:
Expand Down Expand Up @@ -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) {
Expand Down
Loading