Skip to content

Commit 33b9714

Browse files
srawlinsCommit Queue
authored and
Commit Queue
committed
DAS: Move CorrectionUtils.selectionIncludesNonWhitespaceOutsideNode into extract_method
Bug: #53402 Change-Id: I4fca3482c27520328339d1d1f0b6b541399ed612 Cq-Include-Trybots: luci.dart.try:flutter-analyze-try,analyzer-win-release-try,pkg-win-release-try Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/363501 Reviewed-by: Brian Wilkerson <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]>
1 parent 4d6919a commit 33b9714

File tree

5 files changed

+75
-81
lines changed

5 files changed

+75
-81
lines changed

pkg/analysis_server/lib/src/services/correction/dart/extract_local_variable.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
import 'package:analysis_server/src/services/correction/dart/abstract_producer.dart';
66
import 'package:analysis_server/src/services/correction/fix.dart';
7-
import 'package:analysis_server/src/services/correction/util.dart';
7+
import 'package:analysis_server/src/utilities/extensions/ast.dart';
88
import 'package:analyzer/dart/ast/ast.dart';
99
import 'package:analyzer/dart/ast/token.dart';
1010
import 'package:analyzer/dart/ast/visitor.dart';
@@ -139,7 +139,7 @@ class _ExpressionEncoder {
139139
final Map<Element, int> _elementIds = {};
140140

141141
String encode(Expression node) {
142-
var tokens = TokenUtils.getNodeTokens(node);
142+
var tokens = node.tokens;
143143

144144
var tokenToElementMap = Map<Token, Element>.identity();
145145
node.accept(

pkg/analysis_server/lib/src/services/correction/util.dart

Lines changed: 21 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -515,25 +515,24 @@ final class CorrectionUtils {
515515
/// possible) to cover whole lines.
516516
SourceRange getLinesRange(SourceRange sourceRange,
517517
{bool skipLeadingEmptyLines = false}) {
518-
// start
518+
// Calculate the start:
519519
var startOffset = sourceRange.offset;
520520
var startLineOffset = getLineContentStart(startOffset);
521521
if (skipLeadingEmptyLines) {
522522
startLineOffset = _skipEmptyLinesLeft(startLineOffset);
523523
}
524-
// end
524+
// Calculate the end:
525525
var endOffset = sourceRange.end;
526526
var afterEndLineOffset = endOffset;
527527
var lineInfo = _unit.lineInfo;
528528
var lineStart = lineInfo
529529
.getOffsetOfLine(lineInfo.getLocation(startLineOffset).lineNumber - 1);
530530
if (lineStart == startLineOffset) {
531-
// Only consume line ends after the end of the range if there is nothing
532-
// else on the line containing the beginning of the range. Otherwise this
533-
// will end up incorrectly merging two line.
531+
// Only consume line endings after the end of the range if there is
532+
// nothing else on the line containing the beginning of the range.
533+
// Otherwise this will end up incorrectly merging two line.
534534
afterEndLineOffset = getLineContentEnd(endOffset);
535535
}
536-
// range
537536
return range.startOffsetEndOffset(startLineOffset, afterEndLineOffset);
538537
}
539538

@@ -588,14 +587,11 @@ final class CorrectionUtils {
588587
}
589588

590589
/// Returns the text of the given range in the unit.
591-
String getRangeText(SourceRange range) {
592-
return getText(range.offset, range.length);
593-
}
590+
String getRangeText(SourceRange range) => getText(range.offset, range.length);
594591

595592
/// Returns the text of the given range in the unit.
596-
String getText(int offset, int length) {
597-
return _buffer.substring(offset, offset + length);
598-
}
593+
String getText(int offset, int length) =>
594+
_buffer.substring(offset, offset + length);
599595

600596
/// Indents given source left or right.
601597
String indentSourceLeftRight(String source, {bool indentLeft = true}) {
@@ -622,7 +618,7 @@ final class CorrectionUtils {
622618
return sb.toString();
623619
}
624620

625-
/// Return the source of the inverted condition for the given logical
621+
/// Returns the source of the inverted condition for the given logical
626622
/// expression.
627623
String invertCondition(Expression expression) =>
628624
_invertCondition0(expression)._source;
@@ -642,7 +638,7 @@ final class CorrectionUtils {
642638
/// lines).
643639
String replaceSourceIndent(String source, String oldIndent, String newIndent,
644640
{bool includeLeading = false, bool ensureTrailingNewline = false}) {
645-
// prepare STRING token ranges
641+
// Prepare token ranges.
646642
var lineRanges = <SourceRange>[];
647643
{
648644
var tokens = TokenUtils.getTokens(source, _unit.featureSet);
@@ -652,7 +648,7 @@ final class CorrectionUtils {
652648
}
653649
}
654650
}
655-
// re-indent lines
651+
// Re-indent lines.
656652
var sb = StringBuffer();
657653
var eol = endOfLine;
658654
var lines = source.split(eol);
@@ -661,16 +657,16 @@ final class CorrectionUtils {
661657
var line = lines[i];
662658
// Exit early if this is the last line and it's already empty, to avoid
663659
// inserting any whitespace or appending an additional newline if
664-
// [ensureTrailingNewline].
660+
// `ensureTrailingNewline`.
665661
if (i == lines.length - 1 && isEmpty(line)) {
666662
break;
667663
}
668-
// Don't replace whitespace on first line unless [includeLeading].
664+
// Don't replace whitespace on first line unless `includeLeading`.
669665
var doReplaceWhitespace = i != 0 || includeLeading;
670-
// Don't add eol to last line unless [ensureTrailingNewline].
666+
// Don't add eol to last line unless `ensureTrailingNewline`.
671667
var doAppendEol = i != lines.length - 1 || ensureTrailingNewline;
672668

673-
// check if "offset" is in one of the String ranges
669+
// Check if "offset" is in one of the ranges.
674670
var inString = false;
675671
for (var lineRange in lineRanges) {
676672
if (lineOffset > lineRange.offset && lineOffset < lineRange.end) {
@@ -681,11 +677,11 @@ final class CorrectionUtils {
681677
}
682678
}
683679
lineOffset += line.length + eol.length;
684-
// update line indent
680+
// Update line indent.
685681
if (!inString && doReplaceWhitespace) {
686682
line = '$newIndent${removeStart(line, oldIndent)}';
687683
}
688-
// append line
684+
// Append line.
689685
sb.write(line);
690686
if (doAppendEol) {
691687
sb.write(eol);
@@ -717,14 +713,6 @@ final class CorrectionUtils {
717713
ensureTrailingNewline: ensureTrailingNewline);
718714
}
719715

720-
/// Return `true` if [selection] covers [node] and there are any
721-
/// non-whitespace tokens between [selection] and [node] start/end.
722-
bool selectionIncludesNonWhitespaceOutsideNode(
723-
SourceRange selection, AstNode node) {
724-
return _selectionIncludesNonWhitespaceOutsideRange(
725-
selection, range.node(node));
726-
}
727-
728716
/// Returns the [_InvertedCondition] for the given logical expression.
729717
_InvertedCondition _invertCondition0(Expression expression) {
730718
if (expression is BooleanLiteral) {
@@ -793,39 +781,6 @@ final class CorrectionUtils {
793781
return _InvertedCondition._simple(getNodeText(expression));
794782
}
795783

796-
/// Returns whether [range] contains only whitespace or comments.
797-
bool _isJustWhitespaceOrComment(SourceRange range) {
798-
var trimmedText = getRangeText(range).trim();
799-
// may be whitespace
800-
if (trimmedText.isEmpty) {
801-
return true;
802-
}
803-
// may be comment
804-
return TokenUtils.getTokens(trimmedText, _unit.featureSet).isEmpty;
805-
}
806-
807-
/// Return `true` if [selection] covers [range] and there are any
808-
/// non-whitespace tokens between [selection] and [range] start/end.
809-
bool _selectionIncludesNonWhitespaceOutsideRange(
810-
SourceRange selection, SourceRange sourceRange) {
811-
// selection should cover range
812-
if (!selection.covers(sourceRange)) {
813-
return false;
814-
}
815-
// non-whitespace between selection start and range start
816-
if (!_isJustWhitespaceOrComment(
817-
range.startOffsetEndOffset(selection.offset, sourceRange.offset))) {
818-
return true;
819-
}
820-
// non-whitespace after range
821-
if (!_isJustWhitespaceOrComment(
822-
range.startOffsetEndOffset(sourceRange.end, selection.end))) {
823-
return true;
824-
}
825-
// only whitespace in selection around range
826-
return false;
827-
}
828-
829784
/// Skip spaces, tabs and EOLs on the left from [index].
830785
///
831786
/// If [index] is the start of a method, then in the most cases return the end
@@ -848,19 +803,10 @@ final class CorrectionUtils {
848803

849804
/// Utilities to work with [Token]s.
850805
class TokenUtils {
851-
static List<Token> getNodeTokens(AstNode node) {
852-
var result = <Token>[];
853-
for (var token = node.beginToken;; token = token.next!) {
854-
result.add(token);
855-
if (token == node.endToken) {
856-
break;
857-
}
858-
}
859-
return result;
860-
}
861-
862-
/// Return the tokens of the given Dart source, not `null`, may be empty if no
863-
/// tokens or some exception happens.
806+
/// Returns the tokens of the given Dart source, [s].
807+
///
808+
/// The returned list may be empty if there are no tokens, or some exception
809+
/// is caught.
864810
static List<Token> getTokens(String s, FeatureSet featureSet) {
865811
try {
866812
var tokens = <Token>[];

pkg/analysis_server/lib/src/services/refactoring/legacy/extract_local.dart

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -513,7 +513,7 @@ class ExtractLocalRefactoringImpl extends RefactoringImpl
513513
String? selectionSource;
514514
final singleExpression = this.singleExpression;
515515
if (singleExpression != null) {
516-
var tokens = TokenUtils.getNodeTokens(singleExpression);
516+
var tokens = singleExpression.tokens;
517517
selectionSource = _encodeExpressionTokens(singleExpression, tokens);
518518
}
519519
// visit function
@@ -586,8 +586,7 @@ class _OccurrencesVisitor extends GeneralizingAstVisitor<void> {
586586
}
587587

588588
void _tryToFindOccurrence(Expression node) {
589-
var nodeTokens = TokenUtils.getNodeTokens(node);
590-
var nodeSource = ref._encodeExpressionTokens(node, nodeTokens);
589+
var nodeSource = ref._encodeExpressionTokens(node, node.tokens);
591590
if (nodeSource == selectionSource) {
592591
_addOccurrence(range.node(node));
593592
}

pkg/analysis_server/lib/src/services/refactoring/legacy/extract_method.dart

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -671,7 +671,7 @@ final class ExtractMethodRefactoringImpl extends RefactoringImpl
671671
_parentMember = getEnclosingClassOrUnitMember(selectedNode);
672672
// single expression selected
673673
if (selectedNodes.length == 1) {
674-
if (!_utils.selectionIncludesNonWhitespaceOutsideNode(
674+
if (!_selectionIncludesNonWhitespaceOutsideNode(
675675
_selectionRange, selectedNode)) {
676676
if (selectedNode is Expression) {
677677
_selectionExpression = selectedNode;
@@ -987,6 +987,18 @@ final class ExtractMethodRefactoringImpl extends RefactoringImpl
987987
return analyzer.status.isOK;
988988
}
989989

990+
/// Returns whether [range] contains only whitespace or comments.
991+
bool _isJustWhitespaceOrComment(SourceRange range) {
992+
var trimmedText = _utils.getRangeText(range).trim();
993+
// May be whitespace.
994+
if (trimmedText.isEmpty) {
995+
return true;
996+
}
997+
// May be comment.
998+
return TokenUtils.getTokens(trimmedText, _resolveResult.unit.featureSet)
999+
.isEmpty;
1000+
}
1001+
9901002
bool _isParameterNameConflictWithBody(RefactoringMethodParameter parameter) {
9911003
var id = parameter.id;
9921004
var name = parameter.name;
@@ -1041,6 +1053,31 @@ final class ExtractMethodRefactoringImpl extends RefactoringImpl
10411053
}
10421054
}
10431055

1056+
/// Returns whether [selection] covers [node] and there are any
1057+
/// non-whitespace tokens between [selection] and [node]'s `offset` and `end`
1058+
/// respectively.
1059+
bool _selectionIncludesNonWhitespaceOutsideNode(
1060+
SourceRange selection, AstNode node) {
1061+
var sourceRange = range.node(node);
1062+
1063+
// Selection should cover range.
1064+
if (!selection.covers(sourceRange)) {
1065+
return false;
1066+
}
1067+
// Non-whitespace between selection start and range start.
1068+
if (!_isJustWhitespaceOrComment(
1069+
range.startOffsetEndOffset(selection.offset, sourceRange.offset))) {
1070+
return true;
1071+
}
1072+
// Non-whitespace after range.
1073+
if (!_isJustWhitespaceOrComment(
1074+
range.startOffsetEndOffset(sourceRange.end, selection.end))) {
1075+
return true;
1076+
}
1077+
// Only whitespace in selection around range.
1078+
return false;
1079+
}
1080+
10441081
/// Checks if the given [expression] is reasonable to extract as a getter.
10451082
static bool _isExpressionForGetter(Expression? expression) {
10461083
if (expression is BinaryExpression) {

pkg/analysis_server/lib/src/utilities/extensions/ast.dart

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,18 @@ extension AstNodeExtension on AstNode {
162162

163163
bool get inWhileLoop => thisOrAncestorOfType<WhileStatement>() != null;
164164

165+
/// The [Token]s contained within `this`.
166+
List<Token> get tokens {
167+
var result = <Token>[];
168+
for (var token = beginToken;; token = token.next!) {
169+
result.add(token);
170+
if (token == endToken) {
171+
break;
172+
}
173+
}
174+
return result;
175+
}
176+
165177
/// Return this node and all its parents.
166178
Iterable<AstNode> get withParents sync* {
167179
var current = this;

0 commit comments

Comments
 (0)