From 51d7b8477d4abe6cc54b203dcf6fae3a45f0c7db Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Wed, 3 Apr 2024 20:57:51 +0000 Subject: [PATCH] DAS: Move two CorrectionUtils methods to DartEditBuilder `addMethodInsert` and `addCaseClauseAtEndInsert` are only used by code which combines them with DartEditBuilder, so the change is pretty simple. A few utilities are needed in DartEditBuilder, which are also not complicated, such as a utility for getting the existing indentation string on a line. Using this utility in the shared code should improve someo of the existing fixes and assists, which didn't use it before. The same goes for looking at whether left and right brackets are synthetic. So this change comes with a few minor improvements. Change-Id: I5b235021ac234b4a50b09dca6d14a28b3f163e5e Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/360980 Reviewed-by: Brian Wilkerson Reviewed-by: Konstantin Shcheglov Commit-Queue: Samuel Rawlins --- .../add_diagnostic_property_reference.dart | 17 +--- .../dart/add_missing_enum_case_clauses.dart | 25 +---- .../add_missing_enum_like_case_clauses.dart | 13 +-- .../dart/add_missing_switch_cases.dart | 26 ++--- .../correction/dart/create_method.dart | 36 +++---- .../lib/src/services/correction/util.dart | 29 ------ ...dd_diagnostic_property_reference_test.dart | 1 + .../change_builder/change_builder_dart.dart | 94 +++++++++++++++++-- .../change_builder/change_builder_dart.dart | 21 +++++ 9 files changed, 148 insertions(+), 114 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/correction/dart/add_diagnostic_property_reference.dart b/pkg/analysis_server/lib/src/services/correction/dart/add_diagnostic_property_reference.dart index 7ac7c4d1360..f2a86fef9a9 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/add_diagnostic_property_reference.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/add_diagnostic_property_reference.dart @@ -122,26 +122,19 @@ class AddDiagnosticPropertyReference extends ResolvedCorrectionProducer { .where((e) => e.name.lexeme == 'debugFillProperties') .singleOrNull; if (debugFillProperties == null) { - var location = utils.prepareNewMethodLocation(classDeclaration); - if (location == null) { - return; - } - - final insertOffset = location.offset; await builder.addDartFileEdit(file, (builder) { - builder.addInsertion(utils.getLineNext(insertOffset), (builder) { - final declPrefix = - utils.getLinePrefix(classDeclaration.offset) + utils.oneIndent; - final bodyPrefix = declPrefix + utils.oneIndent; + builder.addMethodInsertion(classDeclaration, (builder) { + final declPrefix = utils.oneIndent; + final bodyPrefix = utils.twoIndents; - builder.writeln('$declPrefix@override'); + builder.writeln('@override'); builder.writeln( '${declPrefix}void debugFillProperties(DiagnosticPropertiesBuilder properties) {'); builder .writeln('${bodyPrefix}super.debugFillProperties(properties);'); writePropertyReference(builder, prefix: bodyPrefix, builderName: 'properties'); - builder.writeln('$declPrefix}'); + builder.write('$declPrefix}'); }); }); return; diff --git a/pkg/analysis_server/lib/src/services/correction/dart/add_missing_enum_case_clauses.dart b/pkg/analysis_server/lib/src/services/correction/dart/add_missing_enum_case_clauses.dart index 995d4efa4d1..4a25efc1643 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/add_missing_enum_case_clauses.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/add_missing_enum_case_clauses.dart @@ -82,22 +82,17 @@ class AddMissingEnumCaseClauses extends ResolvedCorrectionProducer { var statementIndent = utils.getLinePrefix(statement.offset); var singleIndent = utils.oneIndent; - var location = utils.newCaseClauseAtEndLocation( - switchKeyword: statement.switchKeyword, - leftBracket: statement.leftBracket, - rightBracket: statement.rightBracket, - ); var prefixString = prefix.isNotEmpty ? '$prefix.' : ''; final enumName_final = '$prefixString$enumName'; - var isLeftBracketSynthetic = statement.leftBracket.isSynthetic; - var insertionOffset = isLeftBracketSynthetic - ? statement.rightParenthesis.end - : location.offset; await builder.addDartFileEdit(file, (builder) { // TODO(brianwilkerson): Consider inserting the names in order into the // switch statement. - builder.addInsertion(insertionOffset, (builder) { + builder.addCaseClauseAtEndInsertion( + switchKeyword: statement.switchKeyword, + rightParenthesis: statement.rightParenthesis, + leftBracket: statement.leftBracket, + rightBracket: statement.rightBracket, (builder) { void addMissingCase(String expression) { builder.write(statementIndent); builder.write(singleIndent); @@ -114,22 +109,12 @@ class AddMissingEnumCaseClauses extends ResolvedCorrectionProducer { builder.writeln('break;'); } - if (isLeftBracketSynthetic) { - builder.write(' {'); - } - builder.write(location.prefix); - for (var constantName in unhandledEnumCases) { addMissingCase('$enumName_final.$constantName'); } if (unhandledNullValue) { addMissingCase('null'); } - - builder.write(location.suffix); - if (statement.rightBracket.isSynthetic) { - builder.write('}'); - } }); }); } diff --git a/pkg/analysis_server/lib/src/services/correction/dart/add_missing_enum_like_case_clauses.dart b/pkg/analysis_server/lib/src/services/correction/dart/add_missing_enum_like_case_clauses.dart index 09d596b9cc4..f5f55e2485e 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/add_missing_enum_like_case_clauses.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/add_missing_enum_like_case_clauses.dart @@ -35,17 +35,15 @@ class AddMissingEnumLikeCaseClauses extends ResolvedCorrectionProducer { var statementIndent = utils.getLinePrefix(node.offset); var singleIndent = utils.oneIndent; - var location = utils.newCaseClauseAtEndLocation( - switchKeyword: node.switchKeyword, - leftBracket: node.leftBracket, - rightBracket: node.rightBracket, - ); await builder.addDartFileEdit(file, (builder) { // TODO(brianwilkerson): Consider inserting the names in order into the // switch statement. - builder.addInsertion(location.offset, (builder) { - builder.write(location.prefix); + builder.addCaseClauseAtEndInsertion( + switchKeyword: node.switchKeyword, + rightParenthesis: node.rightParenthesis, + leftBracket: node.leftBracket, + rightBracket: node.rightBracket, (builder) { for (var name in missingNames) { builder.write(statementIndent); builder.write(singleIndent); @@ -63,7 +61,6 @@ class AddMissingEnumLikeCaseClauses extends ResolvedCorrectionProducer { builder.write(singleIndent); builder.writeln('break;'); } - builder.write(location.suffix); }); }); } diff --git a/pkg/analysis_server/lib/src/services/correction/dart/add_missing_switch_cases.dart b/pkg/analysis_server/lib/src/services/correction/dart/add_missing_switch_cases.dart index 06ab51d4c49..688096c0985 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/add_missing_switch_cases.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/add_missing_switch_cases.dart @@ -59,15 +59,13 @@ class AddMissingSwitchCases extends ResolvedCorrectionProducer { }) async { final lineIndent = utils.getLinePrefix(node.offset); final singleIndent = utils.oneIndent; - final location = utils.newCaseClauseAtEndLocation( - switchKeyword: node.switchKeyword, - leftBracket: node.leftBracket, - rightBracket: node.rightBracket, - ); await builder.addDartFileEdit(file, (builder) { - builder.addInsertion(location.offset, (builder) { - builder.write(location.prefix); + builder.addCaseClauseAtEndInsertion( + switchKeyword: node.switchKeyword, + rightParenthesis: node.rightParenthesis, + leftBracket: node.leftBracket, + rightBracket: node.rightBracket, (builder) { builder.write(lineIndent); builder.write(singleIndent); builder.writeln('// TODO: Handle this case.'); @@ -75,7 +73,6 @@ class AddMissingSwitchCases extends ResolvedCorrectionProducer { builder.write(singleIndent); _writePatternParts(builder, patternParts); builder.writeln(' => throw UnimplementedError(),'); - builder.write(location.suffix); }); }); } @@ -87,15 +84,13 @@ class AddMissingSwitchCases extends ResolvedCorrectionProducer { }) async { final lineIndent = utils.getLinePrefix(node.offset); final singleIndent = utils.oneIndent; - final location = utils.newCaseClauseAtEndLocation( - switchKeyword: node.switchKeyword, - leftBracket: node.leftBracket, - rightBracket: node.rightBracket, - ); await builder.addDartFileEdit(file, (builder) { - builder.addInsertion(location.offset, (builder) { - builder.write(location.prefix); + builder.addCaseClauseAtEndInsertion( + switchKeyword: node.switchKeyword, + rightParenthesis: node.rightParenthesis, + leftBracket: node.leftBracket, + rightBracket: node.rightBracket, (builder) { builder.write(lineIndent); builder.write(singleIndent); builder.write('case '); @@ -105,7 +100,6 @@ class AddMissingSwitchCases extends ResolvedCorrectionProducer { builder.write(singleIndent); builder.write(singleIndent); builder.writeln('// TODO: Handle this case.'); - builder.write(location.suffix); }); }); } diff --git a/pkg/analysis_server/lib/src/services/correction/dart/create_method.dart b/pkg/analysis_server/lib/src/services/correction/dart/create_method.dart index e2b69724622..43112951fc7 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/create_method.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/create_method.dart @@ -4,7 +4,6 @@ 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/analysis/results.dart'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/element/element.dart'; @@ -108,13 +107,12 @@ class CreateMethod extends ResolvedCorrectionProducer { } _memberName = (node as SimpleIdentifier).name; var invocation = node.parent as MethodInvocation; - // prepare environment + // Prepare environment. Element? targetElement; var staticModifier = false; CompilationUnitMember? targetNode; var target = invocation.realTarget; - var utilsForTargetNode = utils; if (target is ExtensionOverride) { targetElement = target.element; if (targetElement is ExtensionElement) { @@ -155,7 +153,7 @@ class CreateMethod extends ResolvedCorrectionProducer { if (targetClassElement.library.isInSdk) { return; } - // prepare target ClassDeclaration + // Prepare target ClassDeclaration. if (targetClassElement is MixinElement) { targetNode = await getMixinDeclaration(targetClassElement); } else if (targetClassElement is ClassElement) { @@ -166,57 +164,49 @@ class CreateMethod extends ResolvedCorrectionProducer { if (targetNode == null) { return; } - // maybe static + // Maybe static. if (target is Identifier) { staticModifier = target.staticElement?.kind == ElementKind.CLASS || target.staticElement?.kind == ElementKind.EXTENSION_TYPE || target.staticElement?.kind == ElementKind.MIXIN; } - // use different utils + // Use different utils. var targetPath = targetClassElement.source.fullName; var targetResolveResult = await unitResult.session.getResolvedUnit(targetPath); if (targetResolveResult is! ResolvedUnitResult) { return; } - utilsForTargetNode = CorrectionUtils(targetResolveResult); } - if (targetElement == null || targetNode == null) { - return; - } - var targetLocation = - utilsForTargetNode.prepareNewMethodLocation(targetNode); - if (targetLocation == null) { - return; - } - var targetSource = targetElement.source; + var targetSource = targetElement?.source; if (targetSource == null) { return; } var targetFile = targetSource.fullName; - // build method source + // Build method source. await builder.addDartFileEdit(targetFile, (builder) { - builder.addInsertion(targetLocation.offset, (builder) { - builder.write(targetLocation.prefix); - // maybe "static" + if (targetNode == null) { + return; + } + builder.addMethodInsertion(targetNode, (builder) { + // Maybe 'static'. if (staticModifier) { builder.write('static '); } - // append return type + // Append return type. { var type = inferUndefinedExpressionType(invocation); if (builder.writeType(type, groupName: 'RETURN_TYPE')) { builder.write(' '); } } - // append name + // Append name. builder.addLinkedEdit('NAME', (builder) { builder.write(_memberName); }); builder.write('('); builder.writeParametersMatchingArguments(invocation.argumentList); builder.write(') {}'); - builder.write(targetLocation.suffix); }); if (targetFile == file) { builder.addLinkedPosition(range.node(node), 'NAME'); diff --git a/pkg/analysis_server/lib/src/services/correction/util.dart b/pkg/analysis_server/lib/src/services/correction/util.dart index 75ceaf885e9..6e1f1746086 100644 --- a/pkg/analysis_server/lib/src/services/correction/util.dart +++ b/pkg/analysis_server/lib/src/services/correction/util.dart @@ -866,25 +866,6 @@ final class CorrectionUtils { String invertCondition(Expression expression) => _invertCondition0(expression)._source; - InsertionLocation newCaseClauseAtEndLocation({ - required Token switchKeyword, - required Token leftBracket, - required Token rightBracket, - }) { - var blockStartLine = getLineThis(leftBracket.offset); - var blockEndLine = getLineThis(rightBracket.offset); - var offset = blockEndLine; - var prefix = ''; - var suffix = ''; - if (blockStartLine == blockEndLine) { - // The switch body is on a single line. - prefix = endOfLine; - offset = leftBracket.end; - suffix = getLinePrefix(switchKeyword.offset); - } - return InsertionLocation(prefix: prefix, offset: offset, suffix: suffix); - } - InsertionLocation prepareEnumNewConstructorLocation( EnumDeclaration enumDeclaration, ) { @@ -966,16 +947,6 @@ final class CorrectionUtils { return prepareNewClassMemberLocation(classDeclaration, shouldSkip); } - InsertionLocation? prepareNewMethodLocation( - CompilationUnitMember declaration) { - return prepareNewClassMemberLocation( - declaration, - (member) => - member is FieldDeclaration || - member is ConstructorDeclaration || - member is MethodDeclaration); - } - /// Returns the source with indentation changed from [oldIndent] to /// [newIndent], keeping indentation of lines relative to each other. /// diff --git a/pkg/analysis_server/test/src/services/correction/fix/add_diagnostic_property_reference_test.dart b/pkg/analysis_server/test/src/services/correction/fix/add_diagnostic_property_reference_test.dart index 1027c4e156a..b87d6c5fffe 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/add_diagnostic_property_reference_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/add_diagnostic_property_reference_test.dart @@ -542,6 +542,7 @@ import 'package:flutter/widgets.dart'; class C extends Widget with Diagnosticable { String field = ''; + @override void debugFillProperties(DiagnosticPropertiesBuilder properties) { super.debugFillProperties(properties); diff --git a/pkg/analyzer_plugin/lib/src/utilities/change_builder/change_builder_dart.dart b/pkg/analyzer_plugin/lib/src/utilities/change_builder/change_builder_dart.dart index 00eef2a0b07..4d5174cb78c 100644 --- a/pkg/analyzer_plugin/lib/src/utilities/change_builder/change_builder_dart.dart +++ b/pkg/analyzer_plugin/lib/src/utilities/change_builder/change_builder_dart.dart @@ -1423,6 +1423,41 @@ class DartFileEditBuilderImpl extends FileEditBuilderImpl @override List get requiredImports => librariesToImport.keys.toList(); + @override + void addCaseClauseAtEndInsertion( + void Function(DartEditBuilder builder) buildEdit, { + required Token switchKeyword, + required Token rightParenthesis, + required Token leftBracket, + required Token rightBracket, + }) { + var blockStart = resolvedUnit.lineInfo.getLocation(leftBracket.offset); + var blockEnd = resolvedUnit.lineInfo.getLocation(rightBracket.offset); + var isBlockSingleLine = blockStart.lineNumber == blockEnd.lineNumber; + int offset; + if (isBlockSingleLine) { + offset = leftBracket.isSynthetic ? rightParenthesis.end : leftBracket.end; + } else { + offset = resolvedUnit.lineInfo.getOffsetOfLine(blockEnd.lineNumber - 1); + } + + addInsertion(offset, (builder) { + if (leftBracket.isSynthetic) { + builder.write(' {'); + } + if (isBlockSingleLine) { + builder.writeln(); + } + buildEdit(builder); + if (isBlockSingleLine) { + builder.write(_linePrefix(switchKeyword.offset)); + } + if (rightBracket.isSynthetic) { + builder.write('}'); + } + }); + } + @override void addFieldInsertion( CompilationUnitMember compilationUnitMember, @@ -1456,6 +1491,20 @@ class DartFileEditBuilderImpl extends FileEditBuilderImpl offset, (builder) => buildEdit(builder as DartEditBuilder), insertBeforeExisting: insertBeforeExisting); + @override + void addMethodInsertion( + CompilationUnitMember compilationUnitMember, + void Function(DartEditBuilder builder) buildEdit, + ) => + _addCompilationUnitMemberInsertion( + compilationUnitMember, + buildEdit, + lastMemberFilter: (member) => + member is FieldDeclaration || + member is ConstructorDeclaration || + member is MethodDeclaration, + ); + @override void addReplacement(SourceRange range, void Function(DartEditBuilder builder) buildEdit) => @@ -1686,9 +1735,9 @@ class DartFileEditBuilderImpl extends FileEditBuilderImpl void Function(DartEditBuilder builder) buildEdit, { required bool Function(ClassMember existingMember) lastMemberFilter, }) { - var preparer = _InsertionPreparer(compilationUnitMember); + var preparer = + _InsertionPreparer(compilationUnitMember, resolvedUnit.lineInfo); var offset = preparer.insertionLocation( - resolvedUnit.lineInfo, lastMemberFilter: lastMemberFilter, ); if (offset == null) { @@ -2201,6 +2250,32 @@ class DartFileEditBuilderImpl extends FileEditBuilderImpl return element.library == resolvedUnit.libraryElement; } + // TODO(srawlins): Move this to a shared extension in new plugin package when + // this code is moved to new plugin package. + bool _isEol(int c) { + return c == 0x0D || c == 0x0A; + } + + // TODO(srawlins): Move this to a shared extension in new plugin package when + // this code is moved to new plugin package. + bool _isSpace(int c) => c == 0x20 || c == 0x09; + + /// Returns the whitespace prefix of the line which contains the given + /// [offset]. + String _linePrefix(int offset) { + var lineStartOffset = resolvedUnit.lineInfo.getOffsetOfLine( + resolvedUnit.lineInfo.getLocation(offset).lineNumber - 1); + resolvedUnit.content; + var length = resolvedUnit.content.length; + var whitespaceEndOffset = lineStartOffset; + while (whitespaceEndOffset < length) { + var c = resolvedUnit.content.codeUnitAt(whitespaceEndOffset); + if (_isEol(c) || !_isSpace(c)) break; + whitespaceEndOffset++; + } + return resolvedUnit.content.substring(lineStartOffset, whitespaceEndOffset); + } + /// Create an edit to replace the return type of the innermost function /// containing the given [node] with the type `Future`. The [typeProvider] is /// used to check the current return type, because if it is already `Future` @@ -2292,9 +2367,10 @@ class _EnclosingElementFinder { /// like a class or mixin. class _InsertionPreparer { final CompilationUnitMember _declaration; + final LineInfo _lineInfo; late final ClassMember? _targetMember; - _InsertionPreparer(this._declaration); + _InsertionPreparer(this._declaration, this._lineInfo); /// Returns the offset of where a new member should be inserted, as a new /// member of [_declaration]. @@ -2303,9 +2379,7 @@ class _InsertionPreparer { /// [lastMemberFilter]. If no existing member matches, then the offset is at /// the beginning of [_declaration], just after it's opening brace. int? insertionLocation( - LineInfo lineInfo, { - required bool Function(ClassMember existingMember) lastMemberFilter, - }) { + {required bool Function(ClassMember existingMember) lastMemberFilter}) { var members = _declaration.members; if (members == null) { assert( @@ -2356,6 +2430,14 @@ class _InsertionPreparer { void writeSuffix(DartEditBuilder builder) { if (_targetMember == null && (_declaration.members?.isNotEmpty ?? false)) { builder.writeln(); + return; + } + + var offsetLine = _lineInfo.getLocation(_declaration.offset).lineNumber; + var endLine = _lineInfo.getLocation(_declaration.end).lineNumber; + var declarationIsSingleLine = endLine == offsetLine; + if (declarationIsSingleLine) { + builder.writeln(); } } } diff --git a/pkg/analyzer_plugin/lib/utilities/change_builder/change_builder_dart.dart b/pkg/analyzer_plugin/lib/utilities/change_builder/change_builder_dart.dart index 16285640e45..1f4aebbcd99 100644 --- a/pkg/analyzer_plugin/lib/utilities/change_builder/change_builder_dart.dart +++ b/pkg/analyzer_plugin/lib/utilities/change_builder/change_builder_dart.dart @@ -3,6 +3,7 @@ // BSD-style license that can be found in the LICENSE file. import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/token.dart'; import 'package:analyzer/dart/element/element.dart'; import 'package:analyzer/dart/element/type.dart'; import 'package:analyzer/dart/element/type_provider.dart'; @@ -322,6 +323,16 @@ abstract class DartFileEditBuilder implements FileEditBuilder { /// edits. List get requiredImports; + /// Adds an insertion for a case clause at the end of a switch statement or + /// switch expression. + void addCaseClauseAtEndInsertion( + void Function(DartEditBuilder builder) buildEdit, { + required Token switchKeyword, + required Token rightParenthesis, + required Token leftBracket, + required Token rightBracket, + }); + /// Adds an insertion for a field. /// /// The field is inserted after the last existing field, or at the beginning @@ -345,6 +356,16 @@ abstract class DartFileEditBuilder implements FileEditBuilder { void addInsertion( int offset, void Function(DartEditBuilder builder) buildEdit); + /// Adds an insertion for a method. + /// + /// The method is inserted after the last existing field, constructor, or + /// method, or at the beginning of [compilationUnitMember], if it has none of + /// these. + void addMethodInsertion( + CompilationUnitMember compilationUnitMember, + void Function(DartEditBuilder builder) buildEdit, + ); + @override void addReplacement( SourceRange range, void Function(DartEditBuilder builder) buildEdit);