From df3df89fd931664334ab4fce32382b30ca6a8e1e Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Wed, 7 Dec 2022 21:11:47 +0000 Subject: [PATCH] [analysis_server] Include type in extract-local-variable if always_specify_types is enabled Fixes https://github.com/Dart-Code/Dart-Code/issues/4177. Change-Id: I8da492c947466677e84d7892113f0bbfbdacfb87 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/274240 Commit-Queue: Brian Wilkerson Reviewed-by: Brian Wilkerson --- .../refactoring/legacy/extract_local.dart | 46 +++++--- .../legacy/extract_local_test.dart | 104 +++++++++++++++++- .../lib/dart/analysis/code_style_options.dart | 3 + .../analysis_options/code_style_options.dart | 3 + 4 files changed, 139 insertions(+), 17 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/refactoring/legacy/extract_local.dart b/pkg/analysis_server/lib/src/services/refactoring/legacy/extract_local.dart index 458f2a93584..6bccab26758 100644 --- a/pkg/analysis_server/lib/src/services/refactoring/legacy/extract_local.dart +++ b/pkg/analysis_server/lib/src/services/refactoring/legacy/extract_local.dart @@ -11,6 +11,7 @@ import 'package:analysis_server/src/services/correction/util.dart'; import 'package:analysis_server/src/services/refactoring/legacy/naming_conventions.dart'; import 'package:analysis_server/src/services/refactoring/legacy/refactoring.dart'; import 'package:analysis_server/src/services/refactoring/legacy/refactoring_internal.dart'; +import 'package:analysis_server/src/utilities/extensions/ast.dart'; import 'package:analysis_server/src/utilities/strings.dart'; import 'package:analyzer/dart/analysis/code_style_options.dart'; import 'package:analyzer/dart/analysis/features.dart'; @@ -73,13 +74,27 @@ class ExtractLocalRefactoringImpl extends RefactoringImpl CompilationUnitElement get unitElement => unit.declaredElement!; - String get _declarationKeyword { - if (_isPartOfConstantExpression(singleExpression)) { - return 'const'; - } else if (codeStyleOptions.makeLocalsFinal) { - return 'final'; + String get _declarationKeywordAndType { + var useConst = stringLiteralPart == null && + _isPartOfConstantExpression(singleExpression); + var useFinal = codeStyleOptions.makeLocalsFinal; + + String? typeString; + if (codeStyleOptions.specifyTypes) { + typeString = singleExpression != null + ? singleExpression?.staticType + ?.getDisplayString(withNullability: unit.isNonNullableByDefault) + : stringLiteralPart != null + ? 'String' + : null; + } + + if (useConst) { + return typeString != null ? 'const $typeString' : 'const'; + } else if (useFinal) { + return typeString != null ? 'final $typeString' : 'final'; } else { - return 'var'; + return typeString ?? 'var'; } } @@ -137,9 +152,9 @@ class ExtractLocalRefactoringImpl extends RefactoringImpl if (singleExpression != null && singleExpression.parent is ExpressionStatement && occurrences.length == 1) { - var keyword = _declarationKeyword; - var declarationSource = '$keyword $name = '; - var edit = SourceEdit(singleExpression.offset, 0, declarationSource); + var keywordAndType = _declarationKeywordAndType; + var declarationCode = '$keywordAndType $name = '; + var edit = SourceEdit(singleExpression.offset, 0, declarationCode); doSourceChange_addElementEdit(change, unitElement, edit); return Future.value(change); } @@ -152,17 +167,15 @@ class ExtractLocalRefactoringImpl extends RefactoringImpl // add variable declaration { - String declarationCode; - int nameOffsetInDeclarationCode; + var keywordAndType = _declarationKeywordAndType; + var declarationCode = '$keywordAndType '; + var nameOffsetInDeclarationCode = declarationCode.length; if (stringLiteralPart != null) { - declarationCode = 'var '; - nameOffsetInDeclarationCode = declarationCode.length; + // TODO(dantup): This does not correctly handle escaping (for example + // unescaped single quotes in a double quoted string). declarationCode += "$name = '$stringLiteralPart';"; } else { - var keyword = _declarationKeyword; var initializerCode = utils.getRangeText(selectionRange); - declarationCode = '$keyword '; - nameOffsetInDeclarationCode = declarationCode.length; declarationCode += '$name = $initializerCode;'; } // prepare location for declaration @@ -197,6 +210,7 @@ class ExtractLocalRefactoringImpl extends RefactoringImpl // prepare replacement var occurrenceReplacement = name; if (stringLiteralPart != null) { + // TODO(dantup): Don't include braces if unnecessary. occurrenceReplacement = '\${$name}'; occurrencesShift += 2; } diff --git a/pkg/analysis_server/test/services/refactoring/legacy/extract_local_test.dart b/pkg/analysis_server/test/services/refactoring/legacy/extract_local_test.dart index a788a4773ed..c770276cad7 100644 --- a/pkg/analysis_server/test/services/refactoring/legacy/extract_local_test.dart +++ b/pkg/analysis_server/test/services/refactoring/legacy/extract_local_test.dart @@ -737,7 +737,109 @@ void f() { expect(refactoring.isAvailable(), isTrue); } - Future test_lint_prefer_final_locals() async { + Future test_lint_alwaysSpecifyTypes() async { + createAnalysisOptionsFile(lints: [LintNames.always_specify_types]); + await indexTestUnit(''' +void f() { + print(1 + 2); +} +'''); + _createRefactoringForString('1 + 2'); + // apply refactoring + return _assertSuccessfulRefactoring(''' +void f() { + int res = 1 + 2; + print(res); +} +'''); + } + + Future test_lint_alwaysSpecifyTypes_const() async { + createAnalysisOptionsFile(lints: [LintNames.always_specify_types]); + await indexTestUnit(''' +void f() { + const [1, 2]; +} +'''); + _createRefactoringForString('1'); + // apply refactoring + return _assertSuccessfulRefactoring(''' +void f() { + const int res = 1; + const [res, 2]; +} +'''); + } + + Future test_lint_alwaysSpecifyTypes_final() async { + createAnalysisOptionsFile( + lints: [LintNames.always_specify_types, LintNames.prefer_final_locals]); + await indexTestUnit(''' +void f() { + print(1 + 2); +} +'''); + _createRefactoringForString('1 + 2'); + // apply refactoring + return _assertSuccessfulRefactoring(''' +void f() { + final int res = 1 + 2; + print(res); +} +'''); + } + + Future test_lint_alwaysSpecifyTypes_functionExpressionBody() async { + createAnalysisOptionsFile(lints: [LintNames.always_specify_types]); + await indexTestUnit(''' +foo(Point p) => p.x * p.x + p.y * p.y; +class Point {int x = 0; int y = 0;} +'''); + _createRefactoringForString('p.x'); + // apply refactoring + return _assertSuccessfulRefactoring(''' +foo(Point p) { + int res = p.x; + return res * res + p.y * p.y; +} +class Point {int x = 0; int y = 0;} +'''); + } + + Future test_lint_alwaysSpecifyTypes_statement() async { + createAnalysisOptionsFile(lints: [LintNames.always_specify_types]); + await indexTestUnit(''' +void f(String p) { + p.toString(); +} +'''); + _createRefactoringForString('p.toString()'); + // apply refactoring + return _assertSuccessfulRefactoring(''' +void f(String p) { + String res = p.toString(); +} +'''); + } + + Future test_lint_alwaysSpecifyTypes_stringLiteralPart() async { + createAnalysisOptionsFile(lints: [LintNames.always_specify_types]); + await indexTestUnit(''' +void f() { + print('abcdefgh'); +} +'''); + _createRefactoringForString('cde'); + // apply refactoring + return _assertSuccessfulRefactoring(r''' +void f() { + String res = 'cde'; + print('ab${res}fgh'); +} +'''); + } + + Future test_lint_preferFinalLocals() async { createAnalysisOptionsFile(lints: [LintNames.prefer_final_locals]); await indexTestUnit(''' void f() { diff --git a/pkg/analyzer/lib/dart/analysis/code_style_options.dart b/pkg/analyzer/lib/dart/analysis/code_style_options.dart index 5c968a61b65..f510dcf8c74 100644 --- a/pkg/analyzer/lib/dart/analysis/code_style_options.dart +++ b/pkg/analyzer/lib/dart/analysis/code_style_options.dart @@ -24,6 +24,9 @@ abstract class CodeStyleOptions { /// class members. bool get sortConstructorsFirst; + /// Return `true` if types should be specified whenever possible. + bool get specifyTypes; + /// Return `true` if the formatter should be used on code changes in this /// context. bool get useFormatter; diff --git a/pkg/analyzer/lib/src/analysis_options/code_style_options.dart b/pkg/analyzer/lib/src/analysis_options/code_style_options.dart index 4225b4873e1..66a02e91c8e 100644 --- a/pkg/analyzer/lib/src/analysis_options/code_style_options.dart +++ b/pkg/analyzer/lib/src/analysis_options/code_style_options.dart @@ -29,6 +29,9 @@ class CodeStyleOptionsImpl implements CodeStyleOptions { @override bool get sortConstructorsFirst => _isLintEnabled('sort_constructors_first'); + @override + bool get specifyTypes => _isLintEnabled('always_specify_types'); + @override bool get useRelativeUris => _isLintEnabled('prefer_relative_imports');