search

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 when `sonar.cs.roslyn.ignoreIssues` is set to true #1211

Closed costin-zaharia-sonarsource closed 2 years ago

costin-zaharia-sonarsource commented 2 years ago

This issue was originally reported on sonar-dotnet: https://github.com/SonarSource/sonar-dotnet/issues/5466

Reproduction steps

  1. Download the repro project from here: https://github.com/SonarSource/sonar-dotnet/issues/5466#issuecomment-1061671771
  2. Make sure sonar.cs.roslyn.ignoreIssues is set to true
  3. call begin step
  4. call dotnet build -> this will fail with the CS8795 error

This happens because Microsoft.Extensions.Logging.Generators.dll is not passed to the compiler.

Looking at the binary logs, the OriginalAnalyzers parameter is:

/usr/share/dotnet/sdk/6.0.200/Sdks/Microsoft.NET.Sdk.Web/analyzers/cs/Microsoft.AspNetCore.Analyzers.dll
/usr/share/dotnet/sdk/6.0.200/Sdks/Microsoft.NET.Sdk.Web/analyzers/cs/Microsoft.AspNetCore.Mvc.Analyzers.dll
/usr/share/dotnet/sdk/6.0.200/Sdks/Microsoft.NET.Sdk.Web/analyzers/cs/Microsoft.AspNetCore.Components.Analyzers.dll
/usr/share/dotnet/sdk/6.0.200/Sdks/Microsoft.NET.Sdk/targets/../analyzers/Microsoft.CodeAnalysis.CSharp.NetAnalyzers.dll
/usr/share/dotnet/sdk/6.0.200/Sdks/Microsoft.NET.Sdk/targets/../analyzers/Microsoft.CodeAnalysis.NetAnalyzers.dll
/usr/share/dotnet/packs/Microsoft.NETCore.App.Ref/6.0.2/analyzers/dotnet/cs/System.Text.Json.SourceGeneration.dll
/usr/share/dotnet/packs/Microsoft.AspNetCore.App.Ref/6.0.2/analyzers/dotnet/cs/Microsoft.AspNetCore.App.Analyzers.dll
/usr/share/dotnet/packs/Microsoft.AspNetCore.App.Ref/6.0.2/analyzers/dotnet/cs/Microsoft.AspNetCore.App.CodeFixes.dll
/usr/share/dotnet/packs/Microsoft.AspNetCore.App.Ref/6.0.2/analyzers/dotnet/roslyn4.0/cs/Microsoft.Extensions.Logging.Generators.dll
/usr/share/dotnet/sdk/6.0.200/Sdks/Microsoft.NET.Sdk.Razor/targets/../source-generators/Microsoft.AspNetCore.Razor.SourceGenerator.Tooling.Internal.dll
/usr/share/dotnet/sdk/6.0.200/Sdks/Microsoft.NET.Sdk.Razor/targets/../source-generators/Microsoft.NET.Sdk.Razor.SourceGenerators.dll

and the OutputItems parameter is:

/tmp/.sonarqube/resources/0/SonarAnalyzer.CSharp.dll
/tmp/.sonarqube/resources/0/SonarAnalyzer.CFG.dll
/tmp/.sonarqube/resources/0/Google.Protobuf.dll
/tmp/.sonarqube/resources/0/SonarAnalyzer.dll
/tmp/.sonarqube/resources/1/SonarAnalyzer.CFG.dll
/tmp/.sonarqube/resources/1/SonarAnalyzer.VisualBasic.dll
/tmp/.sonarqube/resources/1/Google.Protobuf.dll
/tmp/.sonarqube/resources/1/SonarAnalyzer.dll
/tmp/.sonarqube/resources/2/Microsoft.AspNetCore.Razor.Language.dll
/tmp/.sonarqube/resources/2/SonarAnalyzer.Security.dll

All the previous files were removed.

When sonar.cs.roslyn.ignoreIssues is set to false:

OriginalAnalyzers

C:\Program Files\dotnet\sdk\6.0.200\Sdks\Microsoft.NET.Sdk.Web\analyzers\cs\Microsoft.AspNetCore.Analyzers.dll
C:\Program Files\dotnet\sdk\6.0.200\Sdks\Microsoft.NET.Sdk.Web\analyzers\cs\Microsoft.AspNetCore.Mvc.Analyzers.dll
C:\Program Files\dotnet\sdk\6.0.200\Sdks\Microsoft.NET.Sdk.Web\analyzers\cs\Microsoft.AspNetCore.Components.Analyzers.dll
C:\Program Files\dotnet\sdk\6.0.200\Sdks\Microsoft.NET.Sdk\targets\..\analyzers\Microsoft.CodeAnalysis.CSharp.NetAnalyzers.dll
C:\Program Files\dotnet\sdk\6.0.200\Sdks\Microsoft.NET.Sdk\targets\..\analyzers\Microsoft.CodeAnalysis.NetAnalyzers.dll
C:\Program Files\dotnet\packs\Microsoft.NETCore.App.Ref\6.0.2\analyzers/dotnet/cs/System.Text.Json.SourceGeneration.dll
C:\Program Files\dotnet\packs\Microsoft.AspNetCore.App.Ref\6.0.2\analyzers/dotnet/cs/Microsoft.AspNetCore.App.Analyzers.dll
C:\Program Files\dotnet\packs\Microsoft.AspNetCore.App.Ref\6.0.2\analyzers/dotnet/cs/Microsoft.AspNetCore.App.CodeFixes.dll
C:\Program Files\dotnet\packs\Microsoft.AspNetCore.App.Ref\6.0.2\analyzers/dotnet/roslyn4.0/cs/Microsoft.Extensions.Logging.Generators.dll
C:\Program Files\dotnet\sdk\6.0.200\Sdks\Microsoft.NET.Sdk.Razor\targets\..\\source-generators\Microsoft.AspNetCore.Razor.SourceGenerator.Tooling.Internal.dll
C:\Program Files\dotnet\sdk\6.0.200\Sdks\Microsoft.NET.Sdk.Razor\targets\..\\source-generators\Microsoft.NET.Sdk.Razor.SourceGenerators.dll

OutputItems:

C:\Users\costin.zaharia\AppData\Local\Temp\.sonarqube\resources\1\Google.Protobuf.dll
C:\Users\costin.zaharia\AppData\Local\Temp\.sonarqube\resources\1\SonarAnalyzer.CFG.dll
C:\Users\costin.zaharia\AppData\Local\Temp\.sonarqube\resources\1\SonarAnalyzer.CSharp.dll
C:\Users\costin.zaharia\AppData\Local\Temp\.sonarqube\resources\1\SonarAnalyzer.dll
C:\Users\costin.zaharia\AppData\Local\Temp\.sonarqube\resources\2\Google.Protobuf.dll
C:\Users\costin.zaharia\AppData\Local\Temp\.sonarqube\resources\2\SonarAnalyzer.CFG.dll
C:\Users\costin.zaharia\AppData\Local\Temp\.sonarqube\resources\2\SonarAnalyzer.dll
C:\Users\costin.zaharia\AppData\Local\Temp\.sonarqube\resources\2\SonarAnalyzer.VisualBasic.dll
C:\Users\costin.zaharia\AppData\Local\Temp\.sonarqube\resources\3\Microsoft.AspNetCore.Razor.Language.dll
C:\Users\costin.zaharia\AppData\Local\Temp\.sonarqube\resources\3\SonarAnalyzer.Security.dll
C:\Program Files\dotnet\sdk\6.0.200\Sdks\Microsoft.NET.Sdk.Web\analyzers\cs\Microsoft.AspNetCore.Analyzers.dll
C:\Program Files\dotnet\sdk\6.0.200\Sdks\Microsoft.NET.Sdk.Web\analyzers\cs\Microsoft.AspNetCore.Mvc.Analyzers.dll
C:\Program Files\dotnet\sdk\6.0.200\Sdks\Microsoft.NET.Sdk.Web\analyzers\cs\Microsoft.AspNetCore.Components.Analyzers.dll
C:\Program Files\dotnet\sdk\6.0.200\Sdks\Microsoft.NET.Sdk\targets\..\analyzers\Microsoft.CodeAnalysis.CSharp.NetAnalyzers.dll
C:\Program Files\dotnet\sdk\6.0.200\Sdks\Microsoft.NET.Sdk\targets\..\analyzers\Microsoft.CodeAnalysis.NetAnalyzers.dll
C:\Program Files\dotnet\packs\Microsoft.NETCore.App.Ref\6.0.2\analyzers/dotnet/cs/System.Text.Json.SourceGeneration.dll
C:\Program Files\dotnet\packs\Microsoft.AspNetCore.App.Ref\6.0.2\analyzers/dotnet/cs/Microsoft.AspNetCore.App.Analyzers.dll
C:\Program Files\dotnet\packs\Microsoft.AspNetCore.App.Ref\6.0.2\analyzers/dotnet/cs/Microsoft.AspNetCore.App.CodeFixes.dll
C:\Program Files\dotnet\packs\Microsoft.AspNetCore.App.Ref\6.0.2\analyzers/dotnet/roslyn4.0/cs/Microsoft.Extensions.Logging.Generators.dll
C:\Program Files\dotnet\sdk\6.0.200\Sdks\Microsoft.NET.Sdk.Razor\targets\..\\source-generators\Microsoft.AspNetCore.Razor.SourceGenerator.Tooling.Internal.dll
C:\Program Files\dotnet\sdk\6.0.200\Sdks\Microsoft.NET.Sdk.Razor\targets\..\\source-generators\Microsoft.NET.Sdk.Razor.SourceGenerators.dll

Versions used:

dotnet-sonarscanner 5.5.3 dotnet sdk 6.0.200 ubuntu-20.04 hosted on azure devops pipeline Binary logs: logs.zip

duncanp-sonar commented 2 years ago

@costin-zaharia-sonarsource the different behaviour on Linux/Windows is surprising. According the logs in the other ticket, the analyzer settings are being removed in the Linux build because sonar.cs.roslyn.ignoreIssues=true:

image

That is the expected behaviour (although it is arguably now incorrect because of source generators). Did you have this property set when reproducing the issue on Windows?

costin-zaharia-sonarsource commented 2 years ago

Did you have this property set when reproducing the issue on Windows?

No, on my run it was set to false.

sonar.cs.roslyn.ignoreIssues=false

Thanks, @duncanp-sonar. I've updated the title and the description.

glucaci commented 2 years ago

Any update on this one, or can I help with a PR ?

Thanks!

digitalpowers commented 2 years ago

i seem to be having the same problem on a linux build with sonar.cs.roslyn.ignoreIssues set to false. checking the binlog i can definately see my analyzer being removed and not re-added and i am setting /d:sonar.cs.roslyn.ignoreIssues=false when i execute dotnet-sonarscanner begin I can also see it is false in the LocalSettings section of .sonarqube/conf/SonarQubeAnalysisConfig.xml

costin-zaharia-sonarsource commented 2 years ago

Hi @digitalpowers

Could you please create a new thread on our community forum? This issue is related to code generators in the context when sonar.cs.roslyn.ignoreIssues is set to true. The problem you're facing seems to be different.

duncanp-sonar commented 2 years ago

Update: proposed solution:

Changes need to be synchronized with the analyzers.

Note: the proposed solution might also address #1211 - check whether #1211 is still an issue once this bug is fixed.

andrei-epure-sonarsource commented 2 years ago

from discussion with @csaba-sagi-sonarsource and @tom-howlett-sonarsource

Option 1: change behavior of existing parameter.

If we don't remove analyzers and filter on the SQ Plugin side:

The default behavior won't change at all. Some people are doing this.

If we don't update SQ but we update the scanner, 3rd party issues won't work anymore.

We need to have a fallback strategy for SQ LTS users who update the scanner.

Option 2: deprecate existing parameter and add another parameter.

sonar.cs.roslyn.importThirdPartyIssues = true by default

If both are mentioned:

Can we tell which version of SonarQube we are? If 9.6 - never remove analyzers. If before 9.6 - keep existing behavior. If SonarCloud - like SQ 9.6.

And log to the user that importThirdPartyIssues is not supported before SQ 9.6.

DECISION: Option 2

andrei-epure-sonarsource commented 2 years ago

Another discussion with @duncanp-sonar and @csaba-sagi-sonarsource

Option 3: do the filtering in the END step

We can filter the external issue in the END step. This will avoid confusion and dependencies between releases sonar-dotnet and s4net.

Performance-wise we cannot do anything.

We prefer Option 3 now.

andrei-epure-sonarsource commented 2 years ago

We've also discussed that:

pavel-mikula-sonarsource commented 2 years ago

Loading analyzers is a bit overkill from workflow POV. Technically, it's doable and we have the codebase on AutoScan side https://github.com/SonarSource/sonar-dotnet-autoscan/blob/master/AutoScan.NET/Roslyn/AnalyzerCache.cs https://github.com/SonarSource/sonar-dotnet-autoscan/blob/master/AutoScan.NET/Analysis.cs

Eli-Black-Work commented 2 years ago

@costin-zaharia-sonarsource Might you consider changing the title to use "source generators" instead of "code generators"? I think that's the official nomenclature for this feature 😄

Eli-Black-Work commented 2 years ago

Thanks! 🙂