Skip to content

Add custom lint to sanity check analyzer public API #60058

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

Closed
stereotype441 opened this issue Feb 5, 2025 · 1 comment
Closed

Add custom lint to sanity check analyzer public API #60058

stereotype441 opened this issue Feb 5, 2025 · 1 comment
Assignees
Labels
analyzer-api Issues that impact the public API of the analyzer package 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

Comments

@stereotype441
Copy link
Member

stereotype441 commented Feb 5, 2025

After mistakenly exposing some private implementation details of package:_fe_analyzer_shared through the analyzer public API, and having to do a lot of work to clean up my mistake (see #59763), I'd like to add a custom lint to the analyzer, to verify the following:

  • For this lint, I'd like to define the analyzer's public API to be:

    • All top level elements of files in package:analyzer but not in package:analyzer/src whose names are public

    • All elements annotated with @AnalyzerPublicApi(), a new annotation I propose adding. (The annotation will be defined in package:_fe_analyzer_shared so that types in that package can use it. The rationale for this is that some things look like they aren't part of the analyzer's public API but actually are, such as the Token class defined in package:_fe_analyzer_shared.)

    • For any member of the analyzer's public API that defines a type or an extension, all of its public members.

  • The types of the methods, functions, getters, setters, constructors, and supertypes in the analyzer's public API may only refer to types defined in the analyzer's public API (or types defined in dart:core and a few permitted packages, e.g. package:pub_semver).

  • The export directives in package:analyzer but not in package:analyzer/src may only export elements that are in the analyzer's public API.

  • No top level element in package:analyzer but not in package:analyzer/src may have a name ending in Impl. (This prevents us from accidentally exposing "Impl" classes by declaring them in the wrong place.)

  • Any part directives in package:analyzer but not in package:analyzer/src must also point to files in package:analyzer but not in package:analyzer/src. (Without this rule, an element might sneak into the analyzer's public API without being noticed by this lint. Note that the analyzer currently uses part files very rarely, but this may change after augmentation support is added to the language.)

@stereotype441 stereotype441 added analyzer-api Issues that impact the public API of the analyzer package legacy-area-analyzer Use area-devexp instead. labels Feb 5, 2025
@stereotype441 stereotype441 self-assigned this Feb 5, 2025
@stereotype441
Copy link
Member Author

I've done a prototype of this lint and I've found a few violations:

  • PluginConfiguration.diagnosticConfigs has type Map<String, DiagnosticConfig>, but DiagnosticConfig is defined in package:analyzer/src/lint/config.dart. (We could address this by declaring DiagnosticConfig and RuleConfig to be part of the analyzer's public API.)

  • Element.context and ConstantEvaluationTarget.context have type AnalysisContext, but AnalysisContext is defined in package:analyzer/src/generated/engine.dart. (Note: there's a different AnalysisContext defined in package:analyzer/dart/analysis/analysis_context.dart, but it appears unrelated. We could address this by declaring both AnalysisContext classes to be part of the analyzer's public API, but that would (a) be confusing, and (b) expose SourceFactory through the public API; SourceFactory, in turn, would expose DartSdk and UriResolver, and DartSdk would expose SdkLibrary, so I'm not sure whether this is such a good idea.)

  • ContextRoot.workspace has type Workspace, but Workspace is defined in package:analyzer/src/workspace/workspace.dart. (If we tried to address this by declaring Workspace to be part of the analyzer's public API, we would have to also make ApiSignature, DartSdk, SourceFactory, SummaryDataStore, UriResolver, and WorkspacePackage part of the public API, and a lot of those classes don't seem like they're intended to be public.)

  • AnalysisOptions.lintRules has type List<LintRule>, but LintRule is defined in package:analyzer/src/lint/linter.dart. (We could declare LintRule to be part of the analyzer's public API, but that would force a lot of other types to become part of the public API too: AnalysisRule, LinterContext, LintRuleUnitContext, InheritanceManager3, NodeLintRegistry, PSDependency, PSDependencyList, PSEntry, PSEnvironment, PGitRepo, PSHost, PSNode, PSNodeList, PubspecVisitor, State, and WorkspacePackage)

I'll set up a time to discuss these with @bwilkerson.

@keertip keertip added type-enhancement A request for a change that isn't a bug P2 A bug or feature request we're likely to work on labels Feb 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-api Issues that impact the public API of the analyzer package 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
Projects
None yet
Development

No branches or pull requests

2 participants