sonar-dotnet icon indicating copy to clipboard operation
sonar-dotnet copied to clipboard

New Rule Idea: Mark pre-release nuget package as non compliant

Open martin-strecker-sonarsource opened this issue 2 years ago • 4 comments

Community request

Reasoning

Adding pre-release package dependency to a project can cause maintainability and security issues. There is a high risk of changing or dropping APIs before a new stable version is released.

Description

Pre-release is a Nuget and not an assembly concept. There are two options for implementing this (further investigation is needed).

Use the AssemblyInformationalVersionAttribute

Some packages like Fody or EF core apply the Nuget version also to the assemblies via the AssemblyInformationalVersionAttribute. We could use Compilation.References to find the AssemblyInformationalVersionAttribute via the metadata reader of the module of the PortableExecutableReference of external DLLs.

Pros:

  • Would work in the IDE
  • No "additional files" magic needed

Cons:

  • We would warn pre "DLL" which is not the same as per Nuget package (A single package can contain multiple references, see e.g. PDFsharp)
  • The message would be about some dll, which the user needs to map to the Nuget package. We need proper RSpec to help the user do so.
  • We rely on the AssemblyInformationalVersionAttribute which is not present in all Nuget packages (e.g. Syncfusion.Xamarin.Pdf) or does not include the "pre-release" keywords (e.g. GraphQL)
  • Where do we report the issue?
  • Transitive dependencies would show up as well (packages depending on other packages or references caused by packages of a dependent project)

Instrument msbuild in the scanner to add the csproj as an additional file

This would allow us to access the csproj (and reporting there should also just work).

Pros:

  • We would get the version from the <PackageReference Version> attribute, which is the only reliable source of truth
  • No problem with transitive dependencies and multiple DLLs per package
  • We can report on the package reference in the csproj
  • We can create other rules for csproj

Cons:

  • We would rely on changes in the scanner (We need to add the csproj to the additional files, which would need changes in our scanner target file)
  • This will likely not work in the IDE (maybe connected mode would work)

A PoC was implemented here

https://github.com/martin-strecker-sonarsource/ExampleAnalyzer/blob/master/Analyzer/Analyzer/CsprojAnalyzer.cs

Important: See how Analyzer.targets adds the csproj to AdditionalFiles and how the nuget spec adds the target file to the build directory of the nuget

https://github.com/martin-strecker-sonarsource/ExampleAnalyzer/tree/master/Analyzer/Analyzer.Package

Still open to discuss:

  • How do we integrate with the scanner. The scanner does not add the analyzer via package reference and therefore the targets file is not picked up. But the scanner adds AdditionalFiles itself, so we may just need to change the scanner targets file.
  • How do we integrate with SonarLint. Do they need to make changes as well (see the UT for how the solution is changed to add the additional file to the build process in a workspace)?
  • Should we do the same for other "additional files" we currently support like WebConfig. These are not added via the AdditionalFiles mechanism as far as I know (which is probably bad as it does not trigger an analysis if the file changes).

@martin-strecker-sonarsource Ideally you want to rely on additional files. However, is some cases, files are not available as such. You then still want to be able to do the analysis. This can be done by (also rergestering on the compilation action):

We did it like this: https://github.com/dotnet-project-file-analyzers/dotnet-project-file-analyzers/blob/main/src/DotNetProjectFile.Analyzers/Extensions/Microsoft.CodeAnalysis.Diagnostics.AnalysisContext.cs

Corniel avatar Aug 28 '24 10:08 Corniel

Thank you, @Corniel. I looked at the project, and there are some interesting ideas about including some common files for the analysis.

@martin-strecker-sonarsource I know. I started a year ago with this project, and it has been successfully used within my company (and some others) already. I'm working on some rules for the .editorconfig too, and consider adding rules for appsettings, but there nothing concrete yet has been implemented.

It would be nice if Sonar would support its output too, as mentioned: https://community.sonarsource.com/t/suport-roslyn-analyzers-on-additional-files/123957.

Corniel avatar Aug 28 '24 10:08 Corniel

Internal ticket NET-1647