Merge pull request #2163 from guwirth/fix-2161

public API detection: improved detection of comments at the end of last line

Author: guwirth

cxx-squid/src/main/java/org/sonar/cxx/visitors/AbstractCxxPublicApiVisitor.java

@@ -227,22 +227,19 @@ private static String getOperatorId(AstNode operatorFunctionId) {
   private static List<Token> getDeclaratorInlineComment(AstNode declarator) {
     List<Token> comments;
 
-    // inline comments are attached to the next AST node (not sibling,
-    // because the last attribute inline comment is attached to the next
-    // node of the parent)
+    // inline comments are attached to the next AST node (not sibling, because the last attribute inline comment
+    // is attached to the next node of the parent)
     var next = declarator.getNextAstNode();
 
-    // inline documentation may be on the next definition token
-    // or next curly brace
+    // inline documentation may be on the next definition token or next curly brace
     if (next != null) {
       // discard COMMA and SEMICOLON
       if (next.getToken().getType().equals(CxxPunctuator.COMMA)
             || next.getToken().getType().equals(CxxPunctuator.SEMICOLON)) {
         next = next.getNextAstNode();
       }
 
-      comments = getInlineDocumentation(next.getToken(),
-                                        declarator.getTokenLine());
+      comments = getInlineDocumentation(next.getToken());
     } else {
       // could happen on parse error ?
       comments = new ArrayList<>();
@@ -309,21 +306,18 @@ private static boolean isPublicApiMember(AstNode node) {
   }
 
   /**
-   * Check if inline Doxygen documentation is attached to the given token at specified line
+   * Check if inline Doxygen documentation is attached to the end of the line
    *
    * @param token the token to inspect
-   * @param line line of the inlined documentation
    * @return true if documentation is found for specified line, false otherwise
    */
-  private static List<Token> getInlineDocumentation(Token token, int line) {
+  private static List<Token> getInlineDocumentation(Token token) {
     var comments = new ArrayList<Token>();
 
     for (var trivia : token.getTrivia()) {
       if (trivia.isComment()) {
         var triviaToken = trivia.getToken();
-        if ((triviaToken != null)
-              && (triviaToken.getLine() == line)
-              && (isDoxygenInlineComment(triviaToken.getValue()))) {
+        if ((triviaToken != null) && (isDoxygenInlineComment(triviaToken.getValue()))) {
           comments.add(triviaToken);
         }
       }
@@ -806,11 +800,9 @@ private void visitAliasDeclaration(AstNode aliasDeclNode) {
   }
 
   private void visitEnumSpecifier(AstNode enumSpecifierNode) {
-    var enumIdNode = enumSpecifierNode.getFirstDescendant(
-      GenericTokenType.IDENTIFIER);
+    var enumIdNode = enumSpecifierNode.getFirstDescendant(GenericTokenType.IDENTIFIER);
 
-    String enumId = (enumIdNode == null)
-                      ? UNNAMED_ENUM_ID : enumIdNode.getTokenValue();
+    String enumId = (enumIdNode == null) ? UNNAMED_ENUM_ID : enumIdNode.getTokenValue();
 
     if (!isPublicApiMember(enumSpecifierNode)) {
       // not in public API
@@ -823,17 +815,13 @@ private void visitEnumSpecifier(AstNode enumSpecifierNode) {
       docNode = enumSpecifierNode;
     }
 
-    visitPublicApi(
-      enumSpecifierNode, enumId,
-      getBlockDocumentation(docNode));
+    visitPublicApi(enumSpecifierNode, enumId,getBlockDocumentation(docNode));
 
     // deal with enumeration values
-    AstNode enumeratorList = enumSpecifierNode
-      .getFirstDescendant(CxxGrammarImpl.enumeratorList);
+    AstNode enumeratorList = enumSpecifierNode.getFirstDescendant(CxxGrammarImpl.enumeratorList);
 
     if (enumeratorList != null) {
-      for (var definition : enumeratorList
-        .getChildren(CxxGrammarImpl.enumeratorDefinition)) {
+      for (var definition : enumeratorList.getChildren(CxxGrammarImpl.enumeratorDefinition)) {
 
         // look for block documentation
         List<Token> comments = getBlockDocumentation(definition);
@@ -842,24 +830,18 @@ private void visitEnumSpecifier(AstNode enumSpecifierNode) {
         if (comments.isEmpty()) {
           var next = definition.getNextAstNode();
 
-          // inline documentation may be on the next definition token
-          // or next curly brace
+          // inline documentation may be on the next definition token or next curly brace
           if (next != null) {
             // discard COMMA
             if (next.getToken().getType().equals(CxxPunctuator.COMMA)) {
               next = next.getNextAstNode();
             }
 
-            comments = getInlineDocumentation(next.getToken(),
-                                              definition.getTokenLine());
+            comments = getInlineDocumentation(next.getToken());
           }
         }
 
-        visitPublicApi(
-          definition,
-          definition.getFirstDescendant(
-            GenericTokenType.IDENTIFIER).getTokenValue(),
-          comments);
+        visitPublicApi(definition,  definition.getFirstDescendant(  GenericTokenType.IDENTIFIER).getTokenValue(), comments);
       }
     }
   }

cxx-squid/src/test/java/org/sonar/cxx/visitors/CxxPublicApiVisitorTest.java

@@ -67,38 +67,43 @@ public void test_no_matching_suffix() throws IOException {
 
   @Test
   public void doxygen_example() throws IOException {
-    assertThat(testFile("src/test/resources/metrics/doxygen_example.h")).isEqualTo(tuple(13, 0));
+    assertThat(verifyPublicApiOfFile("src/test/resources/metrics/doxygen_example.h")).isEqualTo(tuple(13, 0));
   }
 
   @Test
   public void to_delete() throws IOException {
-    assertThat(testFile("src/test/resources/metrics/public_api.h")).isEqualTo(tuple(47, 0));
+    assertThat(verifyPublicApiOfFile("src/test/resources/metrics/public_api.h")).isEqualTo(tuple(47, 0));
 
   }
 
   @Test
   public void no_doc() throws IOException {
-    assertThat(testFile("src/test/resources/metrics/no_doc.h")).isEqualTo(tuple(22, 22));
+    assertThat(verifyPublicApiOfFile("src/test/resources/metrics/no_doc.h")).isEqualTo(tuple(22, 22));
   }
 
   @Test
   public void template() throws IOException {
-    assertThat(testFile("src/test/resources/metrics/template.h")).isEqualTo(tuple(14, 4));
+    assertThat(verifyPublicApiOfFile("src/test/resources/metrics/template.h")).isEqualTo(tuple(14, 4));
   }
 
   @Test
   public void alias_function_template() throws IOException {
-    assertThat(testFile("src/test/resources/metrics/alias_in_template_func.h")).isEqualTo(tuple(4, 3));
+    assertThat(verifyPublicApiOfFile("src/test/resources/metrics/alias_in_template_func.h")).isEqualTo(tuple(4, 3));
   }
 
   @Test
   public void unnamed_class() throws IOException {
-    assertThat(testFile("src/test/resources/metrics/unnamed_class.h")).isEqualTo(tuple(3, 1));
+    assertThat(verifyPublicApiOfFile("src/test/resources/metrics/unnamed_class.h")).isEqualTo(tuple(3, 1));
   }
 
   @Test
   public void unnamed_enum() throws IOException {
-    assertThat(testFile("src/test/resources/metrics/unnamed_enum.h")).isEqualTo(tuple(1, 1));
+    assertThat(verifyPublicApiOfFile("src/test/resources/metrics/unnamed_enum.h")).isEqualTo(tuple(1, 1));
+  }
+
+  @Test
+  public void multiline() throws IOException {
+    assertThat(verifyPublicApiOfFile("src/test/resources/metrics/multiline.h")).isEqualTo(tuple(9, 0));
   }
 
   @Test
@@ -199,13 +204,12 @@ public void public_api() throws UnsupportedEncodingException, IOException {
   }
 
   /**
-   * Check that CxxPublicApiVisitor correctly counts API for given file.
+   * Check that CxxPublicApiVisitor correctly counts public API for given file.
    *
-   * @param fileName the file to use for test
-   * @param expectedApi expected number of API
-   * @param expectedUndoc expected number of undocumented API
+   * @param fileName file to verify
+   * @return tuple(found public API, thereof undocumented public API)
    */
-  private Tuple testFile(String fileName)
+  private Tuple verifyPublicApiOfFile(String fileName)
     throws UnsupportedEncodingException, IOException {
 
     var squidConfig = new CxxSquidConfiguration();

cxx-squid/src/test/resources/metrics/multiline.h

@@ -0,0 +1,29 @@
+/* multi-line definitions/declarations and comments at the end of last line */
+
+/**
+ docu MyEnum (issue #2161)
+ */
+typedef enum {
+    A = 0, ///< docu A
+    B =
+        1, ///< docu B
+} MyEnum;
+
+/**
+ docu MyClass
+ */
+class MyClass {
+public:
+   void method1() = 0; ///< docu method1
+   void method2() =
+                    0; ///< docu method2
+};
+
+/**
+ docu MyType
+ */
+struct MyType {
+   int var1 = 1; ///< docu var1
+   int var2 =
+              2; ///< docu var2
+};