SonarSource / sonar-scanner-msbuild

SonarScanner for .NET
http://redirect.sonarsource.com/doc/msbuild-sq-runner.html
GNU Lesser General Public License v3.0
361 stars 143 forks source link

The `GetAnalyzerSettings` task is removing existing source generators for test projects when `sonar.dotnet.excludeTestProjects` is set to true #1469

Open csaba-sagi-sonarsource opened 1 year ago

csaba-sagi-sonarsource commented 1 year ago

The issue was originally reported by the community: https://community.sonarsource.com/t/sonarqube-scanner-seems-to-interfere-with-net-source-generators-refit/82712

Reproduction steps

  1. Create a test project that uses classes that are generated by a source generator
  2. Make sure sonar.dotnet.excludeTestProjects is set to true
  3. call begin step
  4. call dotnet build -> this will fail because the source generator was removed by the GetAnalyzerSettings, so the generated type is not generated and is unknown in the test project.

Version used:

dotnet-sonarscanner 5.11.0

csaba-sagi-sonarsource commented 1 year ago

Should have a very similar solution like https://github.com/SonarSource/sonar-scanner-msbuild/issues/1211

PavelFischerCoupa commented 8 months ago

We run into the similar issue: If we use sonar, generator output is not present in the compiled dll. Any plans to fix it, or is there any workarounds? But in our case it is independent of setting. We use version 5.15 for dotnet-sonarscanner and server Version 8.3.1

siewers commented 3 months ago

I just hit the same issue using SonarCloud. Are there any workarounds to this besides setting sonar.dotnet.excludeTestProjects to false?

PavelFischerCoupa commented 3 months ago

In my case it works now with the 6.2 version of scanner. sonar.dotnet.excludeTestProjects is in default value, no additional changes.

siewers commented 3 months ago

That's strange. I'm using 6.2 as well, but I'm seeing the exact error as described. I have a couple of nested partial classes using [LoggerMessage] and they show up with error in the build. If I remove the parameter, the build succeeds.

pavel-mikula-sonarsource commented 3 months ago

You should be able to add <SonarQubeExclude>true</SonarQubeExclude> to your test project csproj/vbproj files instead of setting sonar.dotnet.excludeTestProjects to true.

siewers commented 3 months ago

Yes, but I'm building an Azure DevOps template that should be used with hundreds of projects and I don't want to have everyone update their projects just to get Sonar to skip scanning their test projects. I found the idea of automatic test project detection a great feature, but if I cannot use source generators, it's a showstopper and I have to rely on manual filtering instead.

pavel-mikula-sonarsource commented 3 months ago

@siewers, are you able to detect test projects based on their name? You should be able to do this globally via targets file (either directory.build.tagets, or a file dropped into MSBuild's Microsoft.Comon.targets\ImportBefore directory.

You can go one step further and elaborate more complex target file that would rely on our internal SonarQubeTestProject property to detect the test project. That would require hooking after SonarCategoriseProject and before SonarWriteProjectData to work. See https://github.com/SonarSource/sonar-scanner-msbuild/blob/master/src/SonarScanner.MSBuild.Tasks/Targets/SonarQube.Integration.targets

It's a workaround, but it should work.

pavel-mikula-sonarsource commented 3 months ago

From looking at the code: GetAnalyzerSettings calls CreateDeactivatedProjectSettings that removes all analyzers except Sonar. And sends a deactivated quality profile. We could simply merge the analyzers instead and rely on users to set their external analyzers as they want during analysis. As we can't (easily) distinguish between DLLs with analyzers, DLLs with source generators and single DLLs with both.

Alternatively, we could inspect the DLLs, keep them all and craft a QP file deactivating all found external rules.

siewers commented 3 months ago

@pavel-mikula-sonarsource, we rely on the same project naming scheme, so I can exclude test projects using a glob pattern instead, like **/*Tests*/**/*. It's not ideal, as it restricts the naming conventions our teams can use, but in my case, it's a decided convention, so there are no exceptions to this.

I'm confused about this, though. What is the difference between using sonar.test.inclusions=**/*Tests*/**/* and setting sonar.dotnet.excludeTestProjects to true?

pavel-mikula-sonarsource commented 3 months ago

That's moving out of the scope of this ticket. https://community.sonarsource.com/ is a better place to continue the discussion