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

Custom Orchard Core analyzers (OSOE-346) #15

Open
Piedone opened this issue Dec 18, 2020 · 6 comments
Open

Custom Orchard Core analyzers (OSOE-346) #15

Piedone opened this issue Dec 18, 2020 · 6 comments
Labels
enhancement New feature or request

Comments

@Piedone
Copy link
Member

Piedone commented Dec 18, 2020

Let's develop custom analyzers. This is not that easy, and we'd need to publish them on NuGet since analyzers can't be used directly from the source.

Some ideas:

  • Check if controller actions have authorization either with attributes (e.g. [Admin]) or with an explicit IAuthorizationService call.
  • Check for any unfiltered ISession queries. While not always, usually there should be some kind of filtering instead of a full table retrieval.
  • Check for IContentManager.GetAsync(string id) calls in a loop and suggest IContentManager.GetAsync(IEnumerable<string> contentItemIds, bool latest = false) instead.
  • Check for OrchardCore.ResourceManagenet project/package reference if resource management tag helpers are used. Similarly, check for a missing OrchardCore.DisplayManagement reference and others. See Users admin <script> doesn't get Razor compiled OrchardCMS/OrchardCore#6943.
  • Use IClock instead of DateTime.UtcNow.
  • Interesting not just for Orchard apps but any ASP.NET Core app, so should be under their own category:
    • Check for the injected localizer to have a proper type (not the field/property it's saved to): It must be IStringLocalizer<MyClass> or IHtmlLocalizer<MyClass>. The type parameter mustn't be omitted or have a different type than the current type. This would actually be generally interesting for ASP.NET Core MVC apps. Came up here too: Fix localizer type in tenants ApiController OrchardCMS/OrchardCore#10987
    • Check for the Sdk attribute being Microsoft.NET.Sdk.Razor in the csproj's <Project> element if Razor templates are being used.
    • Private and protected fields should be named _prefixedWithUnderscoreCamelCased except for T and H. We can modify SX1309 for this. The alternative would be to put all localizers into properties instead and enable SX1309.

Jira issue

@hishamco
Copy link

@Piedone is there any analyzers developed yet? If not is there a repo or project that I can start prototype the localizer analyzer

@Piedone
Copy link
Member Author

Piedone commented Jan 10, 2022

Not yet, we'd start in this project, so please fork it.

@hishamco
Copy link

Sure

@six006
Copy link

six006 commented Jan 26, 2022

mark

@github-actions github-actions bot changed the title Custom Orchard Core analyzers Custom Orchard Core analyzers (OSOE-346) Sep 18, 2022
@0liver
Copy link
Contributor

0liver commented Mar 3, 2023

Here's a proposition for a valuable analyzer:

Verify that an exception defined in a catch clause is used at the correct position in _logger.LogError():

  1. _logger.LogError("message", ex) should trigger a violation
  2. A fix should be provided to move ex to the beginning: _logger.LogError(ex, "message").

@Piedone
Copy link
Member Author

Piedone commented May 27, 2024

Tutorial on writing an analyzer: https://www.meziantou.net/roslyn-analyzers-how-to.htm.

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

No branches or pull requests

4 participants