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

A lean linter (for a sustainable future). #57427

Open
1 of 2 tasks
pq opened this issue Feb 2, 2017 · 3 comments
Open
1 of 2 tasks

A lean linter (for a sustainable future). #57427

pq opened this issue Feb 2, 2017 · 3 comments
Labels
analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. contributions-welcome Contributions welcome to help resolve this (the resolution is expected to be clear from the issue) P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug type-question A question about expected behavior or functionality type-task A well-defined stand-alone task

Comments

@pq
Copy link
Member

pq commented Feb 2, 2017

PROPOSAL: remove the linter CLI and reduce the linter package to a kernel of lint rules, tests and testing support.


Sub tasks:


Background: the linter depends on analyzer package internals and so is vulnerable to prolonged build breakages between stable analyzer releases. At the heart of these dependencies is support for the linter command-line tool. This tool was built at the project's outset, when the dartanalyzer did not support running lints (it does now) and has continued to evolve to support linting in particular.

It duplicates a lot of logic in the dartanalyzer CLI. Notably it duplicates all the work done by the analyzer to setup analysis (e.g., creating resolvers, setting package roots, configuring SDK locations, etc). While effort has been made to reduce this duplication and push this functionality into the analyzer kernel, it has been the source of ongoing maintenance.

The linter CLI also provides some refinements to the analyzer CLI with an eye towards supporting linting in particular. The way it displays errors, for example, is a bit more verbose, and it provides a useful --stats option to collect and display lint occurrence counts.

PROS

By removing the CLI bits from the linter, it notably

  • will becomes less vulnerable to breakage from analyzer API changes,
  • can have a greatly reduced dependency set making it less vulnerable to package versioning issues, and
  • will be easier for contributors to reason about (and ideally contribute to)

CONS

On the downside:

  • we lose some CLI user-friendliness.

Because I feel so strongly about getting the build GREEN again (sustainably) I'd like to push hard on removing the CLI. When finished, this would get us a linter that contains:

  • lint rules, rule documentation and tests, with supporting
  • test and doc utilities

(and would IMO be a good candidate for a 1.0.0.)

In cases where CLI niceties are missed, I propose we upstream them to the dartanalyzer CLI proper.

While I'm confident that this is the right thing to do, I realize that this change may not be popular with everyone, notably folks who have grown to depend on command-line refinements. If these folks are many (and vocal), I'd be happy to put some effort into upstreaming.

CC'ing a handful of contributors and community members but needless to say the conversation is open to all. Please chime in.

@alexeieleusis @bwilkerson @matanlurey @ochafik @dikmax @guillermooo @kevmoo @srawlins @yyoon @filiph @a14n @zoechi

Thanks in advance for any thoughts!

@pq pq added contributions-welcome Contributions welcome to help resolve this (the resolution is expected to be clear from the issue) type-question A question about expected behavior or functionality type-task A well-defined stand-alone task labels Feb 2, 2017
@dikmax
Copy link

dikmax commented Feb 2, 2017

I think separate CLI can be removed. dartanalyzer CLI is enough for most tasks.

@kevmoo
Copy link
Member

kevmoo commented Feb 2, 2017 via email

@alexeieleusis
Copy link
Contributor

I only use the linter CLI for testing, so the test utilities mentioned above should be a good replacement. Though often the last test I perform is run a rule against SDK and Flutter codebases, how hard will be to perform that?

Thank you!

@srawlins srawlins added the type-enhancement A request for a change that isn't a bug label Jun 17, 2021
@srawlins srawlins added the P3 A lower priority bug or feature request label Sep 21, 2022
@devoncarew devoncarew added analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. labels Nov 18, 2024
@devoncarew devoncarew transferred this issue from dart-lang/linter Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. contributions-welcome Contributions welcome to help resolve this (the resolution is expected to be clear from the issue) P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug type-question A question about expected behavior or functionality type-task A well-defined stand-alone task
Projects
None yet
Development

No branches or pull requests

6 participants