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
988 stars 364 forks source link

Clarify sonar.cxx.gcc.regex documentation in the wiki #2717

Open cmorve-te opened 1 month ago

cmorve-te commented 1 month ago

https://github.com/SonarOpenCommunity/sonar-cxx/wiki/sonar.cxx.gcc.reportPaths#configure-cxx-plugin says

Create a matching sonar.cxx.gcc.regex, some samples are below.

(I read it as a hard requirement, not something optional you happen to be able to change), and gives two examples, "with column number" and "without column number". But there is already a working default regular expression: https://github.com/SonarOpenCommunity/sonar-cxx/blob/cxx-2.1.2/cxx-sensors/src/main/java/org/sonar/cxx/sensors/compiler/gcc/CxxCompilerGccSensor.java#L44, which isn't even any of the ones given in the documentation.

It's also strange that none of those three (two examples and default) uses the <column> capture group given that's documented in https://github.com/SonarOpenCommunity/sonar-cxx/wiki/sonar.cxx.gcc.regex. If the column information is captured, is it actually used for anything?

Also, before starting using Sonar, I had been using ((.+):(\d+):(\d+)|cc1|cc1plus): warning: (.*) \[-W(.*)\]$ for both gcc and clang for ages (https://plugins.jenkins.io/warnings-ng/), they use the same format. (I don't remember why the cc1|cc1plus thing right now, I will take a look) I have just started looking at sonar-cxx and it's not clear to me whether clang warnings are handled via sonar.cxx.gcc.reportPaths, or not at all (I guess https://github.com/SonarOpenCommunity/sonar-cxx/pull/2606 could also have an effect?). Some kind of mention to clang in https://github.com/SonarOpenCommunity/sonar-cxx/wiki/sonar.cxx.gcc.reportPaths could help.

guwirth commented 1 month ago

Hi @cmorve-te,

thanks for your feedback!

(I read it as a hard requirement, not something optional you happen to be able to change), and gives two examples, "with column number" and "without column number". But there is already a working default regular expression

You are right it’s optional, there is always a default. Maybe I have to state this in the Wiki.

In the past we optimized the regex, that’s maybe the reason that they no more match: https://github.com/SonarOpenCommunity/sonar-cxx/commit/6fd6ef7ee059125be4638efe6e4bd92378c8d3c7

It's also strange that none of those three (two examples and default) uses the capture group

Here we handle the column: https://github.com/SonarOpenCommunity/sonar-cxx/blob/10371d39e5d6d0aabc5598edfae358188cd4b385/cxx-sensors/src/main/java/org/sonar/cxx/sensors/compiler/CxxCompilerSensor.java#L70

Earlier versions of the SQ API had no column and that’s the reasons why not all sensors were supporting it in the past.

for both gcc and clang for ages

The clang sensor has an own sensor, settings and regex. Would not mix this up.

Regards,

reddwarf69 commented 4 weeks ago

The clang sensor has an own sensor, settings and regex. Would not mix this up.

To be clear, there is:

But there is nothing for clang, the compiler. And clang, the compiler,

People maybe have a Jenkins job building for Ubuntu (gcc) and Android (clang). They may be building for 30 targets, some gcc and some clang. They may have all the log files in logs/compiler_log_<target>.log, they may want to capture all the warnings, and they may want to use sonar.cxx.gcc.reportPaths=logs/compiler_log_*.log, without having to keep track of which ones use clang and which ones use gcc.

At the same time there is an argument to be made that's better to keep them separate. But, not sure how this works,

As things are right now, people may reasonably expect sonar.cxx.gcc.reportPaths to handle clang warnings. "If it didn't, surely there would be sonar.cxx.clang.reportPaths also".

guwirth commented 4 weeks ago

Hi @reddwarf69,

But there is nothing for clang, the compiler.

Yes right, not supported. There is always a long discussion about whether compiler warnings should be included or not, or whether they should be checked in a CI/CD system (and should actually be 0).

Happens to use the exact same format as gcc Happens to share the name of plenty of warnings Happens to have some warnings not provided by gcc

Here are some general notes:

At the same time there is an argument to be made that's better to keep them separate.

Following the approach above we always like to have separate sensors for separate tools (or you use one of the generic ones)

There is a warning with key -Warray-bounds ... Can we have another one with the same key

Not totally sure but that should be possible. For my understanding the keys must be only unique per repository (and every sensor has an own repository).

As things are right now, people may reasonably expect sonar.cxx.gcc.reportPaths to handle clang warnings. "If it didn't, surely there would be sonar.cxx.clang.reportPaths also".

Up to now this sonar.cxx.clang was for https://clang.llvm.org/extra/clang-tidy/ only and not for compiler messages. On the other side sonar.cxx.vc.reportPaths is also mixing up compiler messages and static code analysis.

Main question for me is always: Are there different tools and resulting reports behind it or only one? If the reports could have different encodings or formats in one run we must split it.

Regards,

cmorve-te commented 4 weeks ago

Yes right, not supported. There is always a long discussion about whether compiler warnings should be included or not, or whether they should be checked in a CI/CD system (and should actually be 0).

The issue with that logic nowadays is that the lines have become too blurry.

Up to now this sonar.cxx.clang was for https://clang.llvm.org/extra/clang-tidy/ only

Do you mean sonar.cxx.clangtidy? There is no sonar.cxx.clang, right? It would be free to use for "clang the compiler"?

In cxx there is Other (https://github.com/SonarOpenCommunity/sonar-cxx/wiki/sonar.cxx.other.reportPaths), so that any tools can be supported.

The limitations of https://docs.sonarsource.com/sonarqube/latest/analyzing-source-code/importing-external-issues/generic-issue-import-format/ are actually what made me find sonar-cxx. Somehow I had forgotten/skipped over sonar.cxx.other. I have to take another look at it...

guwirth commented 4 weeks ago

Hi @reddwarf69,

Do you mean sonar.cxx.clangtidy? There is no sonar.cxx.clang, right? It would be free to use for "clang the compiler"?

You are right, name should be sonar.cxx.clangtidy

The issue with that logic nowadays is that the lines have become too blurry

Would agree if the result is one report with issues in same format. If different tools copy things together in one file with different issue formats I would always use different sensors even if I would have to read the LOG file twice.

Please have also a look to the discussion in #2325

Regards,