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

What are sensible categories? Should we even have categories? #269

Open
ZedThree opened this issue Jan 10, 2025 · 3 comments
Open

What are sensible categories? Should we even have categories? #269

ZedThree opened this issue Jan 10, 2025 · 3 comments
Labels
question Further information is requested

Comments

@ZedThree
Copy link
Member

Our current categories are:

  • Error
  • Filesystem
  • Style
  • Typing
  • Obsolescent
  • Precision
  • Modules
  • IO
  • Readability
  • Bugprone

Are these correct/sensible? One alternative we've talked about is a hierarchy of levels instead, something like:

  • Error
  • Warning
  • Pedantic

Other linters

Let's have a quick survey of popular linters in other languages to see how they do things:

ruff

Obviously we're heavily inspired by ruff, but we're not keen on their original-implementation categorisation -- although the original linters often did just implement specific categories of checks, e.g. flake8-use-pathlib focuses on using pathlib, tryceratops focuses on exceptions and try statements. So for us, this would mean encouraging more categories with possibly quite narrow focus.

clippy

I thought clippy had something like a hierarchy of levels, but actually it has both levels and categories:

  • levels:

    • allow (ignored)
    • warn
    • deny (hard error)
  • categories:

    • cargo
    • complexity
    • correctness
    • nursery (I think this is our --preview)
    • pedantic
    • perf
    • restriction
    • style
    • suspicious

clang-tidy

Another with many categories, but these are mostly company- or project-specific, e.g. llvm or google. The sort of general purpose categories are:

  • bugprone
  • clang-analyzer
  • concurrency
  • cppcoreguidelines
  • misc
  • modernize
  • mpi
  • openmp
  • performance
  • portability
  • readability

Also note that clang-tidy doesn't implement categories as a concept, these are all just rule prefixes. Rules may also have multiple names, so cppcoreguidelines-avoid-magic-numbers redirects to readability-magic-numbers. This has the downside of needing to ignore both names if you enable both prefixes.

ESLint

ESLint is a javascript linter. Doesn't seem to have categories as a concept, or name prefixes like clang-tidy. Their docs divides up rules into:

  • Possible Problems
  • Suggestions
  • Layout & Formatting
  • Deprecated
  • Removed

RuboCop

Ruby linter. Has "departments" (rules are "cops"), small number of broad categories:

  • Style
  • Layout (basically just whitespace)
  • Lint
  • Metrics
  • Naming
  • Security
  • Bundler (this and gemspec are for packaging, I believe)
  • Gemspec

Also has severity levels:

  • info
  • refactor
  • convention
  • warning
  • error
  • fatal

No real docs as to what each of these levels actually does?


I think my conclusion from looking at other linters is that there's no real industry/meta-community consensus about how best to divvy up warnings. Having some kind of severity level might be nice, but probably not essential.

Our choices are probably between:

  • a few, broad categories
    • probably less useful for enabling/disabling as a whole group?
  • many specific categories
    • issues of where exactly does a given rule go? or make it available in multiple categories?

We seem to be trending towards the second option.

Thoughts, comments?

@ZedThree ZedThree added the question Further information is requested label Jan 10, 2025
@LiamPattinson
Copy link
Collaborator

Thanks for this, it's good to see that other linters run into these problems too. While properly categorising our rules has always been something of a headache, I'm not convinced that any of these methods would be strictly better. Switching them up at this stage would also leave the Ruff/flake8-like rule codes in a bit of an odd place.

In a very early build of Fortitude I included a --strict setting which would switch on more pedantic rules and modify the behaviour of other rules. I immediately found it to be quite annoying to work with, so removed it shortly after adding it. I'm now leaning towards just keeping a single level of rule severity, but also adding a hardcoded default rules list that excludes any rules that might be too pedantic for the average user. I probably could be convinced the other way though.

@ZedThree
Copy link
Member Author

Switching them up at this stage would also leave the Ruff/flake8-like rule codes in a bit of an odd place

That's also a good question -- do we still think the rule codes are a good idea? I have been thinking it might be better to use the names as the primary way of referring to them in the output. It would be good to get some community feedback as to what people prefer -- codes/names/both.

I'm now leaning towards just keeping a single level of rule severity, but also adding a hardcoded default rules list that excludes any rules that might be too pedantic for the average user.

Yep, agreed, I think this best to do.

@imciner2
Copy link
Contributor

Here are two more tools/systems that even advertise Fortran support:

Coverity (Synopsys/BlackDuck)

They have "Groups", but those are even more fine-grained. The actual rules are all just labelled with FC.XXX, where xxx is a number. They do classify into "Impact", but that is not really used to turn things on/off, just to display to the user.

ref: https://community.blackduck.com/s/article/Synopsys-Static-Analysis-Coverity-Fortran-Syntax-Analysis

Codee

They have an Open Catalog of checks https://open-catalog.codee.com/, and they put things into three categories: Correctness, Optimization and Modernization (they do have multiple check prefixes, but I haven't been able to find what they mean). Fortitude rules would probably be in either the Correctness or Modernization categories (optimization is more static analysis not linting).


I'm now leaning towards just keeping a single level of rule severity, but also adding a hardcoded default rules list that excludes any rules that might be too pedantic for the average user.

I think this is probably the more powerful system. I don't think having the code know about severities is that useful, and instead it is more useful to have a nice way of configuring checks that are active so that users can make a custom check set if they want. Also, severity levels can tend to be very subjective.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants