Skip to content

Commit 2635b72

Browse files
srawlinssrawlins
committed
bump to 3.10; add a rule
1 parent 54ab763 commit 2635b72

File tree

10 files changed

+339
-49
lines changed

10 files changed

+339
-49
lines changed

‎pkgs/test_analyzer_plugin/README.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,13 @@ This analyzer plugin provides the following additional analysis:
99
declaration. This can _sometimes_ be detected at runtime. This warning is
1010
reported statically.
1111

12+
* Report a warning when a non-nullable value is matched against `isNotNull` or
13+
`isNull`.
14+
15+
* Report a warning when an `expect` expectation of `true`, `false`, `isTrue`,
16+
or `isFalse` is paired with a `.isEmpty` or `.isNotEmpty` property access on
17+
an actual value. Instead, the `isEmpty` or `isNotEmpty` Matcher should be
18+
used.
19+
1220
* Offer a quick fix in the IDE for the above warning, which moves the violating
1321
`test` or `group` declaration below the containing `test` declaration.

‎pkgs/test_analyzer_plugin/lib/main.dart‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import 'package:analysis_server_plugin/registry.dart';
88
import 'src/fixes.dart';
99
import 'src/rules/non_nullable_is_not_null_rule.dart';
1010
import 'src/rules/test_in_test_rule.dart';
11+
import 'src/rules/use_is_empty_matcher.dart';
1112

1213
final plugin = TestPackagePlugin();
1314

@@ -18,9 +19,12 @@ class TestPackagePlugin extends Plugin {
1819
void register(PluginRegistry registry) {
1920
registry.registerWarningRule(TestInTestRule());
2021
registry.registerFixForRule(
21-
TestInTestRule.code, MoveBelowEnclosingTestCall.new);
22+
TestInTestRule.code,
23+
MoveBelowEnclosingTestCall.new,
24+
);
2225

2326
registry.registerWarningRule(NonNullableIsNotNullRule());
27+
registry.registerWarningRule(UseIsEmptyMatcherRule());
2428
// Should we register a fix for this rule? The only automatic fix I can
2529
// think of would be to delete the entire statement:
2630
// `expect(a, isNotNull);` or `expect(a, isNull);`.
@@ -30,8 +34,6 @@ class TestPackagePlugin extends Plugin {
3034
// or `String`.
3135
// * `expect(7, hasLength(3))` - should only use `hasLength` with `Iterable`
3236
// or `String`.
33-
// * `expect([].isEmpty, isFalse)` - should use `isEmpty` matcher.
34-
// * `expect([].isNotEmpty, isTrue)` - should use `isNotEmpty` matcher.
3537
// * `expect([].contains(7), isFalse)` - should use `contains` matcher.
3638
}
3739
}

‎pkgs/test_analyzer_plugin/lib/src/fixes.dart‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,10 @@ import 'utilities.dart';
1313

1414
class MoveBelowEnclosingTestCall extends ResolvedCorrectionProducer {
1515
static const _moveBelowEnclosingTestCallKind = FixKind(
16-
'dart.fix.moveBelowEnclosingTestCall',
17-
DartFixKindPriority.standard,
18-
"Move below the enclosing 'test' call");
16+
'dart.fix.moveBelowEnclosingTestCall',
17+
DartFixKindPriority.standard,
18+
"Move below the enclosing 'test' call",
19+
);
1920

2021
MoveBelowEnclosingTestCall({required super.context});
2122

@@ -52,7 +53,9 @@ class MoveBelowEnclosingTestCall extends ResolvedCorrectionProducer {
5253
// Move the source for `methodCall` wholsale to be just after `enclosingTestCall`.
5354
builder.addDeletion(range.deletionRange(methodCall));
5455
builder.addSimpleInsertion(
55-
enclosingTestCall.end, '$defaultEol$defaultEol$indent$source');
56+
enclosingTestCall.end,
57+
'$defaultEol$defaultEol$indent$source',
58+
);
5659
});
5760
}
5861
}

‎pkgs/test_analyzer_plugin/lib/src/rules/non_nullable_is_not_null_rule.dart‎

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,6 @@ import 'package:analyzer/error/error.dart';
1212

1313
import '../utilities.dart';
1414

15-
// TODO(srawlins): Several others; use same name or just different codes?
16-
// * `expect(null, isNotNull)` - always false
17-
// * `expect(null, isNull)` - always true
18-
// * `expect(7, isNull)` - always false
1915
class NonNullableIsNotNullRule extends MultiAnalysisRule {
2016
static const LintCode nonNullableIsNotNullCode = LintCode(
2117
'non_nullable_is_not_null',
@@ -30,18 +26,24 @@ class NonNullableIsNotNullRule extends MultiAnalysisRule {
3026
);
3127

3228
NonNullableIsNotNullRule()
33-
: super(
34-
name: 'non_nullable_is_not_null',
35-
description: "Non-nullable values will always pass an 'isNotNull' "
36-
"expectation and never pass an 'isNull' expectation.",
37-
);
29+
: super(
30+
name: 'non_nullable_is_not_null',
31+
description:
32+
"Non-nullable values will always pass an 'isNotNull' "
33+
"expectation and never pass an 'isNull' expectation.",
34+
);
3835

3936
@override
40-
List<LintCode> get diagnosticCodes => [nonNullableIsNotNullCode];
37+
List<LintCode> get diagnosticCodes => [
38+
nonNullableIsNotNullCode,
39+
nonNullableIsNullCode,
40+
];
4141

4242
@override
4343
void registerNodeProcessors(
44-
RuleVisitorRegistry registry, RuleContext context) {
44+
RuleVisitorRegistry registry,
45+
RuleContext context,
46+
) {
4547
var visitor = _Visitor(this, context.typeSystem);
4648
registry.addMethodInvocation(this, visitor);
4749
}
@@ -60,20 +62,25 @@ class _Visitor extends SimpleAstVisitor<void> {
6062
return;
6163
}
6264

63-
if (node.argumentList.arguments
64-
case [var actual, SimpleIdentifier matcher]) {
65+
if (node.argumentList.arguments case [
66+
var actual,
67+
SimpleIdentifier matcher,
68+
]) {
6569
var actualType = actual.staticType;
6670
if (actualType == null) return;
6771
if (typeSystem.isNonNullable(actualType)) {
6872
if (matcher.isNotNull) {
6973
// The actual value will always match this matcher.
70-
rule.reportAtNode(matcher,
71-
diagnosticCode:
72-
NonNullableIsNotNullRule.nonNullableIsNotNullCode);
74+
rule.reportAtNode(
75+
matcher,
76+
diagnosticCode: NonNullableIsNotNullRule.nonNullableIsNotNullCode,
77+
);
7378
} else if (matcher.isNull) {
7479
// The actual value will never match this matcher.
75-
rule.reportAtNode(matcher,
76-
diagnosticCode: NonNullableIsNotNullRule.nonNullableIsNullCode);
80+
rule.reportAtNode(
81+
matcher,
82+
diagnosticCode: NonNullableIsNotNullRule.nonNullableIsNullCode,
83+
);
7784
}
7885
}
7986
}

‎pkgs/test_analyzer_plugin/lib/src/rules/test_in_test_rule.dart‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,19 +19,21 @@ class TestInTestRule extends AnalysisRule {
1919
);
2020

2121
TestInTestRule()
22-
: super(
23-
name: 'test_in_test',
24-
description:
25-
'Tests and groups declared inside of a test are not properly '
26-
'registered in the test framework.',
27-
);
22+
: super(
23+
name: 'test_in_test',
24+
description:
25+
'Tests and groups declared inside of a test are not properly '
26+
'registered in the test framework.',
27+
);
2828

2929
@override
3030
LintCode get diagnosticCode => code;
3131

3232
@override
3333
void registerNodeProcessors(
34-
RuleVisitorRegistry registry, RuleContext context) {
34+
RuleVisitorRegistry registry,
35+
RuleContext context,
36+
) {
3537
var visitor = _Visitor(this);
3638
registry.addMethodInvocation(this, visitor);
3739
}

‎pkgs/test_analyzer_plugin/lib/src/rules/use_is_empty_matcher.dart‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
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+
5+
import 'package:analyzer/analysis_rule/analysis_rule.dart';
6+
import 'package:analyzer/analysis_rule/rule_context.dart';
7+
import 'package:analyzer/analysis_rule/rule_visitor_registry.dart';
8+
import 'package:analyzer/dart/ast/ast.dart';
9+
import 'package:analyzer/dart/ast/visitor.dart';
10+
import 'package:analyzer/dart/element/type_system.dart';
11+
import 'package:analyzer/error/error.dart';
12+
13+
import '../utilities.dart';
14+
15+
class UseIsEmptyMatcherRule extends MultiAnalysisRule {
16+
static const LintCode useIsEmptyMatcherCode = LintCode(
17+
'use_is_empty_matcher',
18+
"Use the 'isEmpty' matcher.",
19+
);
20+
21+
static const LintCode useIsNotEmptyMatcherCode = LintCode(
22+
'use_is_not_empty_matcher',
23+
"Use the 'isNotEmpty' matcher.",
24+
);
25+
26+
UseIsEmptyMatcherRule()
27+
: super(
28+
name: 'use_is_empty_matcher',
29+
description: "Use the built-in 'isEmpty' and 'isNotEmpty' matchers.",
30+
);
31+
32+
@override
33+
List<LintCode> get diagnosticCodes => [
34+
useIsEmptyMatcherCode,
35+
useIsNotEmptyMatcherCode,
36+
];
37+
38+
@override
39+
void registerNodeProcessors(
40+
RuleVisitorRegistry registry,
41+
RuleContext context,
42+
) {
43+
var visitor = _Visitor(this, context.typeSystem);
44+
registry.addMethodInvocation(this, visitor);
45+
}
46+
}
47+
48+
class _Visitor extends SimpleAstVisitor<void> {
49+
final MultiAnalysisRule rule;
50+
51+
final TypeSystem typeSystem;
52+
53+
_Visitor(this.rule, this.typeSystem);
54+
55+
@override
56+
void visitMethodInvocation(MethodInvocation node) {
57+
if (!node.methodName.isExpect) return;
58+
59+
var arguments = node.argumentList.arguments;
60+
if (arguments.isEmpty || arguments.length > 2) return;
61+
var [actual, matcher] = arguments;
62+
63+
bool actualIsIsEmpty;
64+
if (actual is PrefixedIdentifier) {
65+
if (actual.identifier.name == 'isEmpty') {
66+
actualIsIsEmpty = true;
67+
} else if (actual.identifier.name == 'isNotEmpty') {
68+
actualIsIsEmpty = false;
69+
} else {
70+
return;
71+
}
72+
} else if (actual is PropertyAccess) {
73+
if (actual.propertyName.name == 'isEmpty') {
74+
actualIsIsEmpty = true;
75+
} else if (actual.propertyName.name == 'isNotEmpty') {
76+
actualIsIsEmpty = false;
77+
} else {
78+
return;
79+
}
80+
} else {
81+
return;
82+
}
83+
84+
bool matcherValue;
85+
if (matcher is BooleanLiteral) {
86+
matcherValue = matcher.value;
87+
} else if (matcher is SimpleIdentifier && matcher.isIsFalse) {
88+
matcherValue = false;
89+
} else if (matcher is SimpleIdentifier && matcher.isIsTrue) {
90+
matcherValue = true;
91+
} else {
92+
return;
93+
}
94+
95+
if (actualIsIsEmpty == matcherValue) {
96+
// Either `expect(a.isEmpty, isTrue|true)` or
97+
// `expect(a.isNotEmpty, isFalse|false)`.
98+
rule.reportAtNode(
99+
matcher,
100+
diagnosticCode: UseIsEmptyMatcherRule.useIsEmptyMatcherCode,
101+
);
102+
} else {
103+
// Either `expect(a.isEmpty, isFalse|false)` or
104+
// `expect(a.isNotEmpty, isTrue|true)`.
105+
rule.reportAtNode(
106+
matcher,
107+
diagnosticCode: UseIsEmptyMatcherRule.useIsNotEmptyMatcherCode,
108+
);
109+
}
110+
}
111+
}

‎pkgs/test_analyzer_plugin/lib/src/utilities.dart‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,24 @@ extension SimpleIdentifierExtension on SimpleIdentifier {
4444
return element.library?.uri.path.startsWith('matcher/') ?? false;
4545
}
4646

47+
/// Whether this identifier represents the 'isFalse' matcher from the
48+
/// 'matcher' package.
49+
bool get isIsFalse {
50+
final element = this.element;
51+
if (element == null) return false;
52+
if (element.name != 'isFalse') return false;
53+
return element.library?.uri.path.startsWith('matcher/') ?? false;
54+
}
55+
56+
/// Whether this identifier represents the 'isTrue' matcher from the
57+
/// 'matcher' package.
58+
bool get isIsTrue {
59+
final element = this.element;
60+
if (element == null) return false;
61+
if (element.name != 'isTrue') return false;
62+
return element.library?.uri.path.startsWith('matcher/') ?? false;
63+
}
64+
4765
/// Whether this identifier represents the 'isNotNull' constant from the
4866
/// 'matcher' package.
4967
bool get isNotNull {

‎pkgs/test_analyzer_plugin/pubspec.yaml‎

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,16 @@
11
name: test_analyzer_plugin
22
description: An analyzer plugin to report improper usage of the test package.
3-
version: 1.0.0
4-
publish_to: none
3+
version: 0.1.0
54

65
environment:
7-
sdk: '>=3.6.0 <4.0.0'
6+
sdk: ^3.10.0
87

98
dependencies:
109
analysis_server_plugin: ^0.3.0
1110
analyzer: ^8.4.0
1211
analyzer_plugin: ^0.13.5
1312

1413
dev_dependencies:
15-
analyzer_testing: ^0.1.0
14+
analyzer_testing: ^0.1.2
1615
test: any
1716
test_reflective_loader: any
18-
19-
dependency_overrides:
20-
# _fe_analyzer_shared:
21-
# path: /Users/srawlins/code/dart-sdk/sdk/pkg/_fe_analyzer_shared
22-
# analysis_server_plugin:
23-
# path: /Users/srawlins/code/analysis_server_plugin
24-
# analyzer:
25-
# path: /Users/srawlins/code/dart-sdk/sdk/pkg/analyzer
26-
# analyzer_plugin:
27-
# path: /Users/srawlins/code/dart-sdk/sdk/pkg/analyzer_plugin
28-
# analyzer_testing:
29-
# path: /Users/srawlins/code/dart-sdk/sdk/pkg/analyzer_testing

‎pkgs/test_analyzer_plugin/test/rules/non_nullable_is_not_null_test.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

0 commit comments

Comments
 (0)