Merge pull request SonarOpenCommunity#1559 from ivangalkin/fix_1557

Fix MethodName for nested classes
ivangalkin · Oct 6, 2018 · 040a94a · 040a94a
2 parents 2156286 + 1a730d3
commit 040a94a
Show file tree

Hide file tree

Showing 3 changed files with 79 additions and 4 deletions.
diff --git a/cxx-checks/src/main/java/org/sonar/cxx/checks/naming/MethodNameCheck.java b/cxx-checks/src/main/java/org/sonar/cxx/checks/naming/MethodNameCheck.java
@@ -19,10 +19,11 @@
  */
 package org.sonar.cxx.checks.naming;
 
-import com.sonar.sslr.api.AstNode;
-import com.sonar.sslr.api.Grammar;
+import java.util.Optional;
 import java.util.regex.Pattern;
+
 import javax.annotation.Nullable;
+
 import org.sonar.check.Priority;
 import org.sonar.check.Rule;
 import org.sonar.check.RuleProperty;
@@ -32,6 +33,10 @@
 import org.sonar.squidbridge.annotations.SqaleConstantRemediation;
 import org.sonar.squidbridge.checks.SquidCheck;
 
+import com.sonar.sslr.api.AstNode;
+import com.sonar.sslr.api.GenericTokenType;
+import com.sonar.sslr.api.Grammar;
+
 /**
  * MethodNameCheck
  *
@@ -111,16 +116,32 @@ AstNode getInsideMemberDeclaration(AstNode declId) {
     return result;
   }
 
+  private static Optional<AstNode> getMostNestedTypeName(AstNode nestedNameSpecifier) {
+    Optional<AstNode> result = Optional.empty();
+    for (AstNode child : nestedNameSpecifier.getChildren()) {
+      if (
+          // type name was recognized by parser (most probably the least nested type)
+          child.is(CxxGrammarImpl.typeName) ||
+          // type name was recognized as template
+          child.is(CxxGrammarImpl.simpleTemplateId) ||
+          // type name was recognized, but not properly typed
+          GenericTokenType.IDENTIFIER.equals(child.getToken().getType())) {
+        result = Optional.of(child);
+      }
+    }
+    return result;
+  }
+
   private static @Nullable
   AstNode getOutsideMemberDeclaration(AstNode declId) {
     AstNode nestedNameSpecifier = declId.getFirstDescendant(CxxGrammarImpl.nestedNameSpecifier);
     AstNode result = null;
     if (nestedNameSpecifier != null) {
       AstNode idNode = declId.getLastChild(CxxGrammarImpl.className);
       if (idNode != null) {
-        AstNode className = nestedNameSpecifier.getFirstDescendant(CxxGrammarImpl.className);
+        Optional<AstNode> typeName = getMostNestedTypeName(nestedNameSpecifier);
         // if class name is equal to method name then it is a ctor or dtor
-        if ((className != null) && !className.getTokenValue().equals(idNode.getTokenValue())) {
+        if (typeName.isPresent() && !typeName.get().getTokenValue().equals(idNode.getTokenValue())) {
           result = idNode;
         }
       }

diff --git a/cxx-checks/src/test/java/org/sonar/cxx/checks/naming/MethodNameCheckTest.java b/cxx-checks/src/test/java/org/sonar/cxx/checks/naming/MethodNameCheckTest.java
@@ -47,6 +47,9 @@ public void test() throws Exception {
       .next().atLine(26).withMessage(
       "Rename method \"TooLongMethodNameBecauseItHasMoreThan30Characters1\" "
       + "to match the regular expression ^[A-Z][A-Za-z0-9]{2,30}$.")
+      .next().atLine(96).withMessage(
+      "Rename method \"Third_Level_Nested_Class_getX\" "
+      + "to match the regular expression ^[A-Z][A-Za-z0-9]{2,30}$.")
       .noMore();
   }
 

diff --git a/cxx-checks/src/test/resources/checks/MethodName.cc b/cxx-checks/src/test/resources/checks/MethodName.cc
@@ -36,6 +36,24 @@ class My_Class {
 
   ~My_Class();
   ~My_Class() {} // not an error
+
+  class My_Inner_Class {
+    My_Inner_Class();
+    ~My_Inner_Class();
+  };
+
+  template<typename T>
+  class My_Inner_Class_With_Template {
+    My_Inner_Class_With_Template();
+    ~My_Inner_Class_With_Template();
+
+    class Third_Level_Nested_Class {
+      Third_Level_Nested_Class();
+      ~Third_Level_Nested_Class()
+
+      void Third_Level_Nested_Class_getX();
+    };
+  };
 };
 
 My_Class::My_Class() // not an error
@@ -46,4 +64,37 @@ test::My_Class::~My_Class() // not an error
 {
 }
 
+test::My_Class::My_Inner_Class::My_Inner_Class() // not an error
+{
+}
+
+test::My_Class::My_Inner_Class::~My_Inner_Class() // not an error
+{
+}
+
+template<typename T>
+My_Class::My_Inner_Class_With_Template<T>::My_Inner_Class_With_Template() // not an error
+{
+}
+
+template<typename T>
+My_Class::My_Inner_Class_With_Template<T>::~My_Inner_Class_With_Template() // not an error
+{
+}
+
+template<typename T>
+My_Class::My_Inner_Class_With_Template<T>::Third_Level_Nested_Class::Third_Level_Nested_Class() // not an error
+{
+}
+
+template<typename T>
+My_Class::My_Inner_Class_With_Template<T>::Third_Level_Nested_Class::Third_Level_Nested_Class() // not an error
+{
+}
+
+template<typename T>
+void My_Class::My_Inner_Class_With_Template<T>::Third_Level_Nested_Class::Third_Level_Nested_Class_getX() // error
+{
+}
+
 // EOF