diff --git a/java-checks-test-sources/src/main/java/checks/spring/OptionalRestParametersShouldBeObjects.java b/java-checks-test-sources/src/main/java/checks/spring/OptionalRestParametersShouldBeObjects.java index fe67c849265..9e92b8aa737 100644 --- a/java-checks-test-sources/src/main/java/checks/spring/OptionalRestParametersShouldBeObjects.java +++ b/java-checks-test-sources/src/main/java/checks/spring/OptionalRestParametersShouldBeObjects.java @@ -56,11 +56,26 @@ public Article getArticleButSettingADifferentValue(@PathVariable(name = "article return new Article(articleId); } + @GetMapping(value = "{/article/id") + public Article getArticleRequestParamWithDefaultValue(@RequestParam(required = false, defaultValue = "1") int articleId) { // Compliant + return new Article(articleId); + } + @GetMapping(value = {"/article/{id}"}) public Article getArticleButDifferentlyAnnotated(@Nullable int articleId) { // Compliant return new Article(articleId); } + @GetMapping(value = {"/article/{id}"}) + public Article getArticlePathVariableWithValue(@PathVariable("id") int articleId) { // Compliant + return new Article(articleId); + } + + @GetMapping(value = {"/article/{id}"}) + public Article getArticleRequestParamWithValue(@RequestParam("id") int articleId) { // Compliant + return new Article(articleId); + } + record Article(int id) { } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/OptionalRestParametersShouldBeObjectsCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/OptionalRestParametersShouldBeObjectsCheck.java index 278c500f130..048d2477cdd 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/OptionalRestParametersShouldBeObjectsCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/OptionalRestParametersShouldBeObjectsCheck.java @@ -20,6 +20,7 @@ package org.sonar.java.checks.spring; import java.util.List; +import java.util.stream.Stream; import org.sonar.check.Rule; import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; import org.sonar.plugins.java.api.tree.AnnotationTree; @@ -31,10 +32,11 @@ @Rule(key = "S6814") public class OptionalRestParametersShouldBeObjectsCheck extends IssuableSubscriptionVisitor { - + private static final String PATH_VARIABLE_ANNOTATION = "org.springframework.web.bind.annotation.PathVariable"; + private static final String REQUEST_PARAM_ANNOTATION = "org.springframework.web.bind.annotation.RequestParam"; private static final List PARAMETER_ANNOTATIONS = List.of( - "org.springframework.web.bind.annotation.PathVariable", - "org.springframework.web.bind.annotation.RequestParam" + PATH_VARIABLE_ANNOTATION, + REQUEST_PARAM_ANNOTATION ); @Override @@ -53,17 +55,29 @@ public void visitNode(Tree tree) { private static boolean isOptionalPrimitive(VariableTree parameter) { return parameter.type().symbolType().isPrimitive() && parameter.modifiers().annotations().stream() - .anyMatch(OptionalRestParametersShouldBeObjectsCheck::isMarkingAsOptional); + .anyMatch(annotation -> isMarkingAsOptional(annotation) && !hasDefaultValue(annotation)); } private static boolean isMarkingAsOptional(AnnotationTree annotation) { return PARAMETER_ANNOTATIONS.stream().anyMatch(candidate -> annotation.annotationType().symbolType().is(candidate)) && - annotation.arguments().stream().anyMatch(expressionTree -> { - // Because all parameters of the supported annotations are assignments, we can cast safely here. - AssignmentExpressionTree assignment = (AssignmentExpressionTree) expressionTree; + streamAllNamedArguments(annotation).anyMatch(assignment -> { IdentifierTree variable = (IdentifierTree) assignment.variable(); Boolean constant = assignment.expression().asConstant(Boolean.class).orElse(Boolean.TRUE); return "required".equals(variable.name()) && Boolean.FALSE.equals(constant); }); } + + private static boolean hasDefaultValue(AnnotationTree annotation) { + return annotation.annotationType().symbolType().is(REQUEST_PARAM_ANNOTATION) && + streamAllNamedArguments(annotation).anyMatch(assignment -> { + IdentifierTree variable = (IdentifierTree) assignment.variable(); + return "defaultValue".equals(variable.name()); + }); + } + + private static Stream streamAllNamedArguments(AnnotationTree annotation) { + return annotation.arguments().stream() + .filter(expression -> expression.is(Tree.Kind.ASSIGNMENT)) + .map(AssignmentExpressionTree.class::cast); + } }