Modernize LinterContext APIs
Deprecate `LinterContext.analysisOptions` and
`LinterContext.declaredVariables`; these are unused anywhere in the
Dart SDK. I suppose they could be used in an analyzer plugin if there
is a supported way that an analyzer plugin can get access to a
LinterContext?
Deprecate `LinterContext.resolveNameInScope`, replaced by
`LinterContext.resolveNameInScope2`, with a named bool parameter. See
https://dart.dev/effective-dart/design#avoid-positional-boolean-parameters.
Use third-person verbs in doc comments. See
https://dart.dev/effective-dart/documentation#prefer-starting-function-or-method-comments-with-third-person-verbs
Change-Id: Ifb9a02f9f4399b88e67b55c0c1bbc1491f501999
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/334082
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
diff --git a/pkg/analyzer/lib/src/lint/linter.dart b/pkg/analyzer/lib/src/lint/linter.dart
index 28f0035..c368b38 100644
--- a/pkg/analyzer/lib/src/lint/linter.dart
+++ b/pkg/analyzer/lib/src/lint/linter.dart
@@ -232,10 +232,16 @@
abstract class LinterContext {
List<LinterContextUnit> get allUnits;
+ @Deprecated('This field is being removed; for access to the analysis options '
+ 'that apply to `allUnits`, use '
+ '`currentUnit.unit.declaredElement?.session`.')
AnalysisOptions get analysisOptions;
LinterContextUnit get currentUnit;
+ @Deprecated('This field is being removed; for access to the '
+ 'DeclaredVariables that apply to `allUnits`, use '
+ '`currentUnit.unit.declaredElement?.session`.')
DeclaredVariables get declaredVariables;
InheritanceManager3 get inheritanceManager;
@@ -246,7 +252,7 @@
TypeSystem get typeSystem;
- /// Return `true` if it would be valid for the given [expression] to have
+ /// Returns `true` if it would be valid for the given [expression] to have
/// a keyword of `const`.
///
/// The [expression] is expected to be a node within one of the compilation
@@ -256,49 +262,56 @@
/// computationally expensive.
bool canBeConst(Expression expression);
- /// Return `true` if it would be valid for the given constructor declaration
+ /// Returns `true` if it would be valid for the given constructor declaration
/// [node] to have a keyword of `const`.
///
- /// The [node] is expected to be a node within one of the compilation
- /// units in [allUnits].
+ /// The [node] is expected to be a node within one of the compilation units in
+ /// [allUnits].
///
/// Note that this method can cause constant evaluation to occur, which can be
/// computationally expensive.
bool canBeConstConstructor(ConstructorDeclaration node);
- /// Return the result of evaluating the given expression.
+ /// Returns the result of evaluating the given expression.
LinterConstantEvaluationResult evaluateConstant(Expression node);
- /// Return `true` if the given [unit] is in a test directory.
+ /// Returns `true` if the given [unit] is in a test directory.
bool inTestDir(CompilationUnit unit);
- /// Return `true` if the [feature] is enabled in the library being linted.
+ /// Returns `true` if the [feature] is enabled in the library being linted.
bool isEnabled(Feature feature);
- /// Resolve the name `id` or `id=` (if [setter] is `true`) an the location
+ /// Resolves the name `id` or `id=` (if [setter] is `true`) at the location
/// of the [node], according to the "16.35 Lexical Lookup" of the language
/// specification.
+ @Deprecated('Use resolveNameInScope2')
LinterNameInScopeResolutionResult resolveNameInScope(
String id, bool setter, AstNode node);
+
+ /// Resolves the name `id` or `id=` (if [setter] is `true`) at the location
+ /// of the [node], according to the "16.35 Lexical Lookup" of the language
+ /// specification.
+ LinterNameInScopeResolutionResult resolveNameInScope2(
+ String id,
+ AstNode node, {
+ required bool setter,
+ });
}
-/// Implementation of [LinterContext]
class LinterContextImpl implements LinterContext {
@override
final List<LinterContextUnit> allUnits;
- @override
- final AnalysisOptions analysisOptions;
-
+ // TODO(srawlins): Remove when the public accessor, `analysisOption`, is
+ // removed.
+ final AnalysisOptions _analysisOptions;
@override
final LinterContextUnit currentUnit;
- @override
- final DeclaredVariables declaredVariables;
+ final DeclaredVariables _declaredVariables;
@override
final WorkspacePackage? package;
-
@override
final TypeProvider typeProvider;
@@ -308,19 +321,33 @@
@override
final InheritanceManager3 inheritanceManager;
- final List<String> testDirectories;
+ final List<String> _testDirectories;
LinterContextImpl(
this.allUnits,
this.currentUnit,
- this.declaredVariables,
+ DeclaredVariables declaredVariables,
this.typeProvider,
this.typeSystem,
this.inheritanceManager,
- this.analysisOptions,
+ AnalysisOptions analysisOptions,
this.package,
p.Context pathContext,
- ) : testDirectories = LinterContextImpl.getTestDirectories(pathContext);
+ ) : _declaredVariables = declaredVariables,
+ _analysisOptions = analysisOptions,
+ _testDirectories = getTestDirectories(pathContext);
+
+ @override
+ @Deprecated('This field is being removed; for access to the analysis options '
+ 'that apply to `allUnits`, use '
+ '`currentUnit.unit.declaredElement?.session`.')
+ AnalysisOptions get analysisOptions => _analysisOptions;
+
+ @override
+ @Deprecated('This field is being removed; for access to the '
+ 'DeclaredVariables that apply to `allUnits`, use '
+ '`currentUnit.unit.declaredElement?.session`.')
+ DeclaredVariables get declaredVariables => _declaredVariables;
@override
bool canBeConst(Expression expression) {
@@ -367,7 +394,7 @@
);
var evaluationEngine = ConstantEvaluationEngine(
- declaredVariables: declaredVariables,
+ declaredVariables: _declaredVariables,
isNonNullableByDefault: isEnabled(Feature.non_nullable),
configuration: ConstantEvaluationConfiguration(),
);
@@ -378,7 +405,7 @@
);
computeConstants(
- declaredVariables: declaredVariables,
+ declaredVariables: _declaredVariables,
constants: dependencies,
featureSet: libraryElement.featureSet,
configuration: ConstantEvaluationConfiguration(),
@@ -398,7 +425,7 @@
@override
bool inTestDir(CompilationUnit unit) {
var path = unit.declaredElement?.source.fullName;
- return path != null && testDirectories.any(path.contains);
+ return path != null && _testDirectories.any(path.contains);
}
@override
@@ -409,7 +436,15 @@
@override
LinterNameInScopeResolutionResult resolveNameInScope(
- String id, bool setter, AstNode node) {
+ String id, bool setter, AstNode node) =>
+ resolveNameInScope2(id, node, setter: setter);
+
+ @override
+ LinterNameInScopeResolutionResult resolveNameInScope2(
+ String id,
+ AstNode node, {
+ required bool setter,
+ }) {
Scope? scope;
for (AstNode? context = node; context != null; context = context.parent) {
scope = ScopeResolverVisitor.getNodeNameScope(context);
@@ -486,7 +521,7 @@
var dependenciesFinder = ConstantExpressionsDependenciesFinder();
node.accept(dependenciesFinder);
computeConstants(
- declaredVariables: declaredVariables,
+ declaredVariables: _declaredVariables,
constants: dependenciesFinder.dependencies.toList(),
featureSet: libraryElement.featureSet,
configuration: ConstantEvaluationConfiguration(),
@@ -503,7 +538,7 @@
ConstantVerifier(
errorReporter,
libraryElement,
- declaredVariables,
+ _declaredVariables,
),
);
return listener.hasConstError;
@@ -536,13 +571,18 @@
LinterContextParsedImpl(
this.allUnits,
this.currentUnit,
- // this.package,
);
@override
+ @Deprecated('This field is being removed; for access to the analysis options '
+ 'that apply to `allUnits`, use '
+ '`currentUnit.unit.declaredElement?.session`.')
AnalysisOptions get analysisOptions => throw UnimplementedError();
@override
+ @Deprecated('This field is being removed; for access to the '
+ 'DeclaredVariables that apply to `allUnits`, use '
+ '`currentUnit.unit.declaredElement?.session`.')
DeclaredVariables get declaredVariables =>
throw UnsupportedError('LinterContext with parsed results');
@@ -578,6 +618,14 @@
LinterNameInScopeResolutionResult resolveNameInScope(
String id, bool setter, AstNode node) =>
throw UnsupportedError('LinterContext with parsed results');
+
+ @override
+ LinterNameInScopeResolutionResult resolveNameInScope2(
+ String id,
+ AstNode node, {
+ required bool setter,
+ }) =>
+ throw UnsupportedError('LinterContext with parsed results');
}
class LinterContextUnit {
diff --git a/pkg/linter/lib/src/rules/avoid_types_as_parameter_names.dart b/pkg/linter/lib/src/rules/avoid_types_as_parameter_names.dart
index 1221b62..6da7056 100644
--- a/pkg/linter/lib/src/rules/avoid_types_as_parameter_names.dart
+++ b/pkg/linter/lib/src/rules/avoid_types_as_parameter_names.dart
@@ -82,7 +82,7 @@
}
bool _isTypeName(AstNode scope, Token name) {
- var result = context.resolveNameInScope(name.lexeme, false, scope);
+ var result = context.resolveNameInScope2(name.lexeme, scope, setter: false);
if (result.isRequestedName) {
var element = result.element;
return element is ClassElement ||
diff --git a/pkg/linter/lib/src/rules/unnecessary_this.dart b/pkg/linter/lib/src/rules/unnecessary_this.dart
index 368492d..e31af5d 100644
--- a/pkg/linter/lib/src/rules/unnecessary_this.dart
+++ b/pkg/linter/lib/src/rules/unnecessary_this.dart
@@ -111,7 +111,7 @@
var id = element.displayName;
var isSetter = element is PropertyAccessorElement && element.isSetter;
- var result = context.resolveNameInScope(id, isSetter, node);
+ var result = context.resolveNameInScope2(id, node, setter: isSetter);
// No result, definitely no shadowing.
// The requested element is inherited, or from an extension.
