doxygen grouping tags cause false detection (section of code should not be "commented out") and "Undocumented API"

MarkShackman commented 8 years ago

The doxygen grouping tags @{ and @} are not interpreted correctly. The tags themselves lead to false detection (section of code should not be "commented out"), and APIs grouped by the tags are marked as undocumented (in the example below, functionVariantB).

//@{
/**************************************************************************//**
* @brief An API.
* @param[in] addr   Address parameter.
* @return Data requested.
******************************************************************************/
int functionVariantA(uint addr);
int functionVariantB(uint addr);
//@}

See: http://sonarqube-archive.15.x6.nabble.com/False-Positive-for-section-of-code-should-not-be-quot-commented-out-quot-td5007197.html

guwirth commented 8 years ago

@MarkShackman thanks for reporting this issues. These are two issues:

one is a false positive of the CoomentedCodeCheck: https://github.com/SonarOpenCommunity/sonar-cxx/blob/master/cxx-checks/src/main/java/org/sonar/cxx/checks/CommentedCodeCheck.java
The other is missing support of doxygen grouping tags. My feeling is that it is hard to support this and if full doxygen support is needed it would be easier to write doxygen warnings to a report file and create an additional sensor for this use case.

Bertk commented 8 years ago

@guwirth, @MarkShackman I could fix the CommentedCodeCheck but need some help to extend the test file. I do not have a real live sample with doxygen grouping tags. https://github.com/SonarOpenCommunity/sonar-cxx/blob/master/cxx-checks/src/test/resources/checks/commentedCode.cc I guess adding one line should fix the false-positiv.

      if (trivia.isComment()
        && !trivia.getToken().getOriginalValue().startsWith("///")
        && !trivia.getToken().getOriginalValue().startsWith("//!")
        && !trivia.getToken().getOriginalValue().startsWith("/**")
        && !trivia.getToken().getOriginalValue().startsWith("/*!")
        && !trivia.getToken().getOriginalValue().startsWith("//@{")) { ...

MarkShackman commented 8 years ago

Thanks for your help.

I’m not sure that a one line fix is sufficient – there are a number of permutations of the group tag – see [http://www.stack.nl/~dimitri/doxygen/manual/grouping.html] Also need to also add a test for the end-of-group tag.

Bertk commented 8 years ago

Upps. I just noticed the unit test for this checker is broken (https://github.com/SonarOpenCommunity/sonar-cxx/blob/master/cxx-checks/src/test/java/org/sonar/cxx/checks/CommentedCodeCheckTest.java).

public class CommentedCodeCheckTest {

  @Rule
  public CheckMessagesVerifierRule checkMessagesVerifier = new CheckMessagesVerifierRule();

  // @Test
  public void test() throws UnsupportedEncodingException, IOException {
    CxxFileTester tester = CxxFileTesterHelper.CreateCxxFileTester("src/test/resources/checks/commentedCode.cc", ".");       
    SourceFile file = CxxAstScanner.scanSingleFile(tester.cxxFile, tester.sensorContext, new CommentedCodeCheck());

    checkMessagesVerifier.verify(file.getCheckMessages())
      .next().atLine(10).withMessage("Remove this commented out code.")
      .next().atLine(15);
  }

}

After enabling the test again this exception occurs

org.sonar.squidbridge.api.AnalysisException: Unable to analyze file: C:\Github\sonar-cxx\cxx-checks\src\test\resources\checks\commentedCode.cc
    at org.sonar.squidbridge.AstScanner.scanFiles(AstScanner.java:131)
    at org.sonar.squidbridge.AstScanner.scanFile(AstScanner.java:77)
    at org.sonar.cxx.CxxAstScanner.scanSingleFileConfig(CxxAstScanner.java:90)
    at org.sonar.cxx.CxxAstScanner.scanSingleFile(CxxAstScanner.java:74)
    at org.sonar.cxx.checks.CommentedCodeCheckTest.test(CommentedCodeCheckTest.java:39)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
    at java.lang.reflect.Method.invoke(Unknown Source)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.junit.rules.Verifier$1.evaluate(Verifier.java:35)
    at org.junit.rules.RunRules.evaluate(RunRules.java:20)
    at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
    at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:86)
    at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:459)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:678)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:382)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:192)
Caused by: java.lang.NoClassDefFoundError: org/apache/commons/lang/StringUtils
    at org.sonar.squidbridge.recognizer.ContainsDetector.scan(ContainsDetector.java:38)
    at org.sonar.squidbridge.recognizer.Detector.recognition(Detector.java:36)
    at org.sonar.squidbridge.recognizer.CodeRecognizer.recognition(CodeRecognizer.java:39)
    at org.sonar.squidbridge.recognizer.CodeRecognizer.isLineOfCode(CodeRecognizer.java:55)
    at org.sonar.cxx.checks.CommentedCodeCheck.visitToken(CommentedCodeCheck.java:87)
    at com.sonar.sslr.impl.ast.AstWalker.visitToken(AstWalker.java:107)
    at com.sonar.sslr.impl.ast.AstWalker.visit(AstWalker.java:86)
    at com.sonar.sslr.impl.ast.AstWalker.visitChildren(AstWalker.java:99)
    at com.sonar.sslr.impl.ast.AstWalker.visit(AstWalker.java:87)
    at com.sonar.sslr.impl.ast.AstWalker.visitChildren(AstWalker.java:99)
    at com.sonar.sslr.impl.ast.AstWalker.visit(AstWalker.java:87)
    at com.sonar.sslr.impl.ast.AstWalker.visitChildren(AstWalker.java:99)
    at com.sonar.sslr.impl.ast.AstWalker.visit(AstWalker.java:87)
    at com.sonar.sslr.impl.ast.AstWalker.visitChildren(AstWalker.java:99)
    at com.sonar.sslr.impl.ast.AstWalker.visit(AstWalker.java:87)
    at com.sonar.sslr.impl.ast.AstWalker.visitChildren(AstWalker.java:99)
    at com.sonar.sslr.impl.ast.AstWalker.visit(AstWalker.java:87)
    at com.sonar.sslr.impl.ast.AstWalker.visitChildren(AstWalker.java:99)
    at com.sonar.sslr.impl.ast.AstWalker.visit(AstWalker.java:87)
    at com.sonar.sslr.impl.ast.AstWalker.visitChildren(AstWalker.java:99)
    at com.sonar.sslr.impl.ast.AstWalker.visit(AstWalker.java:87)
    at com.sonar.sslr.impl.ast.AstWalker.visitChildren(AstWalker.java:99)
    at com.sonar.sslr.impl.ast.AstWalker.visit(AstWalker.java:87)
    at com.sonar.sslr.impl.ast.AstWalker.walkAndVisit(AstWalker.java:69)
    at org.sonar.squidbridge.AstScanner.scanFiles(AstScanner.java:110)
    ... 29 more
Caused by: java.lang.ClassNotFoundException: org.apache.commons.lang.StringUtils
    at java.net.URLClassLoader.findClass(Unknown Source)
    at java.lang.ClassLoader.loadClass(Unknown Source)
    at sun.misc.Launcher$AppClassLoader.loadClass(Unknown Source)
    at java.lang.ClassLoader.loadClass(Unknown Source)
    ... 54 more

This is related to this line String lineWithoutWhitespaces = StringUtils.deleteWhitespace(line) in org.sonar.squidbridge.recognizer.ContainsDetector class.

// Compiled from StringUtils.java (version 1.3 : 47.0, super bit)
public class org.sonar.api.internal.apachecommons.lang.StringUtils

guwirth commented 8 years ago

Hi @MarkShackman CommentedCodeCheck should be fixed, please try and give us feedback.

guwirth commented 8 years ago

@MarkShackman I close this, please reopen if still an issue.

SonarOpenCommunity / sonar-cxx

doxygen grouping tags cause false detection (section of code should not be "commented out") and "Undocumented API" #990