Improve NavigatingTreeCouldBeReferenceCheck #119

esql-checks/pom.xml

@@ -61,5 +61,4 @@
       </repository>
     </repositories>
 
-
 </project>

esql-checks/src/main/java/com/exxeta/iss/sonar/esql/check/NavigatingTreeCouldBeReferenceCheck.java

@@ -45,12 +45,19 @@ public class NavigatingTreeCouldBeReferenceCheck extends DoubleDispatchVisitorCh
 
     private static final String MESSAGE = "Navigating message tree could be replaced by a reference.";
 
-    private static final int DEFAULT_THRESHOLD = 3;
+    private static final int DEFAULT_ALLOWED_NUMBER_OF_FIELD_REFERENCE_USES = 2;
     @RuleProperty(
-            key = "NavigatingTreeCouldBeReference",
-            description = "Navigating message tree could be replaced by a reference.",
-            defaultValue = "" + DEFAULT_THRESHOLD)
-    public int threshold = DEFAULT_THRESHOLD;
+            key = "AllowedNumberOfReferences",
+            description = "The maximum allowed number of reused field references. Issues will only created if a field reference is used more often.",
+            defaultValue = "" + DEFAULT_ALLOWED_NUMBER_OF_FIELD_REFERENCE_USES)
+    public int allowedNumberOfFieldReferenceUses = DEFAULT_ALLOWED_NUMBER_OF_FIELD_REFERENCE_USES;
+
+    private static final int DEFAULT_MAXIMUM_ALLOWED_PATH_ELEMENTS = 2;
+    @RuleProperty(
+            key = "MaximumAllowedPathElements",
+            description = "The number of allowed of path elements in a field reference for which no reference will be suggested. Issues will only be created for field references with more path elements.",
+            defaultValue = "" + DEFAULT_MAXIMUM_ALLOWED_PATH_ELEMENTS)
+    public int maximumAllowedPathElements = DEFAULT_MAXIMUM_ALLOWED_PATH_ELEMENTS;
 
     private final Multimap<String, FieldReferenceTree> occurrences = ArrayListMultimap.create();
 
@@ -77,14 +84,15 @@ public void visitProgram(ProgramTree tree) {
         super.visitProgram(tree);
 
         List<String> sortedKeys = occurrences.keySet().stream()
+                .filter(a -> StringUtils.countMatches(a, ".") + 1 > maximumAllowedPathElements)
                 .sorted((k1, k2) -> StringUtils.countMatches(k2, ".") - StringUtils.countMatches(k1, "."))
                 .collect(Collectors.toList());
 
         ArrayList<String> issueKeys = new ArrayList<>();
 
         for (String entry : sortedKeys) {
             Collection<FieldReferenceTree> fieldReferenceTrees = occurrences.get(entry);
-            if (fieldReferenceTrees.size() > threshold) {
+            if (fieldReferenceTrees.size() > allowedNumberOfFieldReferenceUses) {
                 boolean alreadyIssued = false;
                 for (String issueKey : issueKeys) {
                     if (issueKey.startsWith(entry + ".")) {

esql-checks/src/test/java/com/exxeta/iss/sonar/esql/check/NavigatingTreeCouldBeReferenceCheckTest.java

@@ -6,38 +6,47 @@
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
- * 
+ *
  *     http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
 package com.exxeta.iss.sonar.esql.check;
-import java.io.File;
 
+import com.exxeta.iss.sonar.esql.checks.verifier.EsqlCheckVerifier;
 import org.junit.Test;
 
-import com.exxeta.iss.sonar.esql.api.EsqlCheck;
-import com.exxeta.iss.sonar.esql.checks.verifier.EsqlCheckVerifier;
+import java.io.File;
+
 /**
- * This java  class is  the test class for NavigatingTreeCouldBeReferenceCheck.java. 
- * @author Sapna. singh
+ * This java  class is  the test class for NavigatingTreeCouldBeReferenceCheck.java.
  *
+ * @author Sapna. singh
  */
 public class NavigatingTreeCouldBeReferenceCheckTest {
 
-
-     @Test
-		public void test() {
-			EsqlCheck check = new NavigatingTreeCouldBeReferenceCheck();
-			EsqlCheckVerifier.issues(check, new File("src/test/resources/NavigatingTreeCouldBeReference.esql"))
-					.next().atLine(4).withMessage("Navigating message tree could be replaced by a reference.")
-					.next().atLine(16).withMessage("Navigating message tree could be replaced by a reference.")
-					.next().atLine(16).withMessage("Navigating message tree could be replaced by a reference.")
-			.noMore();
-		}
+
+    @Test
+    public void test() {
+        NavigatingTreeCouldBeReferenceCheck check = new NavigatingTreeCouldBeReferenceCheck();
+        EsqlCheckVerifier.issues(check, new File("src/test/resources/NavigatingTreeCouldBeReference.esql"))
+                .next().atLine(4).withMessage("Navigating message tree could be replaced by a reference.")
+                .next().atLine(16).withMessage("Navigating message tree could be replaced by a reference.")
+                .next().atLine(16).withMessage("Navigating message tree could be replaced by a reference.")
+                .noMore();
+        check.maximumAllowedPathElements = 3;
+        EsqlCheckVerifier.issues(check, new File("src/test/resources/NavigatingTreeCouldBeReference.esql"))
+                .next().atLine(4).withMessage("Navigating message tree could be replaced by a reference.")
+                .noMore();
+		check.maximumAllowedPathElements = 2;
+        check.allowedNumberOfFieldReferenceUses = 4;
+        EsqlCheckVerifier.issues(check, new File("src/test/resources/NavigatingTreeCouldBeReference.esql"))
+                .next().atLine(16).withMessage("Navigating message tree could be replaced by a reference.")
+                .noMore();
+    }
 }