Skip to content

Commit 25a309a

Browse files
srawlinsCommit Queue
authored andcommitted
linter: Re-implement LinterContext.canBeConst and .canBeConstConstructor
Similar to `computeConstantValue`, an Expression or a ConstructorDeclaration each have enough information on their fields to calculate these values. * This also lets us remove the now unused `CorrectionProducer.getLinterContext`. * Also two functions in prefer_const_constructors_in_immutibles can be made static: `_hasConstConstructorInvocation` and `_hasImmutableAnnotation` (so they get resorted in this change). * Several rules no longer need to pass around a LinterContext. Change-Id: I584c62e2673658c5004283842a6525eea89f805b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/364964 Reviewed-by: Phil Quitslund <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]>
1 parent 51ac900 commit 25a309a

File tree

8 files changed

+128
-141
lines changed

8 files changed

+128
-141
lines changed

pkg/analysis_server/lib/src/services/correction/dart/abstract_producer.dart

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,14 @@ import 'package:analyzer/source/source_range.dart';
2121
import 'package:analyzer/src/dart/analysis/session_helper.dart';
2222
import 'package:analyzer/src/dart/ast/ast.dart';
2323
import 'package:analyzer/src/dart/ast/utilities.dart';
24-
import 'package:analyzer/src/dart/element/inheritance_manager3.dart';
2524
import 'package:analyzer/src/dart/element/type.dart';
26-
import 'package:analyzer/src/dart/element/type_system.dart';
2725
import 'package:analyzer/src/generated/engine.dart';
28-
import 'package:analyzer/src/lint/linter.dart';
2926
import 'package:analyzer_plugin/utilities/assist/assist.dart';
3027
import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
3128
import 'package:analyzer_plugin/utilities/change_builder/change_builder_dart.dart';
3229
import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
3330
import 'package:analyzer_plugin/utilities/range_factory.dart';
3431
import 'package:meta/meta.dart';
35-
import 'package:path/path.dart' as path;
3632

3733
/// An object that can compute a correction (fix or assist) in a Dart file.
3834
abstract class CorrectionProducer<T extends ParsedUnitResult>
@@ -353,20 +349,6 @@ abstract class ResolvedCorrectionProducer
353349
return null;
354350
}
355351

356-
LinterContext getLinterContext(path.Context pathContext) {
357-
return LinterContextImpl(
358-
[] /* allUnits, unused */,
359-
LinterContextUnit(unitResult.content, unitResult.unit),
360-
unitResult.session.declaredVariables,
361-
typeProvider,
362-
typeSystem as TypeSystemImpl,
363-
InheritanceManager3(), // Unused.
364-
analysisOptions,
365-
null,
366-
pathContext,
367-
);
368-
}
369-
370352
/// Returns the mixin declaration for the given [element], or `null` if there
371353
/// is no such mixin.
372354
Future<MixinDeclaration?> getMixinDeclaration(MixinElement element) async {

pkg/analysis_server/lib/src/services/correction/dart/add_const.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import 'package:analyzer/dart/ast/ast.dart';
88
import 'package:analyzer/dart/ast/token.dart';
99
import 'package:analyzer/dart/ast/visitor.dart';
1010
import 'package:analyzer/source/source_range.dart';
11+
import 'package:analyzer/src/lint/linter.dart';
1112
import 'package:analyzer_plugin/src/utilities/change_builder/change_builder_dart.dart';
1213
import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
1314
import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
@@ -59,8 +60,7 @@ class AddConst extends ResolvedCorrectionProducer {
5960
}
6061
if (targetNode is ConstantPattern) {
6162
var expression = targetNode.expression;
62-
var canBeConst =
63-
getLinterContext(resourceProvider.pathContext).canBeConst(expression);
63+
var canBeConst = expression.canBeConst;
6464
if (canBeConst) {
6565
await builder.addDartFileEdit(file, (builder) {
6666
var offset = expression.offset;

pkg/analysis_server/lib/src/services/correction/dart/replace_with_decorated_box.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import 'package:analysis_server/src/services/linter/lint_names.dart';
88
import 'package:analyzer/dart/ast/ast.dart';
99
import 'package:analyzer/dart/ast/token.dart';
1010
import 'package:analyzer/error/error.dart';
11+
import 'package:analyzer/src/lint/linter.dart';
1112
import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
1213
import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
1314
import 'package:analyzer_plugin/utilities/range_factory.dart';
@@ -44,7 +45,6 @@ class ReplaceWithDecoratedBox extends ResolvedCorrectionProducer {
4445

4546
var deletions = <Token>[];
4647
var replacements = <AstNode, String>{};
47-
var linterContext = getLinterContext(resourceProvider.pathContext);
4848

4949
void replace(Expression expression, {required bool addConst}) {
5050
if (expression is InstanceCreationExpression && _hasLint(expression)) {
@@ -57,7 +57,7 @@ class ReplaceWithDecoratedBox extends ResolvedCorrectionProducer {
5757
/// and return whether it can be a `const` or not.
5858
bool canExpressionBeConst(Expression expression,
5959
{required bool isReplace}) {
60-
var canBeConst = linterContext.canBeConst(expression);
60+
var canBeConst = expression.canBeConst;
6161
if (!canBeConst &&
6262
isReplace &&
6363
expression is InstanceCreationExpression &&

pkg/analyzer/lib/src/error/best_practices_verifier.dart

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1155,9 +1155,7 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
11551155
if (constructor == null) {
11561156
return;
11571157
}
1158-
if (!node.isConst &&
1159-
constructor.hasLiteral &&
1160-
_linterContext.canBeConst(node)) {
1158+
if (!node.isConst && constructor.hasLiteral && node.canBeConst) {
11611159
// Echoing jwren's `TODO` from _checkForDeprecatedMemberUse:
11621160
// TODO(jwren): We should modify ConstructorElement.getDisplayName(), or
11631161
// have the logic centralized elsewhere, instead of doing this logic

pkg/analyzer/lib/src/lint/linter.dart

Lines changed: 100 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,7 @@ abstract class LinterContext {
217217
///
218218
/// Note that this method can cause constant evaluation to occur, which can be
219219
/// computationally expensive.
220+
@Deprecated('Use `expression.canBeConst`')
220221
bool canBeConst(Expression expression);
221222

222223
/// Returns `true` if it would be valid for the given constructor declaration
@@ -227,6 +228,7 @@ abstract class LinterContext {
227228
///
228229
/// Note that this method can cause constant evaluation to occur, which can be
229230
/// computationally expensive.
231+
@Deprecated('Use `expression.canBeConstConstructor`')
230232
bool canBeConstConstructor(ConstructorDeclaration node);
231233

232234
/// Returns `true` if the given [unit] is in a test directory.
@@ -304,35 +306,11 @@ class LinterContextImpl implements LinterContext {
304306
DeclaredVariables get declaredVariables => _declaredVariables;
305307

306308
@override
307-
bool canBeConst(Expression expression) {
308-
if (expression is InstanceCreationExpressionImpl) {
309-
return _canBeConstInstanceCreation(expression);
310-
} else if (expression is TypedLiteralImpl) {
311-
return _canBeConstTypedLiteral(expression);
312-
} else {
313-
return false;
314-
}
315-
}
309+
bool canBeConst(Expression expression) => expression.canBeConst;
316310

317311
@override
318-
bool canBeConstConstructor(covariant ConstructorDeclarationImpl node) {
319-
var element = node.declaredElement!;
320-
321-
var classElement = element.enclosingElement;
322-
if (classElement is ClassElement && classElement.hasNonFinalField) {
323-
return false;
324-
}
325-
326-
var oldKeyword = node.constKeyword;
327-
try {
328-
temporaryConstConstructorElements[element] = true;
329-
node.constKeyword = KeywordToken(Keyword.CONST, node.offset);
330-
return !_hasConstantVerifierError(node);
331-
} finally {
332-
temporaryConstConstructorElements[element] = null;
333-
node.constKeyword = oldKeyword;
334-
}
335-
}
312+
bool canBeConstConstructor(covariant ConstructorDeclarationImpl node) =>
313+
node.canBeConst;
336314

337315
@override
338316
bool inTestDir(CompilationUnit unit) {
@@ -389,69 +367,6 @@ class LinterContextImpl implements LinterContext {
389367
return const LinterNameInScopeResolutionResult._none();
390368
}
391369

392-
bool _canBeConstInstanceCreation(InstanceCreationExpressionImpl node) {
393-
//
394-
// Verify that the invoked constructor is a const constructor.
395-
//
396-
var element = node.constructorName.staticElement;
397-
if (element == null || !element.isConst) {
398-
return false;
399-
}
400-
401-
// Ensure that dependencies (e.g. default parameter values) are computed.
402-
var implElement = element.declaration as ConstructorElementImpl;
403-
implElement.computeConstantDependencies();
404-
405-
//
406-
// Verify that the evaluation of the constructor would not produce an
407-
// exception.
408-
//
409-
var oldKeyword = node.keyword;
410-
try {
411-
node.keyword = KeywordToken(Keyword.CONST, node.offset);
412-
return !_hasConstantVerifierError(node);
413-
} finally {
414-
node.keyword = oldKeyword;
415-
}
416-
}
417-
418-
bool _canBeConstTypedLiteral(TypedLiteralImpl node) {
419-
var oldKeyword = node.constKeyword;
420-
try {
421-
node.constKeyword = KeywordToken(Keyword.CONST, node.offset);
422-
return !_hasConstantVerifierError(node);
423-
} finally {
424-
node.constKeyword = oldKeyword;
425-
}
426-
}
427-
428-
/// Returns whether [ConstantVerifier] reports an error for the [node].
429-
bool _hasConstantVerifierError(AstNode node) {
430-
var unitElement = currentUnit.unit.declaredElement!;
431-
var libraryElement = unitElement.library as LibraryElementImpl;
432-
433-
var dependenciesFinder = ConstantExpressionsDependenciesFinder();
434-
node.accept(dependenciesFinder);
435-
computeConstants(
436-
declaredVariables: _declaredVariables,
437-
constants: dependenciesFinder.dependencies.toList(),
438-
featureSet: libraryElement.featureSet,
439-
configuration: ConstantEvaluationConfiguration(),
440-
);
441-
442-
var listener = _ConstantAnalysisErrorListener();
443-
var errorReporter = ErrorReporter(listener, unitElement.source);
444-
445-
node.accept(
446-
ConstantVerifier(
447-
errorReporter,
448-
libraryElement,
449-
_declaredVariables,
450-
),
451-
);
452-
return listener.hasConstError;
453-
}
454-
455370
static List<String> getTestDirectories(p.Context pathContext) {
456371
var separator = pathContext.separator;
457372
return [
@@ -873,7 +788,73 @@ enum _LinterNameInScopeResolutionResultState {
873788
differentName
874789
}
875790

791+
extension on AstNode {
792+
/// Whether [ConstantVerifier] reports an error when computing the value of
793+
/// `this` as a constant.
794+
bool get hasConstantVerifierError {
795+
var unitElement = thisOrAncestorOfType<CompilationUnit>()?.declaredElement;
796+
if (unitElement == null) return false;
797+
var libraryElement = unitElement.library as LibraryElementImpl;
798+
799+
var dependenciesFinder = ConstantExpressionsDependenciesFinder();
800+
accept(dependenciesFinder);
801+
computeConstants(
802+
declaredVariables: unitElement.session.declaredVariables,
803+
constants: dependenciesFinder.dependencies.toList(),
804+
featureSet: libraryElement.featureSet,
805+
configuration: ConstantEvaluationConfiguration(),
806+
);
807+
808+
var listener = _ConstantAnalysisErrorListener();
809+
var errorReporter = ErrorReporter(listener, unitElement.source);
810+
811+
accept(
812+
ConstantVerifier(
813+
errorReporter,
814+
libraryElement,
815+
unitElement.session.declaredVariables,
816+
),
817+
);
818+
return listener.hasConstError;
819+
}
820+
}
821+
822+
extension ConstructorDeclarationExtension on ConstructorDeclaration {
823+
bool get canBeConst {
824+
var element = declaredElement!;
825+
826+
var classElement = element.enclosingElement;
827+
if (classElement is ClassElement && classElement.hasNonFinalField) {
828+
return false;
829+
}
830+
831+
var oldKeyword = constKeyword;
832+
var self = this as ConstructorDeclarationImpl;
833+
try {
834+
temporaryConstConstructorElements[element] = true;
835+
self.constKeyword = KeywordToken(Keyword.CONST, offset);
836+
return !hasConstantVerifierError;
837+
} finally {
838+
temporaryConstConstructorElements[element] = null;
839+
self.constKeyword = oldKeyword;
840+
}
841+
}
842+
}
843+
876844
extension ExpressionExtension on Expression {
845+
/// Whether it would be valid for this expression to have a `const` keyword.
846+
///
847+
/// Note that this method can cause constant evaluation to occur, which can be
848+
/// computationally expensive.
849+
bool get canBeConst {
850+
var self = this;
851+
return switch (self) {
852+
InstanceCreationExpressionImpl() => _canBeConstInstanceCreation(self),
853+
TypedLiteralImpl() => _canBeConstTypedLiteral(self),
854+
_ => false,
855+
};
856+
}
857+
877858
/// Computes the constant value of `this`, if it has one.
878859
///
879860
/// Returns a [LinterConstantEvaluationResult], containing both the computed
@@ -910,4 +891,33 @@ extension ExpressionExtension on Expression {
910891
var dartObject = constant is DartObjectImpl ? constant : null;
911892
return LinterConstantEvaluationResult(dartObject, errorListener.errors);
912893
}
894+
895+
bool _canBeConstInstanceCreation(InstanceCreationExpressionImpl node) {
896+
var element = node.constructorName.staticElement;
897+
if (element == null || !element.isConst) return false;
898+
899+
// Ensure that dependencies (e.g. default parameter values) are computed.
900+
var implElement = element.declaration as ConstructorElementImpl;
901+
implElement.computeConstantDependencies();
902+
903+
// Verify that the evaluation of the constructor would not produce an
904+
// exception.
905+
var oldKeyword = node.keyword;
906+
try {
907+
node.keyword = KeywordToken(Keyword.CONST, offset);
908+
return !hasConstantVerifierError;
909+
} finally {
910+
node.keyword = oldKeyword;
911+
}
912+
}
913+
914+
bool _canBeConstTypedLiteral(TypedLiteralImpl node) {
915+
var oldKeyword = node.constKeyword;
916+
try {
917+
node.constKeyword = KeywordToken(Keyword.CONST, offset);
918+
return !hasConstantVerifierError;
919+
} finally {
920+
node.constKeyword = oldKeyword;
921+
}
922+
}
913923
}

pkg/linter/lib/src/rules/prefer_const_constructors.dart

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import 'package:analyzer/dart/ast/ast.dart';
66
import 'package:analyzer/dart/ast/visitor.dart';
77
import 'package:analyzer/dart/element/element.dart';
88
import 'package:analyzer/dart/element/type.dart';
9+
import 'package:analyzer/src/lint/linter.dart'; // ignore: implementation_imports
910

1011
import '../analyzer.dart';
1112
import '../extensions.dart';
@@ -72,17 +73,15 @@ class PreferConstConstructors extends LintRule {
7273
@override
7374
void registerNodeProcessors(
7475
NodeLintRegistry registry, LinterContext context) {
75-
var visitor = _Visitor(this, context);
76+
var visitor = _Visitor(this);
7677
registry.addInstanceCreationExpression(this, visitor);
7778
}
7879
}
7980

8081
class _Visitor extends SimpleAstVisitor<void> {
8182
final LintRule rule;
8283

83-
final LinterContext context;
84-
85-
_Visitor(this.rule, this.context);
84+
_Visitor(this.rule);
8685

8786
@override
8887
void visitInstanceCreationExpression(InstanceCreationExpression node) {
@@ -120,7 +119,7 @@ class _Visitor extends SimpleAstVisitor<void> {
120119
}
121120
}
122121

123-
if (context.canBeConst(node)) {
122+
if (node.canBeConst) {
124123
rule.reportLint(node);
125124
}
126125
}

0 commit comments

Comments
 (0)