Provide a roslyn analyzers corresponding to the GD0001 and GD0002

[van800]

Fixes JetBrains/godot-support#81

Discussion:
https://chat.godotengine.org/channel/dotnet?msg=dtyXNTdDNij3S7Jvr

I have also added a code-fix

Tried it in Rider and VScode

Benefit for vscode users is that report from analyzer is much nicer then multiple reports from generators.
Old behaviour:

New behaviour:

[van800]

@raulsntos I have cherry-picked your change from #87342

[paulloz]

As discussed previously, this moves the diagnostics we were reporting from within the generators to its own analyzer, right? Essentially doing the job twice (i.e. the generator is already visiting all those classes, and now the analyzer does too).

I think we could probably report only from ScriptPathAttributeGenerator, and fix the source location to not underline the entire class.

[paulloz]

Actually, the more I think about it, the more I'm back to thinking that moving all of our diagnostics to their own analyzers is a good idea. At the same time, we could clean the mess that is Common.cs at the moment (e.g., we're defining diagnostics as static readonly, but not using them depending on from where it is reported, see here).

It'd also reduce the work done by incremental generators once #64899 is a thing. Not to mention that the eventual shift to a GDExtension implementation, as far as I understand, will mean the generated content will be vastly different (the generators are going to change), while the public API should stay compatible (the analyzers/diagnostics should stay the same).

[van800]

@paulloz please take another look, when you have time. I haven't changed anything, just the #87342 was merged, and I rebased on top of master.
I think the code sets a good UX for both Rider and VSCode and sets a good example of roslyn quick fix and tests for analyzer and quick fix.

[paulloz]

I have a CSC error about this analyzer, both when compiling Godot.SourceGenerators, and when opening Godot afterwards. Not entirely sure why.

CSC : warning CS8032: An instance of analyzer Godot.SourceGenerators.ClassPartialModifierAnalyzer cannot be created from E:\Godot Worktree\godot\modules\mono\editor\G
odot.NET.Sdk\Godot.SourceGenerators\bin\Release\netstandard2.0\Godot.SourceGenerators.dll : Could not load type 'Godot.SourceGenerators.ClassPartialModifierAnalyzer' 
from assembly 'Godot.SourceGenerators, Version=4.3.0.0, Culture=neutral, PublicKeyToken=null'.. [E:\Godot Worktree\godot\modules\mono\editor\Godot.NET.Sdk\Godot.Sourc 
eGenerators.Sample\Godot.SourceGenerators.Sample.csproj]

Apart from that, it seems fine.

The only thing I'm a bit scared about is users will now be able to compile broken projects by suppressing GD0001. We say that it shouldn't be suppressed, but still... I wonder if we should find a way to still block compilation here (with something additionally reporting at Location.None, or IDK).

Would be nice for #88295 to be merged before, to prevent regressions. And maybe #80343 too.

[raulsntos]

The only thing I'm a bit scared about is users will now be able to compile broken projects by suppressing GD0001

@paulloz The project should still not compile if we are generating partials of a class not marked as partial.

[paulloz]

But we are skipping non partial classes, so we won't generate the partials.

[raulsntos]

We probably shouldn't, otherwise that means we're still checking if the class is partial in the generator and we said we were moving the check to the analyzer.

That said, it's probably fine to keep the skipping behavior in this PR, changing it is tricky because it's kind of mixed with GD0002 which would increase the scope of this PR. GD0002 should probably be included in the GD0001 analyzer anyway, but I didn't want to delay this PR further.

And to be honest, I don't expect users to disable this diagnostic, doing so would be ignoring an error in your code so whatever happens after that can be considered undefined behavior.

[van800]

Thank you for the review and all the suggestions, I have also cleaned up redundant sealed modifiers and did a cosmetic fix, which bugged me: trimmed the repeating part of Description. Unfortunately the practice of preparing just one commit pull request makes it harder for you to follow, what was changed. For the future, should I keep multiple commits and squash them after the review?

[AThousandShips]

When this is ready to merge you will need to squash :) you can do it now but there will be some code style review once this has been approved (sorry missed you had squashed and thought you asked if you needed to, in the future you will need to always before merge, but until then it's fine to keep separate commits, though generally keep it simple to not clutter)

Also in the future try using the batch suggestions to avoid making multiple commits from suggestions

[van800]

I have also rewritten the GD0002.

[paulloz]

I have a CSC error about this analyzer, both when compiling Godot.SourceGenerators, and when opening Godot afterwards. Not entirely sure why.

CSC : warning CS8032: An instance of analyzer Godot.SourceGenerators.ClassPartialModifierAnalyzer cannot be created from E:\Godot Worktree\godot\modules\mono\editor\G
odot.NET.Sdk\Godot.SourceGenerators\bin\Release\netstandard2.0\Godot.SourceGenerators.dll : Could not load type 'Godot.SourceGenerators.ClassPartialModifierAnalyzer' 
from assembly 'Godot.SourceGenerators, Version=4.3.0.0, Culture=neutral, PublicKeyToken=null'.. [E:\Godot Worktree\godot\modules\mono\editor\Godot.NET.Sdk\Godot.Sourc 
eGenerators.Sample\Godot.SourceGenerators.Sample.csproj]

This isn't happening any more 👍

[raulsntos]

Looks good to me. Thanks!

[akien-mga]

Could you squash the commits? See PR workflow for instructions.

[akien-mga]

Thanks!