Skip to content

Commit 3e95ca4

Browse files
irina-batinic-sonarsourceirina-batinic-sonarsource
authored
SONARJAVA-4645 Add implementation of S6806 (#4487)
1 parent 4924e89 commit 3e95ca4

9 files changed

Lines changed: 272 additions & 2 deletions

File tree

‎its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,15 +180,15 @@ public void javaCheckTestSources() throws Exception {
180180
softly.assertThat(newTotal).isEqualTo(knownTotal);
181181
softly.assertThat(rulesCausingFPs).hasSize(6);
182182
softly.assertThat(rulesNotReporting).hasSize(7);
183-
softly.assertThat(rulesSilenced).hasSize(71);
183+
softly.assertThat(rulesSilenced).hasSize(72);
184184

185185
/**
186186
* 4. Check total number of differences (FPs + FNs)
187187
*
188188
* No differences would mean that we find the same issues with and without the bytecode and libraries
189189
*/
190190
String differences = Files.readString(pathFor(TARGET_ACTUAL + PROJECT_KEY + "-no-binaries_differences"));
191-
softly.assertThat(differences).isEqualTo("Issues differences: 3451");
191+
softly.assertThat(differences).isEqualTo("Issues differences: 3461");
192192

193193
softly.assertAll();
194194
}

‎its/ruling/src/test/resources/autoscan/autoscan-diff-by-rules.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2879,6 +2879,12 @@
28792879
"falseNegatives": 94,
28802880
"falsePositives": 0
28812881
},
2882+
{
2883+
"ruleKey": "S6806",
2884+
"hasTruePositives": false,
2885+
"falseNegatives": 10,
2886+
"falsePositives": 0
2887+
},
28822888
{
28832889
"ruleKey": "S6810",
28842890
"hasTruePositives": false,

‎java-checks-test-sources/src/main/java/checks/spring/ModelAttributeNamingConventionForSpELCheck.java‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
package checks.spring;
2+
3+
import java.io.File;
4+
import java.util.HashMap;
5+
import java.util.Map;
6+
7+
class ModelAttributeNamingConventionForSpELCheck {
8+
9+
private static final String MY_LEGAL_CONSTANT = "legalName";
10+
private static final String MY_ILLEGAL_CONSTANT = "a-b";
11+
12+
public void foo(org.springframework.ui.Model model) {
13+
model.addAllAttributes(
14+
Map.of(" m", 42, // Noncompliant
15+
" a", 22)); // Noncompliant
16+
17+
model.addAllAttributes(Map.of(MY_ILLEGAL_CONSTANT, 42)); // Noncompliant
18+
19+
model.addAttribute(File.separator, 42); // Compliant - can not resolve
20+
model.addAttribute(MY_LEGAL_CONSTANT, 0); // Compliant
21+
model.addAttribute(MY_ILLEGAL_CONSTANT, 0); // Noncompliant
22+
23+
model.addAllAttributes(Map.of("m", 42, "a", 22)); // Compliant
24+
model.addAllAttributes(getMap()); // Compliant
25+
26+
model.addAllAttributes(Map.of("m", 42, " a", 22)); // Noncompliant
27+
28+
model.addAllAttributes(Map.ofEntries(Map.entry("m", 42), Map.entry("a", 22))); // Compliant
29+
model.addAllAttributes(getMap()); // Compliant
30+
31+
model.addAllAttributes(Map.ofEntries(Map.entry(" m", 42), Map.entry(" a", 22))); // Noncompliant
32+
33+
model.addAttribute("", 5); // Noncompliant
34+
model.addAttribute(" a", ""); // Noncompliant [[sc=24;ec=28]] {{Attribute names must begin with a letter (a-z, A-Z), underscore (_), or dollar sign ($) and can be
35+
// followed by letters, digits, underscores, or dollar signs.}}
36+
model.addAttribute("a-b", ""); // Noncompliant
37+
model.addAttribute("1c", 42); // Noncompliant
38+
39+
model.addAttribute("a", 100); // Compliant
40+
model.addAttribute("b", 42); // Compliant
41+
model.addAttribute("_c", 7); // Compliant
42+
model.addAttribute("$d", 8); // Compliant
43+
44+
model.addAllAttributes(new HashMap<>()); // Compliant - test coverage
45+
}
46+
47+
private Map getMap() {
48+
return Map.of("one", "two");
49+
}
50+
51+
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@
138138
import org.sonar.java.checks.spring.AsyncMethodsCalledViaThisCheck;
139139
import org.sonar.java.checks.spring.AsyncMethodsReturnTypeCheck;
140140
import org.sonar.java.checks.spring.ControllerWithSessionAttributesCheck;
141+
import org.sonar.java.checks.spring.ModelAttributeNamingConventionForSpELCheck;
141142
import org.sonar.java.checks.spring.FieldDependencyInjectionCheck;
142143
import org.sonar.java.checks.spring.OptionalRestParametersShouldBeObjectsCheck;
143144
import org.sonar.java.checks.spring.PersistentEntityUsedAsRequestParameterCheck;
@@ -492,6 +493,7 @@ public final class CheckList {
492493
MissingCurlyBracesCheck.class,
493494
MissingDeprecatedCheck.class,
494495
MissingOverridesInRecordWithArrayComponentCheck.class,
496+
ModelAttributeNamingConventionForSpELCheck.class,
495497
ModifiersOrderCheck.class,
496498
ModulusEqualityCheck.class,
497499
MultipleWhitespaceCheck.class,

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

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
/*
2+
* SonarQube Java
3+
* Copyright (C) 2012-2023 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 GNU Lesser General Public
8+
* License as published by the Free Software Foundation; either
9+
* version 3 of the License, or (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
14+
* Lesser General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU Lesser General Public License
17+
* along with this program; if not, write to the Free Software Foundation,
18+
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
19+
*/
20+
package org.sonar.java.checks.spring;
21+
22+
import java.util.regex.Matcher;
23+
import java.util.regex.Pattern;
24+
import org.sonar.check.Rule;
25+
import org.sonar.java.checks.methods.AbstractMethodDetection;
26+
import org.sonar.java.model.LiteralUtils;
27+
import org.sonar.java.model.declaration.VariableTreeImpl;
28+
import org.sonar.plugins.java.api.semantic.MethodMatchers;
29+
import org.sonar.plugins.java.api.tree.ExpressionTree;
30+
import org.sonar.plugins.java.api.tree.IdentifierTree;
31+
import org.sonar.plugins.java.api.tree.LiteralTree;
32+
import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree;
33+
import org.sonar.plugins.java.api.tree.MethodInvocationTree;
34+
import org.sonar.plugins.java.api.tree.Tree;
35+
36+
@Rule(key = "S6806")
37+
public class ModelAttributeNamingConventionForSpELCheck extends AbstractMethodDetection {
38+
39+
private static final Pattern pattern = Pattern.compile("^[a-zA-Z_$][a-zA-Z0-9_$]*$");
40+
41+
private static final MethodMatchers ADD_ATTRIBUTE_MATCHER_WITH_TWO_PARAMS = MethodMatchers.create()
42+
.ofTypes("org.springframework.ui.Model")
43+
.names("addAttribute")
44+
.addParametersMatcher("java.lang.String", "java.lang.Object")
45+
.build();
46+
47+
private static final MethodMatchers ADD_ATTRIBUTE_MATCHER_WITH_ONE_PARAM = MethodMatchers.create()
48+
.ofTypes("org.springframework.ui.Model")
49+
.names("addAllAttributes")
50+
.addParametersMatcher("java.util.Map")
51+
.build();
52+
53+
private static final MethodMatchers MAP_OF = MethodMatchers.create()
54+
.ofTypes("java.util.Map")
55+
.names("of", "ofEntries", "entry")
56+
.withAnyParameters()
57+
.build();
58+
59+
@Override
60+
protected MethodMatchers getMethodInvocationMatchers() {
61+
return MethodMatchers.or(ADD_ATTRIBUTE_MATCHER_WITH_TWO_PARAMS, ADD_ATTRIBUTE_MATCHER_WITH_ONE_PARAM);
62+
}
63+
64+
@Override
65+
protected void onMethodInvocationFound(MethodInvocationTree mit) {
66+
ExpressionTree argumentTree = mit.arguments().get(0);
67+
checkExpression(argumentTree, argumentTree);
68+
}
69+
70+
private void checkExpression(ExpressionTree argumentTree, ExpressionTree reportTree) {
71+
if (argumentTree.is(Tree.Kind.STRING_LITERAL)) {
72+
checkStringLiteralAndReport(argumentTree, reportTree);
73+
} else if (argumentTree.is(Tree.Kind.IDENTIFIER)) {
74+
checkIdentifier((IdentifierTree) argumentTree);
75+
} else if (argumentTree.is(Tree.Kind.MEMBER_SELECT)) {
76+
checkMemberSelect((MemberSelectExpressionTree) argumentTree);
77+
} else if (argumentTree.is(Tree.Kind.METHOD_INVOCATION)) {
78+
checkMethodInvocation((MethodInvocationTree) argumentTree);
79+
}
80+
}
81+
82+
private void checkStringLiteralAndReport(ExpressionTree tree, ExpressionTree reportTree) {
83+
LiteralTree literalTree = (LiteralTree) tree;
84+
String literalValue = LiteralUtils.getAsStringValue(literalTree);
85+
Matcher matcher = pattern.matcher(literalValue);
86+
if (!matcher.matches()) {
87+
reportIssue(reportTree,
88+
"Attribute names must begin with a letter (a-z, A-Z), underscore (_), or dollar sign ($) and can be followed by letters, digits, underscores, or dollar signs.");
89+
}
90+
}
91+
92+
private void checkIdentifier(IdentifierTree identifierTree) {
93+
VariableTreeImpl declaration = (VariableTreeImpl) identifierTree.symbol().declaration();
94+
if (declaration != null) {
95+
checkExpression(declaration.initializer(), identifierTree);
96+
}
97+
}
98+
99+
private void checkMemberSelect(MemberSelectExpressionTree memberSelectExpressionTree) {
100+
checkIdentifier(memberSelectExpressionTree.identifier());
101+
}
102+
103+
private void checkMethodInvocation(MethodInvocationTree methodInvocationTree) {
104+
if (MAP_OF.matches(methodInvocationTree)) {
105+
for (int i = 0; i < methodInvocationTree.arguments().size(); i += 2) {
106+
ExpressionTree key = methodInvocationTree.arguments().get(i);
107+
checkExpression(key, key);
108+
}
109+
}
110+
}
111+
112+
}

‎java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6806.html‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<h2>Why is this an issue?</h2>
2+
<p>Spring Expression Language (SpEL) is an expression language used in the Spring Framework for evaluating and manipulating objects, properties, and
3+
conditions within Spring-based applications.</p>
4+
<p><code>org.springframework.ui.Model</code> is an interface in the Spring Framework that represents a container for data that can be passed between a
5+
controller and a view in a Spring MVC web application, allowing for data sharing during the request-response cycle.</p>
6+
<p>Attributes added to the <code>org.springframework.ui.Model</code> should follow the Java identifier naming convention, which means they must start
7+
with a letter <code>a-z, A-Z</code>, underscore <code>_</code>, or a dollar sign <code>$</code> and may be followed by letters, digits, underscores,
8+
or dollar signs.</p>
9+
<p>Failure to do so may result in SpEL parsing errors when using these attributes in template engines.</p>
10+
<h2>How to fix it</h2>
11+
<p>Follow the Java identifier naming convention.</p>
12+
<h3>Code examples</h3>
13+
<h4>Noncompliant code example</h4>
14+
<pre data-diff-id="1" data-diff-type="noncompliant">
15+
model.addAttribute(" a", 100); // Noncompliant (starts with a space)
16+
model.addAttribute("a-b", 7); // Noncompliant (contains a hyphen)
17+
model.addAttribute("1c", 42); // Noncompliant (starts with a digit)
18+
</pre>
19+
<h4>Compliant solution</h4>
20+
<pre data-diff-id="1" data-diff-type="compliant">
21+
model.addAttribute("a", 100);
22+
model.addAttribute("b", 42);
23+
model.addAttribute("_c", 7);
24+
model.addAttribute("$d", 8);
25+
</pre>
26+
<h2>Resources</h2>
27+
<h3>Documentation</h3>
28+
<ul>
29+
<li> <a href="https://www.oracle.com/java/technologies/javase/codeconventions-namingconventions.html">Java SE - naming conventions</a> </li>
30+
<li> <a href="https://docs.spring.io/spring-framework/reference/core/expressions.html">Spring Expression Language (SpEL)</a> </li>
31+
<li> <a href="https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/ui/Model.html">Spring IO Docs - Interface
32+
Model</a> </li>
33+
</ul>
34+

‎java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6806.json‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
{
2+
"title": "Model attribute naming convention for Spring Expression Language (SpEL)",
3+
"type": "BUG",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "5min"
8+
},
9+
"tags": [],
10+
"defaultSeverity": "Major",
11+
"ruleSpecification": "RSPEC-6806",
12+
"sqKey": "S6806",
13+
"scope": "Main",
14+
"quickfix": "unknown",
15+
"code": {
16+
"impacts": {
17+
"MAINTAINABILITY": "MEDIUM",
18+
"RELIABILITY": "MEDIUM"
19+
},
20+
"attribute": "LOGICAL"
21+
}
22+
}

‎java-checks/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,7 @@
480480
"S6539",
481481
"S6541",
482482
"S6548",
483+
"S6806",
483484
"S6809",
484485
"S6810",
485486
"S6813",

‎java-checks/src/test/java/org/sonar/java/checks/spring/ModelAttributeNamingConventionForSpELCheckTest.java‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/*
2+
* SonarQube Java
3+
* Copyright (C) 2012-2023 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 GNU Lesser General Public
8+
* License as published by the Free Software Foundation; either
9+
* version 3 of the License, or (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
14+
* Lesser General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU Lesser General Public License
17+
* along with this program; if not, write to the Free Software Foundation,
18+
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
19+
*/
20+
package org.sonar.java.checks.spring;
21+
22+
import org.junit.jupiter.api.Test;
23+
import org.sonar.java.checks.verifier.CheckVerifier;
24+
25+
import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath;
26+
27+
class ModelAttributeNamingConventionForSpELCheckTest {
28+
29+
@Test
30+
void test() {
31+
CheckVerifier.newVerifier()
32+
.onFile(mainCodeSourcesPath("checks/spring/ModelAttributeNamingConventionForSpELCheck.java"))
33+
.withCheck(new ModelAttributeNamingConventionForSpELCheck())
34+
.verifyIssues();
35+
CheckVerifier.newVerifier()
36+
.onFile(mainCodeSourcesPath("checks/spring/ModelAttributeNamingConventionForSpELCheck.java"))
37+
.withCheck(new ModelAttributeNamingConventionForSpELCheck())
38+
.withoutSemantic()
39+
.verifyNoIssues();
40+
}
41+
42+
}

0 commit comments

Comments
 (0)