Incorrect decorations in PR (won't fix code smells)

flobernd · August 3, 2022, 2:19pm

Hey there,

when running the CI for a PR (see below URL), incorrect decorations are shown in the PR as well as in the build log.

For example:

These code smells are marked as “won’t fix” and don’t show up as “open” on the SonarCloud panel, but still in the PR annotations:

The build reflects the same problem:

ALM used

GitHub

CI system used

GitHub Actions

Scanner command used when applicable (private details masked)

github.com

flobernd/zysharp-metaprogramming/blob/master/.github/workflows/build.yml#L66


      
          - name: Install SonarCloud Scanner
            if: steps.cache-sonar-scanner.outputs.cache-hit != 'true'
            run: |
              mkdir -p ./.sonar/scanner
              dotnet tool update dotnet-sonarscanner --tool-path ./.sonar/scanner
          
          
- name: SonarCloud Begin Analysis
            env:
              GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
              SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
            run: >
              ./.sonar/scanner/dotnet-sonarscanner begin
              /k:"${{ env.SONAR_PROJECT }}"
              /o:"${{ env.SONAR_ORGANIZATION }}"
              /d:sonar.login="${{ secrets.SONAR_TOKEN }}"
              /d:sonar.host.url="https://sonarcloud.io"
              /d:sonar.cs.opencover.reportsPaths="**/TestResults/**/coverage.opencover.xml"
              /d:sonar.cs.vstest.reportsPaths="**/TestResults/*.trx"
          
          
- name: Cache NuGet Packages
            uses: actions/cache@v3

Languages of the repository

C#

Only if the SonarCloud project is public, the URL

flobernd · August 5, 2022, 10:25am

push …

Colin · August 5, 2022, 3:35pm

Please do not bump threads.

flobernd · August 9, 2022, 9:52am

Sure, no problem. But the issue is still not solved. What do you suggest me in order to get proper help?

flobernd · August 14, 2022, 9:50pm

Sorry, but the issue still exists. As you did not give me any suggestions yet, I have to bump this again.

Colin · August 15, 2022, 9:24am

Hey there.

To be super clear – bumping tends to have the opposite effect in this community (making it less likely to get attention than more). It can take time for the right team to take a look, and this doesn’t appear to be blocking your pull requests from merging.

GitHub itself is decorating your PRs with these issues – not SonarCloud. The way that C# analysis works on SonarCloud is that Roslyn analyzers are executed during your build – and whenever the analyzer finds an issue, it’s emitted as a warning. These warnings are then imported to SonarCloud when you run the final step of analysis, and only then filtered for what issues have already been marked as false-positive (or files excluded, etc.).

So there’s nothing that we can really do here – it looks like this feature is in Beta on GitHub’s side based on the screenshots you’ve shared. Maybe this is feedback worth sharing to them.

flobernd · August 15, 2022, 11:15am

The way that C# analysis works on SonarCloud is that Roslyn analyzers are executed during your build – and whenever the analyzer finds an issue, it’s emitted as a warning. These warnings are then imported to SonarCloud when you run the final step of analysis, and only then filtered for what issues have already been marked as false-positive (or files excluded, etc.).

Thanks for clarification. That’s what I guessed.

GitHub itself is decorating your PRs with these issues – not SonarCloud.

This part confuses me to be honest. The annotations are added by the “check” action called “SonarCloud Code Analysis”.

If I do not run the “sonarscanner /end” step, there is no “check step” in the GitHub Actions and no annotations as well.

I’m currently evaluating SonarCloud for a much bigger project and what we expect is that the Sonar warnings (not the Roslyn ones) are not displayed if we previously marked them as e.g. “won’t fix”.

The wrong annotations are all Sonar warnings and no inbuild Roslyn ones. Some of the Sonar warnings are not showing up, others are still visible.

Couldn’t it be the case that we are facing a bug in the internal filtering?

Colin · August 15, 2022, 12:59pm

Are you sure? That’s not what I see in the screenshot you shared.

There would be no “SonarCloud Code Analysis” check (because the results never hit the server, and therefore the PR isn’t decorated). And I would understand if removing both the begin and end step caused this, but not just removing theend. Do you have a specific PR where you see this behavior?

I want to emphasize that in-line issue annotations aren’t a feature of SonarCloud. It annoys quite a few users and it’s an item on our roadmap, but as far as I can tell this is all coming from GitHub picking up the warnings from the build step.

flobernd · August 15, 2022, 1:33pm

Do you have a specific PR where you see this behavior?

I will try to reproduce this again and come back to you with a link

I want to emphasize that in-line issue annotations aren’t a feature of SonarCloud. It annoys quite a few users and it’s an item on our roadmap, but as far as I can tell this is all coming from GitHub picking up the warnings from the build step.

Mhh okay I see. I thought the Sonar check was actively calling some kind of GitHub API that adds these annotations. They are super useful and I would love to see such feature being implemented in Sonar. I will do some research if- and how GitHub might implicitly grab warnings from the build output.

flobernd · August 15, 2022, 2:05pm

Oof. I found the issue.

Our workflow uses the GitHub - actions/setup-dotnet: Set up your GitHub Actions workflow with a specific version of the .NET core sdk action which apparently registers error matchers for dotnet. This one seem to produce the annotations.

There as well is an open issue that suggests to only show new annotations instead of every single old one:

github.com/actions/toolkit

"Unchanged files with check annotations" is disruptive and should be configurable

opened 12:04PM - 14 May 20 UTC

ThiefMaster

enhancement external

The "Unchanged files with check annotations" feature when using problem matchers… needs to be configurable (for each action/matcher): It's pretty discouraging for a new contributor to see loads of unrelated warnings from a style checker in their first PR that are completely unrelated, not to mention that it also adds noise for checkers where you know that it's not something that *could* be caused in changes from another file in the PR. For example, I'm running stylelint with `no-descending-specificity` warnings enabled. For CSS added in a PR those warnings may be useful, but I *REALLY* don't care about the almost 1000 warnings in our existing codebase (some even in legacy files) when running CI for a new PR! Right now the only workaround for this is to either disable the rule altogether, only show errors but not warnings, or somehow only run the linter on new/changed files when running the action for a PR.

Colin · August 15, 2022, 2:21pm

Thanks for the follow-up! It’s really useful if another user faces the same (and I would have never figured out it was setup-dotnet)

flobernd · August 16, 2022, 8:55am

Took me a while to figure this out as well. Thanks for the hint in the right direction!

For anybody trying to disable the .NET error matcher after using the setup-dotnet action, just add another step to your workflow (after the dotnet setup) and run the following command in the shell:

echo "::remove-matcher owner=csc::"

I upvoted the annotation feature on your roadmap.