Code Analyzer

[agriffard]

Would it be interesting to use a Code Analyzer package to enforce good practices?

Ex: https://github.com/meziantou/Meziantou.Analyzer

How can it be applied to all projects easily?

[hishamco]

I think we add some in editor.config furthermore VS already suggestes some of code suggestions to enforce good practices. What the package will add, than what we have?

[rjpowers10]

This is maybe a better candidate for https://github.com/OrchardCoreContrib/OrchardCoreContrib.PoExtractor but I can describe a few analyzers I made for my project.

1. Use [Display(Name = "")] instead of [DisplayName] - the former will be localized, the latter will not
2. Use correct names for localizers: IHtmlLocalizer - H, IStringLocalizer - S, IViewLocalizer - T, the extractor tool assumes this convention
3. Use literal strings for localizers: The extractor tool will extract T["This is a string"], it will miss expressions like T["This is " + "a string"] or variables like T[myStringVariable]

[rjpowers10]

Another one: Use IClock or ILocalClock instead of DateTime.Now, DateTime.UtcNow, etc.

[Skrypt]

I think we have a stale PR with a Github code analyzer that never made it because it adds a lot of overhead of the time execution for the CI workflows.

/cc @Jetski5822

[Piedone]

See #7950 and Lombiq/.NET-Analyzers#15.

[hishamco]

Use correct names for localizers: IHtmlLocalizer - H, IStringLocalizer - S, IViewLocalizer - T, the extractor tool assumes this convention

BTW using capital letter for fields against the naming rule ;)

[BenedekFarkas]

We'd be happy to take this further by integrating https://github.com/Lombiq/.NET-Analyzers (see the list of packages used in the readme) - we use these in all our projects:

1. We can start with a simple PR that covers the integration with the default configuration and verify that the CI build correctly fails on rule violations (they remain warnings in local development).
2. Disable every rule that has a violation and then merge it, this way probably some rules will remain and can already be put to good use.
3. Iterate through every disabled rule: enable and fix violations (maybe group some of them together where it makes sense) in a PR, merge when ready; repeat until there are no disabled rules.

Step 3 (probably more than 95% of the work) can be a collaborative effort, although merge conflicts can happen.

[BenedekFarkas]

We can start with CodeQL integration first - it might be easier to set up and see how much it overlaps with .NET Analyzers..

[Piedone]

Where was this done @agriffard?

[BenedekFarkas]

I guess it's closed as a duplicate of #7950?