SONARJAVA-4645 Add implementation of S6806 (#4487)

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

@@ -180,15 +180,15 @@ public void javaCheckTestSources() throws Exception {
     softly.assertThat(newTotal).isEqualTo(knownTotal);
     softly.assertThat(rulesCausingFPs).hasSize(6);
     softly.assertThat(rulesNotReporting).hasSize(7);
-    softly.assertThat(rulesSilenced).hasSize(71);
+    softly.assertThat(rulesSilenced).hasSize(72);
 
     /**
      * 4. Check total number of differences (FPs + FNs)
      *
      * No differences would mean that we find the same issues with and without the bytecode and libraries
      */
     String differences = Files.readString(pathFor(TARGET_ACTUAL + PROJECT_KEY + "-no-binaries_differences"));
-    softly.assertThat(differences).isEqualTo("Issues differences: 3451");
+    softly.assertThat(differences).isEqualTo("Issues differences: 3461");
 
     softly.assertAll();
   }

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

@@ -2879,6 +2879,12 @@
     "falseNegatives": 94,
     "falsePositives": 0
   },
+  {
+    "ruleKey": "S6806",
+    "hasTruePositives": false,
+    "falseNegatives": 10,
+    "falsePositives": 0
+  },
   {
     "ruleKey": "S6810",
     "hasTruePositives": false,

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

@@ -0,0 +1,51 @@
+package checks.spring;
+
+import java.io.File;
+import java.util.HashMap;
+import java.util.Map;
+
+class ModelAttributeNamingConventionForSpELCheck {
+
+  private static final String MY_LEGAL_CONSTANT = "legalName";
+  private static final String MY_ILLEGAL_CONSTANT = "a-b";
+
+  public void foo(org.springframework.ui.Model model) {
+    model.addAllAttributes(
+      Map.of(" m", 42, // Noncompliant
+        " a", 22)); // Noncompliant
+
+    model.addAllAttributes(Map.of(MY_ILLEGAL_CONSTANT, 42)); // Noncompliant
+
+    model.addAttribute(File.separator, 42); // Compliant - can not resolve
+    model.addAttribute(MY_LEGAL_CONSTANT, 0); // Compliant
+    model.addAttribute(MY_ILLEGAL_CONSTANT, 0); // Noncompliant
+
+    model.addAllAttributes(Map.of("m", 42, "a", 22)); // Compliant
+    model.addAllAttributes(getMap()); // Compliant
+
+    model.addAllAttributes(Map.of("m", 42, " a", 22)); // Noncompliant
+
+    model.addAllAttributes(Map.ofEntries(Map.entry("m", 42), Map.entry("a", 22))); // Compliant
+    model.addAllAttributes(getMap()); // Compliant
+
+    model.addAllAttributes(Map.ofEntries(Map.entry(" m", 42), Map.entry(" a", 22))); // Noncompliant
+
+    model.addAttribute("", 5); // Noncompliant
+    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
+                                  // followed by letters, digits, underscores, or dollar signs.}}
+    model.addAttribute("a-b", ""); // Noncompliant
+    model.addAttribute("1c", 42); // Noncompliant
+
+    model.addAttribute("a", 100); // Compliant
+    model.addAttribute("b", 42); // Compliant
+    model.addAttribute("_c", 7); // Compliant
+    model.addAttribute("$d", 8); // Compliant
+
+    model.addAllAttributes(new HashMap<>()); // Compliant - test coverage
+  }
+
+  private Map getMap() {
+    return Map.of("one", "two");
+  }
+
+}

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

@@ -138,6 +138,7 @@
 import org.sonar.java.checks.spring.AsyncMethodsCalledViaThisCheck;
 import org.sonar.java.checks.spring.AsyncMethodsReturnTypeCheck;
 import org.sonar.java.checks.spring.ControllerWithSessionAttributesCheck;
+import org.sonar.java.checks.spring.ModelAttributeNamingConventionForSpELCheck;
 import org.sonar.java.checks.spring.FieldDependencyInjectionCheck;
 import org.sonar.java.checks.spring.OptionalRestParametersShouldBeObjectsCheck;
 import org.sonar.java.checks.spring.PersistentEntityUsedAsRequestParameterCheck;
@@ -492,6 +493,7 @@ public final class CheckList {
     MissingCurlyBracesCheck.class,
     MissingDeprecatedCheck.class,
     MissingOverridesInRecordWithArrayComponentCheck.class,
+    ModelAttributeNamingConventionForSpELCheck.class,
     ModifiersOrderCheck.class,
     ModulusEqualityCheck.class,
     MultipleWhitespaceCheck.class,

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

@@ -0,0 +1,112 @@
+/*
+ * SonarQube Java
+ * Copyright (C) 2012-2023 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.java.checks.spring;
+
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import org.sonar.check.Rule;
+import org.sonar.java.checks.methods.AbstractMethodDetection;
+import org.sonar.java.model.LiteralUtils;
+import org.sonar.java.model.declaration.VariableTreeImpl;
+import org.sonar.plugins.java.api.semantic.MethodMatchers;
+import org.sonar.plugins.java.api.tree.ExpressionTree;
+import org.sonar.plugins.java.api.tree.IdentifierTree;
+import org.sonar.plugins.java.api.tree.LiteralTree;
+import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree;
+import org.sonar.plugins.java.api.tree.MethodInvocationTree;
+import org.sonar.plugins.java.api.tree.Tree;
+
+@Rule(key = "S6806")
+public class ModelAttributeNamingConventionForSpELCheck extends AbstractMethodDetection {
+
+  private static final Pattern pattern = Pattern.compile("^[a-zA-Z_$][a-zA-Z0-9_$]*$");
+
+  private static final MethodMatchers ADD_ATTRIBUTE_MATCHER_WITH_TWO_PARAMS = MethodMatchers.create()
+    .ofTypes("org.springframework.ui.Model")
+    .names("addAttribute")
+    .addParametersMatcher("java.lang.String", "java.lang.Object")
+    .build();
+
+  private static final MethodMatchers ADD_ATTRIBUTE_MATCHER_WITH_ONE_PARAM = MethodMatchers.create()
+    .ofTypes("org.springframework.ui.Model")
+    .names("addAllAttributes")
+    .addParametersMatcher("java.util.Map")
+    .build();
+
+  private static final MethodMatchers MAP_OF = MethodMatchers.create()
+    .ofTypes("java.util.Map")
+    .names("of", "ofEntries", "entry")
+    .withAnyParameters()
+    .build();
+
+  @Override
+  protected MethodMatchers getMethodInvocationMatchers() {
+    return MethodMatchers.or(ADD_ATTRIBUTE_MATCHER_WITH_TWO_PARAMS, ADD_ATTRIBUTE_MATCHER_WITH_ONE_PARAM);
+  }
+
+  @Override
+  protected void onMethodInvocationFound(MethodInvocationTree mit) {
+    ExpressionTree argumentTree = mit.arguments().get(0);
+    checkExpression(argumentTree, argumentTree);
+  }
+
+  private void checkExpression(ExpressionTree argumentTree, ExpressionTree reportTree) {
+    if (argumentTree.is(Tree.Kind.STRING_LITERAL)) {
+      checkStringLiteralAndReport(argumentTree, reportTree);
+    } else if (argumentTree.is(Tree.Kind.IDENTIFIER)) {
+      checkIdentifier((IdentifierTree) argumentTree);
+    } else if (argumentTree.is(Tree.Kind.MEMBER_SELECT)) {
+      checkMemberSelect((MemberSelectExpressionTree) argumentTree);
+    } else if (argumentTree.is(Tree.Kind.METHOD_INVOCATION)) {
+      checkMethodInvocation((MethodInvocationTree) argumentTree);
+    }
+  }
+
+  private void checkStringLiteralAndReport(ExpressionTree tree, ExpressionTree reportTree) {
+    LiteralTree literalTree = (LiteralTree) tree;
+    String literalValue = LiteralUtils.getAsStringValue(literalTree);
+    Matcher matcher = pattern.matcher(literalValue);
+    if (!matcher.matches()) {
+      reportIssue(reportTree,
+        "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.");
+    }
+  }
+
+  private void checkIdentifier(IdentifierTree identifierTree) {
+    VariableTreeImpl declaration = (VariableTreeImpl) identifierTree.symbol().declaration();
+    if (declaration != null) {
+      checkExpression(declaration.initializer(), identifierTree);
+    }
+  }
+
+  private void checkMemberSelect(MemberSelectExpressionTree memberSelectExpressionTree) {
+    checkIdentifier(memberSelectExpressionTree.identifier());
+  }
+
+  private void checkMethodInvocation(MethodInvocationTree methodInvocationTree) {
+    if (MAP_OF.matches(methodInvocationTree)) {
+      for (int i = 0; i < methodInvocationTree.arguments().size(); i += 2) {
+        ExpressionTree key = methodInvocationTree.arguments().get(i);
+        checkExpression(key, key);
+      }
+    }
+  }
+
+}

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

@@ -0,0 +1,34 @@
+<h2>Why is this an issue?</h2>
+<p>Spring Expression Language (SpEL) is an expression language used in the Spring Framework for evaluating and manipulating objects, properties, and
+conditions within Spring-based applications.</p>
+<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
+controller and a view in a Spring MVC web application, allowing for data sharing during the request-response cycle.</p>
+<p>Attributes added to the <code>org.springframework.ui.Model</code> should follow the Java identifier naming convention, which means they must start
+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,
+or dollar signs.</p>
+<p>Failure to do so may result in SpEL parsing errors when using these attributes in template engines.</p>
+<h2>How to fix it</h2>
+<p>Follow the Java identifier naming convention.</p>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+model.addAttribute(" a", 100); // Noncompliant (starts with a space)
+model.addAttribute("a-b", 7);  // Noncompliant (contains a hyphen)
+model.addAttribute("1c", 42);  // Noncompliant (starts with a digit)
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+model.addAttribute("a", 100);
+model.addAttribute("b", 42);
+model.addAttribute("_c", 7);
+model.addAttribute("$d", 8);
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li> <a href="https://www.oracle.com/java/technologies/javase/codeconventions-namingconventions.html">Java SE - naming conventions</a> </li>
+  <li> <a href="https://docs.spring.io/spring-framework/reference/core/expressions.html">Spring Expression Language (SpEL)</a> </li>
+  <li> <a href="https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/ui/Model.html">Spring IO Docs - Interface
+  Model</a> </li>
+</ul>
+

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

@@ -0,0 +1,22 @@
+{
+  "title": "Model attribute naming convention for Spring Expression Language (SpEL)",
+  "type": "BUG",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-6806",
+  "sqKey": "S6806",
+  "scope": "Main",
+  "quickfix": "unknown",
+  "code": {
+    "impacts": {
+      "MAINTAINABILITY": "MEDIUM",
+      "RELIABILITY": "MEDIUM"
+    },
+    "attribute": "LOGICAL"
+  }
+}

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

@@ -480,6 +480,7 @@
     "S6539",
     "S6541",
     "S6548",
+    "S6806",
     "S6809",
     "S6810",
     "S6813",

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

@@ -0,0 +1,42 @@
+/*
+ * SonarQube Java
+ * Copyright (C) 2012-2023 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.java.checks.spring;
+
+import org.junit.jupiter.api.Test;
+import org.sonar.java.checks.verifier.CheckVerifier;
+
+import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath;
+
+class ModelAttributeNamingConventionForSpELCheckTest {
+
+  @Test
+  void test() {
+    CheckVerifier.newVerifier()
+      .onFile(mainCodeSourcesPath("checks/spring/ModelAttributeNamingConventionForSpELCheck.java"))
+      .withCheck(new ModelAttributeNamingConventionForSpELCheck())
+      .verifyIssues();
+    CheckVerifier.newVerifier()
+      .onFile(mainCodeSourcesPath("checks/spring/ModelAttributeNamingConventionForSpELCheck.java"))
+      .withCheck(new ModelAttributeNamingConventionForSpELCheck())
+      .withoutSemantic()
+      .verifyNoIssues();
+  }
+
+}