X Tutup
Skip to content

Commit 69a309f

Browse files
leonardo-pilastri-sonarsourceleonardo-pilastri-sonarsource
authored
SONARJAVA-5755 FP on S1133 when using forRemoval=false (#5292)
1 parent d3d2ae3 commit 69a309f

File tree

7 files changed

+125
-12
lines changed

7 files changed

+125
-12
lines changed

‎java-checks/src/main/java/org/sonar/java/checks/DeprecatedTagPresenceCheck.java‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,17 @@
1818

1919
import java.util.Arrays;
2020
import java.util.List;
21+
import java.util.Optional;
2122
import org.sonar.check.Rule;
2223
import org.sonar.java.ast.visitors.PublicApiChecker;
2324
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
25+
import org.sonar.plugins.java.api.tree.AnnotationTree;
2426
import org.sonar.plugins.java.api.tree.Tree;
2527

2628
import static org.sonar.java.checks.helpers.DeprecatedCheckerHelper.deprecatedAnnotation;
27-
import static org.sonar.java.checks.helpers.DeprecatedCheckerHelper.reportTreeForDeprecatedTree;
29+
import static org.sonar.java.checks.helpers.DeprecatedCheckerHelper.getAnnotationAttributeValue;
2830
import static org.sonar.java.checks.helpers.DeprecatedCheckerHelper.hasJavadocDeprecatedTag;
31+
import static org.sonar.java.checks.helpers.DeprecatedCheckerHelper.reportTreeForDeprecatedTree;
2932

3033
@Rule(key = "S1133")
3134
public class DeprecatedTagPresenceCheck extends IssuableSubscriptionVisitor {
@@ -43,7 +46,17 @@ public void visitNode(Tree tree) {
4346
}
4447

4548
private static boolean hasDeprecatedAnnotation(Tree tree) {
46-
return deprecatedAnnotation(tree) != null;
49+
AnnotationTree annotation = deprecatedAnnotation(tree);
50+
if (annotation == null) {
51+
return false;
52+
}
53+
Optional<Boolean> forRemovalValue = getAnnotationAttributeValue(annotation, "forRemoval", Boolean.class);
54+
// If forRemoval was not specified, we consider the deprecated code should be removed in the future.
55+
if (forRemovalValue.isEmpty()) {
56+
return true;
57+
}
58+
// Otherwise, we check the value of forRemoval and return that, so that only @Deprecated(forRemoval = true) is considered.
59+
return Boolean.TRUE.equals(forRemovalValue.get());
4760
}
4861

4962
}

‎java-checks/src/main/java/org/sonar/java/checks/helpers/DeprecatedCheckerHelper.java‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,21 @@
1616
*/
1717
package org.sonar.java.checks.helpers;
1818

19+
import java.util.Optional;
1920
import javax.annotation.CheckForNull;
2021
import org.sonar.java.ast.visitors.PublicApiChecker;
22+
import org.sonar.java.model.expression.AssignmentExpressionTreeImpl;
2123
import org.sonar.plugins.java.api.tree.AnnotationTree;
2224
import org.sonar.plugins.java.api.tree.ClassTree;
25+
import org.sonar.plugins.java.api.tree.ExpressionTree;
2326
import org.sonar.plugins.java.api.tree.IdentifierTree;
2427
import org.sonar.plugins.java.api.tree.MethodTree;
2528
import org.sonar.plugins.java.api.tree.Tree;
2629
import org.sonar.plugins.java.api.tree.Tree.Kind;
2730
import org.sonar.plugins.java.api.tree.VariableTree;
2831

32+
import static org.sonar.java.model.ExpressionUtils.annotationAttributeName;
33+
2934
public class DeprecatedCheckerHelper {
3035

3136
private static final Kind[] CLASS_KINDS = PublicApiChecker.classKinds();
@@ -94,4 +99,24 @@ private static boolean isDeprecated(AnnotationTree tree) {
9499
"Deprecated".equals(((IdentifierTree) tree.annotationType()).name());
95100
}
96101

102+
/**
103+
* @param annotationTree The annotation tree to check
104+
* @param attributeName The attribute name of the attribute to get the value for
105+
* @return an Optional containing the value of the attribute if found, otherwise an empty Optional
106+
*/
107+
public static <T> Optional<T> getAnnotationAttributeValue(AnnotationTree annotationTree, String attributeName, Class<T> valueType) {
108+
Optional<ExpressionTree> valueExpression = annotationTree.arguments().stream()
109+
.filter(argument -> attributeName.equals(annotationAttributeName(argument)))
110+
.map(argument -> {
111+
// arguments of an annotation are either an assignment (name=value) or an expression (ofter a literal value)
112+
if (argument.is(Kind.ASSIGNMENT)) {
113+
return ((AssignmentExpressionTreeImpl) argument).expression();
114+
} else {
115+
return argument;
116+
}
117+
})
118+
.findFirst();
119+
return valueExpression.flatMap(expressionTree -> expressionTree.asConstant(valueType));
120+
}
121+
97122
}

‎java-checks/src/main/java/org/sonar/java/checks/spring/SpringComposedRequestMappingCheck.java‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@
3737
import org.sonar.plugins.java.api.tree.NewArrayTree;
3838
import org.sonar.plugins.java.api.tree.Tree;
3939

40+
import static org.sonar.java.model.ExpressionUtils.annotationAttributeName;
41+
4042
@Rule(key = "S4488")
4143
public class SpringComposedRequestMappingCheck extends IssuableSubscriptionVisitor implements DependencyVersionAware {
4244

@@ -62,7 +64,7 @@ public void visitNode(Tree tree) {
6264
AnnotationTree annotation = (AnnotationTree) tree;
6365
if (annotation.symbolType().is("org.springframework.web.bind.annotation.RequestMapping")) {
6466
List<ExpressionTree> methodValues = annotation.arguments().stream()
65-
.filter(argument -> "method".equals(attributeName(argument)))
67+
.filter(argument -> "method".equals(annotationAttributeName(argument)))
6668
.flatMap(SpringComposedRequestMappingCheck::extractValues)
6769
.toList();
6870

@@ -94,15 +96,6 @@ private static String getRequestMethodEnumEntry(ExpressionTree requestMethod) {
9496
return "";
9597
}
9698

97-
private static String attributeName(ExpressionTree expression) {
98-
if (expression.is(Tree.Kind.ASSIGNMENT)) {
99-
AssignmentExpressionTree assignment = (AssignmentExpressionTree) expression;
100-
// assignment.variable() in annotation is always a Tree.Kind.IDENTIFIER
101-
return ((IdentifierTree) assignment.variable()).name();
102-
}
103-
return "value";
104-
}
105-
10699
private static Stream<ExpressionTree> extractValues(ExpressionTree argument) {
107100
ExpressionTree expression = argument;
108101
if (expression.is(Tree.Kind.ASSIGNMENT)) {

‎java-checks/src/test/files/checks/DeprecatedTagPresenceCheck.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
class Foo {
22

3+
@Deprecated(forRemoval = true)
4+
public String getName; // Noncompliant
5+
6+
@Deprecated(forRemoval = false)
7+
public String getName; // Compliant
8+
39
@Deprecated
410
public int foo; // Noncompliant {{Do not forget to remove this deprecated code someday.}}
511
// ^^^

‎java-checks/src/test/java/org/sonar/java/checks/helpers/DeprecatedCheckerHelperTest.java‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/*
2+
* SonarQube Java
3+
* Copyright (C) 2012-2025 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.java.checks.helpers;
18+
19+
import org.junit.jupiter.api.Test;
20+
import org.sonar.plugins.java.api.tree.ClassTree;
21+
import org.sonar.plugins.java.api.tree.CompilationUnitTree;
22+
23+
import static org.junit.jupiter.api.Assertions.assertEquals;
24+
25+
class DeprecatedCheckerHelperTest {
26+
27+
@Test
28+
void getAnnotationAttributeValue() {
29+
assertValue("@Deprecated(forRemoval = true)", "forRemoval", true, Boolean.class);
30+
assertValue("@Deprecated(forRemoval = false)", "forRemoval", false, Boolean.class);
31+
assertValue("@Deprecated(value = \"test\")", "value", "test", String.class);
32+
assertValue("@Deprecated(since = 42)", "since", 42, Integer.class);
33+
assertValue("@Deprecated(\"Descr\")", "value", "Descr", String.class);
34+
}
35+
36+
private <T> void assertValue(String annotationSourceCode, String attributeName, T expectedValue, Class<T> type) {
37+
var classTree = parseClass(annotationSourceCode + " class A {}");
38+
var annotation = DeprecatedCheckerHelper.deprecatedAnnotation(classTree);
39+
T value = DeprecatedCheckerHelper.getAnnotationAttributeValue(annotation, attributeName, type).orElse(null);
40+
assertEquals(expectedValue, value);
41+
}
42+
43+
private ClassTree parseClass(String code) {
44+
CompilationUnitTree compilationUnitTree = JParserTestUtils.parse(code);
45+
return (ClassTree) compilationUnitTree.types().get(0);
46+
}
47+
48+
}

‎java-frontend/src/main/java/org/sonar/java/model/ExpressionUtils.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,4 +390,13 @@ private static Object resolveOr(BinaryExpressionTree binaryExpression) {
390390
return null;
391391
}
392392

393+
public static String annotationAttributeName(ExpressionTree expression) {
394+
if (expression.is(Tree.Kind.ASSIGNMENT)) {
395+
AssignmentExpressionTree assignment = (AssignmentExpressionTree) expression;
396+
// assignment.variable() in annotation is always a Tree.Kind.IDENTIFIER
397+
return ((IdentifierTree) assignment.variable()).name();
398+
}
399+
return "value";
400+
}
401+
393402
}

‎java-frontend/src/test/java/org/sonar/java/model/ExpressionUtilsTest.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import static java.lang.reflect.Modifier.isFinal;
4545
import static java.lang.reflect.Modifier.isPrivate;
4646
import static org.assertj.core.api.Assertions.assertThat;
47+
import static org.junit.jupiter.api.Assertions.assertEquals;
4748
import static org.junit.jupiter.api.Assertions.assertThrows;
4849
import static org.sonar.java.model.ExpressionUtils.isInvocationOnVariable;
4950
import static org.sonar.java.model.ExpressionUtils.skipParenthesesUpwards;
@@ -469,4 +470,22 @@ void areVariablesSame_unknown_symbol() {
469470
assertThat(ExpressionUtils.areVariablesSame(initializer.falseExpression(), condition.rightOperand(), false)).isFalse();
470471
}
471472

473+
@Test
474+
void testAnnotationAttributeName(){
475+
var unit = JParserTestUtils.parse("""
476+
interface A {
477+
@Deprecated(forRemoval = true)
478+
void m();
479+
@MyAnnotation("noArgName")
480+
void m2();
481+
}""");
482+
var classTree = (ClassTree) unit.types().get(0);
483+
var methodTree = (MethodTree) classTree.members().get(0);
484+
var annotation = methodTree.modifiers().annotations().get(0).arguments().get(0);
485+
assertEquals("forRemoval", ExpressionUtils.annotationAttributeName(annotation));
486+
methodTree = (MethodTree) classTree.members().get(1);
487+
annotation = methodTree.modifiers().annotations().get(0).arguments().get(0);
488+
assertEquals("value", ExpressionUtils.annotationAttributeName(annotation));
489+
}
490+
472491
}

0 commit comments

Comments
 (0)
X Tutup