Refactoring: issue context (addLocation/addFlow)

guwirth commented 5 years ago

Like to review and refactor issue context. Not sure if we are using additional location and flow always in the right way.

Manual of SQ is saying: https://docs.sonarqube.org/latest/user-guide/issues/#header-1

Sometimes, issues are self-evident once they're pointed out. For instance, if your team has agreed to a init-lower, camelCase variable naming convention, and an issue is raised on My_variable, you don't need a lot of context to understand the problem. But in other situations context may be essential to understanding why an issue was raised. That's why SonarQube supports not just the primary issue location, where the issue message is shown, but also secondary issue locations. For instance, secondary issues locations are used to mark the pieces of code in a method which add Cognitive Complexity to a method.

But there are times when a simple laundry list of contributing locations isn't enough to understand an issue. For instance, when a null pointer can be dereferenced on some paths through the code, what you really need are issue flows. Each flow is a set of secondary locations ordered to show the exact path through the code on which a problem can happen. And because there can be multiple paths through the code on which, for instance a resource is not released, SonarQube supports multiple flows.

Comments in source code of SQ framework: https://github.com/SonarSource/sonarqube/blob/master/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/issue/NewIssue.java

public interface NewIssue {
 ...
/**
   * Primary location for this issue.
   * @since 5.2
   */
  NewIssue at(NewIssueLocation primaryLocation);

  /**
   * Add a secondary location for this issue. Several secondary locations can be registered.
   * @since 5.2
   */
  NewIssue addLocation(NewIssueLocation secondaryLocation);

  /**
   * Register a flow for this issue. A flow is an ordered list of issue locations that help to understand the issue.
   * It should be a <b>path that backtracks the issue from its primary location to the start of the flow</b>. 
   * Several flows can be registered.
   * @since 5.2
   */
NewIssue addFlow(Iterable<NewIssueLocation> flowLocations);
...
}

guwirth commented 5 years ago

rise a question on SonarSource Commuity 12275

guwirth commented 5 years ago

Report:

1 warning generated.
/tmp/static-analyzer.cpp:11:18: warning: Dereference of null pointer (loaded from variable 'x') [clang-analyzer-core.NullDereference]
    printf("%d", *x);
                 ^
/tmp/static-analyzer.cpp:7:5: note: 'x' initialized to a null pointer value
    int* x = nullptr;
    ^
/tmp/static-analyzer.cpp:8:9: note: Assuming 'argc' is <= 1
    if (argc > 1) {
        ^
/tmp/static-analyzer.cpp:8:5: note: Taking false branch
    if (argc > 1) {
    ^
/tmp/static-analyzer.cpp:11:18: note: Dereference of null pointer (loaded from variable 'x')
    printf("%d", *x);
                 ^

Test 1 with clang-tidy addFlow:

UI:

grafik

Test 2 with clang-tidy addLocation:

UI:

grafik

SonarOpenCommunity / sonar-cxx

Refactoring: issue context (addLocation/addFlow) #1743