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

Can this be implemented as Roslyn Analysers? #720

Open
Meligy opened this issue Aug 18, 2022 · 11 comments
Open

Can this be implemented as Roslyn Analysers? #720

Meligy opened this issue Aug 18, 2022 · 11 comments
Labels
help wanted type:enhancement New feature or request

Comments

@Meligy
Copy link

Meligy commented Aug 18, 2022

If this is possible, we'd get a lot of benefits:

  • C# integration via the C# extension itself
  • We might not need anything special for MSBuild
  • Possible support of dotnet format (maybe just need to turn off some built-in analyzers or something)

Forgive my ignorance if this is already implemented this way and still doesn't achieve these goals.

@Meligy Meligy changed the title Can this be implemented as Roslyn Anallyzers? Can this be implemented as Roslyn Analysers? Aug 18, 2022
@belav
Copy link
Owner

belav commented Sep 4, 2022

I haven't had time to check this out yet, but my one worry is that analyzers run on a specific node type. This could be an analyzer on the CompilationUnitSyntax, but I don't know if a file would be re-analyzed when a line inside of a method changes. Even without that it would still work as part of the build, and may even be faster than the current MsBuild package.

@belav belav added this to the 0.20.0 milestone Sep 4, 2022
@Meligy
Copy link
Author

Meligy commented Sep 5, 2022

Thanks a lot. I mention this because it's how dotnet roslynator format works for example. It might also help with getting access to editorconfig style rules and using them.

@belav
Copy link
Owner

belav commented Oct 3, 2022

I tried creating an Analyzer in the CSharpier solution, but couldn't get past this error. There is some weirdness already with the source generators needing very specific versions of Microsoft.CodeAnalysis

  An instance of analyzer Insite.Analyzers.Net6Rules.CSharpierAnalyzer cannot be created from
C:\projects\csharpier\CSharpier.Analyzer\bin\Debug\net6.0\CSharpier.Analyzer.dll : 
Could not load file or assembly 'Microsoft.CodeAnalysis, Version=4.3.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35'.
The system cannot find the file specified..

I tried creating an empty solution for the analyzer, and referencing CSharpier from nuget. When I try to use CSharpier in the analyzer project, I get this error. But CSharpier can be referenced just fine from within a console app.

  Analyzer 'Insite.Analyzers.Net6Rules.CSharpierAnalyzer' threw an exception of type 'System.IO.FileNotFoundException'
with message 'Could not load file or assembly 'CSharpier, Version=0.18.0.0, Culture=neutral, PublicKeyToken=null'.
The system cannot find the file specified.'.

I'm not sure how to get past either of those errors.

@belav belav removed this from the 0.20.0 milestone Oct 3, 2022
@belav belav added type:enhancement New feature or request help wanted labels Oct 3, 2022
belav added a commit that referenced this issue Oct 3, 2022
@OneCyrus
Copy link

OneCyrus commented Oct 4, 2022

to use nuget packages inside of source generators it's a bit complicated.

https://www.youtube.com/watch?v=wp-dxZXRkJ4

@shocklateboy92
Copy link
Collaborator

shocklateboy92 commented Oct 24, 2022

I brought this up before with them, but didn't get any response:
dotnet/roslyn#54099

When I pinged the roslyn devs offline, they were against the idea since csharpier does't handle code with syntax errors.

@gat-cs
Copy link

gat-cs commented Oct 26, 2022

Source generators can generate new source files to add to the compilation, but very importantly, they cannot modify existing files (at least, last time I checked). As a result, a source generator cannot be used to format the code.

From this FAQ on source generators:

The key difference is that Source Generators don’t allow you rewrite user code. We view this limitation as a significant benefit, since it keeps user code predictable with respect to what it actually does at runtime. We recognize that rewriting user code is a very powerful feature, but we’re unlikely to enable Source Generators to do that.

Code analyzers lets you analyze the code and produce diagnostics and the associated code fixes. If anything, this can be done as an analyzer, but not as a source generator.

@Meligy
Copy link
Author

Meligy commented Oct 27, 2022

If anything, this can be done as an analyzer, but not as a source generator.

I totally agree. Code generators are for generated code. Analyzers are for human-written code.

@belav
Copy link
Owner

belav commented Dec 19, 2023

I ran across this which could help figuring out how to get the required nuget packages into CSharpier as an analyzer

https://www.meziantou.net/packaging-a-roslyn-analyzer-with-nuget-dependencies.htm

@scabana
Copy link

scabana commented Sep 12, 2024

I'm very interested in this issue. This would allow integration into dotnet format. As an analyser can provide automatic code fixes, dotnet format could most likely format the c# code. It would also work well within ecosystems that already have dotnet format --verify-no-changes configured in the build pipeline. Is this something that the maintainers would be willing to have?

This could also lower the burden on the extensions as they would, in theory, not be needed anymore since an analyser + refactoring would just show up in all IDEs. This would resolve issues where some devs don't have the extension installed for example. It would also support sharing the config through analyser globalconfig and not have the line length parameter copied all over the place.

@belav
Copy link
Owner

belav commented Sep 12, 2024

I'm very interested in this issue. This would allow integration into dotnet format. As an analyser can provide automatic code fixes, dotnet format could most likely format the c# code. It would also work well within ecosystems that already have dotnet format --verify-no-changes configured in the build pipeline. Is this something that the maintainers would be willing to have?

This could also lower the burden on the extensions as they would, in theory, not be needed anymore since an analyser + refactoring would just show up in all IDEs. This would resolve issues where some devs don't have the extension installed for example. It would also support sharing the config through analyser globalconfig and not have the line length parameter copied all over the place.

I'm not opposed to this being avaiable as an analyzer but I don't see it replacing the current CLI. On our large work project, dotnet-format never finishes. It changes some files and eventually hangs. In comparison dotnet csharpier runs in 2s when it has a primed cache, 20s with no cache.

I also don't know that "quick fix analyzers on save" is a thing in IDEs. Format on save is incredibly useful and I can't image life without it.

I'm not familiar with globalconfig, but it seems like they aren't meant for style options

Unlike EditorConfig files, global configuration files can't be used to configure editor style settings for IDEs, such as indent size or whether to trim trailing whitespace. Instead, they're designed purely for specifying project-level analyzer configuration options.

@scabana
Copy link

scabana commented Sep 13, 2024

I'm not familiar with globalconfig, but it seems like they aren't meant for style options

My understanding, and I might be wrong, is that IDEs (or their editorconfig support) won't be fed from the globalconfig file. But, roslyn analysers and quick code fixes will.

I'm not opposed to this being avaiable as an analyzer but I don't see it replacing the current CLI. On our large work project, dotnet-format never finishes. It changes some files and eventually hangs. In comparison dotnet csharpier runs in 2s when it has a primed cache, 20s with no cache.
This is quite sad indeed for the delays / not even working dotnet format on larger projects. We have multiple smaller projects on our side which does not suffer the same fate.

I also don't know that "quick fix analyzers on save" is a thing in IDEs. Format on save is incredibly useful and I can't image life without it.
You are right. I'd love to have as an analyser to make sure people who don't know they should have the extensions still get some hint that they should. Now, I fully agree that for the best experience, you'll want csharpier on save as it's available through the extensions.

WIth all this said, I'll take a look if I can come up with something acceptable that would tie it all together without making it too complex for the maintenance of this project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted type:enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants