odftoolkit icon indicating copy to clipboard operation
odftoolkit copied to clipboard

Run static analysis on merge/commit

Open xzel23 opened this issue 7 months ago • 5 comments

I think we should add some static analysis tool that detects common programming errors to the Github pipeline. There are several options free of charge for Open Source projects, including free tools like SpotBugs, PMD, and CheckStyle, and even commercial ones like SonarQube and Qodana.

I have some experience with both Qodana (setting it up on GitHub and using it) and SonarQube (only as a user - the CI pipeline at my day job uses it) and could prepare something.

I also run SpotBugs on my personal projects, both local and in CI, but SB shows a lot of false positives which can lead people to ignore the SB warnings completely or simply disabling it for whole classes.

xzel23 avatar May 21 '25 08:05 xzel23

@xzel23 I do like the idea! We have currently already a normalisation of white space (pure syntax, while you talk about semantics - but it is simliar static test), as there was in the past a lot of whitespace noise, of people changing a line, and the IDE is doing some auto-indent to the preferred configuration of the IDE - tabs vs. spaces, any indent at will...

see https://github.com/tdf/odftoolkit/blob/master/pom.xml#L361

svanteschubert avatar May 21 '25 11:05 svanteschubert

Ah, I learned something about a neat new plugin today, thank you!

I'll try to set up Qodana (because that's what I know) on my fork first and when it works, let's move it upstream.

xzel23 avatar May 21 '25 13:05 xzel23

for LO we have used Coverity and others before, but not as part of CI; if this should run in CI we need pointers to documentation about how to suppress any inevitable false positives that the tools may detect - they usually provide special comments for this - somewhere prominent in the README or so.

mistmist avatar May 22 '25 07:05 mistmist

OK, so I have set up both Qodana and Sonar:

Qodana

I really like the interface to drill down based on categories, but the problem is, to see the report, it seems you need to have a JetBrains account. That's no problem for me as I have one, but other people on the project might not.

Qodana in the default configuration reports 1097 issues for the main branch with the default configuration.

So as much as I like Qodana, I think we should use another scanner.

SonarCloud

SonarCloud only scans the main branch on the free tier. I configured the scan so that it only runs on the main branch because otherwise the scan runs but the results cannot be accessed, which seems like a waste of resources.

Sonar reports more than 4000 issues for the main branch.

The results of the scan of my fork of ODF Toolkit are publicly available on SonarCloud.

The interface might need some time to get used to, but IMHO it is worth running the analysis and getting used to Sonar - it's also a good thing to know the job market ;)

False positives: there certainly will be some from time to time, but in my experience, this is rather rare. These can be marked right in the interface with an explanation why it is not an issue. As long as we don't introduce Quality Gates (that means failing the build based on inspection results), I'd rather not add comments that disable inspections to the codebase. If a certain rule simply doesn't fit with the project, we should change the Sonar configuration.

Issues found by Sonar are put under different categories and the interface lets you apply filters to focus on what is relevant to you. But I recommend using the SonarQube plugin for your IDE and connect it to the SonarCloud instance. This way, whenever you open a source file, the inspection results for the opened file are displayed in the sonar tool window, along with information about the issue, like here in IntelliJ (Eclipse should look almost the same, but I haven't used Eclipse in three years):

Image

Configuration

I cannot prepare a ready-to-merge pull request for this one because the repository owner has to do some steps to integrate the SonarCloud analysis.

  • The repository owner has to sign up for a Sonar account (several options are available, including authentication by GitHub) and create an organization (or maybe the OpenDocument Foundation already has one?) and a project. Check the detailed instructions.

  • The change to the POM described in the documentation is not necessary - we just set the necessary fields directly on the mvn command line in the YML file. Replace maven.yml with the attached file (rename to maven.yml because I cannot attach yml files) and change these two lines to your Sonar organization and project:

          -Dsonar.projectKey=xzel23_odftoolkit
          -Dsonar.organization=xzel23

maven.txt

  • You also have to add a GitHub secret "SONAR_TOKEN" in the repository settings. If you have not created a token when signing up, you can generate one on your Sonar profile:
Image
  • Then copy the token text and create a GitHub repository secret with name "SONAR_TOKEN" and the token text you copied:
Image
  • When ready, push the updated maven.yml to master. Check the GitHub "Action" tab to see if everything works.

  • Try it out and if everything works, the Sonar coordinates should be published on the project page so that every developer can connect to the instance and get the issues right in their IDE.

xzel23 avatar May 24 '25 14:05 xzel23

@cloph TDF has a Sonar account - this needs some project settings such as SONAR_TOKEN to enable it - Axel's PR is in https://github.com/tdf/odftoolkit/pull/407

mistmist avatar Aug 04 '25 10:08 mistmist