belav / csharpier

CSharpier is an opinionated code formatter for c#.
https://csharpier.com
MIT License
1.41k stars 99 forks source link

Can this be implemented as Roslyn Analysers? #720

Open Meligy opened 2 years ago

Meligy commented 2 years ago

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

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

belav commented 2 years ago

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.

Meligy commented 2 years ago

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 commented 2 years ago

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.

OneCyrus commented 2 years ago

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

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

shocklateboy92 commented 2 years ago

I brought this up before with them, but didn't get any response: https://github.com/dotnet/roslyn/discussions/54099

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

gat-cs commented 2 years ago

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 commented 2 years ago

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 commented 11 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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.

scabana commented 3 weeks ago

So, after just trying to use the CSharpier.MSBuild for a while now, I think it's enough as a degraded experience when no extension has been installed.