-
Notifications
You must be signed in to change notification settings - Fork 29
Add ignoreDeadCode option
#633
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
base: master
Are you sure you want to change the base?
Changes from 16 commits
7102456
bd2e5e7
58f4773
1d1c348
2540cd3
02c69a8
a3e4013
73de62f
30638eb
6392013
decd355
f7b1bb5
a93748b
1576672
3571c39
fa183ec
f678255
2117ffc
aff7485
1508ca6
bf1652c
85d3a9a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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 -AignoreDeadCode command-line argument is used. */ | ||
| public class NullnessIgnoreDeadCodeTest extends CheckerFrameworkPerDirectoryTest { | ||
| /** | ||
| * Create a NullnessNullMarkedTest. | ||
|
aosen-xiong marked this conversation as resolved.
Outdated
|
||
| * | ||
| * @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"}; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| class DeadBranchNPE { | ||
| void test1() { | ||
| Object obj = null; | ||
| if (true) { | ||
| // :: error: (dereference.of.nullable) | ||
| obj.toString(); | ||
| } else { | ||
| // TODO: This is a dead branch should not issue error, the currently it does | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As this is the point of the PR, we should fix this issue before merging. |
||
| // obj.toString(); | ||
| } | ||
| } | ||
|
|
||
| void test2() { | ||
| Object obj = null; | ||
| // :: error: (dereference.of.nullable) | ||
| obj.toString(); | ||
| // The following loop is dead code because the loop condition is false. | ||
| for (int i = 0; i < 0; i++) { | ||
| obj.toString(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also note that this also doesn't produce a warning before this PR - so does this PR do anything?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should add the same test to the normal Nullness Checker tests and show the actual difference in behavior between the two.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see. I thought we want to enable the check first. I will write dead branch analysis in this PR. I think Jenny's thesis contains case study for dead branch analysis but not sure why it is not in the dataflow framework.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you mean? The
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this issue comes from when we fetch upstream changes. They only implement reachablenodes to determine whether it includes exception node and its Successors. See
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That upstream change is already in eisop. Are you saying there are other changes that have not been merged yet?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I am trying to say the upstream change is not enough to check dead branch for |
||
| } | ||
| } | ||
|
|
||
| void test3() { | ||
| Object obj = null; | ||
| // :: error: (dereference.of.nullable) | ||
| obj.toString(); | ||
| while (obj != null) { | ||
| obj.toString(); | ||
| } | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.