Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactoring: issue context (addLocation/addFlow) #1743

Open
guwirth opened this issue Jul 23, 2019 · 2 comments
Open

Refactoring: issue context (addLocation/addFlow) #1743

guwirth opened this issue Jul 23, 2019 · 2 comments

Comments

@guwirth
Copy link
Collaborator

guwirth commented Jul 23, 2019

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
Copy link
Collaborator Author

guwirth commented Jul 24, 2019

rise a question on SonarSource Commuity 12275

@guwirth
Copy link
Collaborator Author

guwirth commented Jul 24, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
1 participant