DAS: Simplify public API of CorrectionProducer and friends
* CorrectionProducer:
* Remove `errorMessage`; unused.
* Make `configure` abstract; it is only here for the doc comment.
* MultiCorrectionProducer: remove `typeProvider`, `typeSystem`.
* ResolvedCorrectionProducer: make `coreTypeBool` private.
* _AbstractCorrectionProducer:
* Move `invalidNodes`; only used once.
* Remove `strictCasts`; unused.
* Remove `displayStringForType`; unnecessary convenience.
* Move `mightBeImplicitConstructor` to be a private extension
getter.
* Move `nameOfType` to be a private extension getter.
* Move `shouldWrapParenthesisBeforeAnd` to be a public extension
getter.
* Move DartFileEditBuilderExtension to be a private extension.
Change-Id: Icd1e17ccf398fafd1d9fee88d4b6a430a2deb50a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/367363
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
diff --git a/pkg/analysis_server/lib/src/services/correction/dart/abstract_producer.dart b/pkg/analysis_server/lib/src/services/correction/dart/abstract_producer.dart
index 8bdbc27..595f694 100644
--- a/pkg/analysis_server/lib/src/services/correction/dart/abstract_producer.dart
+++ b/pkg/analysis_server/lib/src/services/correction/dart/abstract_producer.dart
@@ -25,9 +25,7 @@
import 'package:analyzer/src/generated/engine.dart';
import 'package:analyzer_plugin/utilities/assist/assist.dart';
import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
-import 'package:analyzer_plugin/utilities/change_builder/change_builder_dart.dart';
import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
-import 'package:analyzer_plugin/utilities/range_factory.dart';
import 'package:meta/meta.dart';
/// How broadly a [CorrectionProducer] can be applied.
@@ -117,17 +115,12 @@
bool get canBeAppliedAutomatically =>
applicability == CorrectionApplicability.automatically;
- /// The length of the error message being fixed, or `null` if there is no
- /// diagnostic.
+ /// The length of the source range associated with the error message being
+ /// fixed, or `null` if there is no diagnostic.
int? get errorLength => diagnostic?.problemMessage.length;
- /// The text of the error message being fixed, or `null` if there is no
- /// diagnostic.
- String? get errorMessage =>
- diagnostic?.problemMessage.messageText(includeUrl: true);
-
- /// The offset of the error message being fixed, or `null` if there is no
- /// diagnostic.
+ /// The offset of the source range associated with the error message being
+ /// fixed, or `null` if there is no diagnostic.
int? get errorOffset => diagnostic?.problemMessage.offset;
/// The arguments that should be used when composing the message for a fix, or
@@ -162,9 +155,7 @@
///
/// If the fix needs to dynamically set [fixKind], it should be done here.
@override
- void configure(CorrectionProducerContext<T> context) {
- super.configure(context);
- }
+ void configure(CorrectionProducerContext<T> context);
}
class CorrectionProducerContext<UnitResult extends ParsedUnitResult> {
@@ -176,11 +167,10 @@
final AnalysisSessionHelper _sessionHelper;
final UnitResult _unitResult;
- // TODO(migration): Make it non-nullable, specialize "fix" context?
final DartFixContext? dartFixContext;
- /// A flag indicating whether the correction producers will be run in the
- /// context of applying bulk fixes.
+ /// Whether the correction producers are run in the context of applying bulk
+ /// fixes.
final bool _applyingBulkFixes;
final Diagnostic? _diagnostic;
@@ -191,13 +181,13 @@
CorrectionProducerContext._({
required UnitResult unitResult,
- bool applyingBulkFixes = false,
- this.dartFixContext,
- Diagnostic? diagnostic,
+ required bool applyingBulkFixes,
+ required this.dartFixContext,
+ required Diagnostic? diagnostic,
required AstNode node,
required Token token,
- int selectionOffset = -1,
- int selectionLength = 0,
+ required int selectionOffset,
+ required int selectionLength,
}) : _unitResult = unitResult,
_sessionHelper = AnalysisSessionHelper(unitResult.session),
_utils = CorrectionUtils(unitResult),
@@ -301,12 +291,6 @@
/// The individual producers generated by this producer.
Future<List<ResolvedCorrectionProducer>> get producers;
-
- TypeProvider get typeProvider => unitResult.typeProvider;
-
- /// The type system appropriate to the library in which the correction is
- /// requested.
- TypeSystem get typeSystem => unitResult.typeSystem;
}
/// An object that can compute a correction (fix or assist) in a Dart file using
@@ -322,9 +306,6 @@
sessionHelper.session.analysisContext
.getAnalysisOptionsForFile(unitResult.file) as AnalysisOptionsImpl;
- /// The type for the class `bool` from `dart:core`.
- DartType get coreTypeBool => unitResult.typeProvider.boolType;
-
/// Whether [node] is in a static context.
bool get inStaticContext {
// Constructor initializers cannot reference `this`.
@@ -351,6 +332,9 @@
/// requested.
TypeSystem get typeSystem => unitResult.typeSystem;
+ /// The type for the class `bool` from `dart:core`.
+ DartType get _coreTypeBool => typeProvider.boolType;
+
/// Returns the class declaration for the given [element], or `null` if there
/// is no such class.
Future<ClassDeclaration?> getClassDeclaration(ClassElement element) async {
@@ -486,35 +470,35 @@
if (parent is AssertStatement) {
var statement = parent;
if (statement.condition == expression) {
- return coreTypeBool;
+ return _coreTypeBool;
}
}
// `if ( myFunction() ) {}`.
if (parent is IfStatement) {
var statement = parent;
if (statement.expression == expression) {
- return coreTypeBool;
+ return _coreTypeBool;
}
}
// `while ( myFunction() ) {}`.
if (parent is WhileStatement) {
var statement = parent;
if (statement.condition == expression) {
- return coreTypeBool;
+ return _coreTypeBool;
}
}
// `do {} while ( myFunction() );`.
if (parent is DoStatement) {
var statement = parent;
if (statement.condition == expression) {
- return coreTypeBool;
+ return _coreTypeBool;
}
}
// `!myFunction()`.
if (parent is PrefixExpression) {
var prefixExpression = parent;
if (prefixExpression.operator.type == TokenType.BANG) {
- return coreTypeBool;
+ return _coreTypeBool;
}
}
// Binary expression `&&` or `||`.
@@ -523,7 +507,7 @@
var operatorType = binaryExpression.operator.type;
if (operatorType == TokenType.AMPERSAND_AMPERSAND ||
operatorType == TokenType.BAR_BAR) {
- return coreTypeBool;
+ return _coreTypeBool;
}
}
}
@@ -577,11 +561,6 @@
String get file => _context.path;
- /// See [CompilationUnitImpl.invalidNodes].
- List<AstNode> get invalidNodes {
- return (unit as CompilationUnitImpl).invalidNodes;
- }
-
AstNode get node => _context._node;
ResourceProvider get resourceProvider => unitResult.session.resourceProvider;
@@ -594,15 +573,6 @@
AnalysisSessionHelper get sessionHelper => _context._sessionHelper;
- bool get strictCasts {
- var file = _context.dartFixContext?.resolvedResult.file;
- // TODO(pq): can this ever happen?
- if (file == null) return false;
- var analysisOptions = _context._unitResult.session.analysisContext
- .getAnalysisOptionsForFile(file) as AnalysisOptionsImpl;
- return analysisOptions.strictCasts;
- }
-
Token get token => _context._token;
CompilationUnit get unit => _context._unitResult.unit;
@@ -611,16 +581,12 @@
CorrectionUtils get utils => _context._utils;
- /// Configure this producer based on the [context].
+ /// Configures this producer based on the [context].
@mustCallSuper
void configure(CorrectionProducerContext<T> context) {
_context = context;
}
- /// Returns the text that should be displayed to users when referring to the
- /// given [type].
- String displayStringForType(DartType type) => type.getDisplayString();
-
CodeStyleOptions getCodeStyleOptions(File file) =>
sessionHelper.session.analysisContext
.getAnalysisOptionsForFile(file)
@@ -650,19 +616,17 @@
}
/// Returns the text of the given [range] in the unit.
- String getRangeText(SourceRange range) {
- return utils.getRangeText(range);
- }
+ String getRangeText(SourceRange range) => utils.getRangeText(range);
/// Returns the mapping from a library (that is available to this context) to
/// a top-level declaration that is exported (not necessary declared) by this
- /// library, and has the requested base name. For getters and setters the
- /// corresponding top-level variable is returned.
+ /// library, and has the requested base name.
+ ///
+ /// For getters and setters the corresponding top-level variable is returned.
Future<Map<LibraryElement, Element>> getTopLevelDeclarations(
String baseName,
- ) {
- return _context.dartFixContext!.getTopLevelDeclarations(baseName);
- }
+ ) =>
+ _context.dartFixContext!.getTopLevelDeclarations(baseName);
/// Returns whether the selection covers an operator of the given
/// [binaryExpression].
@@ -686,73 +650,9 @@
return false;
}
- /// Return libraries with extensions that declare non-static public
+ /// Returns libraries with extensions that declare non-static public
/// extension members with the [memberName].
Stream<LibraryElement> librariesWithExtensions(String memberName) {
return _context.dartFixContext!.librariesWithExtensions(memberName);
}
-
- /// Return `true` if the given [node] is in a location where an implicit
- /// constructor invocation would be allowed.
- bool mightBeImplicitConstructor(AstNode node) {
- if (node is SimpleIdentifier) {
- var parent = node.parent;
- if (parent is MethodInvocation) {
- return parent.realTarget == null;
- }
- }
- return false;
- }
-
- /// If the [node] might be a type name, return its name.
- String? nameOfType(AstNode node) {
- if (node is SimpleIdentifier) {
- var name = node.name;
- if (node.parent is NamedType || _isNameOfType(name)) {
- return name;
- }
- }
- return null;
- }
-
- /// Return `true` if the given [expression] should be wrapped with parenthesis
- /// when we want to use it as operand of a logical `and` expression.
- bool shouldWrapParenthesisBeforeAnd(Expression expression) {
- if (expression is BinaryExpression) {
- var binary = expression;
- var precedence = binary.operator.type.precedence;
- return precedence < TokenClass.LOGICAL_AND_OPERATOR.precedence;
- }
- return false;
- }
-
- /// Return `true` if the [name] is capitalized.
- bool _isNameOfType(String name) {
- if (name.isEmpty) {
- return false;
- }
- var firstLetter = name.substring(0, 1);
- if (firstLetter.toUpperCase() != firstLetter) {
- return false;
- }
- return true;
- }
-}
-
-extension DartFileEditBuilderExtension on DartFileEditBuilder {
- /// Add edits to the [builder] to remove any parentheses enclosing the
- /// [expression].
- // TODO(brianwilkerson): Consider moving this to DartFileEditBuilder.
- void removeEnclosingParentheses(Expression expression) {
- var precedence = getExpressionPrecedence(expression);
- while (expression.parent is ParenthesizedExpression) {
- var parenthesized = expression.parent as ParenthesizedExpression;
- if (getExpressionParentPrecedence(parenthesized) > precedence) {
- break;
- }
- addDeletion(range.token(parenthesized.leftParenthesis));
- addDeletion(range.token(parenthesized.rightParenthesis));
- expression = parenthesized;
- }
- }
}
diff --git a/pkg/analysis_server/lib/src/services/correction/dart/change_type_annotation.dart b/pkg/analysis_server/lib/src/services/correction/dart/change_type_annotation.dart
index c264f2c..a256c59 100644
--- a/pkg/analysis_server/lib/src/services/correction/dart/change_type_annotation.dart
+++ b/pkg/analysis_server/lib/src/services/correction/dart/change_type_annotation.dart
@@ -48,8 +48,8 @@
if (newType is InterfaceType ||
newType is FunctionType ||
newType is RecordType) {
- _oldAnnotation = displayStringForType(typeNode.typeOrThrow);
- _newAnnotation = displayStringForType(newType);
+ _oldAnnotation = typeNode.typeOrThrow.getDisplayString();
+ _newAnnotation = newType.getDisplayString();
await builder.addDartFileEdit(file, (builder) {
if (builder.canWriteType(newType)) {
builder.addReplacement(range.node(typeNode), (builder) {
diff --git a/pkg/analysis_server/lib/src/services/correction/dart/create_class.dart b/pkg/analysis_server/lib/src/services/correction/dart/create_class.dart
index 45a4a1b..417f37f 100644
--- a/pkg/analysis_server/lib/src/services/correction/dart/create_class.dart
+++ b/pkg/analysis_server/lib/src/services/correction/dart/create_class.dart
@@ -55,14 +55,14 @@
className = targetNode.name2.lexeme;
requiresConstConstructor |= _requiresConstConstructor(targetNode);
} else if (targetNode is SimpleIdentifier) {
- className = nameOfType(targetNode);
+ className = targetNode.nameOfType;
requiresConstConstructor |= _requiresConstConstructor(targetNode);
} else if (targetNode is PrefixedIdentifier) {
prefixElement = targetNode.prefix.staticElement;
if (prefixElement == null) {
return;
}
- className = nameOfType(targetNode.identifier);
+ className = targetNode.identifier.nameOfType;
} else {
return;
}
@@ -159,3 +159,29 @@
return false;
}
}
+
+extension on AstNode {
+ /// If this might be a type name, return its name.
+ String? get nameOfType {
+ var self = this;
+ if (self is SimpleIdentifier) {
+ var name = self.name;
+ if (self.parent is NamedType || _isNameOfType(name)) {
+ return name;
+ }
+ }
+ return null;
+ }
+
+ /// Return `true` if the [name] is capitalized.
+ static bool _isNameOfType(String name) {
+ if (name.isEmpty) {
+ return false;
+ }
+ var firstLetter = name.substring(0, 1);
+ if (firstLetter.toUpperCase() != firstLetter) {
+ return false;
+ }
+ return true;
+ }
+}
diff --git a/pkg/analysis_server/lib/src/services/correction/dart/import_library.dart b/pkg/analysis_server/lib/src/services/correction/dart/import_library.dart
index 97f1604..b21c67b 100644
--- a/pkg/analysis_server/lib/src/services/correction/dart/import_library.dart
+++ b/pkg/analysis_server/lib/src/services/correction/dart/import_library.dart
@@ -137,7 +137,7 @@
ElementKind.MIXIN,
ElementKind.TYPE_ALIAS,
]);
- } else if (mightBeImplicitConstructor(targetNode)) {
+ } else if (targetNode.mightBeImplicitConstructor) {
var typeName = (targetNode as SimpleIdentifier).name;
await _importLibraryForElement(producers, typeName, const [
ElementKind.CLASS,
@@ -147,7 +147,6 @@
return producers;
}
- @override
String? nameOfType(AstNode node) {
var parent = node.parent;
switch (node) {
@@ -584,3 +583,17 @@
});
}
}
+
+extension on AstNode {
+ /// Whether this [AstNode] is in a location where an implicit constructor
+ /// invocation would be allowed.
+ bool get mightBeImplicitConstructor {
+ if (this is SimpleIdentifier) {
+ var parent = this.parent;
+ if (parent is MethodInvocation) {
+ return parent.realTarget == null;
+ }
+ }
+ return false;
+ }
+}
diff --git a/pkg/analysis_server/lib/src/services/correction/dart/join_if_with_inner.dart b/pkg/analysis_server/lib/src/services/correction/dart/join_if_with_inner.dart
index f0b4744..b24b285 100644
--- a/pkg/analysis_server/lib/src/services/correction/dart/join_if_with_inner.dart
+++ b/pkg/analysis_server/lib/src/services/correction/dart/join_if_with_inner.dart
@@ -47,10 +47,10 @@
var innerCondition = innerIfStatement.expression;
var targetConditionSource = utils.getNodeText(targetCondition);
var innerConditionSource = utils.getNodeText(innerCondition);
- if (shouldWrapParenthesisBeforeAnd(targetCondition)) {
+ if (targetCondition.shouldWrapParenthesisBeforeAnd) {
targetConditionSource = '($targetConditionSource)';
}
- if (shouldWrapParenthesisBeforeAnd(innerCondition)) {
+ if (innerCondition.shouldWrapParenthesisBeforeAnd) {
innerConditionSource = '($innerConditionSource)';
}
var condition = '$targetConditionSource && $innerConditionSource';
diff --git a/pkg/analysis_server/lib/src/services/correction/dart/join_if_with_outer.dart b/pkg/analysis_server/lib/src/services/correction/dart/join_if_with_outer.dart
index 4a3a812..45dd05f 100644
--- a/pkg/analysis_server/lib/src/services/correction/dart/join_if_with_outer.dart
+++ b/pkg/analysis_server/lib/src/services/correction/dart/join_if_with_outer.dart
@@ -53,10 +53,10 @@
var outerCondition = outerIfStatement.expression;
var targetConditionSource = utils.getNodeText(targetCondition);
var outerConditionSource = utils.getNodeText(outerCondition);
- if (shouldWrapParenthesisBeforeAnd(targetCondition)) {
+ if (targetCondition.shouldWrapParenthesisBeforeAnd) {
targetConditionSource = '($targetConditionSource)';
}
- if (shouldWrapParenthesisBeforeAnd(outerCondition)) {
+ if (outerCondition.shouldWrapParenthesisBeforeAnd) {
outerConditionSource = '($outerConditionSource)';
}
var condition = '$outerConditionSource && $targetConditionSource';
diff --git a/pkg/analysis_server/lib/src/services/correction/dart/remove_constructor.dart b/pkg/analysis_server/lib/src/services/correction/dart/remove_constructor.dart
index fdb62ba..21b39f6 100644
--- a/pkg/analysis_server/lib/src/services/correction/dart/remove_constructor.dart
+++ b/pkg/analysis_server/lib/src/services/correction/dart/remove_constructor.dart
@@ -4,8 +4,8 @@
import 'package:analysis_server/src/services/correction/dart/abstract_producer.dart';
import 'package:analysis_server/src/services/correction/fix.dart';
-import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/token.dart';
+import 'package:analyzer/src/dart/ast/ast.dart';
import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
import 'package:analyzer_plugin/utilities/range_factory.dart';
@@ -51,8 +51,9 @@
return null;
}
+ var invalidNodes = (unit as CompilationUnitImpl).invalidNodes;
for (var constructor in invalidNodes) {
- if (constructor is ConstructorDeclaration) {
+ if (constructor is ConstructorDeclarationImpl) {
if (range.node(constructor).contains(errorOffset)) {
return constructor;
}
diff --git a/pkg/analysis_server/lib/src/services/correction/dart/remove_unnecessary_cast.dart b/pkg/analysis_server/lib/src/services/correction/dart/remove_unnecessary_cast.dart
index fc8b40f..052dbcf 100644
--- a/pkg/analysis_server/lib/src/services/correction/dart/remove_unnecessary_cast.dart
+++ b/pkg/analysis_server/lib/src/services/correction/dart/remove_unnecessary_cast.dart
@@ -4,8 +4,10 @@
import 'package:analysis_server/src/services/correction/dart/abstract_producer.dart';
import 'package:analysis_server/src/services/correction/fix.dart';
+import 'package:analysis_server/src/services/correction/util.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
+import 'package:analyzer_plugin/utilities/change_builder/change_builder_dart.dart';
import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
import 'package:analyzer_plugin/utilities/range_factory.dart';
@@ -35,3 +37,20 @@
});
}
}
+
+extension on DartFileEditBuilder {
+ /// Adds edits to this [DartFileEditBuilder] to remove any parentheses
+ /// enclosing the [expression].
+ void removeEnclosingParentheses(Expression expression) {
+ var precedence = getExpressionPrecedence(expression);
+ while (expression.parent is ParenthesizedExpression) {
+ var parenthesized = expression.parent as ParenthesizedExpression;
+ if (getExpressionParentPrecedence(parenthesized) > precedence) {
+ break;
+ }
+ addDeletion(range.token(parenthesized.leftParenthesis));
+ addDeletion(range.token(parenthesized.rightParenthesis));
+ expression = parenthesized;
+ }
+ }
+}
diff --git a/pkg/analysis_server/lib/src/utilities/extensions/ast.dart b/pkg/analysis_server/lib/src/utilities/extensions/ast.dart
index 35140cd..320ffaf 100644
--- a/pkg/analysis_server/lib/src/utilities/extensions/ast.dart
+++ b/pkg/analysis_server/lib/src/utilities/extensions/ast.dart
@@ -8,6 +8,7 @@
import 'package:analyzer/dart/ast/visitor.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/source/source.dart';
+import 'package:analyzer/src/dart/ast/token.dart';
import 'package:analyzer/src/dart/ast/utilities.dart';
import 'package:analyzer/src/generated/source.dart';
import 'package:analyzer/src/utilities/extensions/collection.dart';
@@ -350,6 +351,17 @@
}
return false;
}
+
+ /// Whether this [Expression] should be wrapped with parentheses when we want
+ /// to use it as operand of a logical and-expression.
+ bool get shouldWrapParenthesisBeforeAnd {
+ var self = this;
+ if (self is! BinaryExpression) {
+ return false;
+ }
+ var precedence = self.operator.type.precedence;
+ return precedence < TokenClass.LOGICAL_AND_OPERATOR.precedence;
+ }
}
extension FunctionBodyExtension on FunctionBody {
