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

Best way to add/extend Clang Tidy rules defined in this plugin ? #2378

Closed
aallrd opened this issue May 24, 2022 · 4 comments
Closed

Best way to add/extend Clang Tidy rules defined in this plugin ? #2378

aallrd opened this issue May 24, 2022 · 4 comments
Labels

Comments

@aallrd
Copy link

aallrd commented May 24, 2022

Hello,

I am using your plugin to parse and display Clang Tidy / Warnings (diagnostics) reports in SonarQube and it is working great.
As far as I understood, the rules used to match the content from the above reports come from the plugin itself:

From what I gathered, this file is generated using generate_clangtidy_resources.cmd and with every new release of LLVM/Clang this XML rule file is rebuilt and a new release of the plugin is done to update the list/content of Clang Tidy / Warnings (diagnostics) rules.

I have a custom build of LLVM/Clang with my own custom Clang Tidy checks defined besides the standard ones, and I would like to add rules in SonarQube to display these checks along the default ones.

The ideas I had to do that:

  1. Manually create the rule using the Rule template for Clang-Tidy custom rules

Cumbersome and error prone, but seems like the only manual/UI solution available...

  1. Use the provided generate_clangtidy_resources.sh to generate the clangtidy.xml for my custom LLVM/Clang build

Seems ideal because reusing the same automated workflow used by the plugin, however I have no clue what to do next once I have generated my custom clangtidy.xml, do I rebuild the plugin or simply copy/paste this file on the SonarQube server?

  1. Create my own CXX plugin following https://github.com/SonarOpenCommunity/cxx-custom-checks-example-plugin

Seems a bit too much just to add a few rules.

  1. Something else?

What would be the best way to proceed in your opinion?

Thank you.

@aallrd
Copy link
Author

aallrd commented May 24, 2022

An additional question on option 2 (which for now I think is the best way to move forward): running the generate_clangtidy_resources script generates two files:

  • clangtidy_new.xml (clang-tidy rules)
  • diagnostic_new.xml (clang diagnostics rules)

Do you have a documented way of merging these two files into the single clangtidy.xml that is used by the plugin?
It reminds me your comments on #2325 where you suggested to keep these rules separate.

@guwirth
Copy link
Collaborator

guwirth commented May 24, 2022

Hello @aallrd,

As far as I understood, the rules used to match the content from the above reports come from the plugin itself: clangtidy.xml

Yes, this file is embedded into the plugin.

From what I gathered, this file is generated using generate_clangtidy_resources.cmd

Yes, the script creates the files clangtidy_new.xml and diagnostic_new.xml. Merging it into the final clangtidy.xml is a manual step. 2325 is a n idea for the future to split these files.

What would be the best way to proceed in your opinion?

First of all I think merging your custom rules into the cxx plugin makes no sense. Means you have to create a fork.

The official way is to convert it to the Generic Issue Format and read it with this sensor then:
https://docs.sonarqube.org/latest/analysis/generic-issue/. Maybe this helps also for a better understanding: https://community.sonarsource.com/t/createrepository-vs-createexternalrepository/26102.

I was also thinking about creating a new sensor supporting the External issues format:

  1. External issue created without (external) rule definition (an empty one will be created automatically) using newExternalIssue(). That is what Generic Issue Format is doing.
  2. External rule + External issue created during analysis (=adhoc rule). Using newAdHocRule() 2 + newExternalIssue()
  3. External rule defined in advance in the SQ server using createExternalRepository() then external issues created during the analysis.

For 2 and 3 a new sensor is necessary. My idea was to reuse (read) the already existing files from sonar.cxx.other.rules with createExternalRepository. Using the External API the file could be local and extendable.

Your proposed option 2 has the disadvantage that you have to build a new plugin every time your rules are changing. You have to deploy it on your server(s) and have to restart the server and have to add it to a profile before they are usable.

Regards,

@aallrd
Copy link
Author

aallrd commented May 30, 2022

Hello,

After giving it more thought, I think the best way for my use case (custom Clang Tidy rules from our own LLVM/Clang fork) is the option 2 from my initial post:

  • Replacing the clangtidy.xml file inside the Sonar CXX release JAR by my custom one.

There are several pros to this solution:

  • Using the project official scripts to produce the rules directly from the LLVM/Clang sources (PR Adding mergerules command to generate a merged rule file for Clang rules #2382 pending)
  • Using our own LLVM/Clang fork directly as source for the Sonar CXX Clang rules
  • The set of rules is strictly aligned with the LLVM/Clang compiler we use in production
  • Same nice HTLML output for our custom Clang Tidy rules in Sonar since built from the LLVM/Clang .rst files

The only cons are:

  • Need to patch the Sonar CXX release JAR to update the clangtidy.xml
    • Easy to do with jar -uf for a single file
  • Need to restart the Sonar server whenever we need to update the rules
    • Depends on the rules update frequency: very low currently in my case
    • A maintenance process can be put in place to handle this task

@guwirth guwirth closed this as completed May 31, 2022
@guwirth guwirth reopened this May 31, 2022
@guwirth
Copy link
Collaborator

guwirth commented May 31, 2022

Hi @aallrd,

If that works for you, there's nothing wrong with it.
When I have time, I'll try whether adhoc rules work, stay tuned.

Regards,

@guwirth guwirth closed this as completed May 31, 2022
@guwirth guwirth unpinned this issue May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants