Skip to content

Could the various Lint related classes in package:analyzer be made public? #50986

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.

Sign up for GitHub

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

Jump to bottom
Open
rrousselGit opened this issue Jan 11, 2023 · 8 comments
Open

Could the various Lint related classes in package:analyzer be made public? #50986

rrousselGit opened this issue Jan 11, 2023 · 8 comments
Assignees
@srawlins
Labels
analyzer-api Issues that impact the public API of the analyzer package area-dart-model For issues related to conformance to the language spec in the parser, compilers or the CLI analyzer. P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug

Comments

@rrousselGit
Copy link

rrousselGit commented Jan 11, 2023 •
edited
Loading

Hello!

I am working on enabling the implementation of custom lint rules in a syntax similar to how the Dart SDK defines them.
And while investigating how the SDK handles lints, I realized a large amount of classes related to them are private.

Here's a non-exhaustive of potential classes that I feel would be valuable if made public:

  • LintRule
  • LinterContext

and their equally valuable dependencies, such as InterfaceManagger3, LinterVisitor, ...

All of these combined contain a large number of valuable utilities. Yet these objects aren't easily extractable out of the SDK. They are often entangled with the implementation classes of other aspects of the analyzer.
@srawlins
Copy link
Member

CC @pq @scheglov @bwilkerson

I think I agree. It would be perfectly reasonable to use these while writing a diagnostic in a plugin, right?

@srawlins srawlins added legacy-area-analyzer Use area-devexp instead. P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug analyzer-api Issues that impact the public API of the analyzer package labels Jan 11, 2023
@rrousselGit
Copy link
Author

rrousselGit commented Jan 12, 2023 •
edited
Loading

On that note, ErrorReporter is public and has a public constructor which accepts a Source. But Source isn't public

I believe Source is also available on various other places (Element)

@rrousselGit
Copy link
Author

Some equally valuable utilities to expose would be the fixes / CorrectionProducers

There's a large amount of them, and diagnostic plugins would otherwise have to reimplement them for their use-case.

@bwilkerson
Copy link
Member

It's my opinion that we don't want to commit to supporting the lint related classes as part of the public API for the analyzer package, or to make CorrectionProducer public, until we have resolved the issue of whether and how to support plugins.

@srawlins
Copy link
Member

We've got working designs for how to support plugins using LintRule, LintCode, CorrectionProducer, et al, so I will start working to make the necessary lint-related classes public. CC @pq

@srawlins srawlins self-assigned this Jun 14, 2024
@rrousselGit
Copy link
Author

This is hype! I'll definitely try this whenever it's available :)

copybara-service bot pushed a commit that referenced this issue Jun 20, 2024
@srawlins
Tidy up LintCode docs before making public API
ae3de54 
Work towards #50986

Cq-Include-Trybots: luci.dart.try:flutter-analyze-try,analyzer-win-release-try,pkg-win-release-try
Change-Id: I60d378da27570f6b76a554685802d3b888edee0d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/372182
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Sam Rawlins <[email protected]>
copybara-service bot pushed a commit that referenced this issue Jul 5, 2024
@srawlins
linter: require LintRules to either declare 'lintCode' or 'lintCodes'
f274b8a 
Work towards #50986

This found a few lint rules that had not declared a `lintCode` or
`lintCodes`! So I corrent them as well, and a few grammar nits in their
descriptions.

Cq-Include-Trybots: luci.dart.try:flutter-analyze-try,analyzer-win-release-try,pkg-win-release-try
Change-Id: I2bca3bbc382d1f25a3291326eb3b660c06d8917d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/372183
Commit-Queue: Samuel Rawlins <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
copybara-service bot pushed a commit that referenced this issue Jul 9, 2024
@srawlins
analyzer: Expose LintCode as public API, for plugins
0e39ed6 
Work towards #50986

Cq-Include-Trybots: luci.dart.try:flutter-analyze-try,analyzer-win-release-try,pkg-win-release-try
Change-Id: Ie5783240e3fc2c9f8076b3efb13f79a3b8fa5000
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/375060
Reviewed-by: Phil Quitslund <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
@bwilkerson
Copy link
Member

@srawlins Should we close this now?

@srawlins
Copy link
Member

There are a few classes that still need tidying, and in particular, renaming.

@johnniwinther johnniwinther added area-dart-model For issues related to conformance to the language spec in the parser, compilers or the CLI analyzer. and removed legacy-area-analyzer Use area-devexp instead. labels Mar 6, 2025
copybara-service bot pushed a commit that referenced this issue May 8, 2025
@srawlins
Introduce two subclasses of 'LintRule' for handling multiple lint codes
abd4e77 
Before this change, we had an enforcement that either 'get lintCode' or
'get lintCodes' is implemented, but it was a runtime enforcement. And it
was a little roundabout.

The new system is simpler and more typical:
* `AbstractLintRule` is the parent type, and it's sealed. It provides
  `get lintCodes` as an interface. Code outside of the lint rules
  requires this getter, in registering codes, etc.
* `LintRule` is the new subclass which is the "90% case." It declares
  a `get lintRule`, and offers simpler reporting methods for reporting
  a single code everywhere.
* `MultiLintRule` is the new subclass which represents that last
  "10% case." It only declares the reporting methods which all require
  a LintCode parameter, since there is no concept of a "default lint
  code among all my lint codes."

Work towards #50986

Change-Id: I63cbbfc1d936bb4428aa0b8f5fb0638c9b61b3dc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/426284
Reviewed-by: Brian Wilkerson <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
copybara-service bot pushed a commit that referenced this issue May 13, 2025
@srawlins
analyzer: Convert getPubspecVisitor to a getter named pubspecVisitor
da58ca0 
Work towards #50986

Change-Id: I1bcdc6b32efef0f7e70bbb49c3cce046f95351fb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/428225
Commit-Queue: Samuel Rawlins <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>
copybara-service bot pushed a commit that referenced this issue May 13, 2025
@srawlins
linter: privatize LintRuleUnitContext.file
cfb4fed 
Work towards #50986

Also remove under-used libraryFragment getter.

Change-Id: I6804bacc62abb483a339b28b8d5a226b689aa68d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/428224
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
copybara-service bot pushed a commit that referenced this issue May 13, 2025
@srawlins
analyzer: Swap LintRule and typedef AnalysisRule
c2fe957 
Work towards #50986

Change-Id: Ie9b2130d48929f8407a2ef20e23576699ecea2b9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/428223
Reviewed-by: Brian Wilkerson <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
copybara-service bot pushed a commit that referenced this issue May 13, 2025
@srawlins
analyzer: Remove unnecessary InheritanceManager3 from LinterContext
0d1fe0f 
Work towards #50986

At the same time, migrate `redeclare_verifier` from InheritanceManager3,
so that the impl is the same as the `annotate_redeclares` lint rule.

Change-Id: I9c5c77dcf50aa54820473874327bc3a7b93fd987
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/428380
Commit-Queue: Samuel Rawlins <[email protected]>
Reviewed-by: Paul Berry <[email protected]>
@srawlins srawlins mentioned this issue May 14, 2025
Closed
copybara-service bot pushed a commit that referenced this issue May 15, 2025
@srawlins
analyzer: Deprecate AbstractAnalysisRule.lintCodes
a3b1b4f 
Work towards #50986

Change-Id: I4c86dbf806964c98f4a1c5854eb53c6c8cfe002b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/428624
Commit-Queue: Samuel Rawlins <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>
copybara-service bot pushed a commit that referenced this issue May 15, 2025
@srawlins
analyzer: Simplify interface of WorkspacePackage, use File for root
701c611 
Work towards #50986

Change-Id: I56c14bc941e1204b9d9cc13086f5ed92a1228585
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/428623
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
copybara-service bot pushed a commit that referenced this issue May 15, 2025
@srawlins
linter: deprecate AnalysisRule.lintCode
22453ba 
Work towards #50986

Change-Id: Iedf1127b9f04aec910647c7f35597f38c6bfe1d2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/428846
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
copybara-service bot pushed a commit that referenced this issue May 21, 2025
@srawlins
analyzer: make fields final, in pubspec lint rule code
0d0cac4 
Work towards #50986

Various classes have mutable fields which should be final. Some of the
fixes require using factory constructors:

* _PSDependency, _PSEnvironment, _PSGitRepo, _PSHost, _Pubspec
* _PSNode: resourceProvider can be private and non-null
* Doc comments tidied here and there

Change-Id: Ib11a059e72670c80a4172e1c688493c1067328c8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/429582
Commit-Queue: Samuel Rawlins <[email protected]>
Reviewed-by: Paul Berry <[email protected]>
copybara-service bot pushed a commit that referenced this issue May 22, 2025
@srawlins
analyzer: Rename NodeLintRegistry to RuleVisitorRegistry, provide alias
87f5185 
Work towards #50986

Change-Id: Id0053b4721ca0517e315d5118ae48839dfaa3027
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/429465
Reviewed-by: Konstantin Shcheglov <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
copybara-service bot pushed a commit that referenced this issue May 22, 2025
@srawlins
analyzer: rename AnalysisRule 'errorCode' parameters
612c9f6 
Work towards #50986

Change-Id: I3d5a55f6638b18e2941a574a71c89fc142f0c206
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/430060
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
copybara-service bot pushed a commit that referenced this issue May 28, 2025
@srawlins
analyzer: Move RuleState into the public API
eba91c5 
Work towards #50986

Change-Id: I32e1d9b6dbb0a6ef91569ae65100365f775ec0f9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/430544
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
copybara-service bot pushed a commit that referenced this issue Jun 2, 2025
@srawlins
Remove underused exports from analyzer.dart
bd9ab13 
Work towards #50986

The few rules that need this or that can import it themselves; this
helps us understand what parts of private API are needed more and less;
plugin authors should not be importing private API; this gives us
signal about which private API should maybe be offered as public API.

Change-Id: Id5ac0e48132d7e10e67e1f122ab63e5c2287f226
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/429060
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@srawlins srawlins

Labels
analyzer-api Issues that impact the public API of the analyzer package area-dart-model For issues related to conformance to the language spec in the parser, compilers or the CLI analyzer. P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@srawlins @bwilkerson @johnniwinther @rrousselGit