Skip to content

Commit 0f41778

Browse files
srawlinsCommit Queue
authored andcommitted
linter: merge most of the impl of discarded and unawaited futures rules
Change-Id: I784b383dc14f2f74036a4c70cafee6b9454c22eb Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/428561 Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]>
1 parent 0da11d5 commit 0f41778

File tree

4 files changed

+147
-196
lines changed

4 files changed

+147
-196
lines changed

pkg/linter/lib/src/rules/discarded_futures.dart

Lines changed: 14 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,10 @@
33
// BSD-style license that can be found in the LICENSE file.
44
import 'package:analyzer/analysis_rule/rule_context.dart';
55
import 'package:analyzer/dart/ast/ast.dart';
6-
import 'package:analyzer/dart/ast/visitor.dart';
7-
import 'package:analyzer/dart/element/element.dart';
8-
import 'package:analyzer/dart/element/type.dart';
96
import 'package:analyzer/error/error.dart';
107

118
import '../analyzer.dart';
12-
import '../extensions.dart';
13-
import '../utils.dart';
9+
import '../util/unused_futures.dart';
1410

1511
const _desc =
1612
'There should be no `Future`-returning calls in synchronous functions unless they '
@@ -25,96 +21,21 @@ class DiscardedFutures extends LintRule {
2521

2622
@override
2723
void registerNodeProcessors(NodeLintRegistry registry, RuleContext context) {
28-
var visitor = _Visitor(this);
24+
var visitor = UnusedFuturesVisitor(
25+
rule: this,
26+
isInteresting: (node) {
27+
var type = node.staticType;
28+
// This rule does concern itself with `FutureOr`.
29+
if (type == null || !type.isOrImplementsFutureOrFutureOr) {
30+
return false;
31+
}
32+
// This rule is only concerned with code in sync functions.
33+
return node.thisOrAncestorOfType<FunctionBody>()?.isSynchronous ??
34+
false;
35+
},
36+
);
2937
registry.addExpressionStatement(this, visitor);
3038
registry.addCascadeExpression(this, visitor);
3139
registry.addInterpolationExpression(this, visitor);
3240
}
3341
}
34-
35-
class _Visitor extends SimpleAstVisitor<void> {
36-
final LintRule rule;
37-
38-
_Visitor(this.rule);
39-
40-
@override
41-
void visitCascadeExpression(CascadeExpression node) {
42-
var sections = node.cascadeSections;
43-
for (var i = 0; i < sections.length; i++) {
44-
_visit(sections[i]);
45-
}
46-
}
47-
48-
@override
49-
void visitExpressionStatement(ExpressionStatement node) {
50-
var expr = node.expression;
51-
if (expr is AssignmentExpression) return;
52-
if (_isEnclosedInAsyncFunctionBody(node)) return;
53-
if (expr is AwaitExpression) return;
54-
if (expr.isAwaitNotRequired) return;
55-
56-
var type = expr.staticType;
57-
if (type == null) {
58-
return;
59-
}
60-
if (type.isFutureOrFutureOr) {
61-
// Ignore a couple of special known cases.
62-
if (_isFutureDelayedInstanceCreationWithComputation(expr) ||
63-
_isMapPutIfAbsentInvocation(expr)) {
64-
return;
65-
}
66-
67-
reportOnExpression(rule, expr);
68-
}
69-
}
70-
71-
@override
72-
void visitInterpolationExpression(InterpolationExpression node) {
73-
_visit(node.expression);
74-
}
75-
76-
bool _isEnclosedInAsyncFunctionBody(AstNode node) {
77-
var enclosingFunctionBody = node.thisOrAncestorOfType<FunctionBody>();
78-
return enclosingFunctionBody?.isAsynchronous ?? false;
79-
}
80-
81-
/// Detects `Future.delayed(duration, [computation])` creations with a
82-
/// computation.
83-
bool _isFutureDelayedInstanceCreationWithComputation(Expression expr) =>
84-
expr is InstanceCreationExpression &&
85-
(expr.staticType.isFutureOrFutureOr) &&
86-
expr.constructorName.name?.name == 'delayed' &&
87-
expr.argumentList.arguments.length == 2;
88-
89-
bool _isMapClass(Element? e) =>
90-
e is ClassElement && e.name3 == 'Map' && e.library2.name3 == 'dart.core';
91-
92-
/// Detects Map.putIfAbsent invocations.
93-
bool _isMapPutIfAbsentInvocation(Expression expr) =>
94-
expr is MethodInvocation &&
95-
expr.methodName.name == 'putIfAbsent' &&
96-
_isMapClass(expr.methodName.element?.enclosingElement);
97-
98-
void _visit(Expression expr) {
99-
if (expr.isAwaitNotRequired) {
100-
return;
101-
}
102-
103-
if ((expr.staticType.isFutureOrFutureOr) &&
104-
!_isEnclosedInAsyncFunctionBody(expr) &&
105-
expr is! AssignmentExpression) {
106-
// TODO(srawlins): Take `@awaitNotRequired` into account.
107-
reportOnExpression(rule, expr);
108-
}
109-
}
110-
}
111-
112-
extension on DartType? {
113-
bool get isFutureOrFutureOr {
114-
var self = this;
115-
if (self == null) return false;
116-
if (self.isDartAsyncFuture) return true;
117-
if (self.isDartAsyncFutureOr) return true;
118-
return false;
119-
}
120-
}

pkg/linter/lib/src/rules/unawaited_futures.dart

Lines changed: 14 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,12 @@
44

55
import 'package:analyzer/analysis_rule/rule_context.dart';
66
import 'package:analyzer/dart/ast/ast.dart';
7-
import 'package:analyzer/dart/ast/visitor.dart';
87
import 'package:analyzer/dart/element/element.dart';
98
import 'package:analyzer/dart/element/type.dart';
109
import 'package:analyzer/error/error.dart';
1110

1211
import '../analyzer.dart';
13-
import '../extensions.dart';
14-
import '../utils.dart';
12+
import '../util/unused_futures.dart';
1513

1614
const _desc =
1715
r'`Future` results in `async` function bodies must be '
@@ -26,96 +24,25 @@ class UnawaitedFutures extends LintRule {
2624

2725
@override
2826
void registerNodeProcessors(NodeLintRegistry registry, RuleContext context) {
29-
var visitor = _Visitor(this);
27+
var visitor = UnusedFuturesVisitor(
28+
rule: this,
29+
isInteresting: (node) {
30+
var type = node.staticType;
31+
// This rule is not currently concerned with `FutureOr`.
32+
if (type == null || !type.isOrImplementsFuture) {
33+
return false;
34+
}
35+
// This rule is only concerned with code in async functions.
36+
return node.thisOrAncestorOfType<FunctionBody>()?.isAsynchronous ??
37+
false;
38+
},
39+
);
3040
registry.addExpressionStatement(this, visitor);
3141
registry.addCascadeExpression(this, visitor);
3242
registry.addInterpolationExpression(this, visitor);
3343
}
3444
}
3545

36-
class _Visitor extends SimpleAstVisitor<void> {
37-
final LintRule rule;
38-
39-
_Visitor(this.rule);
40-
41-
@override
42-
void visitCascadeExpression(CascadeExpression node) {
43-
var sections = node.cascadeSections;
44-
for (var i = 0; i < sections.length; i++) {
45-
_visit(sections[i]);
46-
}
47-
}
48-
49-
@override
50-
void visitExpressionStatement(ExpressionStatement node) {
51-
var expr = node.expression;
52-
if (expr is AssignmentExpression) return;
53-
54-
var type = expr.staticType;
55-
if (type == null || !type.isOrImplementsFuture) {
56-
return;
57-
}
58-
59-
// Ignore a couple of special known cases.
60-
if (_isFutureDelayedInstanceCreationWithComputation(expr) ||
61-
_isMapPutIfAbsentInvocation(expr)) {
62-
return;
63-
}
64-
65-
if (expr.isAwaitNotRequired) {
66-
return;
67-
}
68-
69-
if (_isEnclosedInAsyncFunctionBody(node)) {
70-
// Future expression statement that isn't awaited in an async function:
71-
// while this is legal, it's a very frequent sign of an error.
72-
reportOnExpression(rule, node.expression);
73-
}
74-
}
75-
76-
@override
77-
void visitInterpolationExpression(InterpolationExpression node) {
78-
_visit(node.expression);
79-
}
80-
81-
bool _isEnclosedInAsyncFunctionBody(AstNode node) {
82-
var enclosingFunctionBody = node.thisOrAncestorOfType<FunctionBody>();
83-
return enclosingFunctionBody?.isAsynchronous ?? false;
84-
}
85-
86-
/// Detects `Future.delayed(duration, [computation])` creations with a
87-
/// computation.
88-
bool _isFutureDelayedInstanceCreationWithComputation(Expression expr) =>
89-
expr is InstanceCreationExpression &&
90-
(expr.staticType?.isDartAsyncFuture ?? false) &&
91-
expr.constructorName.name?.name == 'delayed' &&
92-
expr.argumentList.arguments.length == 2;
93-
94-
bool _isMapClass(Element? e) =>
95-
e is ClassElement && e.name3 == 'Map' && e.library2.name3 == 'dart.core';
96-
97-
/// Detects `Map.putIfAbsent` invocations.
98-
bool _isMapPutIfAbsentInvocation(Expression expr) =>
99-
expr is MethodInvocation &&
100-
expr.methodName.name == 'putIfAbsent' &&
101-
_isMapClass(expr.methodName.element?.enclosingElement);
102-
103-
void _visit(Expression expr) {
104-
if (expr.isAwaitNotRequired) {
105-
return;
106-
}
107-
108-
var type = expr.staticType;
109-
if (type == null || !type.isOrImplementsFuture) {
110-
return;
111-
}
112-
113-
if (_isEnclosedInAsyncFunctionBody(expr) && expr is! AssignmentExpression) {
114-
reportOnExpression(rule, expr);
115-
}
116-
}
117-
}
118-
11946
extension on DartType {
12047
/// Whether this type is `Future` from dart:async, or is a subtype thereof.
12148
bool get isOrImplementsFuture {
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
// Copyright (c) 2025, 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/visitor.dart';
6+
import 'package:analyzer/dart/element/element.dart';
7+
import 'package:analyzer/dart/element/type.dart';
8+
9+
import '../analyzer.dart';
10+
import '../extensions.dart';
11+
12+
/// A function that returns whether a given [Expression] is "interesting," and
13+
/// should be considered for a lint report.
14+
typedef IsInterestingFilter = bool Function(Expression node);
15+
16+
/// A shared visitor for the `discarded_futures` and `unawaited_futures` lint
17+
/// rules.
18+
///
19+
/// The two rules instantiate this with different [IsInterestingFilter]
20+
/// predicates (`discarded_futures` is concerned with _synchronous_ functions;
21+
/// `unawaited_futures` is concerned with _asynchronous_ functions).
22+
class UnusedFuturesVisitor extends SimpleAstVisitor<void> {
23+
final LintRule _rule;
24+
25+
/// Returns whether an [Expression] is "interesting," that is, whether we
26+
/// might report on it.
27+
final IsInterestingFilter _isInteresting;
28+
29+
UnusedFuturesVisitor({
30+
required LintRule rule,
31+
required IsInterestingFilter isInteresting,
32+
}) : _rule = rule,
33+
_isInteresting = isInteresting;
34+
35+
@override
36+
void visitCascadeExpression(CascadeExpression node) {
37+
var sections = node.cascadeSections;
38+
for (var i = 0; i < sections.length; i++) {
39+
_visit(sections[i]);
40+
}
41+
}
42+
43+
@override
44+
void visitExpressionStatement(ExpressionStatement node) {
45+
var expr = node.expression;
46+
if (expr is AssignmentExpression) return;
47+
if (expr is AwaitExpression) return;
48+
49+
if (!_isInteresting(expr)) return;
50+
if (expr.isAwaitNotRequired) return;
51+
52+
// Ignore a couple of special known cases.
53+
if (_isFutureDelayedInstanceCreationWithComputation(expr) ||
54+
_isMapPutIfAbsentInvocation(expr)) {
55+
return;
56+
}
57+
58+
_reportOnExpression(expr);
59+
}
60+
61+
@override
62+
void visitInterpolationExpression(InterpolationExpression node) {
63+
_visit(node.expression);
64+
}
65+
66+
/// Detects `Future.delayed(duration, [computation])` creations with a
67+
/// computation.
68+
bool _isFutureDelayedInstanceCreationWithComputation(Expression expr) =>
69+
expr is InstanceCreationExpression &&
70+
(expr.staticType?.isDartAsyncFuture ?? false) &&
71+
expr.constructorName.name?.name == 'delayed' &&
72+
expr.argumentList.arguments.length == 2;
73+
74+
bool _isMapClass(Element? e) =>
75+
e is ClassElement && e.name3 == 'Map' && e.library2.name3 == 'dart.core';
76+
77+
/// Detects `Map.putIfAbsent` invocations.
78+
bool _isMapPutIfAbsentInvocation(Expression expr) =>
79+
expr is MethodInvocation &&
80+
expr.methodName.name == 'putIfAbsent' &&
81+
_isMapClass(expr.methodName.element?.enclosingElement);
82+
83+
void _reportOnExpression(Expression expr) {
84+
_rule.reportAtNode(switch (expr) {
85+
MethodInvocation(:var methodName) => methodName,
86+
InstanceCreationExpression(:var constructorName) => constructorName,
87+
FunctionExpressionInvocation(:var function) => function,
88+
PrefixedIdentifier(:var identifier) => identifier,
89+
PropertyAccess(:var propertyName) => propertyName,
90+
_ => expr,
91+
});
92+
}
93+
94+
void _visit(Expression expr) {
95+
if (expr.isAwaitNotRequired) {
96+
return;
97+
}
98+
99+
var type = expr.staticType;
100+
if (type != null &&
101+
type.isOrImplementsFutureOrFutureOr &&
102+
_isInteresting(expr) &&
103+
expr is! AssignmentExpression) {
104+
_reportOnExpression(expr);
105+
}
106+
}
107+
}
108+
109+
extension DartTypeExtension on DartType {
110+
/// Whether this type is `Future` or `FutureOr` from dart:async, or is a
111+
/// subtype of `Future`.
112+
bool get isOrImplementsFutureOrFutureOr {
113+
var typeElement = element3;
114+
if (typeElement is! InterfaceElement) return false;
115+
return isDartAsyncFuture ||
116+
isDartAsyncFutureOr ||
117+
typeElement.allSupertypes.any((t) => t.isDartAsyncFuture);
118+
}
119+
}

0 commit comments

Comments
 (0)