Ericsson / codechecker

CodeChecker is an analyzer tooling, defect database and viewer extension for static and dynamic analyzer tools.
https://codechecker.readthedocs.io
Apache License 2.0
2.27k stars 381 forks source link

Clang-Tidy fixits don't store code neccessary to understand fixit #799

Open whisperity opened 7 years ago

whisperity commented 7 years ago
projects/thrift-0.10.0/compiler/cpp/src/thrift/generate/t_rb_generator.cc:91:18: warning: do not use 'compare' to test equality of strings; use the string equality operator instead [misc-string-compare]
      } else if( iter->first.compare("namespaced") == 0) {
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~    ~~
                 iter->first                          "namespaced"

Currently, only

iter->first                          "namespaced"

is stored as the fixit's message (actually, as

iter->first                          "namespaced" (fixit)

) and showed in the viewer. Clang-tidy outputs the string as above, making use of the surrounding code.

Storing the exact leftside whitespaces, and the full line with the arrow and showing it appropriately in the webGUI would help this issue. Currently, the whitespace between first and "namespaced" is eliminated. The plist contains the whitespace, but the browser gets it as

instead of equal number of  s, so the browser eliminates them, resulting in this:

image

tmsblgh commented 7 years ago

Hi,

I am working on this and I would like to ask your opinion.

Now it's looks like this:

screen shot 2017-10-07 at 14 36 13

(1) I would like to move the lines more left till the arrow will under the 'i' character. (2) The FIXIT label can be on the right side.

What do you think?

whisperity commented 7 years ago

Can you please also provide an example on how it looks in the command-line? (CodeChecker parse)

I think it would be better if the labels moved to the left a bit. The other, even better – but I think harder to implement – option is to create a separate tag that somehow fits into the source code better, with less padding. I'm thinking of something that fits as if it were part of the source code, like how the standard output of the analyzer handles it. Because the font sizes and the spacing is so different in these tags and the code viewer...

But I'm thinking of something even better, which would make the thing even more kickass. @Xazax-hun, I need your input on how Clang-Tidy works in this regards, but here is what I think:

Considering we already know it is a fix-it tag by some logic...

Considering Clang-Tidy gives us an output like:

if ( blabla == foobar )
     ^~~~~~    ~~~~~~
     aaaaaaaa  bbbbbbbbb

I think it would be a much more "kick-ass" feature if there was a way to, with a push of a button, or by hovering over the line... we would actually draw/print the fixed version as suggested by Clang-Tidy? So we basically parse and execute the string replace that was given to us in the output of the analyzer.

Do not use compare to test equality of strings; use the string equality operator instead. [Explain]

Clicking on Explain shows the fixed version. The location to be changed is already highlighted, because the red line points to the line in question.

tmsblgh commented 7 years ago

At the moment:

CodeChecker parse --verbose debug -t plist ~/results/test.cpp_e6aceab7b99acd9007ba62ea491884c1.plist
[HIGH] test.cpp:14:13: do not use 'compare' to test equality of strings; use the string equality operator instead [misc-string-compare]
        if (iter->first.compare("namespace") == 0)
            ^

Found 1 defect(s) while analyzing UNKNOWN

The output is the same without my modifications.

plist file:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
    <key>diagnostics</key>
    <array>
        <dict>
            <key>category</key>
            <string>misc</string>
            <key>check_name</key>
            <string>misc-string-compare</string>
            <key>description</key>
            <string>do not use 'compare' to test equality of strings; use the string equality operator instead</string>
            <key>location</key>
            <dict>
                <key>col</key>
                <integer>13</integer>
                <key>file</key>
                <integer>0</integer>
                <key>line</key>
                <integer>14</integer>
            </dict>
            <key>path</key>
            <array>
                <dict>
                    <key>depth</key>
                    <integer>0</integer>
                    <key>kind</key>
                    <string>event</string>
                    <key>location</key>
                    <dict>
                        <key>col</key>
                        <integer>13</integer>
                        <key>file</key>
                        <integer>0</integer>
                        <key>line</key>
                        <integer>14</integer>
                    </dict>
                    <key>message</key>
                    <string>^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~    ~~ (fixit)</string>
                </dict>
                <dict>
                    <key>depth</key>
                    <integer>0</integer>
                    <key>kind</key>
                    <string>event</string>
                    <key>location</key>
                    <dict>
                        <key>col</key>
                        <integer>13</integer>
                        <key>file</key>
                        <integer>0</integer>
                        <key>line</key>
                        <integer>14</integer>
                    </dict>
                    <key>message</key>
                    <string>iter-&gt;first                         "namespace" (fixit)</string>
                </dict>
                <dict>
                    <key>depth</key>
                    <integer>0</integer>
                    <key>kind</key>
                    <string>event</string>
                    <key>location</key>
                    <dict>
                        <key>col</key>
                        <integer>13</integer>
                        <key>file</key>
                        <integer>0</integer>
                        <key>line</key>
                        <integer>14</integer>
                    </dict>
                    <key>message</key>
                    <string>do not use 'compare' to test equality of strings; use the string equality operator instead</string>
                </dict>
            </array>
            <key>type</key>
            <string>clang-tidy</string>
        </dict>
    </array>
    <key>files</key>
    <array>
        <string>/Users/tamasbalogh/Desktop/test.cpp</string>
    </array>
</dict>
</plist>
Xazax-hun commented 7 years ago

I think printing the fixed version as a fix-it hint with the changed parts highlighted would be great.