SonarOpenCommunity / sonar-cxx

SonarQube C++ Community plugin (cxx plugin): This plugin adds C++ support to SonarQube with the focus on integration of existing C++ tools.
GNU Lesser General Public License v3.0
994 stars 364 forks source link

Contributing: add new cyclomatic complexity metrics #1391

Closed ericlemes closed 6 years ago

ericlemes commented 6 years ago

Hi all,

I'm planning to get some extra metrics and contribute the code back to the plugin. I'm just trying to understand the code structure and standards.

I'm not sure if this is the right place to post this kind of question, but according to the contributing guide, it is ;-)

So, my question is: Usuarlly measures that appears on "Measures" tab of sonar are obtained from MeasureComputer class, which is not the case of this plugin. How the cyclomatic complexity and size are obtained for C++ code? I understand the structure of the parser, I'm just not sure how these particular metrics are sent to the server.

Can you help me with that?

Regards,

Eric

guwirth commented 6 years ago

Hi @ericlemes,

I'm planning to get some extra metrics and contribute the code back to the plugin.

you are welcome!

How the cyclomatic complexity and size are obtained for C++ code?

Easiest is to search for "metric" in the repository to get a feeling how we are doing it: https://github.com/SonarOpenCommunity/sonar-cxx/search?utf8=%E2%9C%93&q=metric&type=

What exactly do you wanna do?

Regards,

ericlemes commented 6 years ago

Hi @guwirth

I found the concept of "Visitor" in the cxx.sensors. Apparently fits well for me.

I'm trying to do a number of things and apparently won't be a clean implementation, so, I guess there will be some discussion to "accept" this code, but I'm happy to implement it at least to extract numbers myself.

So, I'll split this in several parts.

1) Get the list of functions ranked by cyclomatic complexity. I know I don't have where to put this is sonar, but my first idea is just dump to a file. My "MVP" for this feature would be to expose this list as an artifact in the CI. 2) Get the number of functions over a threshold of CC 3) Get the list of functions ranked by LoC. Same problem of #1 4) Get the number of functions over a threshold of LoC

To make this not very bad in terms of polluting the code, I had some ideas, not sure which ones to put in practice first. 1) Create a command line parameter for people that doesn't want to dump the list for features #1 and #3 2) Create another command line parameter where you can feed the thresholds for CC and LoC and deliver #2 and #4. 3) If it is still not acceptable, implement a sort of a hook that allows to plug 3rd party Sensors, so I can implement this in a way that doesn't interfere too much in the plugin.

I was trying to publish #1 and #3 as a "DATA" metric to Sonar, but didn't figure out how to work with blobs.

It would be good to hear your thoughts on these ideas.

Cheers

Eric

Bertk commented 6 years ago

Hi @ericlemes,

additional metrics are appreciated and should be visible in a dashboard on SonarQube. I feel some of the aspects from your comments are already available (see screenshot) and maybe need some refinement.

The C++ plugin has some complexity checks and the threshold for the checker rule can be customized. These checks will create SonarQube issues with the current complexity number. SonarSource also defined a new metric Cognitive Complexity which is very helpful.

Reporting is somehow a weak topic but could be done using the SonarQube web API to query thecomplexity related issues. I am not sure how you want to display this information on the CI - see also MMF-1123.

By the way, SonarSource removed some complexity metrics with SQ 6.7 (LTS) - Sonar-8331

Best regards Bert

query-complexity

ericlemes commented 6 years ago

@Bertk

Thanks for all the feedback and support.

I agree about the metrics should be visible in the dashboard and ideally is what I'm trying to achieve. Unfortunately I found quite annoying that Sonar doesn't support any metric per function. I found that the aggregated view of cyclomatic complexity useless, and the average per function also not very helpful. That's why for me the % of methods "not compliant" would be the ideal way of viewing. I knew about the possibility of enabling the check to highlight the bad metrics, but although helpful to get the problem fixed, is not helpful for the "metric" point of view.

Ultimately, what I'd like is to establish a "score" for different code bases, in a way that the developers agree with some basic metrics. I personally find much of the things classified as "issue" or "code smell" very opinion based.

The cognitive complexity is definitely interesting and I'll have a look in mid-long term. I'm working with the cyclomatic complexity now just because is easily available and more popular.

I had some progress Today and expecting to have some code to review by the end of the week.

By the way, is any way to obtain the code coverage report locally? Java stack is definitely not my best one :-(

Regards,

Eric

guwirth commented 6 years ago

@ericlemes first of all you should distiguish between rules (checks) and metrics. SonarSource removed metrics step by step over the last versions. If you want a frequency distribution you need metrics. I see only a small chance to add this to SQ because also the UI elements has been removed.

What you can do is to define template rules with the threshold as an parameter. But all what you can do is to display the results is a list - other UI Elements are not possible.

What you are talking about are analysis functions. SonarSource has removed their focus more on the quality gates and the water leak method.

For adding new metrics it's also the question if they are generic enough that others also wanna use it.

ericlemes commented 6 years ago

@guwirth, based on some PoC's I did, it can fit in the Measures tab. I understand that sonar removed a lot of things and that was the reason why I had issues with sonar in the past. A lot of metrics, a lot of information, but you couldn't get simple metrics that easily show you what is going on.

On the other hand, I personally don't like the issues workflow. I see a clear distinction between a check if the source code meets some standard or not (similar to what compiler checks does) and code metrics. For me metrics are more broad and give a high level indication about how good is the source code.

And yes, I understand they need to be generic. That's why I was expecting some questions whilst trying to integrate this source code. But I think is part of the process.

I just feel that if high level metrics are not welcome to sonar anymore, more and more this tool will become a central repository of compiler warnings. In my perspective code metrics are more like high level information that shows how health your code is.

If different projects uses completely different methodologies and understands an issue or code smell differently (which I think is correct), it will be really hard to find common measures between projects.

One example is a team that I worked that loved rules. They used things like "your usings clauses must be alphabetically sorted", which in Sonar way, will be counted as an issue, maybe a code smell. Not severe, but will count. Other team will consider dead code as not an issue. If your quality is based on number of issues you will never be able to find a common ground between these 2 very different worlds.

Anyway, I'll move forward with the implementation and later on with the pull request, maybe looking to the final result will help to understand what I'm trying to achieve.

I believe that the metrics I'm trying to add fits the quality gates and water leak methods.

Thanks for all the support.

guwirth commented 6 years ago

@ericlemes don't understand me wrong: I also like and need this analysis functions (hotspot analysis) beside the pure rule violation counting (water leak). Question for me is only if it make sense to fight against the tool and strategy of SonarSource? Maybe it's at the end simply the wrong tool in future?

But yes please provide your ideas and code?

ericlemes commented 6 years ago

@guwirth you have a good point. I don't like to fight against their strategy. That's exactly why I was reluctant to use Sonar in the past, and apparently I'm falling in the same trap again ;-)

I have one small project in my github called "MSBuildCodeMetrics". That was my naive strategy in the past, scan the code, spit some metrics in whatever CI tool. As soon as I start getting good numbers then I go to the second problem which is how to publish the values. Unfortunately, it was very focused on C#, and I was using some Visual Studio tool to get the metrics which was based in IL. When I started using C++, a whole new set of problems started.

Then I tried antler and the C++ grammar, and found a lot of new problems. Excessive memory consumption, bugs in the runtime which makes you change generated code, etc.

Then for my surprise sonar parsers was more or less taking care of the job (thanks for the good work you guys did here) and I got the basic very easy. But then all this strict approach about what kind of metrics you can push frustrated me.

I'm pretty sure I'll get this numbers. If using something that makes Sonar happy, I'm not totally sure ;-)

But in terms of the whole "Metrics" approach, I think the community is not quite there yet about "what" to observe, hence the problem. I didn't find many books (apart from Clean Code) that discuss what good code is like and how to measure it. That's why I'm more interested in getting the numbers, compare other quality information (bugs, crashes, user perception) with the numbers, until I get to the point that I know what to observe. Then probably will be a good time to have tool problem.

My problem with the "water leak" approach is that you can just look to a code base and a "2000 issues" number. Is that good or bad? Looks pretty bad. But if 1900 of those is just files with more than 80 columns, I don't think so.

I just don't understand why Sonar didn't leave a more "open" approach about what to publish on the dashboard. I think the existing concept of metric (that shows on measure panel on the UI) with few extensions could deliver most of the cases. By extensions I mean: properties, key value pairs that you can set on your project or quality profile and get on the scanner and the ability of using any kind of custom measure instead of only files (which the scanner could decide between functions, classes, etc). The idea is just a drill down of the metric.

These 2 things would make possible to add everything I need and probably a lot more.

guwirth commented 6 years ago

... These 2 things would make possible to add everything I need and probably a lot more.

@ericlemes besides the actions here it's maybe also a good idea to complain such things in the official SQ communities: https://www.sonarqube.org/community/

ericlemes commented 6 years ago

I've created a pull request for part of the metrics that are working. I'll breakdown in 3 pull requests to make the discussion easier. Hope it helps. Also the CI tools broke during the sonar startup. I'd like to have a look at the logs if possible, So I can chase any potential bug.