From 4d0fa1e40238f8b32baeec533cde54ea8e678825 Mon Sep 17 00:00:00 2001 From: pq Date: Fri, 6 Sep 2019 13:24:09 +0000 Subject: [PATCH] fix for slash_for_doc_comments Change-Id: I05ed8f878736f3afeb545940e047ac03dfcbc1f9 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/115771 Commit-Queue: Phil Quitslund Reviewed-by: Brian Wilkerson --- .../services/correction/assist_internal.dart | 79 +------ .../services/correction/base_processor.dart | 218 +++++++++++------- .../lib/src/services/correction/fix.dart | 2 + .../src/services/correction/fix_internal.dart | 8 + .../lib/src/services/linter/lint_names.dart | 1 + .../convert_documentation_into_line_test.dart | 17 ++ .../convert_documentation_into_line_test.dart | 47 ++++ .../src/services/correction/fix/test_all.dart | 3 + 8 files changed, 224 insertions(+), 151 deletions(-) create mode 100644 pkg/analysis_server/test/src/services/correction/fix/convert_documentation_into_line_test.dart diff --git a/pkg/analysis_server/lib/src/services/correction/assist_internal.dart b/pkg/analysis_server/lib/src/services/correction/assist_internal.dart index 8830af4b30cde..4689bd8ddbf72 100644 --- a/pkg/analysis_server/lib/src/services/correction/assist_internal.dart +++ b/pkg/analysis_server/lib/src/services/correction/assist_internal.dart @@ -64,7 +64,11 @@ class AssistProcessor extends BaseProcessor { await _addProposal_assignToLocalVariable(); await _addProposal_convertClassToMixin(); await _addProposal_convertDocumentationIntoBlock(); - await _addProposal_convertDocumentationIntoLine(); + if (!_containsErrorCode( + {LintNames.slash_for_doc_comments}, + )) { + await _addProposal_convertDocumentationIntoLine(); + } await _addProposal_convertIntoFinalField(); await _addProposal_convertIntoGetter(); await _addProposal_convertListConstructorToListLiteral(); @@ -464,64 +468,7 @@ class AssistProcessor extends BaseProcessor { } Future _addProposal_convertDocumentationIntoLine() async { - Comment comment = node.thisOrAncestorOfType(); - if (comment == null || - !comment.isDocumentation || - comment.tokens.length != 1) { - _coverageMarker(); - return; - } - Token token = comment.tokens.first; - if (token.type != TokenType.MULTI_LINE_COMMENT) { - _coverageMarker(); - return; - } - String text = token.lexeme; - List lines = text.split('\n'); - String prefix = utils.getNodePrefix(comment); - List newLines = []; - bool firstLine = true; - String linePrefix = ''; - for (String line in lines) { - if (firstLine) { - firstLine = false; - String expectedPrefix = '/**'; - if (!line.startsWith(expectedPrefix)) { - _coverageMarker(); - return; - } - line = line.substring(expectedPrefix.length).trim(); - if (line.isNotEmpty) { - newLines.add('/// $line'); - linePrefix = eol + prefix; - } - } else { - if (line.startsWith(prefix + ' */')) { - break; - } - String expectedPrefix = prefix + ' *'; - if (!line.startsWith(expectedPrefix)) { - _coverageMarker(); - return; - } - line = line.substring(expectedPrefix.length); - if (line.isEmpty) { - newLines.add('$linePrefix///'); - } else { - newLines.add('$linePrefix///$line'); - } - linePrefix = eol + prefix; - } - } - - var changeBuilder = _newDartChangeBuilder(); - await changeBuilder.addFileEdit(file, (DartFileEditBuilder builder) { - builder.addReplacement(range.node(comment), (DartEditBuilder builder) { - for (String newLine in newLines) { - builder.write(newLine); - } - }); - }); + final changeBuilder = await createBuilder_convertDocumentationIntoLine(); _addAssistFromBuilder( changeBuilder, DartAssistKind.CONVERT_DOCUMENTATION_INTO_LINE); } @@ -3715,20 +3662,6 @@ class AssistProcessor extends BaseProcessor { return true; } - /** - * Configures [utils] using given [target]. - */ - void _configureTargetLocation(Object target) { - utils.targetClassElement = null; - if (target is AstNode) { - ClassDeclaration targetClassDeclaration = - target.thisOrAncestorOfType(); - if (targetClassDeclaration != null) { - utils.targetClassElement = targetClassDeclaration.declaredElement; - } - } - } - bool _containsErrorCode(Set errorCodes) { final fileOffset = node.offset; for (var error in context.resolveResult.errors) { diff --git a/pkg/analysis_server/lib/src/services/correction/base_processor.dart b/pkg/analysis_server/lib/src/services/correction/base_processor.dart index 94c305a4aa003..1fff4801c7643 100644 --- a/pkg/analysis_server/lib/src/services/correction/base_processor.dart +++ b/pkg/analysis_server/lib/src/services/correction/base_processor.dart @@ -61,77 +61,50 @@ abstract class BaseProcessor { (session.analysisContext.analysisOptions as AnalysisOptionsImpl) .experimentStatus; - /// This method does nothing, but we invoke it in places where Dart VM - /// coverage agent fails to provide coverage information - such as almost - /// all "return" statements. - /// - /// https://code.google.com/p/dart/issues/detail?id=19912 - static void _coverageMarker() {} - - /// Configures [utils] using given [target]. - void configureTargetLocation(Object target) { - utils.targetClassElement = null; - if (target is AstNode) { - ClassDeclaration targetClassDeclaration = - target.thisOrAncestorOfType(); - if (targetClassDeclaration != null) { - utils.targetClassElement = targetClassDeclaration.declaredElement; - } - } - } - Future - createBuilder_addTypeAnnotation_VariableDeclaration() async { - AstNode node = this.node; - // prepare VariableDeclarationList - VariableDeclarationList declarationList = - node.thisOrAncestorOfType(); - if (declarationList == null) { - _coverageMarker(); - return null; - } - // may be has type annotation already - if (declarationList.type != null) { - _coverageMarker(); - return null; - } - // prepare single VariableDeclaration - List variables = declarationList.variables; - if (variables.length != 1) { - _coverageMarker(); - return null; + createBuilder_addTypeAnnotation_DeclaredIdentifier() async { + DeclaredIdentifier declaredIdentifier = + node.thisOrAncestorOfType(); + if (declaredIdentifier == null) { + ForStatement forEach = node.thisOrAncestorMatching( + (node) => node is ForStatement && node.forLoopParts is ForEachParts); + ForEachParts forEachParts = forEach?.forLoopParts; + int offset = node.offset; + if (forEach != null && + forEachParts.iterable != null && + offset < forEachParts.iterable.offset) { + declaredIdentifier = forEachParts is ForEachPartsWithDeclaration + ? forEachParts.loopVariable + : null; + } } - VariableDeclaration variable = variables[0]; - // must be not after the name of the variable - if (selectionOffset > variable.name.end) { + if (declaredIdentifier == null) { _coverageMarker(); return null; } - // we need an initializer to get the type from - Expression initializer = variable.initializer; - if (initializer == null) { + // Ensure that there isn't already a type annotation. + if (declaredIdentifier.type != null) { _coverageMarker(); return null; } - DartType type = initializer.staticType; - // prepare type source - if ((type is! InterfaceType || type.isDartCoreNull) && - type is! FunctionType) { + DartType type = declaredIdentifier.identifier.staticType; + if (type is! InterfaceType && type is! FunctionType) { _coverageMarker(); return null; } - configureTargetLocation(node); + _configureTargetLocation(node); var changeBuilder = _newDartChangeBuilder(); bool validChange = true; await changeBuilder.addFileEdit(file, (DartFileEditBuilder builder) { - Token keyword = declarationList.keyword; - if (keyword?.keyword == Keyword.VAR) { + Token keyword = declaredIdentifier.keyword; + if (keyword.keyword == Keyword.VAR) { builder.addReplacement(range.token(keyword), (DartEditBuilder builder) { validChange = builder.writeType(type); }); } else { - builder.addInsertion(variable.offset, (DartEditBuilder builder) { + builder.addInsertion(declaredIdentifier.identifier.offset, + (DartEditBuilder builder) { validChange = builder.writeType(type); builder.write(' '); }); @@ -166,7 +139,7 @@ abstract class BaseProcessor { return null; } // prepare type source - configureTargetLocation(node); + _configureTargetLocation(node); var changeBuilder = _newDartChangeBuilder(); bool validChange = true; @@ -180,49 +153,57 @@ abstract class BaseProcessor { } Future - createBuilder_addTypeAnnotation_DeclaredIdentifier() async { - DeclaredIdentifier declaredIdentifier = - node.thisOrAncestorOfType(); - if (declaredIdentifier == null) { - ForStatement forEach = node.thisOrAncestorMatching( - (node) => node is ForStatement && node.forLoopParts is ForEachParts); - ForEachParts forEachParts = forEach?.forLoopParts; - int offset = node.offset; - if (forEach != null && - forEachParts.iterable != null && - offset < forEachParts.iterable.offset) { - declaredIdentifier = forEachParts is ForEachPartsWithDeclaration - ? forEachParts.loopVariable - : null; - } + createBuilder_addTypeAnnotation_VariableDeclaration() async { + AstNode node = this.node; + // prepare VariableDeclarationList + VariableDeclarationList declarationList = + node.thisOrAncestorOfType(); + if (declarationList == null) { + _coverageMarker(); + return null; } - if (declaredIdentifier == null) { + // may be has type annotation already + if (declarationList.type != null) { _coverageMarker(); return null; } - // Ensure that there isn't already a type annotation. - if (declaredIdentifier.type != null) { + // prepare single VariableDeclaration + List variables = declarationList.variables; + if (variables.length != 1) { _coverageMarker(); return null; } - DartType type = declaredIdentifier.identifier.staticType; - if (type is! InterfaceType && type is! FunctionType) { + VariableDeclaration variable = variables[0]; + // must be not after the name of the variable + if (selectionOffset > variable.name.end) { _coverageMarker(); return null; } - configureTargetLocation(node); + // we need an initializer to get the type from + Expression initializer = variable.initializer; + if (initializer == null) { + _coverageMarker(); + return null; + } + DartType type = initializer.staticType; + // prepare type source + if ((type is! InterfaceType || type.isDartCoreNull) && + type is! FunctionType) { + _coverageMarker(); + return null; + } + _configureTargetLocation(node); var changeBuilder = _newDartChangeBuilder(); bool validChange = true; await changeBuilder.addFileEdit(file, (DartFileEditBuilder builder) { - Token keyword = declaredIdentifier.keyword; - if (keyword.keyword == Keyword.VAR) { + Token keyword = declarationList.keyword; + if (keyword?.keyword == Keyword.VAR) { builder.addReplacement(range.token(keyword), (DartEditBuilder builder) { validChange = builder.writeType(type); }); } else { - builder.addInsertion(declaredIdentifier.identifier.offset, - (DartEditBuilder builder) { + builder.addInsertion(variable.offset, (DartEditBuilder builder) { validChange = builder.writeType(type); builder.write(' '); }); @@ -231,6 +212,68 @@ abstract class BaseProcessor { return validChange ? changeBuilder : null; } + Future createBuilder_convertDocumentationIntoLine() async { + Comment comment = node.thisOrAncestorOfType(); + if (comment == null || + !comment.isDocumentation || + comment.tokens.length != 1) { + _coverageMarker(); + return null; + } + Token token = comment.tokens.first; + if (token.type != TokenType.MULTI_LINE_COMMENT) { + _coverageMarker(); + return null; + } + String text = token.lexeme; + List lines = text.split('\n'); + String prefix = utils.getNodePrefix(comment); + List newLines = []; + bool firstLine = true; + String linePrefix = ''; + for (String line in lines) { + if (firstLine) { + firstLine = false; + String expectedPrefix = '/**'; + if (!line.startsWith(expectedPrefix)) { + _coverageMarker(); + return null; + } + line = line.substring(expectedPrefix.length).trim(); + if (line.isNotEmpty) { + newLines.add('/// $line'); + linePrefix = eol + prefix; + } + } else { + if (line.startsWith(prefix + ' */')) { + break; + } + String expectedPrefix = prefix + ' *'; + if (!line.startsWith(expectedPrefix)) { + _coverageMarker(); + return null; + } + line = line.substring(expectedPrefix.length); + if (line.isEmpty) { + newLines.add('$linePrefix///'); + } else { + newLines.add('$linePrefix///$line'); + } + linePrefix = eol + prefix; + } + } + + var changeBuilder = _newDartChangeBuilder(); + await changeBuilder.addFileEdit(file, (DartFileEditBuilder builder) { + builder.addReplacement(range.node(comment), (DartEditBuilder builder) { + for (String newLine in newLines) { + builder.write(newLine); + } + }); + }); + return changeBuilder; + } + @protected Future createBuilder_useCurlyBraces() async { Future doStatement(DoStatement node) async { @@ -365,6 +408,25 @@ abstract class BaseProcessor { return node != null; } + /// Configures [utils] using given [target]. + void _configureTargetLocation(Object target) { + utils.targetClassElement = null; + if (target is AstNode) { + ClassDeclaration targetClassDeclaration = + target.thisOrAncestorOfType(); + if (targetClassDeclaration != null) { + utils.targetClassElement = targetClassDeclaration.declaredElement; + } + } + } + DartChangeBuilder _newDartChangeBuilder() => DartChangeBuilderImpl.forWorkspace(workspace); + + /// This method does nothing, but we invoke it in places where Dart VM + /// coverage agent fails to provide coverage information - such as almost + /// all "return" statements. + /// + /// https://code.google.com/p/dart/issues/detail?id=19912 + static void _coverageMarker() {} } diff --git a/pkg/analysis_server/lib/src/services/correction/fix.dart b/pkg/analysis_server/lib/src/services/correction/fix.dart index 0317f6c02d7e7..f15eba6fd9e56 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix.dart @@ -185,6 +185,8 @@ class DartFixKind { const FixKind('CONVERT_FLUTTER_CHILD', 50, "Convert to children:"); static const CONVERT_FLUTTER_CHILDREN = const FixKind('CONVERT_FLUTTER_CHILDREN', 50, "Convert to child:"); + static const CONVERT_TO_LINE_COMMENT = const FixKind( + 'CONVERT_TO_LINE_COMMENT', 50, "Convert to line documentation comment"); static const CONVERT_TO_NAMED_ARGUMENTS = const FixKind( 'CONVERT_TO_NAMED_ARGUMENTS', 50, "Convert to named arguments"); static const CREATE_CLASS = diff --git a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart index 1e60ee2606259..fa2654544ddfe 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart @@ -628,6 +628,9 @@ class FixProcessor extends BaseProcessor { await _addFix_addConst(); await _addFix_replaceNewWithConst(); } + if (errorCode.name == LintNames.slash_for_doc_comments) { + await _addFix_convertDocumentationIntoLine(); + } if (name == LintNames.type_init_formals) { await _addFix_removeTypeAnnotation(); } @@ -1247,6 +1250,11 @@ class FixProcessor extends BaseProcessor { } } + Future _addFix_convertDocumentationIntoLine() async { + final changeBuilder = await createBuilder_convertDocumentationIntoLine(); + _addFixFromBuilder(changeBuilder, DartFixKind.CONVERT_TO_LINE_COMMENT); + } + Future _addFix_convertFlutterChild() async { NamedExpression named = flutter.findNamedExpression(node, 'child'); if (named == null) { diff --git a/pkg/analysis_server/lib/src/services/linter/lint_names.dart b/pkg/analysis_server/lib/src/services/linter/lint_names.dart index ccc863e449e4f..80a479f4b6bc6 100644 --- a/pkg/analysis_server/lib/src/services/linter/lint_names.dart +++ b/pkg/analysis_server/lib/src/services/linter/lint_names.dart @@ -37,6 +37,7 @@ class LintNames { static const String prefer_final_locals = 'prefer_final_locals'; static const String prefer_is_empty = 'prefer_is_empty'; static const String prefer_is_not_empty = 'prefer_is_not_empty'; + static const String slash_for_doc_comments = 'slash_for_doc_comments'; static const String type_annotate_public_apis = 'type_annotate_public_apis'; static const String type_init_formals = 'type_init_formals'; static const String unawaited_futures = 'unawaited_futures'; diff --git a/pkg/analysis_server/test/src/services/correction/assist/convert_documentation_into_line_test.dart b/pkg/analysis_server/test/src/services/correction/assist/convert_documentation_into_line_test.dart index b13e2693ddc0a..df6c4e75d6431 100644 --- a/pkg/analysis_server/test/src/services/correction/assist/convert_documentation_into_line_test.dart +++ b/pkg/analysis_server/test/src/services/correction/assist/convert_documentation_into_line_test.dart @@ -3,6 +3,7 @@ // BSD-style license that can be found in the LICENSE file. import 'package:analysis_server/src/services/correction/assist.dart'; +import 'package:analysis_server/src/services/linter/lint_names.dart'; import 'package:analyzer_plugin/utilities/assist/assist.dart'; import 'package:test_reflective_loader/test_reflective_loader.dart'; @@ -90,6 +91,22 @@ class A { '''); } + test_onText_no_assist_with_lint() async { + createAnalysisOptionsFile(lints: [LintNames.slash_for_doc_comments]); + verifyNoTestUnitErrors = false; + await resolveTestUnit(''' +class A { + /** + * AAAAAAA [int] AAAAAAA + * BBBBBBBB BBBB BBBB + * CCC [A] CCCCCCCCCCC + */ + mmm() {} +} +'''); + await assertNoAssist(); + } + test_onText_hasFirstLine() async { await resolveTestUnit(''' class A { diff --git a/pkg/analysis_server/test/src/services/correction/fix/convert_documentation_into_line_test.dart b/pkg/analysis_server/test/src/services/correction/fix/convert_documentation_into_line_test.dart new file mode 100644 index 0000000000000..8acb794fd1b08 --- /dev/null +++ b/pkg/analysis_server/test/src/services/correction/fix/convert_documentation_into_line_test.dart @@ -0,0 +1,47 @@ +// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:analysis_server/src/services/correction/fix.dart'; +import 'package:analysis_server/src/services/linter/lint_names.dart'; +import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +import 'fix_processor.dart'; + +main() { + defineReflectiveSuite(() { + defineReflectiveTests(ConvertDocumentationIntoLineTest); + }); +} + +@reflectiveTest +class ConvertDocumentationIntoLineTest extends FixProcessorLintTest { + @override + FixKind get kind => DartFixKind.CONVERT_TO_LINE_COMMENT; + + @override + String get lintCode => LintNames.slash_for_doc_comments; + + // More coverage in the `convert_to_documentation_line_test.dart` assist test. + test_onText() async { + await resolveTestUnit(''' +class A { + /** + * /*LINT*/AAAAAAA [int] AAAAAAA + * BBBBBBBB BBBB BBBB + * CCC [A] CCCCCCCCCCC + */ + mmm() {} +} +'''); + await assertHasFix(''' +class A { + /// /*LINT*/AAAAAAA [int] AAAAAAA + /// BBBBBBBB BBBB BBBB + /// CCC [A] CCCCCCCCCCC + mmm() {} +} +'''); + } +} diff --git a/pkg/analysis_server/test/src/services/correction/fix/test_all.dart b/pkg/analysis_server/test/src/services/correction/fix/test_all.dart index 80b7a210cea70..33d0324934477 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/test_all.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/test_all.dart @@ -36,6 +36,8 @@ import 'change_type_annotation_test.dart' as change_type_annotation; import 'convert_flutter_child_test.dart' as convert_flutter_child; import 'convert_flutter_children_test.dart' as convert_flutter_children; import 'convert_to_named_arguments_test.dart' as convert_to_named_arguments; +import 'convert_documentation_into_line_test.dart' + as convert_documentation_into_line; import 'create_class_test.dart' as create_class; import 'create_constructor_for_final_fields_test.dart' as create_constructor_for_final_field; @@ -139,6 +141,7 @@ main() { change_to_nearest_precise_value.main(); change_to_static_access.main(); change_type_annotation.main(); + convert_documentation_into_line.main(); convert_flutter_child.main(); convert_flutter_children.main(); convert_to_named_arguments.main();