search

CarterCommunity / Carter

Carter is framework that is a thin layer of extension methods and functionality over ASP.NET Core allowing code to be more explicit and most importantly more enjoyable.
MIT License
2.05k stars 172 forks source link

Add Roslyn Analyzer project #349

Closed CollinAlpert closed 1 month ago

CollinAlpert commented 1 month ago

As promised, I have created a Roslyn Analyzer which emits a warning when a type implementing ICarterModule has a constructor with parameters. Feel free to look over the test cases and see if you agree with the behavior.

CollinAlpert commented 1 month ago

I managed to pack the analyzer with the Carter package and still keep the analyzer project netstandard2.0. This will ensure maximum compatibility across Visual Studio versions.

Also since the analyzer is now also added to the Carter project, we got a hit. The CarterModule class has a constructor with a parameter and thus the analyzer flags this. I've disabled the warning for now, however let me know if you want me to tweak the analyzer in this regard.

jchannon commented 1 month ago

Looking good - excuse my ignorance but any reason why we can't delete the analyzer project and just put the analyzer classes inside Carter?

CollinAlpert commented 1 month ago

No worries! We can do that, however the analyzer might not work for a very small amount of users due to the targeting of net8.0 instead of netstandard2.0 as recommended. But like I said, I see only minimal risk so I am fine with it. I will make the changes.

jchannon commented 1 month ago

If I put in a ctor dependency in the Carter.Sample project that has a project ref to Carter will I still get a warning?

CollinAlpert commented 1 month ago

Yes, you will, however the project reference must look like this to include analyzer behavior: <ProjectReference Include="../../src/Carter/Carter.csproj" OutputItemType="Analyzer" />

OutputItemType is not necessary for PackageReference elements, since we pack the DLL in the analyzer directory, something that is not possible to ProjectReference elements.

jchannon commented 1 month ago

Should rename it to Carter from CARTER1? Screenshot 2024-05-24 at 12 06 37

CollinAlpert commented 1 month ago

We could, however I wouldn't recommend it. The guidelines for diagnostic IDs recommend using the project name + a number. If you decide you want another analyzer to warn about something else in the future, you'd need to distinguish it from this one, e.g. by naming it CARTER2. The compiler diagnostics (CSXXXX), code analysis diagnostics (CAXXXX) and system library diagnostics (SYSLIBXXXX) all follow this pattern.

jchannon commented 1 month ago

Can you rebase, I've added some permissions to the actions workflow file :)

jchannon commented 1 month ago

Actually GH has a button to do it :)

jchannon commented 1 month ago

Thanks for this!! Appreciate it 👍 👍

CollinAlpert commented 1 month ago

Happy to be of assistance! Ping me if you want any other rules implemented 😄