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

clang-tidy: missing rule [clang-diagnostic-warning] #1702

Closed
stalb opened this issue Apr 15, 2019 · 11 comments
Closed

clang-tidy: missing rule [clang-diagnostic-warning] #1702

stalb opened this issue Apr 15, 2019 · 11 comments
Assignees
Milestone

Comments

@stalb
Copy link
Contributor

stalb commented Apr 15, 2019

Please turn debug info on and reproduce your issue.
The debug info in the log file could help to solve or locate the issue.

Description

Some Clang-Tidy warnings -- [clang-diagnostic-warning] -- are not reported in SonarQube by the Community SonarQube C plugin

Steps to reproduce the problem

  1. On SonarQube instance activate all Clang-Tidy rules
  2. Create a C program which produces some warnings:
    main.c
/* test file */

/* missing semicolon at end of struct or union */
typedef struct {
	int a;
	int b
} my_struct;

int main(){
	/* unused variable */
	int i;
	return 0;
}
  1. Produce report using clang-tidy
main.c:6:7: warning: expected ';' at end of declaration list [clang-diagnostic-warning]
        int b
             ^
             ;
main.c:11:6: warning: unused variable 'i' [clang-diagnostic-unused-variable]
        int i;
  1. Send report to SonarQube instance

Exemple repo (CI is defined for gitlab) is here: https://github.com/stalb/demo_missing_rule_sonar_cxx_plugin

Expected behavior

Community SonarQube C plugin and SonarQube should report 2 issues on main.c.

Actual behavior

Only the 2nd warning (unused variable) is reported.

Known workarounds

none (there is no template rule for Clang-Tidy)

LOG file

Any warnings/errors in the LOG file? None

12:50:50.330 INFO: Load active rules (done) | time=1851ms
12:50:50.332 INFO: Load metrics repository
12:50:50.347 DEBUG: GET 200 https://SonarCE.instance.org/sonar/api/metrics/search?f=name,description,direction,qualitative,custom&ps=500&p=1 | time=15ms
12:50:50.362 INFO: Load metrics repository (done) | time=30ms
12:50:50.424 INFO: Branch name: master, type: long living
12:50:50.483 DEBUG: Declared extensions of language Python were converted to sonar.lang.patterns.py : **/*.py
12:50:50.483 DEBUG: Declared extensions of language C (Community) were converted to sonar.lang.patterns.c : **/*.c,**/*.h
12:50:50.484 DEBUG: Declared extensions of language JavaScript were converted to sonar.lang.patterns.js : **/*.js,**/*.jsx,**/*.vue
12:50:50.484 DEBUG: Declared extensions of language C# were converted to sonar.lang.patterns.cs : **/*.cs
12:50:50.484 DEBUG: Declared extensions of language Java were converted to sonar.lang.patterns.java : **/*.java,**/*.jav
12:50:50.484 DEBUG: Declared extensions of language Flex were converted to sonar.lang.patterns.flex : **/*.as
12:50:50.484 DEBUG: Declared extensions of language XML were converted to sonar.lang.patterns.xml : **/*.xml,**/*.xsd,**/*.xsl
12:50:50.485 DEBUG: Declared extensions of language PHP were converted to sonar.lang.patterns.php : **/*.php,**/*.php3,**/*.php4,**/*.php5,**/*.phtml,**/*.inc
12:50:50.485 DEBUG: Declared extensions of language TypeScript were converted to sonar.lang.patterns.ts : **/*.ts,**/*.tsx
12:50:50.485 INFO: Language is forced to c
12:50:50.489 INFO: Indexing files...
12:50:50.489 INFO: Project configuration:
12:50:50.491 INFO:   Excluded sources: gcc_report.log, clangtidy_report.log
12:50:50.510 DEBUG: 'main.c' indexed with language 'c'
12:50:50.510 WARN: File '/builds/stalb/demo_missing_rule_sonar_cxx_plugin/sonar-project.properties' is ignored because it doesn't belong to the forced language 'c'
12:50:50.511 INFO: 1 file indexed
12:50:50.511 INFO: 2 files ignored because of inclusion/exclusion patterns
12:50:50.515 INFO: Quality profile for c: Sonar extended way
12:50:50.516 INFO: ------------- Run sensors on module demo_missing_rule_sonar_cxx_plugin
12:50:52.553 DEBUG: 'JavaSquidSensor' skipped because there is no related file in current project
12:50:52.554 DEBUG: 'Import external issues report' skipped because one of the required properties is missing
12:50:52.555 DEBUG: 'Python Squid Sensor' skipped because there is no related file in current project
12:50:52.555 DEBUG: 'Cobertura Sensor for Python coverage' skipped because there is no related file in current project
12:50:52.555 DEBUG: 'PythonXUnitSensor' skipped because there is no related file in current project
12:50:52.555 DEBUG: 'PylintSensor' skipped because there is no related file in current project
12:50:52.556 DEBUG: 'PylintImportSensor' skipped because there is no related file in current project
12:50:52.557 DEBUG: 'Import of Bandit issues' skipped because there is no related file in current project
12:50:52.558 DEBUG: 'C (Community) RatsSensor' skipped because one of the required properties is missing
12:50:52.559 DEBUG: 'C (Community) CppCheckSensor' skipped because one of the required properties is missing
12:50:52.559 DEBUG: 'C (Community) PCLintSensor' skipped because there is no related rule activated in the quality profile
12:50:52.560 DEBUG: 'C (Community) DrMemorySensor' skipped because there is no related rule activated in the quality profile
12:50:52.561 DEBUG: 'C (Community) CxxCompilerVcSensor' skipped because there is no related rule activated in the quality profile
12:50:52.562 DEBUG: 'C (Community) VeraxxSensor' skipped because one of the required properties is missing
12:50:52.563 DEBUG: 'C (Community) ValgrindSensor' skipped because one of the required properties is missing
12:50:52.564 DEBUG: 'C (Community) ClangSASensor' skipped because one of the required properties is missing
12:50:52.565 DEBUG: 'C (Community) ExternalRulesSensor' skipped because there is no related rule activated in the quality profile
12:50:52.566 DEBUG: 'C (Community) CoverageSensor' skipped because one of the required properties is missing
12:50:52.567 DEBUG: 'SonarJS' skipped because there is no related file in current project
12:50:52.567 DEBUG: 'ESLint-based SonarJS' skipped because there is no related file in current project
12:50:52.568 DEBUG: 'SonarJS Coverage' skipped because there is no related file in current project
12:50:52.568 DEBUG: 'Import of ESLint issues' skipped because one of the required properties is missing
12:50:52.568 DEBUG: 'C# Properties' skipped because there is no related file in current project
12:50:52.570 DEBUG: 'Import of Checkstyle issues' skipped because there is no related file in current project
12:50:52.571 DEBUG: 'Import of PMD issues' skipped because one of the required properties is missing
12:50:52.571 DEBUG: 'Import of SpotBugs issues' skipped because there is no related file in current project
12:50:52.571 DEBUG: 'SurefireSensor' skipped because there is no related file in current project
12:50:52.572 DEBUG: 'JaCoCoSensor' skipped because there is no related file in current project
12:50:52.573 DEBUG: 'Flex' skipped because there is no related file in current project
12:50:52.573 DEBUG: 'Flex Cobertura' skipped because there is no related file in current project
12:50:52.573 DEBUG: 'XML Sensor' skipped because there is no related file in current project
12:50:52.574 DEBUG: 'PHP sensor' skipped because there is no related file in current project
12:50:52.574 DEBUG: 'Analyzer for "php.ini" files' skipped because there is no related file in current project
12:50:52.574 DEBUG: 'SonarTS' skipped because there is no related file in current project
12:50:52.575 DEBUG: 'SonarTS Coverage' skipped because there is no related file in current project
12:50:52.576 DEBUG: 'Import of TSLint issues' skipped because one of the required properties is missing
12:50:52.581 DEBUG: 'Generic Coverage Report' skipped because one of the required properties is missing
12:50:52.581 DEBUG: 'Generic Test Executions Report' skipped because one of the required properties is missing
12:50:52.582 DEBUG: 'C (Community) XunitSensor' skipped because one of the required properties is missing
12:50:52.582 DEBUG: 'C#' skipped because there is no related file in current project
12:50:52.582 DEBUG: 'C# Tests Coverage Report Import' skipped because there is no related file in current project
12:50:52.582 DEBUG: '[Deprecated] C# Integration Tests Coverage Report Import' skipped because there is no related file in current project
12:50:52.583 DEBUG: 'C# Unit Test Results Import' skipped because there is no related file in current project
12:50:52.583 DEBUG: Sensors : C (Community) SquidSensor -> C (Community) CxxCompilerGccSensor -> C (Community) ClangTidySensor -> JavaXmlSensor -> Zero Coverage Sensor
12:50:52.585 INFO: Sensor C (Community) SquidSensor [c]
12:50:52.592 WARN: Property 'sonar.c.includeDirectories' is not declared as multi-values/property set but was read using 'getStringArray' method. The SonarQube plugin declaring this property should be updated.
12:50:52.593 WARN: Property 'sonar.c.forceIncludes' is not declared as multi-values/property set but was read using 'getStringArray' method. The SonarQube plugin declaring this property should be updated.
12:50:52.593 WARN: Property 'sonar.c.cFilesPatterns' is not declared as multi-values/property set but was read using 'getStringArray' method. The SonarQube plugin declaring this property should be updated.
12:50:52.703 DEBUG: Cyclomatic complexity threshold: 10
12:50:52.704 DEBUG: Function size threshold: 20
12:50:52.718 DEBUG: global settings for: '/builds/stalb/demo_missing_rule_sonar_cxx_plugin/main.c'
12:50:52.718 DEBUG: Parse 'main.c' as C file, ends in '.c'
12:50:52.760 DEBUG: API File: main.c
12:50:52.760 DEBUG: Header file suffixes: [.h]
12:50:52.760 DEBUG: finished preprocessing '/builds/stalb/demo_missing_rule_sonar_cxx_plugin/main.c'
12:50:52.778 DEBUG: 'main.c' generated metadata with charset 'UTF-8'
12:50:52.802 DEBUG: Not enough content in 'main.c' to have CPD blocks, it will not be part of the duplication detection
12:50:52.810 WARN: Metric 'comment_lines_data' is deprecated. Provided value is ignored.
12:50:52.823 DEBUG: CxxFileLinesVisitor: '/builds/stalb/demo_missing_rule_sonar_cxx_plugin/main.c'
12:50:52.823 DEBUG:    lines:           '13'
12:50:52.823 DEBUG:    executableLines: '[12]'
12:50:52.823 DEBUG:    linesOfCode:     '[9, 11, 12]'
12:50:52.823 DEBUG:    linesOfComments: '[1, 3, 10]'
12:50:52.847 INFO: Sensor C (Community) SquidSensor [c] (done) | time=263ms
12:50:52.847 INFO: Sensor C (Community) CxxCompilerGccSensor [c]
12:50:52.847 INFO: Searching reports by relative path with basedir '/builds/stalb/demo_missing_rule_sonar_cxx_plugin' and search prop 'sonar.c.gcc.reportPath'
12:50:52.850 DEBUG: Normalized report includes to '[/builds/stalb/demo_missing_rule_sonar_cxx_plugin/gcc_report.log]'
12:50:52.850 DEBUG: Scanner uses normalized report path(s): '/builds/stalb/demo_missing_rule_sonar_cxx_plugin/gcc_report.log'
12:50:52.873 INFO: Parser will parse '1' report file(s)
12:50:52.873 INFO: Processing report '/builds/stalb/demo_missing_rule_sonar_cxx_plugin/gcc_report.log'
12:50:52.874 INFO: Parsing 'GCC' initialized with report '/builds/stalb/demo_missing_rule_sonar_cxx_plugin/gcc_report.log', Charset= 'UTF-8'
12:50:52.874 INFO: Using pattern : '(?<file>.*):(?<line>[0-9]+):[0-9]+:\x20warning:\x20(?<message>.*)\x20\[(?<id>.*)\]'
12:50:52.877 DEBUG: Scanner-matches file='main.c' line='11' id='-Wunused-variable' msg=unused variable 'i'
12:50:52.902 DEBUG: C-COMPILER-GCC processed = 1
12:50:52.902 INFO: C-COMPILER-GCC processed = 1
12:50:52.903 INFO: Sensor C (Community) CxxCompilerGccSensor [c] (done) | time=56ms
12:50:52.903 INFO: Sensor C (Community) ClangTidySensor [c]
12:50:52.903 INFO: Searching reports by relative path with basedir '/builds/stalb/demo_missing_rule_sonar_cxx_plugin' and search prop 'sonar.c.clangtidy.reportPath'
12:50:52.904 DEBUG: Normalized report includes to '[/builds/stalb/demo_missing_rule_sonar_cxx_plugin/clangtidy_report.log]'
12:50:52.904 DEBUG: Scanner uses normalized report path(s): '/builds/stalb/demo_missing_rule_sonar_cxx_plugin/clangtidy_report.log'
12:50:52.904 INFO: Parser will parse '1' report file(s)
12:50:52.904 INFO: Processing report '/builds/stalb/demo_missing_rule_sonar_cxx_plugin/clangtidy_report.log'
12:50:52.905 DEBUG: Parsing 'clang-tidy' report, CharSet= 'UTF-8'
12:50:52.907 DEBUG: C-CLANG-TIDY processed = 2
12:50:52.907 INFO: C-CLANG-TIDY processed = 2
12:50:52.907 INFO: Sensor C (Community) ClangTidySensor [c] (done) | time=4ms
12:50:52.907 INFO: Sensor JavaXmlSensor [java]
12:50:52.909 INFO: Sensor JavaXmlSensor [java] (done) | time=2ms
12:50:52.909 INFO: Sensor Zero Coverage Sensor
12:50:52.927 INFO: Sensor Zero Coverage Sensor (done) | time=18ms
12:50:52.930 INFO: ------------- Run sensors on project
12:50:52.938 DEBUG: 'Java CPD Block Indexer' skipped because there is no related file in current project
12:50:52.938 DEBUG: Sensors : 
12:50:52.943 INFO: SCM provider for this project is: git
12:50:52.944 INFO: 1 files to be analyzed
12:50:53.014 DEBUG: Blame file main.c
12:50:53.166 INFO: 1/1 files analyzed
12:50:53.169 INFO: 1 file had no CPD blocks
12:50:53.169 INFO: Calculating CPD for 0 files
12:50:53.170 INFO: CPD calculation finished
12:50:53.350 INFO: Analysis report generated in 179ms, dir size=234 KB
12:50:53.370 INFO: Analysis report compressed in 20ms, zip size=35 KB
12:50:53.371 INFO: Analysis report generated in /builds/stalb/demo_missing_rule_sonar_cxx_plugin/.scannerwork/scanner-report
12:50:53.371 DEBUG: Upload report
12:50:53.406 DEBUG: POST 200 https://SonarCE.instance.org/sonar/api/ce/submit?projectKey=demo_missing_rule_sonar_cxx_plugin&projectName=demo_missing_rule_sonar_cxx_plugin&characteristic=branch%3Dmaster&characteristic=branchType%3DLONG | time=33ms
12:50:53.408 INFO: Analysis report uploaded in 37ms
12:50:53.410 INFO: ANALYSIS SUCCESSFUL, you can browse https://SonarCE.instance.org/sonar/dashboard?id=demo_missing_rule_sonar_cxx_plugin&branch=master
12:50:53.411 INFO: Note that you will be able to access the updated dashboard once the server has processed the submitted analysis report
12:50:53.411 INFO: More about the report processing at https://SonarCE.instance.org/sonar/api/ce/task?id=AWohDOFU4uFZCednbAYV
12:50:53.412 DEBUG: Report metadata written to /builds/stalb/demo_missing_rule_sonar_cxx_plugin/.scannerwork/report-task.txt
12:50:53.553 DEBUG: 'GitLab Commit Issue Publisher' skipped because one of the required properties is missing
12:50:53.555 DEBUG: Post-jobs : Final report
12:50:53.555 INFO: Executing post-job 'Final report'
12:50:53.555 WARN: Source code parser: 2 syntax error(s) detected. Syntax errors could cause invalid software metric values. Root cause are typically missing includes, missing macros or compiler specific extensions.
12:50:53.558 INFO: Analysis total time: 6.184 s
12:50:53.644 INFO: ------------------------------------------------------------------------
12:50:53.644 INFO: EXECUTION SUCCESS
12:50:53.644 INFO: ------------------------------------------------------------------------
12:50:53.645 INFO: Total time: 13.045s
12:50:53.779 INFO: Final Memory: 24M/342M
12:50:53.780 INFO: ------------------------------------------------------------------------
Job succeeded

Related information

  • c plugin version: 1.2.2 (build 1653)
  • SonarQube version: 7.6.0.21501
@guwirth
Copy link
Collaborator

guwirth commented Apr 16, 2019

Hello @stalb,

thanks for your feedback.

Maybe you can provide an update?

Regards,

@stalb
Copy link
Contributor Author

stalb commented Apr 16, 2019

clang version 3.8.1-24 (tags/RELEASE_381/final)

I tried the script, but my warning isn't in the generated list.

@stalb
Copy link
Contributor Author

stalb commented Apr 16, 2019

Hi @guwirth

I also tried to use clangtidy_createrules.py script.
The warning is in include/clang/Basic/DiagnosticParseKinds.td
Using Tablegen to create the the json file is also working (the error is in the generated json file).

However using clangtidy_createrules.py on the json doesn't put it in the generated xml rule file (and it's not the only one...).

@guwirth
Copy link
Collaborator

guwirth commented Apr 19, 2019

Hi @stalb,

Think the idea of the Python script is to run clang -cc1 -analyzer-checker-help and extract the messages from the resulting output.

I’m not using clang. So maybe first test would be if missing messages are listed in the output?
If not: Is this an general issue that not all messages are listed or an bug of clang?

Second point: precondition is that source code is error free. In your sample the missing ; creates an syntax error. Syntax analysis is normally not the job of an static code analyzer.

Regards,

@stalb
Copy link
Contributor Author

stalb commented Apr 19, 2019

Think the idea of the Python script is to run clang -cc1 -analyzer-checker-help and extract the messages from the resulting output.

It definitively isn't in this list.

I’m not using clang. So maybe first test would be if missing messages are listed in the output?
If not: Is this an general issue that not all messages are listed or an bug of clang?
So you have code that compiles with warnings but the plugin won't take them into account because the script which is in charge of generating the rules has chosen not to. Seems like an issue for me.

Clang reports the warnings, clang-tidy reports them also with the id clang-diagnostic-warning. The plugin discards them because there isn't any corresponding rule.

The script in charge of defining clang-tidy rules based on clang warnings, is clangtidy_createrules.py script.
For that it uses clang/include/clang/Basic/Diagnostic.td file which lists all clang warnings, errors, notes and remarks. However by design, it has chosen to ignore all warnings that are not associated with a specific warning switch (as -Wunused-variable or -Wextra-semi).

We could solve the problem by adding manualy a rule with id clang-diagnostic-warning, but since the other rules are generated with the script, I'm not sure it would be acceptable.

Second point: precondition is that source code is error free. In your sample the missing ; creates an syntax error. Syntax analysis is normally not the job of an static code analyzer.

I'm ok with that, however the code compiles, with warnings but compiles.
So what, the plugin will not report warnings about missing semicolon because it's syntaxic but reports warnings about semicolon in excess since that's truly static ?

Regards,

@guwirth
Copy link
Collaborator

guwirth commented Apr 20, 2019

However by design, it has chosen to ignore all warnings that are not associated with a specific warning switch (as -Wunused-variable or -Wextra-semi).

Hi @stalb for me it would be ok to extend the script to support also these warnings. You were saying there are also warnings without id? Here we should follow the GCC approach from #1708.

@guwirth guwirth added this to the 1.3.0 milestone Apr 20, 2019
@stalb
Copy link
Contributor Author

stalb commented Apr 26, 2019

As GCC, CLANG has warnings enabled with activation switches and warnings without activation switch.
For example compiling the test sample with clang produces the following output:

$clang -c -Wall main.c -o main
main.c:6:7: warning: expected ';' at end of declaration list
        int b
             ^
             ;
main.c:11:6: warning: unused variable 'i' [-Wunused-variable]
        int i;
            ^

Where the 1st warning has no activation switch and the 2nd is associated to -Wunused-variable.

Clang-tidy maps CLANG warnings with names starting with clang-diagnostic-.
So using clang-tidy on the same example the following output is produced:

$clang-tidy main.c
2 warnings generated.
main.c:6:7: warning: expected ';' at end of declaration list [clang-diagnostic-warning]
        int b
             ^
             ;
main.c:11:6: warning: unused variable 'i' [clang-diagnostic-unused-variable]
        int i;
            ^

So we have clang-diagnostic-warning for the first warning (which don't have a warning option) and
clang-diagnostic-unused-variable for the 2nd warning (which activation switch is -Wunused-variable).

Because the first warning don't have a warning option (as a hundred others) , it is ignored by the rule generation script and no rule with id clang-diagnostic-warning is generated.

So actually there is only one rule missing, since all clang warnings without warning option are mapped to clang-diagnostic-warning .
Do you think it's worth changing the script for a single missing rule? It would be simpler to just add the missing rule in clangtidy.xml

@guwirth
Copy link
Collaborator

guwirth commented Apr 26, 2019

Hi @stalb,

adding the rule manually has a high risk for regression. Next time someone is using the script to create clangtidy.xml the rule will be missing again. Adding clang-diagnostic-warning should at least be verified with a unit test.

Regards,

@cjmang
Copy link

cjmang commented Jul 3, 2019

@guwirth I've tried my hand at working on this issue. I've updated the clangtidy_createrules.py to add the clang-diagnostic-warning rule directly to the clangtidy_rules.xml file. However, when I run the script to actually regenerate the clangtidy rules I'm getting a huge amount of changes to the clangtidy.xml file. Is this expected? Also, which method of generating the rulesets is preferred, using the clang-tidy RST files, or using the clang diagnostics?

@stalb
Copy link
Contributor Author

stalb commented Jul 5, 2019

The rules of clang-tidy were last updated 9 months ago.
Clang and Clang-Tidy have both evolved since...

For the clang-diagnostic-warning rule, since it's related to clang warnings I would add it in the clang diagnostics rules:

  • clang diagnostics rules correspond to clang warnings
  • clang-tidy RST files correspond to clang-tidy own checks

My understanding is you need to run both generating method in order to update the ruleset.

@guwirth
Copy link
Collaborator

guwirth commented Jul 12, 2019

closed with #1730

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

No branches or pull requests

3 participants