Skip to content

Commit fe04e8b

Browse files
authored
Move a bunch of functions on AST nodes over to extensions. (#1111)
* Move a bunch of functions on AST nodes over to extensions. These were mostly arbitrarily defined in SourceVisitor and that class is huge enough as it is, so this moves them out. * Remove done TODO.
1 parent a350882 commit fe04e8b

File tree

4 files changed

+259
-243
lines changed

4 files changed

+259
-243
lines changed

lib/src/argument_list_visitor.dart

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import 'dart:math' as math;
66
import 'package:analyzer/dart/ast/ast.dart';
77
import 'package:analyzer/dart/ast/token.dart';
88

9+
import 'ast_extensions.dart';
910
import 'chunk.dart';
1011
import 'rule/argument.dart';
1112
import 'rule/rule.dart';
@@ -131,7 +132,7 @@ class ArgumentListVisitor {
131132
_visitor.visit(argument);
132133

133134
// Write the following comma.
134-
if (_visitor.hasCommaAfter(argument)) {
135+
if (argument.hasCommaAfter) {
135136
_visitor.token(argument.endToken.next);
136137
}
137138
}
@@ -480,7 +481,7 @@ class ArgumentSublist {
480481
}
481482

482483
// Write the following comma.
483-
if (visitor.hasCommaAfter(argument)) {
484+
if (argument.hasCommaAfter) {
484485
visitor.token(argument.endToken.next);
485486
}
486487
}

lib/src/ast_extensions.dart

Lines changed: 227 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,227 @@
1+
// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
import 'package:analyzer/dart/ast/ast.dart';
5+
import 'package:analyzer/dart/ast/token.dart';
6+
7+
extension AstNodeExtensions on AstNode {
8+
/// The comma token immediately following this if there is one, or `null`.
9+
Token? get commaAfter {
10+
var next = endToken.next!;
11+
if (next.type == TokenType.COMMA) return next;
12+
13+
// TODO(sdk#38990): endToken doesn't include the "?" on a nullable
14+
// function-typed formal, so check for that case and handle it.
15+
if (next.type == TokenType.QUESTION && next.next!.type == TokenType.COMMA) {
16+
return next.next;
17+
}
18+
19+
return null;
20+
}
21+
22+
/// Whether there is a comma token immediately following this.
23+
bool get hasCommaAfter => commaAfter != null;
24+
25+
bool get isControlFlowElement => this is IfElement || this is ForElement;
26+
27+
/// Whether this is immediately contained within an anonymous
28+
/// [FunctionExpression].
29+
bool get isFunctionExpressionBody =>
30+
parent is FunctionExpression && parent!.parent is! FunctionDeclaration;
31+
32+
/// Whether [node] is a spread of a non-empty collection literal.
33+
bool get isSpreadCollection => spreadCollectionBracket != null;
34+
35+
/// If this is a spread of a non-empty collection literal, then returns the
36+
/// token for the opening bracket of the collection, as in:
37+
///
38+
/// [ ...[a, list] ]
39+
/// // ^
40+
///
41+
/// Otherwise, returns `null`.
42+
Token? get spreadCollectionBracket {
43+
var node = this;
44+
if (node is SpreadElement) {
45+
var expression = node.expression;
46+
if (expression is ListLiteral) {
47+
if (!expression.elements.isEmptyBody(expression.rightBracket)) {
48+
return expression.leftBracket;
49+
}
50+
} else if (expression is SetOrMapLiteral) {
51+
if (!expression.elements.isEmptyBody(expression.rightBracket)) {
52+
return expression.leftBracket;
53+
}
54+
}
55+
}
56+
57+
return null;
58+
}
59+
}
60+
61+
extension AstIterableExtensions on Iterable<AstNode> {
62+
/// Whether there is a comma token immediately following this.
63+
bool get hasCommaAfter => isNotEmpty && last.hasCommaAfter;
64+
65+
/// Whether the collection literal or block containing these nodes and
66+
/// terminated by [rightBracket] is empty or not.
67+
///
68+
/// An empty collection must have no elements or comments inside. Collections
69+
/// like that are treated specially because they cannot be split inside.
70+
bool isEmptyBody(Token rightBracket) =>
71+
isEmpty && rightBracket.precedingComments == null;
72+
}
73+
74+
extension ExpressionExtensions on Expression {
75+
/// Whether [expression] is a collection literal, or a call with a trailing
76+
/// comma in an argument list.
77+
///
78+
/// In that case, when the expression is a target of a cascade, we don't
79+
/// force a split before the ".." as eagerly to avoid ugly results like:
80+
///
81+
/// [
82+
/// 1,
83+
/// 2,
84+
/// ]..addAll(numbers);
85+
bool get isCollectionLike {
86+
var expression = this;
87+
if (expression is ListLiteral) return false;
88+
if (expression is SetOrMapLiteral) return false;
89+
90+
// If the target is a call with a trailing comma in the argument list,
91+
// treat it like a collection literal.
92+
ArgumentList? arguments;
93+
if (expression is InvocationExpression) {
94+
arguments = expression.argumentList;
95+
} else if (expression is InstanceCreationExpression) {
96+
arguments = expression.argumentList;
97+
}
98+
99+
// TODO(rnystrom): Do we want to allow an invocation where the last
100+
// argument is a collection literal? Like:
101+
//
102+
// foo(argument, [
103+
// element
104+
// ])..cascade();
105+
106+
return arguments == null || !arguments.arguments.hasCommaAfter;
107+
}
108+
109+
/// Whether this is an argument in an argument list with a trailing comma.
110+
bool get isTrailingCommaArgument {
111+
var parent = this.parent;
112+
if (parent is NamedExpression) parent = parent.parent;
113+
114+
return parent is ArgumentList && parent.arguments.hasCommaAfter;
115+
}
116+
117+
/// Whether this is a method invocation that looks like it might be a static
118+
/// method or constructor call without a `new` keyword.
119+
///
120+
/// With optional `new`, we can no longer reliably identify constructor calls
121+
/// statically, but we still don't want to mix named constructor calls into
122+
/// a call chain like:
123+
///
124+
/// Iterable
125+
/// .generate(...)
126+
/// .toList();
127+
///
128+
/// And instead prefer:
129+
///
130+
/// Iterable.generate(...)
131+
/// .toList();
132+
///
133+
/// So we try to identify these calls syntactically. The heuristic we use is
134+
/// that a target that's a capitalized name (possibly prefixed by "_") is
135+
/// assumed to be a class.
136+
///
137+
/// This has the effect of also keeping static method calls with the class,
138+
/// but that tends to look pretty good too, and is certainly better than
139+
/// splitting up named constructors.
140+
bool get looksLikeStaticCall {
141+
var node = this;
142+
if (node is! MethodInvocation) return false;
143+
if (node.target == null) return false;
144+
145+
// A prefixed unnamed constructor call:
146+
//
147+
// prefix.Foo();
148+
if (node.target is SimpleIdentifier &&
149+
_looksLikeClassName(node.methodName.name)) {
150+
return true;
151+
}
152+
153+
// A prefixed or unprefixed named constructor call:
154+
//
155+
// Foo.named();
156+
// prefix.Foo.named();
157+
var target = node.target;
158+
if (target is PrefixedIdentifier) target = target.identifier;
159+
160+
return target is SimpleIdentifier && _looksLikeClassName(target.name);
161+
}
162+
163+
/// Whether [name] appears to be a type name.
164+
///
165+
/// Type names begin with a capital letter and contain at least one lowercase
166+
/// letter (so that we can distinguish them from SCREAMING_CAPS constants).
167+
static bool _looksLikeClassName(String name) {
168+
// Handle the weird lowercase corelib names.
169+
if (name == 'bool') return true;
170+
if (name == 'double') return true;
171+
if (name == 'int') return true;
172+
if (name == 'num') return true;
173+
174+
// TODO(rnystrom): A simpler implementation is to test against the regex
175+
// "_?[A-Z].*?[a-z]". However, that currently has much worse performance on
176+
// AOT: https://github.com/dart-lang/sdk/issues/37785.
177+
const underscore = 95;
178+
const capitalA = 65;
179+
const capitalZ = 90;
180+
const lowerA = 97;
181+
const lowerZ = 122;
182+
183+
var start = 0;
184+
var firstChar = name.codeUnitAt(start++);
185+
186+
// It can be private.
187+
if (firstChar == underscore) {
188+
if (name.length == 1) return false;
189+
firstChar = name.codeUnitAt(start++);
190+
}
191+
192+
// It must start with a capital letter.
193+
if (firstChar < capitalA || firstChar > capitalZ) return false;
194+
195+
// And have at least one lowercase letter in it. Otherwise it could be a
196+
// SCREAMING_CAPS constant.
197+
for (var i = start; i < name.length; i++) {
198+
var char = name.codeUnitAt(i);
199+
if (char >= lowerA && char <= lowerZ) return true;
200+
}
201+
202+
return false;
203+
}
204+
}
205+
206+
extension CascadeExpressionExtensions on CascadeExpression {
207+
/// Whether a cascade should be allowed to be inline as opposed to moving the
208+
/// section to the next line.
209+
bool get allowInline {
210+
// Cascades with multiple sections are handled elsewhere and are never
211+
// inline.
212+
assert(cascadeSections.length == 1);
213+
214+
// If the receiver is an expression that makes the cascade's very low
215+
// precedence confusing, force it to split. For example:
216+
//
217+
// a ? b : c..d();
218+
//
219+
// Here, the cascade is applied to the result of the conditional, not "c".
220+
if (target is ConditionalExpression) return false;
221+
if (target is BinaryExpression) return false;
222+
if (target is PrefixExpression) return false;
223+
if (target is AwaitExpression) return false;
224+
225+
return true;
226+
}
227+
}

lib/src/call_chain_visitor.dart

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import 'package:analyzer/dart/ast/ast.dart';
55
import 'package:analyzer/dart/ast/token.dart';
66

77
import 'argument_list_visitor.dart';
8+
import 'ast_extensions.dart';
89
import 'rule/argument.dart';
910
import 'rule/rule.dart';
1011
import 'source_visitor.dart';
@@ -311,7 +312,7 @@ class CallChainVisitor {
311312
var argument = argumentList.arguments.last;
312313

313314
// If the argument list has a trailing comma, treat it like a collection.
314-
if (_visitor.hasCommaAfter(argument)) return false;
315+
if (argument.hasCommaAfter) return false;
315316

316317
if (argument is NamedExpression) {
317318
argument = argument.expression;
@@ -534,7 +535,7 @@ Expression _unwrapTarget(Expression node, List<_Selector> calls) {
534535
// Don't include things that look like static method or constructor
535536
// calls in the call chain because that tends to split up named
536537
// constructors from their class.
537-
if (SourceVisitor.looksLikeStaticCall(node)) return node;
538+
if (node.looksLikeStaticCall) return node;
538539

539540
// Selectors.
540541
if (node is MethodInvocation && node.target != null) {

0 commit comments

Comments
 (0)