Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support as Roslyn Analyzer #619

Open
rjmurillo opened this issue Jun 12, 2024 · 3 comments
Open

Add support as Roslyn Analyzer #619

rjmurillo opened this issue Jun 12, 2024 · 3 comments

Comments

@rjmurillo
Copy link
Member

Is your feature request related to a problem? Please describe.
The experience is clunky when using DevSkim. I either use the command line and output a SARIF file, then use a SARIF viewer, or use a Visual Studio or VS Code extension to integrate. This does not help me because I use JetBrains Rider.

Describe the solution you'd like
What would be great if the analyzers show up as Roslyn Code analyzer so it works when I just run dotnet build without all the extra steps.

Describe alternatives you've considered

  • Creating a VS Code workspace definition with the required extensions (not everyone uses workspaces or VS Code)
  • Creating some onboarding documentation that requires contributors to configure their IDE a certain way (we have mechanisms for that, but not for extensions specifically)
  • Creating a shell script that contributors can call to analyze (the experience is clunky)
  • Hooking MSBuild so that the tooling is run from dotnet tool during regular build (the experience is also clunky)

Additional context
rjmurillo/moq.analyzers#83

@gfs
Copy link
Contributor

gfs commented Jun 13, 2024

Thanks for the feedback and suggestion. The GitHub Action is aimed to close the gap here for users who either don't prefer the IDE extension or who use an editor that we don't currently support.

That said, this is an interesting request and sounds like it could be a cool integration. However, I'm not sure about the feasibility of packaging DevSkim as a Roslyn analyzer. I don't have a lot of experience with analyzers, but from the documentation I found at first on authoring roslyn analyzers (https://devblogs.microsoft.com/dotnet/how-to-write-a-roslyn-analyzer/) it looks like analyzers are defined by registering call back methods when specific AST nodes are encountered but DevSkim is primarily regular expression based, so it doesn't look like a clean repackaging at first blush.

If you have some domain specific knowledge here that I'm missing that means that the repackage is a simpler task than it seems, I'm certainly open to the idea.

@rjmurillo
Copy link
Member Author

Architecturally it could work (we have rules that are RegEx based in the Moq Analyzers project). The difference is understanding more about which symbols are applicable. So it's not a context-less scan of code. There are cases defined in your must-match that are narrowing enough, for example. It also can avoid false positives.

For example: we use NerdBank Git Version which stamps the git commit as an assembly property.

internal static partial class ThisAssembly {
    internal const string GitCommitId = "9429e11d88c89cc1ecd460f03f8003ba2cca1b53";
}

This gets flagged incorrectly as a DS173237 because it's hex based and 30 char or more

@gfs
Copy link
Contributor

gfs commented Jun 13, 2024

I'll think on this a bit more. It seems like it might be able to slot in like the structured data queries feature, narrowed down by a new field for the roslyn class to trigger on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants