diff --git a/its/autoscan/src/test/resources/autoscan/autoscan-diff-by-rules.json b/its/autoscan/src/test/resources/autoscan/autoscan-diff-by-rules.json index aa9baecbe3c..5108ce8880d 100644 --- a/its/autoscan/src/test/resources/autoscan/autoscan-diff-by-rules.json +++ b/its/autoscan/src/test/resources/autoscan/autoscan-diff-by-rules.json @@ -1046,7 +1046,7 @@ { "ruleKey": "S2187", "hasTruePositives": true, - "falseNegatives": 11, + "falseNegatives": 12, "falsePositives": 1 }, { @@ -1400,7 +1400,7 @@ { "ruleKey": "S2699", "hasTruePositives": true, - "falseNegatives": 143, + "falseNegatives": 151, "falsePositives": 1 }, { @@ -1466,7 +1466,7 @@ { "ruleKey": "S2924", "hasTruePositives": false, - "falseNegatives": 8, + "falseNegatives": 11, "falsePositives": 0 }, { @@ -1682,7 +1682,7 @@ { "ruleKey": "S3577", "hasTruePositives": true, - "falseNegatives": 44, + "falseNegatives": 46, "falsePositives": 0 }, { diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S2187.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S2187.json index 60bfd65954f..abb5aacb756 100644 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S2187.json +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S2187.json @@ -1,6 +1,6 @@ { "ruleKey": "S2187", "hasTruePositives": true, - "falseNegatives": 11, + "falseNegatives": 12, "falsePositives": 1 -} \ No newline at end of file +} diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S2699.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S2699.json index cfb1224504f..d361a3c2311 100644 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S2699.json +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S2699.json @@ -1,6 +1,6 @@ { "ruleKey": "S2699", "hasTruePositives": true, - "falseNegatives": 150, + "falseNegatives": 151, "falsePositives": 1 -} \ No newline at end of file +} diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S2924.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S2924.json index a69e3a6b183..3fb52f5ed23 100644 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S2924.json +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S2924.json @@ -1,6 +1,6 @@ { "ruleKey": "S2924", "hasTruePositives": false, - "falseNegatives": 8, + "falseNegatives": 11, "falsePositives": 0 -} \ No newline at end of file +} diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S3577.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S3577.json index 74e0ab8608f..ec7fce02570 100644 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S3577.json +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S3577.json @@ -1,6 +1,6 @@ { "ruleKey": "S3577", "hasTruePositives": true, - "falseNegatives": 45, + "falseNegatives": 46, "falsePositives": 0 -} \ No newline at end of file +} diff --git a/java-checks-test-sources/default/src/test/java/checks/tests/UnusedTestRuleCheck_Protected.java b/java-checks-test-sources/default/src/test/java/checks/tests/UnusedTestRuleCheck_Protected.java new file mode 100644 index 00000000000..e7536c1a391 --- /dev/null +++ b/java-checks-test-sources/default/src/test/java/checks/tests/UnusedTestRuleCheck_Protected.java @@ -0,0 +1,47 @@ +package checks.tests; + +import java.nio.file.Path; +import org.junit.jupiter.api.io.TempDir; + +// https://community.sonarsource.com/t/possible-fp-for-java-tempdir-declared-in-super-class/60836 +// https://sonarsource.atlassian.net/browse/SONARJAVA-4238 +public abstract class UnusedTestRuleCheck_Protected { + @TempDir + protected Path tempDirOldFP; // Compliant FP as used in a subclass - compliant because not private +} + +// Test abstract private +abstract class AbstractTestCase { + + @TempDir + private Path tempDir; // Noncompliant {{Remove this unused "TempDir".}} + // increases AutoScan FNs + + void test() { + } + +} + +// Test non-abstract private +class ClassTestCase { // increases AutoScan FNs + + @TempDir + private Path tempDir; // Noncompliant {{Remove this unused "TempDir".}} + // increases AutoScan FNs + + void test() { + } + +} + +// Test non-abstract protected +class ClassTestCase2 { + + @TempDir + protected Path tempDir; // Noncompliant {{Remove this unused "TempDir".}} + // increases AutoScan FNs + + void test() { + } + +} diff --git a/java-checks-test-sources/default/src/test/java/checks/tests/UnusedTestRuleCheck_UseProtected.java b/java-checks-test-sources/default/src/test/java/checks/tests/UnusedTestRuleCheck_UseProtected.java new file mode 100644 index 00000000000..80409ba8559 --- /dev/null +++ b/java-checks-test-sources/default/src/test/java/checks/tests/UnusedTestRuleCheck_UseProtected.java @@ -0,0 +1,17 @@ +package checks.tests; + +import java.nio.file.Files; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +// AutoScan S3577 Increases FN to 46 +public class UnusedTestRuleCheck_UseProtected extends UnusedTestRuleCheck_Protected { + @BeforeEach + void setup() throws Exception { + Files.createTempFile(tempDirOldFP, "test", ""); + } + @Test + void test() { + // AutoScan S2699 Increases FN to 151 + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/unused/UnusedTestRuleCheck.java b/java-checks/src/main/java/org/sonar/java/checks/unused/UnusedTestRuleCheck.java index 179c5ba4bec..3f2669a272b 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/unused/UnusedTestRuleCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/unused/UnusedTestRuleCheck.java @@ -23,13 +23,15 @@ import java.util.List; import java.util.Set; import org.sonar.check.Rule; -import org.sonarsource.analyzer.commons.collections.SetUtils; +import org.sonar.java.model.ModifiersUtils; import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; import org.sonar.plugins.java.api.semantic.Symbol; import org.sonar.plugins.java.api.tree.ClassTree; import org.sonar.plugins.java.api.tree.MethodTree; +import org.sonar.plugins.java.api.tree.Modifier; import org.sonar.plugins.java.api.tree.Tree; import org.sonar.plugins.java.api.tree.VariableTree; +import org.sonarsource.analyzer.commons.collections.SetUtils; @Rule(key = "S2924") public class UnusedTestRuleCheck extends IssuableSubscriptionVisitor { @@ -47,11 +49,16 @@ public List nodesToVisit() { @Override public void visitNode(Tree tree) { ClassTree classTree = (ClassTree) tree; + boolean isAbstract = ModifiersUtils.hasModifier(classTree.modifiers(), Modifier.ABSTRACT); for (Tree member : classTree.members()) { if (member.is(Tree.Kind.VARIABLE)) { VariableTree variableTree = (VariableTree) member; Symbol symbol = variableTree.symbol(); if ((isTestNameOrTemporaryFolderRule(symbol) || hasTempDirAnnotation(symbol)) && symbol.usages().isEmpty()) { + // if class is abstract, then we need to check modifier - if not private, then it's okay + if (isAbstract && !ModifiersUtils.hasModifier(variableTree.modifiers(), Modifier.PRIVATE)) { + continue; + } reportIssue(variableTree.simpleName(), "Remove this unused \"" + getSymbolType(symbol) + "\"."); } } else if (member.is(Tree.Kind.METHOD, Tree.Kind.CONSTRUCTOR)) { diff --git a/java-checks/src/test/java/org/sonar/java/checks/unused/UnusedTestRuleCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/unused/UnusedTestRuleCheckTest.java index cf01b0962fa..6f2179dad66 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/unused/UnusedTestRuleCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/unused/UnusedTestRuleCheckTest.java @@ -42,6 +42,14 @@ void test_JUnit5() { .verifyIssues(); } + @Test + void test_UseProtected() { + CheckVerifier.newVerifier() + .onFile(testCodeSourcesPath("checks/tests/UnusedTestRuleCheck_Protected.java")) + .withCheck(new UnusedTestRuleCheck()) + .verifyIssues(); + } + @Test void test_no_issues_without_semantic() { CheckVerifier.newVerifier()