Clean up 'Extract Local' refactoring.

We don't support expression fragments anymore, also don't need to
re-tokenize code.

R=brianwilkerson@google.com

Change-Id: Ic8bf930a2d7b81862ce08925405bdd479db8eaba
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/115180
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
This commit is contained in:
Konstantin Shcheglov 2019-09-02 20:07:47 +00:00 committed by commit-bot@chromium.org
parent fe3fc9ed15
commit 6705a135e5
3 changed files with 64 additions and 178 deletions

View file

@ -1358,6 +1358,17 @@ class CorrectionUtils_InsertDesc {
* Utilities to work with [Token]s.
*/
class TokenUtils {
static List<Token> getNodeTokens(AstNode node) {
var result = <Token>[];
for (var token = node.beginToken;; token = token.next) {
result.add(token);
if (token == node.endToken) {
break;
}
}
return result;
}
/**
* @return [Token]s of the given Dart source, not <code>null</code>, may be empty if no
* tokens or some exception happens.

View file

@ -7,7 +7,6 @@ import 'dart:collection';
import 'package:analysis_server/src/protocol_server.dart' hide Element;
import 'package:analysis_server/src/services/correction/name_suggestion.dart';
import 'package:analysis_server/src/services/correction/selection_analyzer.dart';
import 'package:analysis_server/src/services/correction/status.dart';
import 'package:analysis_server/src/services/correction/strings.dart';
import 'package:analysis_server/src/services/correction/util.dart';
@ -47,9 +46,8 @@ class ExtractLocalRefactoringImpl extends RefactoringImpl
final List<int> offsets = <int>[];
final List<int> lengths = <int>[];
Expression rootExpression;
FunctionBody coveringFunctionBody;
Expression singleExpression;
bool wholeStatementExpression = false;
String stringLiteralPart;
final List<SourceRange> occurrences = <SourceRange>[];
final Map<Element, int> elementIds = <Element, int>{};
@ -71,7 +69,7 @@ class ExtractLocalRefactoringImpl extends RefactoringImpl
CompilationUnitElement get unitElement => unit.declaredElement;
String get _declarationKeyword {
if (_isPartOfConstantExpression(rootExpression)) {
if (_isPartOfConstantExpression(singleExpression)) {
return "const";
} else if (_isLintEnabled(LintNames.prefer_final_locals)) {
return "final";
@ -130,7 +128,8 @@ class ExtractLocalRefactoringImpl extends RefactoringImpl
occurrences.sort((a, b) => a.offset - b.offset);
// If the whole expression of a statement is selected, like '1 + 2',
// then convert it into a variable declaration statement.
if (wholeStatementExpression && occurrences.length == 1) {
if (singleExpression?.parent is ExpressionStatement &&
occurrences.length == 1) {
String keyword = _declarationKeyword;
String declarationSource = '$keyword $name = ';
SourceEdit edit =
@ -237,20 +236,42 @@ class ExtractLocalRefactoringImpl extends RefactoringImpl
'The selection end offset must be less then the length of the file.');
}
String selectionStr;
var selectionStr = utils.getRangeText(selectionRange);
// exclude whitespaces
{
selectionStr = utils.getRangeText(selectionRange);
int numLeading = countLeadingWhitespaces(selectionStr);
int numTrailing = countTrailingWhitespaces(selectionStr);
int offset = selectionRange.offset + numLeading;
int end = selectionRange.end - numTrailing;
selectionRange = new SourceRange(offset, end - offset);
}
// get covering node
AstNode coveringNode =
new NodeLocator(selectionRange.offset, selectionRange.end)
.searchWithin(unit);
// We need an enclosing function.
// If it has a block body, we can add a new variable declaration statement
// into this block. If it has an expression body, we can convert it into
// the block body first.
coveringFunctionBody = coveringNode?.thisOrAncestorOfType<FunctionBody>();
if (coveringFunctionBody == null) {
return new RefactoringStatus.fatal(
'An expression inside a function must be selected '
'to activate this refactoring.');
}
// part of string literal
if (coveringNode is StringLiteral) {
if (selectionRange.length != 0 &&
selectionRange.offset > coveringNode.offset &&
selectionRange.end < coveringNode.end) {
stringLiteralPart = selectionStr;
return new RefactoringStatus();
}
}
// compute covering expressions
for (AstNode node = coveringNode; node != null; node = node.parent) {
AstNode parent = node.parent;
@ -262,7 +283,7 @@ class ExtractLocalRefactoringImpl extends RefactoringImpl
continue;
}
if (node is ConstructorName || node is Label || node is TypeName) {
rootExpression = null;
singleExpression = null;
coveringExpressionOffsets.clear();
coveringExpressionLengths.clear();
continue;
@ -283,7 +304,7 @@ class ExtractLocalRefactoringImpl extends RefactoringImpl
if (element is ExecutableElement &&
element.returnType != null &&
element.returnType.isVoid) {
if (rootExpression == null) {
if (singleExpression == null) {
return new RefactoringStatus.fatal(
'Cannot extract the void expression.',
newLocation_fromNode(node));
@ -311,37 +332,16 @@ class ExtractLocalRefactoringImpl extends RefactoringImpl
}
}
// set selected expression
if (coveringExpressionOffsets.isEmpty) {
rootExpression = node;
if (singleExpression == null) {
singleExpression = node;
}
// add the expression range
coveringExpressionOffsets.add(node.offset);
coveringExpressionLengths.add(node.length);
}
// We need an enclosing function.
// If it has a block body, we can add a new variable declaration statement
// into this block. If it has an expression body, we can convert it into
// the block body first.
if (coveringNode == null ||
coveringNode.thisOrAncestorOfType<FunctionBody>() == null) {
return new RefactoringStatus.fatal(
'An expression inside a function must be selected '
'to activate this refactoring.');
}
// part of string literal
if (coveringNode is StringLiteral) {
if (selectionRange.length != 0 &&
selectionRange.offset > coveringNode.offset &&
selectionRange.end < coveringNode.end) {
stringLiteralPart = selectionStr;
return new RefactoringStatus();
}
}
// single node selected
if (rootExpression != null) {
singleExpression = rootExpression;
if (singleExpression != null) {
selectionRange = range.node(singleExpression);
wholeStatementExpression = singleExpression.parent is ExpressionStatement;
return new RefactoringStatus();
}
// invalid selection
@ -455,15 +455,6 @@ class ExtractLocalRefactoringImpl extends RefactoringImpl
return null;
}
/**
* Checks if it is OK to extract the node with the given [SourceRange].
*/
bool _isExtractable(SourceRange range) {
_ExtractExpressionAnalyzer analyzer = new _ExtractExpressionAnalyzer(range);
utils.unit.accept(analyzer);
return analyzer.status.isOK;
}
bool _isLintEnabled(String name) {
var analysisOptions = unitElement.context.analysisOptions;
return analysisOptions.isLintEnabled(name);
@ -508,24 +499,15 @@ class ExtractLocalRefactoringImpl extends RefactoringImpl
void _prepareOccurrences() {
occurrences.clear();
elementIds.clear();
// prepare selection
String selectionSource;
{
String rawSelectionSource = utils.getRangeText(selectionRange);
List<Token> selectionTokens =
TokenUtils.getTokens(rawSelectionSource, unit.featureSet);
selectionSource =
_encodeExpressionTokens(rootExpression, selectionTokens);
}
// prepare enclosing function
AstNode enclosingFunction;
{
AstNode selectionNode =
new NodeLocator(selectionOffset).searchWithin(unit);
enclosingFunction = getEnclosingExecutableNode(selectionNode);
if (singleExpression != null) {
var tokens = TokenUtils.getNodeTokens(singleExpression);
selectionSource = _encodeExpressionTokens(singleExpression, tokens);
}
// visit function
enclosingFunction.accept(new _OccurrencesVisitor(
coveringFunctionBody.accept(new _OccurrencesVisitor(
this, occurrences, selectionSource, unit.featureSet));
}
@ -539,79 +521,6 @@ class ExtractLocalRefactoringImpl extends RefactoringImpl
}
}
/**
* [SelectionAnalyzer] for [ExtractLocalRefactoringImpl].
*/
class _ExtractExpressionAnalyzer extends SelectionAnalyzer {
final RefactoringStatus status = new RefactoringStatus();
_ExtractExpressionAnalyzer(SourceRange selection) : super(selection);
/**
* Records fatal error with given message.
*/
void invalidSelection(String message) {
_invalidSelection(message, null);
}
@override
Object visitAssignmentExpression(AssignmentExpression node) {
super.visitAssignmentExpression(node);
Expression lhs = node.leftHandSide;
if (_isFirstSelectedNode(lhs)) {
_invalidSelection('Cannot extract the left-hand side of an assignment.',
newLocation_fromNode(lhs));
}
return null;
}
@override
Object visitSimpleIdentifier(SimpleIdentifier node) {
super.visitSimpleIdentifier(node);
if (_isFirstSelectedNode(node)) {
// name of declaration
if (node.inDeclarationContext()) {
invalidSelection('Cannot extract the name part of a declaration.');
}
// method name
Element element = node.staticElement;
if (element is FunctionElement || element is MethodElement) {
invalidSelection('Cannot extract a single method name.');
}
// name in property access
AstNode parent = node.parent;
if (parent is PrefixedIdentifier && identical(parent.identifier, node)) {
invalidSelection('Cannot extract name part of a property access.');
}
if (parent is PropertyAccess && identical(parent.propertyName, node)) {
invalidSelection('Cannot extract name part of a property access.');
}
}
return null;
}
/**
* Records fatal error with given [message] and [location].
*/
void _invalidSelection(String message, Location location) {
status.addFatalError(message, location);
reset();
}
bool _isFirstSelectedNode(AstNode node) => node == firstSelectedNode;
}
class _HasStatementVisitor extends GeneralizingAstVisitor {
bool result = false;
_HasStatementVisitor();
@override
visitStatement(Statement node) {
result = true;
}
}
class _OccurrencesVisitor extends GeneralizingAstVisitor<void> {
final ExtractLocalRefactoringImpl ref;
final List<SourceRange> occurrences;
@ -622,20 +531,19 @@ class _OccurrencesVisitor extends GeneralizingAstVisitor<void> {
this.ref, this.occurrences, this.selectionSource, this.featureSet);
@override
void visitBinaryExpression(BinaryExpression node) {
if (!_hasStatements(node)) {
_tryToFindOccurrenceFragments(node);
return;
}
super.visitBinaryExpression(node);
void visitExpression(Expression node) {
_tryToFindOccurrence(node);
super.visitExpression(node);
}
@override
void visitExpression(Expression node) {
if (ref._isExtractable(range.node(node))) {
_tryToFindOccurrence(node);
void visitSimpleIdentifier(SimpleIdentifier node) {
var parent = node.parent;
if (parent is VariableDeclaration && parent.name == node ||
parent is AssignmentExpression && parent.leftHandSide == node) {
return;
}
super.visitExpression(node);
super.visitSimpleIdentifier(node);
}
@override
@ -667,48 +575,13 @@ class _OccurrencesVisitor extends GeneralizingAstVisitor<void> {
}
}
bool _hasStatements(AstNode root) {
_HasStatementVisitor visitor = new _HasStatementVisitor();
root.accept(visitor);
return visitor.result;
}
void _tryToFindOccurrence(Expression node) {
String nodeSource = ref.utils.getNodeText(node);
List<Token> nodeTokens = TokenUtils.getTokens(nodeSource, featureSet);
nodeSource = ref._encodeExpressionTokens(node, nodeTokens);
var nodeTokens = TokenUtils.getNodeTokens(node);
var nodeSource = ref._encodeExpressionTokens(node, nodeTokens);
if (nodeSource == selectionSource) {
_addOccurrence(range.node(node));
}
}
void _tryToFindOccurrenceFragments(Expression node) {
int nodeOffset = node.offset;
String nodeSource = ref.utils.getNodeText(node);
List<Token> nodeTokens = TokenUtils.getTokens(nodeSource, featureSet);
nodeSource = ref._encodeExpressionTokens(node, nodeTokens);
// find "selection" in "node" tokens
int lastIndex = 0;
while (true) {
// find next occurrence
int index = nodeSource.indexOf(selectionSource, lastIndex);
if (index == -1) {
break;
}
lastIndex = index + selectionSource.length;
// find start/end tokens
int startTokenIndex =
countMatches(nodeSource.substring(0, index), _TOKEN_SEPARATOR);
int endTokenIndex =
countMatches(nodeSource.substring(0, lastIndex), _TOKEN_SEPARATOR);
Token startToken = nodeTokens[startTokenIndex];
Token endToken = nodeTokens[endTokenIndex - 1];
// add occurrence range
int start = nodeOffset + startToken.offset;
int end = nodeOffset + endToken.end;
_addOccurrence(range.startOffsetEndOffset(start, end));
}
}
}
class _TokenLocalElementVisitor extends RecursiveAstVisitor {

View file

@ -91,9 +91,11 @@ main() {
test_checkInitialConditions_namePartOfDeclaration_function() async {
await indexTestUnit('''
void main() {}
void main() {
void foo() {}
}
''');
_createRefactoringWithSuffix('main', '()');
_createRefactoringWithSuffix('foo', '()');
// check conditions
RefactoringStatus status = await refactoring.checkAllConditions();
assertRefactoringStatus(status, RefactoringProblemSeverity.FATAL,