From e04eec7977725f73cf21ef2e3312822b97846a2f Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Tue, 25 Feb 2025 11:19:20 -0800 Subject: [PATCH 01/13] Introduce an analyzer plugin for the test package. --- pkgs/test_analyzer_plugin/README.md | 13 +++++ pkgs/test_analyzer_plugin/lib/main.dart | 20 +++++++ pkgs/test_analyzer_plugin/lib/src/fixes.dart | 58 +++++++++++++++++++ pkgs/test_analyzer_plugin/lib/src/rules.dart | 53 +++++++++++++++++ .../lib/src/utilities.dart | 37 ++++++++++++ pkgs/test_analyzer_plugin/pubspec.yaml | 12 ++++ 6 files changed, 193 insertions(+) create mode 100644 pkgs/test_analyzer_plugin/README.md create mode 100644 pkgs/test_analyzer_plugin/lib/main.dart create mode 100644 pkgs/test_analyzer_plugin/lib/src/fixes.dart create mode 100644 pkgs/test_analyzer_plugin/lib/src/rules.dart create mode 100644 pkgs/test_analyzer_plugin/lib/src/utilities.dart create mode 100644 pkgs/test_analyzer_plugin/pubspec.yaml diff --git a/pkgs/test_analyzer_plugin/README.md b/pkgs/test_analyzer_plugin/README.md new file mode 100644 index 000000000..1f157181a --- /dev/null +++ b/pkgs/test_analyzer_plugin/README.md @@ -0,0 +1,13 @@ +# test_analyzer_plugin + +This package is an analyzer plugin that provides additional static analysis for +usage of the test package. + +This analyzer plugin provides the following additional analysis: + +* Report a warning when a `test` or a `group` is declared inside a `test` + declaration. This can _sometimes_ be detected at runtime. This warning is + reported statically. + +* Offer a quick fix in the IDE for the above warning, which moves the violating + `test` or `group` declaration below the containing `test` declaration. \ No newline at end of file diff --git a/pkgs/test_analyzer_plugin/lib/main.dart b/pkgs/test_analyzer_plugin/lib/main.dart new file mode 100644 index 000000000..a7445cf0b --- /dev/null +++ b/pkgs/test_analyzer_plugin/lib/main.dart @@ -0,0 +1,20 @@ +// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:analysis_server_plugin/plugin.dart'; +import 'package:analysis_server_plugin/registry.dart'; + +import 'src/fixes.dart'; +import 'src/rules.dart'; + +final plugin = TestPackagePlugin(); + +class TestPackagePlugin extends Plugin { + @override + void register(PluginRegistry registry) { + registry.registerWarningRule(TestInTestRule()); + registry.registerFixForRule( + TestInTestRule.code, MoveBelowEnclosingTestCall.new); + } +} diff --git a/pkgs/test_analyzer_plugin/lib/src/fixes.dart b/pkgs/test_analyzer_plugin/lib/src/fixes.dart new file mode 100644 index 000000000..98af4aa3e --- /dev/null +++ b/pkgs/test_analyzer_plugin/lib/src/fixes.dart @@ -0,0 +1,58 @@ +// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:analysis_server_plugin/edit/dart/correction_producer.dart'; +import 'package:analysis_server_plugin/edit/dart/dart_fix_kind_priority.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart'; +import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; +import 'package:analyzer_plugin/utilities/range_factory.dart'; + +import 'utilities.dart'; + +class MoveBelowEnclosingTestCall extends ResolvedCorrectionProducer { + static const _wrapInQuotesKind = FixKind( + 'dart.fix.moveBelowEnclosingTestCall', + DartFixKindPriority.standard, + "Move below the enclosing 'test' call"); + + MoveBelowEnclosingTestCall({required super.context}); + + @override + CorrectionApplicability get applicability => + // This fix may break code by moving references to variables away from the + // scope in which they are declared. + CorrectionApplicability.singleLocation; + + @override + FixKind get fixKind => _wrapInQuotesKind; + + @override + Future compute(ChangeBuilder builder) async { + var methodCall = node; + if (methodCall is! MethodInvocation) return; + AstNode? enclosingTestCall = findEnclosingTestCall(methodCall); + if (enclosingTestCall == null) return; + + if (enclosingTestCall.parent is ExpressionStatement) { + // Move the 'test' call to below the outer 'test' call _statement_. + enclosingTestCall = enclosingTestCall.parent!; + } + + if (methodCall.parent is ExpressionStatement) { + // Move the whole statement (don't leave the semicolon dangling). + methodCall = methodCall.parent!; + } + + await builder.addDartFileEdit(file, (builder) { + var indent = utils.getLinePrefix(enclosingTestCall!.offset); + var source = utils.getRangeText(range.node(methodCall)); + + // Move the source for `methodCall` wholsale to be just after `enclosingTestCall`. + builder.addDeletion(range.deletionRange(methodCall)); + builder.addSimpleInsertion( + enclosingTestCall.end, '$eol$eol$indent$source'); + }); + } +} diff --git a/pkgs/test_analyzer_plugin/lib/src/rules.dart b/pkgs/test_analyzer_plugin/lib/src/rules.dart new file mode 100644 index 000000000..bf4634ec0 --- /dev/null +++ b/pkgs/test_analyzer_plugin/lib/src/rules.dart @@ -0,0 +1,53 @@ +// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/src/dart/error/lint_codes.dart'; +import 'package:analyzer/src/lint/linter.dart'; + +import 'utilities.dart'; + +class TestInTestRule extends AnalysisRule { + static const LintCode code = LintCode( + 'test_in_test', + "Do not declare a 'test' or a 'group' inside a 'test'", + correctionMessage: "Try moving 'test' or 'group' outside of 'test'", + ); + + TestInTestRule() + : super( + name: 'test_in_test', + description: + 'Tests and groups declared inside of a test are not properly ' + 'registered in the test framework.', + ); + + @override + LintCode get lintCode => code; + + @override + void registerNodeProcessors( + NodeLintRegistry registry, LinterContext context) { + var visitor = _Visitor(this); + registry.addMethodInvocation(this, visitor); + } +} + +class _Visitor extends SimpleAstVisitor { + final AnalysisRule rule; + + _Visitor(this.rule); + + @override + void visitMethodInvocation(MethodInvocation node) { + if (!node.methodName.isTest && !node.methodName.isGroup) { + return; + } + var enclosingTestCall = findEnclosingTestCall(node); + if (enclosingTestCall != null) { + rule.reportLint(node); + } + } +} diff --git a/pkgs/test_analyzer_plugin/lib/src/utilities.dart b/pkgs/test_analyzer_plugin/lib/src/utilities.dart new file mode 100644 index 000000000..0de22044c --- /dev/null +++ b/pkgs/test_analyzer_plugin/lib/src/utilities.dart @@ -0,0 +1,37 @@ +// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:analyzer/dart/ast/ast.dart'; + +/// Finds an enclosing call to the 'test' function, if there is one. +MethodInvocation? findEnclosingTestCall(MethodInvocation node) { + var ancestor = node.parent?.thisOrAncestorOfType(); + while (ancestor != null) { + if (ancestor.methodName.isTest) { + return ancestor; + } + ancestor = ancestor.parent?.thisOrAncestorOfType(); + } + return null; +} + +extension SimpleIdentifierExtension on SimpleIdentifier { + /// Whether this identifier represents the 'test' function from the + /// 'test_core' package. + bool get isTest { + final element = this.element; + if (element == null) return false; + if (element.name3 != 'test') return false; + return element.library2?.uri.path.startsWith('test_core/') ?? false; + } + + /// Whether this identifier represents the 'group' function from the + /// 'test_core' package. + bool get isGroup { + final element = this.element; + if (element == null) return false; + if (element.name3 != 'group') return false; + return element.library2?.uri.path.startsWith('test_core/') ?? false; + } +} diff --git a/pkgs/test_analyzer_plugin/pubspec.yaml b/pkgs/test_analyzer_plugin/pubspec.yaml new file mode 100644 index 000000000..9a434a305 --- /dev/null +++ b/pkgs/test_analyzer_plugin/pubspec.yaml @@ -0,0 +1,12 @@ +name: test_analyzer_plugin +description: An analyzer plugin to report improper usage of the test package. +version: 1.0.0 +publish_to: none + +environment: + sdk: '>=3.6.0 <4.0.0' + +dependencies: + analysis_server_plugin: any + analyzer: ^7.2.0 + From 6cc66f1fcb3e3f4c43fb836818cc84d6ce1d63d5 Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Tue, 4 Mar 2025 10:56:59 -0800 Subject: [PATCH 02/13] feedback --- pkgs/test_analyzer_plugin/lib/src/fixes.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/test_analyzer_plugin/lib/src/fixes.dart b/pkgs/test_analyzer_plugin/lib/src/fixes.dart index 98af4aa3e..87d35f55b 100644 --- a/pkgs/test_analyzer_plugin/lib/src/fixes.dart +++ b/pkgs/test_analyzer_plugin/lib/src/fixes.dart @@ -12,7 +12,7 @@ import 'package:analyzer_plugin/utilities/range_factory.dart'; import 'utilities.dart'; class MoveBelowEnclosingTestCall extends ResolvedCorrectionProducer { - static const _wrapInQuotesKind = FixKind( + static const _moveBelowEnclosingTestCallKind = FixKind( 'dart.fix.moveBelowEnclosingTestCall', DartFixKindPriority.standard, "Move below the enclosing 'test' call"); @@ -26,7 +26,7 @@ class MoveBelowEnclosingTestCall extends ResolvedCorrectionProducer { CorrectionApplicability.singleLocation; @override - FixKind get fixKind => _wrapInQuotesKind; + FixKind get fixKind => _moveBelowEnclosingTestCallKind; @override Future compute(ChangeBuilder builder) async { From eaedfb3e02aba61dbcf2accd2882cf74bb051cde Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Tue, 8 Jul 2025 12:13:24 -0700 Subject: [PATCH 03/13] Add another rule: non_nullable_is_not_null_rule --- pkgs/test_analyzer_plugin/lib/main.dart | 17 +++- .../rules/non_nullable_is_not_null_rule.dart | 81 ++++++++++++++++++ .../test_in_test_rule.dart} | 14 ++-- .../lib/src/utilities.dart | 31 ++++++- pkgs/test_analyzer_plugin/pubspec.yaml | 17 ++++ .../rules/non_nullable_is_not_null_test.dart | 84 +++++++++++++++++++ 6 files changed, 235 insertions(+), 9 deletions(-) create mode 100644 pkgs/test_analyzer_plugin/lib/src/rules/non_nullable_is_not_null_rule.dart rename pkgs/test_analyzer_plugin/lib/src/{rules.dart => rules/test_in_test_rule.dart} (76%) create mode 100644 pkgs/test_analyzer_plugin/test/rules/non_nullable_is_not_null_test.dart diff --git a/pkgs/test_analyzer_plugin/lib/main.dart b/pkgs/test_analyzer_plugin/lib/main.dart index a7445cf0b..952ea3575 100644 --- a/pkgs/test_analyzer_plugin/lib/main.dart +++ b/pkgs/test_analyzer_plugin/lib/main.dart @@ -6,7 +6,8 @@ import 'package:analysis_server_plugin/plugin.dart'; import 'package:analysis_server_plugin/registry.dart'; import 'src/fixes.dart'; -import 'src/rules.dart'; +import 'src/rules/non_nullable_is_not_null_rule.dart'; +import 'src/rules/test_in_test_rule.dart'; final plugin = TestPackagePlugin(); @@ -16,5 +17,19 @@ class TestPackagePlugin extends Plugin { registry.registerWarningRule(TestInTestRule()); registry.registerFixForRule( TestInTestRule.code, MoveBelowEnclosingTestCall.new); + + registry.registerWarningRule(NonNullableIsNotNullRule()); + // Should we register a fix for this rule? The only automatic fix I can + // think of would be to delete the entire statement: + // `expect(a, isNotNull);` or `expect(a, isNull);`. + + // TODO(srawlins): More rules to catch: + // * `expect(7, contains(6))` - should only use `hasLength` with `Iterable` + // or `String`. + // * `expect(7, hasLength(3))` - should only use `hasLength` with `Iterable` + // or `String`. + // * `expect([].isEmpty, isFalse)` - should use `isEmpty` matcher. + // * `expect([].isNotEmpty, isTrue)` - should use `isNotEmpty` matcher. + // * `expect([].contains(7), isFalse)` - should use `contains` matcher. } } diff --git a/pkgs/test_analyzer_plugin/lib/src/rules/non_nullable_is_not_null_rule.dart b/pkgs/test_analyzer_plugin/lib/src/rules/non_nullable_is_not_null_rule.dart new file mode 100644 index 000000000..ddd6d6a49 --- /dev/null +++ b/pkgs/test_analyzer_plugin/lib/src/rules/non_nullable_is_not_null_rule.dart @@ -0,0 +1,81 @@ +// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/dart/element/type_system.dart'; +import 'package:analyzer/error/error.dart'; + +import '../utilities.dart'; + +// TODO(srawlins): Several others; use same name or just different codes? +// * `expect(null, isNotNull)` - always false +// * `expect(null, isNull)` - always true +// * `expect(7, isNull)` - always false +class NonNullableIsNotNullRule extends MultiAnalysisRule { + static const LintCode nonNullableIsNotNullCode = LintCode( + 'non_nullable_is_not_null', + 'Do not check whether a non-nullable value isNotNull', + correctionMessage: 'Try changing the expectation, or removing it', + ); + + static const LintCode nonNullableIsNullCode = LintCode( + 'non_nullable_is_null', + 'Do not check whether a non-nullable value isNull', + correctionMessage: 'Try changing the expectation, or removing it', + ); + + NonNullableIsNotNullRule() + : super( + name: 'non_nullable_is_not_null', + description: "Non-nullable values will always pass an 'isNotNull' " + "expectation and never pass an 'isNull' expectation.", + ); + + @override + List get diagnosticCodes => [nonNullableIsNotNullCode]; + + @override + void registerNodeProcessors( + RuleVisitorRegistry registry, RuleContext context) { + var visitor = _Visitor(this, context.typeSystem); + registry.addMethodInvocation(this, visitor); + } +} + +class _Visitor extends SimpleAstVisitor { + final MultiAnalysisRule rule; + + final TypeSystem typeSystem; + + _Visitor(this.rule, this.typeSystem); + + @override + void visitMethodInvocation(MethodInvocation node) { + if (!node.methodName.isExpect) { + return; + } + + if (node.argumentList.arguments + case [var actual, SimpleIdentifier matcher]) { + var actualType = actual.staticType; + if (actualType == null) return; + if (typeSystem.isNonNullable(actualType)) { + if (matcher.isNotNull) { + // The actual value will always match this matcher. + rule.reportAtNode(matcher, + diagnosticCode: + NonNullableIsNotNullRule.nonNullableIsNotNullCode); + } else if (matcher.isNull) { + // The actual value will never match this matcher. + rule.reportAtNode(matcher, + diagnosticCode: NonNullableIsNotNullRule.nonNullableIsNullCode); + } + } + } + } +} diff --git a/pkgs/test_analyzer_plugin/lib/src/rules.dart b/pkgs/test_analyzer_plugin/lib/src/rules/test_in_test_rule.dart similarity index 76% rename from pkgs/test_analyzer_plugin/lib/src/rules.dart rename to pkgs/test_analyzer_plugin/lib/src/rules/test_in_test_rule.dart index bf4634ec0..9c3ae89cf 100644 --- a/pkgs/test_analyzer_plugin/lib/src/rules.dart +++ b/pkgs/test_analyzer_plugin/lib/src/rules/test_in_test_rule.dart @@ -2,12 +2,14 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; -import 'package:analyzer/src/dart/error/lint_codes.dart'; -import 'package:analyzer/src/lint/linter.dart'; +import 'package:analyzer/error/error.dart'; -import 'utilities.dart'; +import '../utilities.dart'; class TestInTestRule extends AnalysisRule { static const LintCode code = LintCode( @@ -25,11 +27,11 @@ class TestInTestRule extends AnalysisRule { ); @override - LintCode get lintCode => code; + LintCode get diagnosticCode => code; @override void registerNodeProcessors( - NodeLintRegistry registry, LinterContext context) { + RuleVisitorRegistry registry, RuleContext context) { var visitor = _Visitor(this); registry.addMethodInvocation(this, visitor); } @@ -47,7 +49,7 @@ class _Visitor extends SimpleAstVisitor { } var enclosingTestCall = findEnclosingTestCall(node); if (enclosingTestCall != null) { - rule.reportLint(node); + rule.reportAtNode(node); } } } diff --git a/pkgs/test_analyzer_plugin/lib/src/utilities.dart b/pkgs/test_analyzer_plugin/lib/src/utilities.dart index 0de22044c..5226a4949 100644 --- a/pkgs/test_analyzer_plugin/lib/src/utilities.dart +++ b/pkgs/test_analyzer_plugin/lib/src/utilities.dart @@ -23,7 +23,7 @@ extension SimpleIdentifierExtension on SimpleIdentifier { final element = this.element; if (element == null) return false; if (element.name3 != 'test') return false; - return element.library2?.uri.path.startsWith('test_core/') ?? false; + return element.library?.uri.path.startsWith('test_core/') ?? false; } /// Whether this identifier represents the 'group' function from the @@ -32,6 +32,33 @@ extension SimpleIdentifierExtension on SimpleIdentifier { final element = this.element; if (element == null) return false; if (element.name3 != 'group') return false; - return element.library2?.uri.path.startsWith('test_core/') ?? false; + return element.library?.uri.path.startsWith('test_core/') ?? false; + } + + /// Whether this identifier represents the 'expect' function from the + /// 'matcher' package. + bool get isExpect { + final element = this.element; + if (element == null) return false; + if (element.name3 != 'expect') return false; + return element.library?.uri.path.startsWith('matcher/') ?? false; + } + + /// Whether this identifier represents the 'isNotNull' constant from the + /// 'matcher' package. + bool get isNotNull { + final element = this.element; + if (element == null) return false; + if (element.name3 != 'isNotNull') return false; + return element.library?.uri.path.startsWith('matcher/') ?? false; + } + + /// Whether this identifier represents the 'isNull' constant from the + /// 'matcher' package. + bool get isNull { + final element = this.element; + if (element == null) return false; + if (element.name3 != 'isNull') return false; + return element.library?.uri.path.startsWith('matcher/') ?? false; } } diff --git a/pkgs/test_analyzer_plugin/pubspec.yaml b/pkgs/test_analyzer_plugin/pubspec.yaml index 9a434a305..69e1131bf 100644 --- a/pkgs/test_analyzer_plugin/pubspec.yaml +++ b/pkgs/test_analyzer_plugin/pubspec.yaml @@ -9,4 +9,21 @@ environment: dependencies: analysis_server_plugin: any analyzer: ^7.2.0 + analyzer_plugin: + analyzer_testing: +dev_dependencies: + test: any + test_reflective_loader: any + +dependency_overrides: + _fe_analyzer_shared: + path: /Users/srawlins/code/dart-sdk/sdk/pkg/_fe_analyzer_shared + analysis_server_plugin: + path: /Users/srawlins/code/dart-sdk/sdk/pkg/analysis_server_plugin + analyzer: + path: /Users/srawlins/code/dart-sdk/sdk/pkg/analyzer + analyzer_plugin: + path: /Users/srawlins/code/dart-sdk/sdk/pkg/analyzer_plugin + analyzer_testing: + path: /Users/srawlins/code/dart-sdk/sdk/pkg/analyzer_testing diff --git a/pkgs/test_analyzer_plugin/test/rules/non_nullable_is_not_null_test.dart b/pkgs/test_analyzer_plugin/test/rules/non_nullable_is_not_null_test.dart new file mode 100644 index 000000000..98828b171 --- /dev/null +++ b/pkgs/test_analyzer_plugin/test/rules/non_nullable_is_not_null_test.dart @@ -0,0 +1,84 @@ +// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +// ignore_for_file: non_constant_identifier_names + +import 'package:analyzer/src/lint/registry.dart'; +import 'package:analyzer/utilities/package_config_file_builder.dart'; +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:test_analyzer_plugin/src/rules/non_nullable_is_not_null_rule.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(NonNullableIsNotNullTest); + }); +} + +@reflectiveTest +class NonNullableIsNotNullTest extends AnalysisRuleTest { + @override + void setUp() { + Registry.ruleRegistry.registerLintRule(NonNullableIsNotNullRule()); + + super.setUp(); + + var matcherPath = '/packages/matcher'; + newFile('$matcherPath/lib/matcher.dart', ''' +void expect(dynamic actual, dynamic matcher) {} + +const isNotNull = 0; +const isNull = 0; +'''); + writeTestPackageConfig( + PackageConfigFileBuilder() + ..add(name: 'matcher', rootPath: convertPath(matcherPath)), + ); + } + + @override + String get analysisRule => 'non_nullable_is_not_null'; + + void test_nullableValue_isNotNullMatcher() async { + await assertNoDiagnostics(r''' +import 'package:matcher/matcher.dart'; +void f(String? p) { + expect(p, isNotNull); +} +'''); + } + + void test_nonNullableValue_isNotNullMatcher() async { + await assertDiagnostics( + r''' +import 'package:matcher/matcher.dart'; +void f() { + expect(123, isNotNull); +} +''', + [lint(64, 9)], + ); + } + + void test_nullableValue_isNullMatcher() async { + await assertNoDiagnostics(r''' +import 'package:matcher/matcher.dart'; +void f(String? p) { + expect(p, isNull); +} +'''); + } + + void test_nonNullableValue_isNullMatcher() async { + await assertDiagnostics( + r''' +import 'package:matcher/matcher.dart'; +void f() { + expect(123, isNull); +} +''', + [lint(64, 6, name: 'non_nullable_is_null')], + ); + } +} From f128a0d908c03b22140cff5547bd94f0d16b0012 Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Fri, 15 Aug 2025 11:04:48 -0700 Subject: [PATCH 04/13] Bump to analysis_server_plugin 0.2.2 --- pkgs/test_analyzer_plugin/lib/src/fixes.dart | 2 +- .../lib/src/utilities.dart | 10 +++---- pkgs/test_analyzer_plugin/pubspec.yaml | 30 +++++++++---------- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/pkgs/test_analyzer_plugin/lib/src/fixes.dart b/pkgs/test_analyzer_plugin/lib/src/fixes.dart index 87d35f55b..a470c6404 100644 --- a/pkgs/test_analyzer_plugin/lib/src/fixes.dart +++ b/pkgs/test_analyzer_plugin/lib/src/fixes.dart @@ -52,7 +52,7 @@ class MoveBelowEnclosingTestCall extends ResolvedCorrectionProducer { // Move the source for `methodCall` wholsale to be just after `enclosingTestCall`. builder.addDeletion(range.deletionRange(methodCall)); builder.addSimpleInsertion( - enclosingTestCall.end, '$eol$eol$indent$source'); + enclosingTestCall.end, '$defaultEol$defaultEol$indent$source'); }); } } diff --git a/pkgs/test_analyzer_plugin/lib/src/utilities.dart b/pkgs/test_analyzer_plugin/lib/src/utilities.dart index 5226a4949..3ff142b3d 100644 --- a/pkgs/test_analyzer_plugin/lib/src/utilities.dart +++ b/pkgs/test_analyzer_plugin/lib/src/utilities.dart @@ -22,7 +22,7 @@ extension SimpleIdentifierExtension on SimpleIdentifier { bool get isTest { final element = this.element; if (element == null) return false; - if (element.name3 != 'test') return false; + if (element.name != 'test') return false; return element.library?.uri.path.startsWith('test_core/') ?? false; } @@ -31,7 +31,7 @@ extension SimpleIdentifierExtension on SimpleIdentifier { bool get isGroup { final element = this.element; if (element == null) return false; - if (element.name3 != 'group') return false; + if (element.name != 'group') return false; return element.library?.uri.path.startsWith('test_core/') ?? false; } @@ -40,7 +40,7 @@ extension SimpleIdentifierExtension on SimpleIdentifier { bool get isExpect { final element = this.element; if (element == null) return false; - if (element.name3 != 'expect') return false; + if (element.name != 'expect') return false; return element.library?.uri.path.startsWith('matcher/') ?? false; } @@ -49,7 +49,7 @@ extension SimpleIdentifierExtension on SimpleIdentifier { bool get isNotNull { final element = this.element; if (element == null) return false; - if (element.name3 != 'isNotNull') return false; + if (element.name != 'isNotNull') return false; return element.library?.uri.path.startsWith('matcher/') ?? false; } @@ -58,7 +58,7 @@ extension SimpleIdentifierExtension on SimpleIdentifier { bool get isNull { final element = this.element; if (element == null) return false; - if (element.name3 != 'isNull') return false; + if (element.name != 'isNull') return false; return element.library?.uri.path.startsWith('matcher/') ?? false; } } diff --git a/pkgs/test_analyzer_plugin/pubspec.yaml b/pkgs/test_analyzer_plugin/pubspec.yaml index 69e1131bf..9c7051546 100644 --- a/pkgs/test_analyzer_plugin/pubspec.yaml +++ b/pkgs/test_analyzer_plugin/pubspec.yaml @@ -7,23 +7,23 @@ environment: sdk: '>=3.6.0 <4.0.0' dependencies: - analysis_server_plugin: any - analyzer: ^7.2.0 - analyzer_plugin: - analyzer_testing: + analysis_server_plugin: ^0.2.2 + analyzer: ^8.0.0 + analyzer_plugin: ^0.13.6 + analyzer_testing: ^0.1.2 dev_dependencies: test: any test_reflective_loader: any -dependency_overrides: - _fe_analyzer_shared: - path: /Users/srawlins/code/dart-sdk/sdk/pkg/_fe_analyzer_shared - analysis_server_plugin: - path: /Users/srawlins/code/dart-sdk/sdk/pkg/analysis_server_plugin - analyzer: - path: /Users/srawlins/code/dart-sdk/sdk/pkg/analyzer - analyzer_plugin: - path: /Users/srawlins/code/dart-sdk/sdk/pkg/analyzer_plugin - analyzer_testing: - path: /Users/srawlins/code/dart-sdk/sdk/pkg/analyzer_testing + #dependency_overrides: + # _fe_analyzer_shared: + # path: /Users/srawlins/code/dart-sdk/sdk/pkg/_fe_analyzer_shared + # analysis_server_plugin: + # path: /Users/srawlins/code/dart-sdk/sdk/pkg/analysis_server_plugin + # analyzer: + # path: /Users/srawlins/code/dart-sdk/sdk/pkg/analyzer + # analyzer_plugin: + # path: /Users/srawlins/code/dart-sdk/sdk/pkg/analyzer_plugin + # analyzer_testing: + # path: /Users/srawlins/code/dart-sdk/sdk/pkg/analyzer_testing From 54ab763ecb43dcf2b913cc885929e4b23fc074c9 Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Fri, 24 Oct 2025 12:26:54 -0700 Subject: [PATCH 05/13] Bump to work with analysis_server_plugin 0.3.0 --- pkgs/test_analyzer_plugin/lib/main.dart | 2 ++ pkgs/test_analyzer_plugin/pubspec.yaml | 12 ++++++------ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/pkgs/test_analyzer_plugin/lib/main.dart b/pkgs/test_analyzer_plugin/lib/main.dart index 952ea3575..1a9caa14b 100644 --- a/pkgs/test_analyzer_plugin/lib/main.dart +++ b/pkgs/test_analyzer_plugin/lib/main.dart @@ -12,6 +12,8 @@ import 'src/rules/test_in_test_rule.dart'; final plugin = TestPackagePlugin(); class TestPackagePlugin extends Plugin { + String get name => 'Test package plugin'; + @override void register(PluginRegistry registry) { registry.registerWarningRule(TestInTestRule()); diff --git a/pkgs/test_analyzer_plugin/pubspec.yaml b/pkgs/test_analyzer_plugin/pubspec.yaml index 9c7051546..337bac63b 100644 --- a/pkgs/test_analyzer_plugin/pubspec.yaml +++ b/pkgs/test_analyzer_plugin/pubspec.yaml @@ -7,20 +7,20 @@ environment: sdk: '>=3.6.0 <4.0.0' dependencies: - analysis_server_plugin: ^0.2.2 - analyzer: ^8.0.0 - analyzer_plugin: ^0.13.6 - analyzer_testing: ^0.1.2 + analysis_server_plugin: ^0.3.0 + analyzer: ^8.4.0 + analyzer_plugin: ^0.13.5 dev_dependencies: + analyzer_testing: ^0.1.0 test: any test_reflective_loader: any - #dependency_overrides: +dependency_overrides: # _fe_analyzer_shared: # path: /Users/srawlins/code/dart-sdk/sdk/pkg/_fe_analyzer_shared # analysis_server_plugin: - # path: /Users/srawlins/code/dart-sdk/sdk/pkg/analysis_server_plugin + # path: /Users/srawlins/code/analysis_server_plugin # analyzer: # path: /Users/srawlins/code/dart-sdk/sdk/pkg/analyzer # analyzer_plugin: From 2635b72e1ff9088f86284ae91232d36558212d8f Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Tue, 11 Nov 2025 22:06:58 -0800 Subject: [PATCH 06/13] bump to 3.10; add a rule --- pkgs/test_analyzer_plugin/README.md | 8 + pkgs/test_analyzer_plugin/lib/main.dart | 8 +- pkgs/test_analyzer_plugin/lib/src/fixes.dart | 11 +- .../rules/non_nullable_is_not_null_rule.dart | 43 ++--- .../lib/src/rules/test_in_test_rule.dart | 16 +- .../lib/src/rules/use_is_empty_matcher.dart | 111 +++++++++++++ .../lib/src/utilities.dart | 18 +++ pkgs/test_analyzer_plugin/pubspec.yaml | 19 +-- .../rules/non_nullable_is_not_null_test.dart | 2 +- .../test/rules/use_is_empty_matcher_test.dart | 152 ++++++++++++++++++ 10 files changed, 339 insertions(+), 49 deletions(-) create mode 100644 pkgs/test_analyzer_plugin/lib/src/rules/use_is_empty_matcher.dart create mode 100644 pkgs/test_analyzer_plugin/test/rules/use_is_empty_matcher_test.dart diff --git a/pkgs/test_analyzer_plugin/README.md b/pkgs/test_analyzer_plugin/README.md index 1f157181a..e0540b9a2 100644 --- a/pkgs/test_analyzer_plugin/README.md +++ b/pkgs/test_analyzer_plugin/README.md @@ -9,5 +9,13 @@ This analyzer plugin provides the following additional analysis: declaration. This can _sometimes_ be detected at runtime. This warning is reported statically. +* Report a warning when a non-nullable value is matched against `isNotNull` or + `isNull`. + +* Report a warning when an `expect` expectation of `true`, `false`, `isTrue`, + or `isFalse` is paired with a `.isEmpty` or `.isNotEmpty` property access on + an actual value. Instead, the `isEmpty` or `isNotEmpty` Matcher should be + used. + * Offer a quick fix in the IDE for the above warning, which moves the violating `test` or `group` declaration below the containing `test` declaration. \ No newline at end of file diff --git a/pkgs/test_analyzer_plugin/lib/main.dart b/pkgs/test_analyzer_plugin/lib/main.dart index 1a9caa14b..343e2c5f8 100644 --- a/pkgs/test_analyzer_plugin/lib/main.dart +++ b/pkgs/test_analyzer_plugin/lib/main.dart @@ -8,6 +8,7 @@ import 'package:analysis_server_plugin/registry.dart'; import 'src/fixes.dart'; import 'src/rules/non_nullable_is_not_null_rule.dart'; import 'src/rules/test_in_test_rule.dart'; +import 'src/rules/use_is_empty_matcher.dart'; final plugin = TestPackagePlugin(); @@ -18,9 +19,12 @@ class TestPackagePlugin extends Plugin { void register(PluginRegistry registry) { registry.registerWarningRule(TestInTestRule()); registry.registerFixForRule( - TestInTestRule.code, MoveBelowEnclosingTestCall.new); + TestInTestRule.code, + MoveBelowEnclosingTestCall.new, + ); registry.registerWarningRule(NonNullableIsNotNullRule()); + registry.registerWarningRule(UseIsEmptyMatcherRule()); // Should we register a fix for this rule? The only automatic fix I can // think of would be to delete the entire statement: // `expect(a, isNotNull);` or `expect(a, isNull);`. @@ -30,8 +34,6 @@ class TestPackagePlugin extends Plugin { // or `String`. // * `expect(7, hasLength(3))` - should only use `hasLength` with `Iterable` // or `String`. - // * `expect([].isEmpty, isFalse)` - should use `isEmpty` matcher. - // * `expect([].isNotEmpty, isTrue)` - should use `isNotEmpty` matcher. // * `expect([].contains(7), isFalse)` - should use `contains` matcher. } } diff --git a/pkgs/test_analyzer_plugin/lib/src/fixes.dart b/pkgs/test_analyzer_plugin/lib/src/fixes.dart index a470c6404..f2d36f8a1 100644 --- a/pkgs/test_analyzer_plugin/lib/src/fixes.dart +++ b/pkgs/test_analyzer_plugin/lib/src/fixes.dart @@ -13,9 +13,10 @@ import 'utilities.dart'; class MoveBelowEnclosingTestCall extends ResolvedCorrectionProducer { static const _moveBelowEnclosingTestCallKind = FixKind( - 'dart.fix.moveBelowEnclosingTestCall', - DartFixKindPriority.standard, - "Move below the enclosing 'test' call"); + 'dart.fix.moveBelowEnclosingTestCall', + DartFixKindPriority.standard, + "Move below the enclosing 'test' call", + ); MoveBelowEnclosingTestCall({required super.context}); @@ -52,7 +53,9 @@ class MoveBelowEnclosingTestCall extends ResolvedCorrectionProducer { // Move the source for `methodCall` wholsale to be just after `enclosingTestCall`. builder.addDeletion(range.deletionRange(methodCall)); builder.addSimpleInsertion( - enclosingTestCall.end, '$defaultEol$defaultEol$indent$source'); + enclosingTestCall.end, + '$defaultEol$defaultEol$indent$source', + ); }); } } diff --git a/pkgs/test_analyzer_plugin/lib/src/rules/non_nullable_is_not_null_rule.dart b/pkgs/test_analyzer_plugin/lib/src/rules/non_nullable_is_not_null_rule.dart index ddd6d6a49..4c826ae1e 100644 --- a/pkgs/test_analyzer_plugin/lib/src/rules/non_nullable_is_not_null_rule.dart +++ b/pkgs/test_analyzer_plugin/lib/src/rules/non_nullable_is_not_null_rule.dart @@ -12,10 +12,6 @@ import 'package:analyzer/error/error.dart'; import '../utilities.dart'; -// TODO(srawlins): Several others; use same name or just different codes? -// * `expect(null, isNotNull)` - always false -// * `expect(null, isNull)` - always true -// * `expect(7, isNull)` - always false class NonNullableIsNotNullRule extends MultiAnalysisRule { static const LintCode nonNullableIsNotNullCode = LintCode( 'non_nullable_is_not_null', @@ -30,18 +26,24 @@ class NonNullableIsNotNullRule extends MultiAnalysisRule { ); NonNullableIsNotNullRule() - : super( - name: 'non_nullable_is_not_null', - description: "Non-nullable values will always pass an 'isNotNull' " - "expectation and never pass an 'isNull' expectation.", - ); + : super( + name: 'non_nullable_is_not_null', + description: + "Non-nullable values will always pass an 'isNotNull' " + "expectation and never pass an 'isNull' expectation.", + ); @override - List get diagnosticCodes => [nonNullableIsNotNullCode]; + List get diagnosticCodes => [ + nonNullableIsNotNullCode, + nonNullableIsNullCode, + ]; @override void registerNodeProcessors( - RuleVisitorRegistry registry, RuleContext context) { + RuleVisitorRegistry registry, + RuleContext context, + ) { var visitor = _Visitor(this, context.typeSystem); registry.addMethodInvocation(this, visitor); } @@ -60,20 +62,25 @@ class _Visitor extends SimpleAstVisitor { return; } - if (node.argumentList.arguments - case [var actual, SimpleIdentifier matcher]) { + if (node.argumentList.arguments case [ + var actual, + SimpleIdentifier matcher, + ]) { var actualType = actual.staticType; if (actualType == null) return; if (typeSystem.isNonNullable(actualType)) { if (matcher.isNotNull) { // The actual value will always match this matcher. - rule.reportAtNode(matcher, - diagnosticCode: - NonNullableIsNotNullRule.nonNullableIsNotNullCode); + rule.reportAtNode( + matcher, + diagnosticCode: NonNullableIsNotNullRule.nonNullableIsNotNullCode, + ); } else if (matcher.isNull) { // The actual value will never match this matcher. - rule.reportAtNode(matcher, - diagnosticCode: NonNullableIsNotNullRule.nonNullableIsNullCode); + rule.reportAtNode( + matcher, + diagnosticCode: NonNullableIsNotNullRule.nonNullableIsNullCode, + ); } } } diff --git a/pkgs/test_analyzer_plugin/lib/src/rules/test_in_test_rule.dart b/pkgs/test_analyzer_plugin/lib/src/rules/test_in_test_rule.dart index 9c3ae89cf..05f639daa 100644 --- a/pkgs/test_analyzer_plugin/lib/src/rules/test_in_test_rule.dart +++ b/pkgs/test_analyzer_plugin/lib/src/rules/test_in_test_rule.dart @@ -19,19 +19,21 @@ class TestInTestRule extends AnalysisRule { ); TestInTestRule() - : super( - name: 'test_in_test', - description: - 'Tests and groups declared inside of a test are not properly ' - 'registered in the test framework.', - ); + : super( + name: 'test_in_test', + description: + 'Tests and groups declared inside of a test are not properly ' + 'registered in the test framework.', + ); @override LintCode get diagnosticCode => code; @override void registerNodeProcessors( - RuleVisitorRegistry registry, RuleContext context) { + RuleVisitorRegistry registry, + RuleContext context, + ) { var visitor = _Visitor(this); registry.addMethodInvocation(this, visitor); } diff --git a/pkgs/test_analyzer_plugin/lib/src/rules/use_is_empty_matcher.dart b/pkgs/test_analyzer_plugin/lib/src/rules/use_is_empty_matcher.dart new file mode 100644 index 000000000..99035e53c --- /dev/null +++ b/pkgs/test_analyzer_plugin/lib/src/rules/use_is_empty_matcher.dart @@ -0,0 +1,111 @@ +// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/dart/element/type_system.dart'; +import 'package:analyzer/error/error.dart'; + +import '../utilities.dart'; + +class UseIsEmptyMatcherRule extends MultiAnalysisRule { + static const LintCode useIsEmptyMatcherCode = LintCode( + 'use_is_empty_matcher', + "Use the 'isEmpty' matcher.", + ); + + static const LintCode useIsNotEmptyMatcherCode = LintCode( + 'use_is_not_empty_matcher', + "Use the 'isNotEmpty' matcher.", + ); + + UseIsEmptyMatcherRule() + : super( + name: 'use_is_empty_matcher', + description: "Use the built-in 'isEmpty' and 'isNotEmpty' matchers.", + ); + + @override + List get diagnosticCodes => [ + useIsEmptyMatcherCode, + useIsNotEmptyMatcherCode, + ]; + + @override + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, + ) { + var visitor = _Visitor(this, context.typeSystem); + registry.addMethodInvocation(this, visitor); + } +} + +class _Visitor extends SimpleAstVisitor { + final MultiAnalysisRule rule; + + final TypeSystem typeSystem; + + _Visitor(this.rule, this.typeSystem); + + @override + void visitMethodInvocation(MethodInvocation node) { + if (!node.methodName.isExpect) return; + + var arguments = node.argumentList.arguments; + if (arguments.isEmpty || arguments.length > 2) return; + var [actual, matcher] = arguments; + + bool actualIsIsEmpty; + if (actual is PrefixedIdentifier) { + if (actual.identifier.name == 'isEmpty') { + actualIsIsEmpty = true; + } else if (actual.identifier.name == 'isNotEmpty') { + actualIsIsEmpty = false; + } else { + return; + } + } else if (actual is PropertyAccess) { + if (actual.propertyName.name == 'isEmpty') { + actualIsIsEmpty = true; + } else if (actual.propertyName.name == 'isNotEmpty') { + actualIsIsEmpty = false; + } else { + return; + } + } else { + return; + } + + bool matcherValue; + if (matcher is BooleanLiteral) { + matcherValue = matcher.value; + } else if (matcher is SimpleIdentifier && matcher.isIsFalse) { + matcherValue = false; + } else if (matcher is SimpleIdentifier && matcher.isIsTrue) { + matcherValue = true; + } else { + return; + } + + if (actualIsIsEmpty == matcherValue) { + // Either `expect(a.isEmpty, isTrue|true)` or + // `expect(a.isNotEmpty, isFalse|false)`. + rule.reportAtNode( + matcher, + diagnosticCode: UseIsEmptyMatcherRule.useIsEmptyMatcherCode, + ); + } else { + // Either `expect(a.isEmpty, isFalse|false)` or + // `expect(a.isNotEmpty, isTrue|true)`. + rule.reportAtNode( + matcher, + diagnosticCode: UseIsEmptyMatcherRule.useIsNotEmptyMatcherCode, + ); + } + } +} diff --git a/pkgs/test_analyzer_plugin/lib/src/utilities.dart b/pkgs/test_analyzer_plugin/lib/src/utilities.dart index 3ff142b3d..bb110d294 100644 --- a/pkgs/test_analyzer_plugin/lib/src/utilities.dart +++ b/pkgs/test_analyzer_plugin/lib/src/utilities.dart @@ -44,6 +44,24 @@ extension SimpleIdentifierExtension on SimpleIdentifier { return element.library?.uri.path.startsWith('matcher/') ?? false; } + /// Whether this identifier represents the 'isFalse' matcher from the + /// 'matcher' package. + bool get isIsFalse { + final element = this.element; + if (element == null) return false; + if (element.name != 'isFalse') return false; + return element.library?.uri.path.startsWith('matcher/') ?? false; + } + + /// Whether this identifier represents the 'isTrue' matcher from the + /// 'matcher' package. + bool get isIsTrue { + final element = this.element; + if (element == null) return false; + if (element.name != 'isTrue') return false; + return element.library?.uri.path.startsWith('matcher/') ?? false; + } + /// Whether this identifier represents the 'isNotNull' constant from the /// 'matcher' package. bool get isNotNull { diff --git a/pkgs/test_analyzer_plugin/pubspec.yaml b/pkgs/test_analyzer_plugin/pubspec.yaml index 337bac63b..a9a276c4d 100644 --- a/pkgs/test_analyzer_plugin/pubspec.yaml +++ b/pkgs/test_analyzer_plugin/pubspec.yaml @@ -1,10 +1,9 @@ name: test_analyzer_plugin description: An analyzer plugin to report improper usage of the test package. -version: 1.0.0 -publish_to: none +version: 0.1.0 environment: - sdk: '>=3.6.0 <4.0.0' + sdk: ^3.10.0 dependencies: analysis_server_plugin: ^0.3.0 @@ -12,18 +11,6 @@ dependencies: analyzer_plugin: ^0.13.5 dev_dependencies: - analyzer_testing: ^0.1.0 + analyzer_testing: ^0.1.2 test: any test_reflective_loader: any - -dependency_overrides: - # _fe_analyzer_shared: - # path: /Users/srawlins/code/dart-sdk/sdk/pkg/_fe_analyzer_shared - # analysis_server_plugin: - # path: /Users/srawlins/code/analysis_server_plugin - # analyzer: - # path: /Users/srawlins/code/dart-sdk/sdk/pkg/analyzer - # analyzer_plugin: - # path: /Users/srawlins/code/dart-sdk/sdk/pkg/analyzer_plugin - # analyzer_testing: - # path: /Users/srawlins/code/dart-sdk/sdk/pkg/analyzer_testing diff --git a/pkgs/test_analyzer_plugin/test/rules/non_nullable_is_not_null_test.dart b/pkgs/test_analyzer_plugin/test/rules/non_nullable_is_not_null_test.dart index 98828b171..abac2b0cf 100644 --- a/pkgs/test_analyzer_plugin/test/rules/non_nullable_is_not_null_test.dart +++ b/pkgs/test_analyzer_plugin/test/rules/non_nullable_is_not_null_test.dart @@ -1,4 +1,4 @@ -// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. diff --git a/pkgs/test_analyzer_plugin/test/rules/use_is_empty_matcher_test.dart b/pkgs/test_analyzer_plugin/test/rules/use_is_empty_matcher_test.dart new file mode 100644 index 000000000..68d898919 --- /dev/null +++ b/pkgs/test_analyzer_plugin/test/rules/use_is_empty_matcher_test.dart @@ -0,0 +1,152 @@ +// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +// ignore_for_file: non_constant_identifier_names + +import 'package:analyzer/src/lint/registry.dart'; +import 'package:analyzer/utilities/package_config_file_builder.dart'; +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:test_analyzer_plugin/src/rules/use_is_empty_matcher.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(UseIsEmptyMatcherTest); + }); +} + +@reflectiveTest +class UseIsEmptyMatcherTest extends AnalysisRuleTest { + @override + String get analysisRule => 'use_is_empty_matcher'; + + @override + void setUp() { + Registry.ruleRegistry.registerLintRule(UseIsEmptyMatcherRule()); + + super.setUp(); + + var matcherPath = '/packages/matcher'; + newFile('$matcherPath/lib/matcher.dart', ''' +void expect(dynamic actual, dynamic matcher) {} + +const isNotNull = 0; +const isNull = 0; + +const isEmpty = 0; +const isFalse = 0; +const isNotEmpty = 0; +const isTrue = 0; +'''); + writeTestPackageConfig( + PackageConfigFileBuilder() + ..add(name: 'matcher', rootPath: convertPath(matcherPath)), + ); + } + + void test_isEmpty_false() async { + await assertDiagnostics( + r''' +import 'package:matcher/matcher.dart'; +void f() { + expect(''.isEmpty, false); +} +''', + [lint(71, 5, name: 'use_is_not_empty_matcher')], + ); + } + + void test_isEmpty_isFalse() async { + await assertDiagnostics( + r''' +import 'package:matcher/matcher.dart'; +void f() { + expect(''.isEmpty, isFalse); +} +''', + [lint(71, 7, name: 'use_is_not_empty_matcher')], + ); + } + + void test_isEmpty_isTrue() async { + await assertDiagnostics( + r''' +import 'package:matcher/matcher.dart'; +void f() { + expect(''.isEmpty, isTrue); +} +''', + [lint(71, 6, messageContains: 'isEmpty')], + ); + } + + void test_isEmpty_true() async { + await assertDiagnostics( + r''' +import 'package:matcher/matcher.dart'; +void f() { + expect(''.isEmpty, true); +} +''', + [lint(71, 4, messageContains: 'isEmpty')], + ); + } + + void test_isEmptyMatcher() async { + await assertNoDiagnostics(r''' +import 'package:matcher/matcher.dart'; +void f() { + expect('', isEmpty); +} +'''); + } + + void test_isNotEmpty_false() async { + await assertDiagnostics( + r''' +import 'package:matcher/matcher.dart'; +void f() { + expect(''.isNotEmpty, false); +} +''', + [lint(74, 5, messageContains: 'isEmpty')], + ); + } + + void test_isNotEmpty_isFalse() async { + await assertDiagnostics( + r''' +import 'package:matcher/matcher.dart'; +void f() { + expect(''.isNotEmpty, isFalse); +} +''', + [lint(74, 7, messageContains: 'isEmpty')], + ); + } + + void test_isNotEmpty_isTrue() async { + await assertDiagnostics( + r''' +import 'package:matcher/matcher.dart'; +void f() { + expect(''.isNotEmpty, isTrue); +} +''', + [lint(74, 6, name: 'use_is_not_empty_matcher')], + ); + } + + void test_isNotEmpty_true() async { + await assertDiagnostics( + r''' +import 'package:matcher/matcher.dart'; +void f() { + expect(''.isNotEmpty, true); +} +''', + [lint(74, 4, name: 'use_is_not_empty_matcher')], + ); + } +} From cf5e4df4124ceeee3ca21bf9ee26da312c83545b Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Thu, 20 Nov 2025 14:04:01 -0800 Subject: [PATCH 07/13] Add "test body goes last" rule --- pkgs/test_analyzer_plugin/lib/main.dart | 3 + .../rules/non_nullable_is_not_null_rule.dart | 4 +- .../src/rules/test_body_goes_last_rule.dart | 66 ++++++++++ .../lib/src/rules/test_in_test_rule.dart | 5 +- pkgs/test_analyzer_plugin/pubspec.yaml | 4 +- .../rules/non_nullable_is_not_null_test.dart | 7 +- .../test/rules/test_body_goes_last_test.dart | 120 ++++++++++++++++++ .../test/rules/use_is_empty_matcher_test.dart | 23 ++-- 8 files changed, 208 insertions(+), 24 deletions(-) create mode 100644 pkgs/test_analyzer_plugin/lib/src/rules/test_body_goes_last_rule.dart create mode 100644 pkgs/test_analyzer_plugin/test/rules/test_body_goes_last_test.dart diff --git a/pkgs/test_analyzer_plugin/lib/main.dart b/pkgs/test_analyzer_plugin/lib/main.dart index 343e2c5f8..d7dc3e7b4 100644 --- a/pkgs/test_analyzer_plugin/lib/main.dart +++ b/pkgs/test_analyzer_plugin/lib/main.dart @@ -7,12 +7,14 @@ import 'package:analysis_server_plugin/registry.dart'; import 'src/fixes.dart'; import 'src/rules/non_nullable_is_not_null_rule.dart'; +import 'src/rules/test_body_goes_last_rule.dart'; import 'src/rules/test_in_test_rule.dart'; import 'src/rules/use_is_empty_matcher.dart'; final plugin = TestPackagePlugin(); class TestPackagePlugin extends Plugin { + @override String get name => 'Test package plugin'; @override @@ -25,6 +27,7 @@ class TestPackagePlugin extends Plugin { registry.registerWarningRule(NonNullableIsNotNullRule()); registry.registerWarningRule(UseIsEmptyMatcherRule()); + registry.registerLintRule(TestBodyGoesLastRule()); // Should we register a fix for this rule? The only automatic fix I can // think of would be to delete the entire statement: // `expect(a, isNotNull);` or `expect(a, isNull);`. diff --git a/pkgs/test_analyzer_plugin/lib/src/rules/non_nullable_is_not_null_rule.dart b/pkgs/test_analyzer_plugin/lib/src/rules/non_nullable_is_not_null_rule.dart index 4c826ae1e..7bdfaa0e3 100644 --- a/pkgs/test_analyzer_plugin/lib/src/rules/non_nullable_is_not_null_rule.dart +++ b/pkgs/test_analyzer_plugin/lib/src/rules/non_nullable_is_not_null_rule.dart @@ -58,9 +58,7 @@ class _Visitor extends SimpleAstVisitor { @override void visitMethodInvocation(MethodInvocation node) { - if (!node.methodName.isExpect) { - return; - } + if (!node.methodName.isExpect) return; if (node.argumentList.arguments case [ var actual, diff --git a/pkgs/test_analyzer_plugin/lib/src/rules/test_body_goes_last_rule.dart b/pkgs/test_analyzer_plugin/lib/src/rules/test_body_goes_last_rule.dart new file mode 100644 index 000000000..34c9dc5ef --- /dev/null +++ b/pkgs/test_analyzer_plugin/lib/src/rules/test_body_goes_last_rule.dart @@ -0,0 +1,66 @@ +// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/error/error.dart'; + +import '../utilities.dart'; + +class TestBodyGoesLastRule extends AnalysisRule { + static const LintCode code = LintCode( + 'test_body_goes_last', + "The body of a '{0}' should go after the other arguments", + correctionMessage: + "Try moving the body argument below the other '{0}' arguments", + ); + + TestBodyGoesLastRule() + : super( + name: 'test_body_goes_last', + description: + 'The body of a test or group should go after the other arguments', + ); + + @override + LintCode get diagnosticCode => code; + + @override + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, + ) { + var visitor = _Visitor(this); + registry.addMethodInvocation(this, visitor); + } +} + +class _Visitor extends SimpleAstVisitor { + final AnalysisRule rule; + + _Visitor(this.rule); + + @override + void visitMethodInvocation(MethodInvocation node) { + if (!node.methodName.isTest && !node.methodName.isGroup) return; + + final arguments = node.argumentList.arguments; + + for (var i = 0; i < arguments.length; i++) { + final argument = arguments[i]; + if (argument.correspondingParameter?.name == 'body') { + if (i == arguments.length - 1) return; + final errorNode = argument is FunctionExpression + ? argument.parameters + : argument; + rule.reportAtNode(errorNode, arguments: [node.methodName.name]); + // Do not keep iterating through the arguments. + return; + } + } + } +} diff --git a/pkgs/test_analyzer_plugin/lib/src/rules/test_in_test_rule.dart b/pkgs/test_analyzer_plugin/lib/src/rules/test_in_test_rule.dart index 05f639daa..2b0b99969 100644 --- a/pkgs/test_analyzer_plugin/lib/src/rules/test_in_test_rule.dart +++ b/pkgs/test_analyzer_plugin/lib/src/rules/test_in_test_rule.dart @@ -46,9 +46,8 @@ class _Visitor extends SimpleAstVisitor { @override void visitMethodInvocation(MethodInvocation node) { - if (!node.methodName.isTest && !node.methodName.isGroup) { - return; - } + if (!node.methodName.isTest && !node.methodName.isGroup) return; + var enclosingTestCall = findEnclosingTestCall(node); if (enclosingTestCall != null) { rule.reportAtNode(node); diff --git a/pkgs/test_analyzer_plugin/pubspec.yaml b/pkgs/test_analyzer_plugin/pubspec.yaml index a9a276c4d..b97c07650 100644 --- a/pkgs/test_analyzer_plugin/pubspec.yaml +++ b/pkgs/test_analyzer_plugin/pubspec.yaml @@ -7,10 +7,10 @@ environment: dependencies: analysis_server_plugin: ^0.3.0 - analyzer: ^8.4.0 + analyzer: ^9.0.0 analyzer_plugin: ^0.13.5 dev_dependencies: - analyzer_testing: ^0.1.2 + analyzer_testing: ^0.1.7 test: any test_reflective_loader: any diff --git a/pkgs/test_analyzer_plugin/test/rules/non_nullable_is_not_null_test.dart b/pkgs/test_analyzer_plugin/test/rules/non_nullable_is_not_null_test.dart index abac2b0cf..761ea752f 100644 --- a/pkgs/test_analyzer_plugin/test/rules/non_nullable_is_not_null_test.dart +++ b/pkgs/test_analyzer_plugin/test/rules/non_nullable_is_not_null_test.dart @@ -4,7 +4,6 @@ // ignore_for_file: non_constant_identifier_names -import 'package:analyzer/src/lint/registry.dart'; import 'package:analyzer/utilities/package_config_file_builder.dart'; import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; import 'package:test_analyzer_plugin/src/rules/non_nullable_is_not_null_rule.dart'; @@ -20,8 +19,7 @@ void main() { class NonNullableIsNotNullTest extends AnalysisRuleTest { @override void setUp() { - Registry.ruleRegistry.registerLintRule(NonNullableIsNotNullRule()); - + rule = NonNullableIsNotNullRule(); super.setUp(); var matcherPath = '/packages/matcher'; @@ -37,9 +35,6 @@ const isNull = 0; ); } - @override - String get analysisRule => 'non_nullable_is_not_null'; - void test_nullableValue_isNotNullMatcher() async { await assertNoDiagnostics(r''' import 'package:matcher/matcher.dart'; diff --git a/pkgs/test_analyzer_plugin/test/rules/test_body_goes_last_test.dart b/pkgs/test_analyzer_plugin/test/rules/test_body_goes_last_test.dart new file mode 100644 index 000000000..fc7dc3f22 --- /dev/null +++ b/pkgs/test_analyzer_plugin/test/rules/test_body_goes_last_test.dart @@ -0,0 +1,120 @@ +// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +// ignore_for_file: non_constant_identifier_names + +import 'package:analyzer/utilities/package_config_file_builder.dart'; +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:test_analyzer_plugin/src/rules/test_body_goes_last_rule.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(TestBodyGoesLastTest); + }); +} + +@reflectiveTest +class TestBodyGoesLastTest extends AnalysisRuleTest { + @override + void setUp() { + rule = TestBodyGoesLastRule(); + super.setUp(); + + var matcherPath = '/packages/test_core'; + newFile('$matcherPath/lib/test_core.dart', ''' +void group( + Object? description, + dynamic body(), { + String? testOn, + Object? /*Timeout?*/ timeout, + Object? skip, + Object? tags, + Map? onPlatform, + int? retry, + Object? /*TestLocation?*/ location, + bool solo = false, +}) + +void test( + Object? description, + dynamic body(), { + String? testOn, + Object? /*Timeout?*/ timeout, + Object? skip, + Object? tags, + Map? onPlatform, + int? retry, + Object? /*TestLocation?*/ location, + bool solo = false, +}) +'''); + writeTestPackageConfig( + PackageConfigFileBuilder() + ..add(name: 'test_core', rootPath: convertPath(matcherPath)), + ); + } + + void test_groupBeforeSkip() async { + await assertDiagnostics( + r''' +import 'package:test_core/test_core.dart'; +void f() { + group('description', + () { + // Test case. + }, + skip: true, + ); +} +''', + [lint(81, 2)], + ); + } + + void test_groupLast() async { + await assertNoDiagnostics(r''' +import 'package:test_core/test_core.dart'; +void f() { + group('description', + skip: true, + () { + // Test case. + }, + ); +} +'''); + } + + void test_testBeforeSkip() async { + await assertDiagnostics( + r''' +import 'package:test_core/test_core.dart'; +void f() { + test('description', + () { + // Test case. + }, + skip: true, + ); +} +''', + [lint(80, 2)], + ); + } + + void test_testLast() async { + await assertNoDiagnostics(r''' +import 'package:test_core/test_core.dart'; +void f() { + test('description', + skip: true, + () { + // Test case. + }, + ); +} +'''); + } +} diff --git a/pkgs/test_analyzer_plugin/test/rules/use_is_empty_matcher_test.dart b/pkgs/test_analyzer_plugin/test/rules/use_is_empty_matcher_test.dart index 68d898919..a7197dd6e 100644 --- a/pkgs/test_analyzer_plugin/test/rules/use_is_empty_matcher_test.dart +++ b/pkgs/test_analyzer_plugin/test/rules/use_is_empty_matcher_test.dart @@ -4,7 +4,6 @@ // ignore_for_file: non_constant_identifier_names -import 'package:analyzer/src/lint/registry.dart'; import 'package:analyzer/utilities/package_config_file_builder.dart'; import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; import 'package:test_analyzer_plugin/src/rules/use_is_empty_matcher.dart'; @@ -18,13 +17,9 @@ void main() { @reflectiveTest class UseIsEmptyMatcherTest extends AnalysisRuleTest { - @override - String get analysisRule => 'use_is_empty_matcher'; - @override void setUp() { - Registry.ruleRegistry.registerLintRule(UseIsEmptyMatcherRule()); - + rule = UseIsEmptyMatcherRule(); super.setUp(); var matcherPath = '/packages/matcher'; @@ -77,7 +72,9 @@ void f() { expect(''.isEmpty, isTrue); } ''', - [lint(71, 6, messageContains: 'isEmpty')], + [ + lint(71, 6, messageContainsAll: ['isEmpty']), + ], ); } @@ -89,7 +86,9 @@ void f() { expect(''.isEmpty, true); } ''', - [lint(71, 4, messageContains: 'isEmpty')], + [ + lint(71, 4, messageContainsAll: ['isEmpty']), + ], ); } @@ -110,7 +109,9 @@ void f() { expect(''.isNotEmpty, false); } ''', - [lint(74, 5, messageContains: 'isEmpty')], + [ + lint(74, 5, messageContainsAll: ['isEmpty']), + ], ); } @@ -122,7 +123,9 @@ void f() { expect(''.isNotEmpty, isFalse); } ''', - [lint(74, 7, messageContains: 'isEmpty')], + [ + lint(74, 7, messageContainsAll: ['isEmpty']), + ], ); } From a333c19454385e5f96f685770dbc86917231c986 Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Thu, 20 Nov 2025 14:30:47 -0800 Subject: [PATCH 08/13] improved tests --- .../test/rules/test_body_goes_last_test.dart | 38 +-------- .../test/rules/test_in_test_test.dart | 84 +++++++++++++++++++ .../test/with_test_package.dart | 42 ++++++++++ 3 files changed, 129 insertions(+), 35 deletions(-) create mode 100644 pkgs/test_analyzer_plugin/test/rules/test_in_test_test.dart create mode 100644 pkgs/test_analyzer_plugin/test/with_test_package.dart diff --git a/pkgs/test_analyzer_plugin/test/rules/test_body_goes_last_test.dart b/pkgs/test_analyzer_plugin/test/rules/test_body_goes_last_test.dart index fc7dc3f22..4686b9050 100644 --- a/pkgs/test_analyzer_plugin/test/rules/test_body_goes_last_test.dart +++ b/pkgs/test_analyzer_plugin/test/rules/test_body_goes_last_test.dart @@ -4,11 +4,12 @@ // ignore_for_file: non_constant_identifier_names -import 'package:analyzer/utilities/package_config_file_builder.dart'; import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; import 'package:test_analyzer_plugin/src/rules/test_body_goes_last_rule.dart'; import 'package:test_reflective_loader/test_reflective_loader.dart'; +import '../with_test_package.dart'; + void main() { defineReflectiveSuite(() { defineReflectiveTests(TestBodyGoesLastTest); @@ -16,44 +17,11 @@ void main() { } @reflectiveTest -class TestBodyGoesLastTest extends AnalysisRuleTest { +class TestBodyGoesLastTest extends AnalysisRuleTest with WithTestPackage { @override void setUp() { rule = TestBodyGoesLastRule(); super.setUp(); - - var matcherPath = '/packages/test_core'; - newFile('$matcherPath/lib/test_core.dart', ''' -void group( - Object? description, - dynamic body(), { - String? testOn, - Object? /*Timeout?*/ timeout, - Object? skip, - Object? tags, - Map? onPlatform, - int? retry, - Object? /*TestLocation?*/ location, - bool solo = false, -}) - -void test( - Object? description, - dynamic body(), { - String? testOn, - Object? /*Timeout?*/ timeout, - Object? skip, - Object? tags, - Map? onPlatform, - int? retry, - Object? /*TestLocation?*/ location, - bool solo = false, -}) -'''); - writeTestPackageConfig( - PackageConfigFileBuilder() - ..add(name: 'test_core', rootPath: convertPath(matcherPath)), - ); } void test_groupBeforeSkip() async { diff --git a/pkgs/test_analyzer_plugin/test/rules/test_in_test_test.dart b/pkgs/test_analyzer_plugin/test/rules/test_in_test_test.dart new file mode 100644 index 000000000..a488cfaf6 --- /dev/null +++ b/pkgs/test_analyzer_plugin/test/rules/test_in_test_test.dart @@ -0,0 +1,84 @@ +// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +// ignore_for_file: non_constant_identifier_names + +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:test_analyzer_plugin/src/rules/test_in_test_rule.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +import '../with_test_package.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(TestInTestTest); + }); +} + +@reflectiveTest +class TestInTestTest extends AnalysisRuleTest with WithTestPackage { + @override + void setUp() { + rule = TestInTestRule(); + super.setUp(); + } + + void test_groupInGroup() async { + await assertNoDiagnostics(r''' +import 'package:test_core/test_core.dart'; +void f() { + group('one', + () { + group('two', () {}); + }, + ); +} +'''); + } + + void test_groupInTest() async { + await assertDiagnostics( + r''' +import 'package:test_core/test_core.dart'; +void f() { + test('one', + () { + group('two', () {}); + }, + ); +} +''', + [lint(83, 19)], + ); + } + + void test_testInGroup() async { + await assertNoDiagnostics(r''' +import 'package:test_core/test_core.dart'; +void f() { + group('one', + () { + test('two', () {}); + }, + ); +} +'''); + } + + void test_testInTest() async { + await assertDiagnostics( + r''' +import 'package:test_core/test_core.dart'; +void f() { + test('one', + () { + test('two', () {}); + }, + ); +} +''', + [lint(83, 18)], + ); + } +} diff --git a/pkgs/test_analyzer_plugin/test/with_test_package.dart b/pkgs/test_analyzer_plugin/test/with_test_package.dart new file mode 100644 index 000000000..f189a5f2f --- /dev/null +++ b/pkgs/test_analyzer_plugin/test/with_test_package.dart @@ -0,0 +1,42 @@ +import 'package:analyzer/utilities/package_config_file_builder.dart'; +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; + +mixin WithTestPackage on AnalysisRuleTest { + @override + void setUp() { + super.setUp(); + + var testCorePath = '/packages/test_core'; + newFile('$testCorePath/lib/test_core.dart', ''' +void group( + Object? description, + dynamic body(), { + String? testOn, + Object? /*Timeout?*/ timeout, + Object? skip, + Object? tags, + Map? onPlatform, + int? retry, + Object? /*TestLocation?*/ location, + bool solo = false, +}) {} + +void test( + Object? description, + dynamic body(), { + String? testOn, + Object? /*Timeout?*/ timeout, + Object? skip, + Object? tags, + Map? onPlatform, + int? retry, + Object? /*TestLocation?*/ location, + bool solo = false, +}) {} +'''); + writeTestPackageConfig( + PackageConfigFileBuilder() + ..add(name: 'test_core', rootPath: convertPath(testCorePath)), + ); + } +} From 4e793ae4f7af1f020f853516a074049967ac0141 Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Thu, 20 Nov 2025 14:41:48 -0800 Subject: [PATCH 09/13] Add CHANGELOG --- pkgs/test_analyzer_plugin/CHANGELOG.md | 21 +++++++++++++++++++++ pkgs/test_analyzer_plugin/README.md | 7 ++++++- 2 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 pkgs/test_analyzer_plugin/CHANGELOG.md diff --git a/pkgs/test_analyzer_plugin/CHANGELOG.md b/pkgs/test_analyzer_plugin/CHANGELOG.md new file mode 100644 index 000000000..cfd92b281 --- /dev/null +++ b/pkgs/test_analyzer_plugin/CHANGELOG.md @@ -0,0 +1,21 @@ +## 0.1.0 + +- Initial release + +- Available rules: + - `test_in_test`: Report a warning when a `test` or a `group` is declared inside a `test` + declaration. + + - `non_nullable_is_not_null`: Report a warning when a non-nullable value is + matched against `isNotNull` or `isNull`. + + - `use_is_empty_matcher`: Report a warning when an `expect` expectation of + `true`, `false`, `isTrue`, or `isFalse` is paired with a `.isEmpty` or + `.isNotEmpty` property access on an actual value. Instead, the `isEmpty` or + `isNotEmpty` Matcher should be used. + + - `test_body_goes_last`: Report a lint when the body argument of a `test` or + `group` is not last. + +- Quick fixes + - Offer a quick fix in the IDE for `test_in_test`. diff --git a/pkgs/test_analyzer_plugin/README.md b/pkgs/test_analyzer_plugin/README.md index e0540b9a2..f41ef7975 100644 --- a/pkgs/test_analyzer_plugin/README.md +++ b/pkgs/test_analyzer_plugin/README.md @@ -17,5 +17,10 @@ This analyzer plugin provides the following additional analysis: an actual value. Instead, the `isEmpty` or `isNotEmpty` Matcher should be used. +* Report a lint when the body argument of a `test` or `group` is not last. Lint + rules are not enabled by default and must be actively enabled in analysis + options. + * Offer a quick fix in the IDE for the above warning, which moves the violating - `test` or `group` declaration below the containing `test` declaration. \ No newline at end of file + `test` or `group` declaration below the containing `test` declaration. + From e14ed7272c4f677668b2f85baba7ce6279e26424 Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Thu, 20 Nov 2025 15:11:24 -0800 Subject: [PATCH 10/13] LICENSE --- pkgs/test_analyzer_plugin/LICENSE | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 pkgs/test_analyzer_plugin/LICENSE diff --git a/pkgs/test_analyzer_plugin/LICENSE b/pkgs/test_analyzer_plugin/LICENSE new file mode 100644 index 000000000..2b76736b4 --- /dev/null +++ b/pkgs/test_analyzer_plugin/LICENSE @@ -0,0 +1,27 @@ +Copyright 2025, the Dart project authors. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + + * Redistributions of source code must retain the above copyright + notice, this list of conditions and the following disclaimer. + * Redistributions in binary form must reproduce the above + copyright notice, this list of conditions and the following + disclaimer in the documentation and/or other materials provided + with the distribution. + * Neither the name of Google LLC nor the names of its + contributors may be used to endorse or promote products derived + from this software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. From 1c578edefa42b010bc739c52cf13735743dcb2d7 Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Fri, 21 Nov 2025 07:07:03 -0800 Subject: [PATCH 11/13] pubspec links --- pkgs/test_analyzer_plugin/pubspec.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkgs/test_analyzer_plugin/pubspec.yaml b/pkgs/test_analyzer_plugin/pubspec.yaml index b97c07650..5183c5ebd 100644 --- a/pkgs/test_analyzer_plugin/pubspec.yaml +++ b/pkgs/test_analyzer_plugin/pubspec.yaml @@ -1,5 +1,7 @@ name: test_analyzer_plugin description: An analyzer plugin to report improper usage of the test package. +repository: https://github.com/dart-lang/test/tree/master/pkgs/test_core +issue_tracker: https://github.com/dart-lang/test/issues?q=is%3Aissue+is%3Aopen version: 0.1.0 environment: From 7897eb6901dc2bba3e55a64a22cc8faf9e1cae9b Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Fri, 21 Nov 2025 09:43:30 -0800 Subject: [PATCH 12/13] New rule: use_contains_matcher --- pkgs/test_analyzer_plugin/CHANGELOG.md | 7 +- pkgs/test_analyzer_plugin/README.md | 15 +- pkgs/test_analyzer_plugin/lib/main.dart | 11 +- .../src/rules/use_contains_matcher_rule.dart | 107 +++++++++++++ .../test/rules/use_contains_matcher_test.dart | 150 ++++++++++++++++++ 5 files changed, 279 insertions(+), 11 deletions(-) create mode 100644 pkgs/test_analyzer_plugin/lib/src/rules/use_contains_matcher_rule.dart create mode 100644 pkgs/test_analyzer_plugin/test/rules/use_contains_matcher_test.dart diff --git a/pkgs/test_analyzer_plugin/CHANGELOG.md b/pkgs/test_analyzer_plugin/CHANGELOG.md index cfd92b281..071f946f1 100644 --- a/pkgs/test_analyzer_plugin/CHANGELOG.md +++ b/pkgs/test_analyzer_plugin/CHANGELOG.md @@ -9,8 +9,13 @@ - `non_nullable_is_not_null`: Report a warning when a non-nullable value is matched against `isNotNull` or `isNull`. + - `use_contains_matcher`: Report a warning when an `expect` expectation of + `true`, `false`, `isTrue`, or `isFalse` is paired with a `.contains` method + call on an actual value (maybe wrapped with a `!`). Instead, the `contains` + Matcher (maybe wrapped with the `isNot()` Matcher) should be used. + - `use_is_empty_matcher`: Report a warning when an `expect` expectation of - `true`, `false`, `isTrue`, or `isFalse` is paired with a `.isEmpty` or + `true`, `false`, `isTrue`, or `isFalse` is paired with an `.isEmpty` or `.isNotEmpty` property access on an actual value. Instead, the `isEmpty` or `isNotEmpty` Matcher should be used. diff --git a/pkgs/test_analyzer_plugin/README.md b/pkgs/test_analyzer_plugin/README.md index f41ef7975..9622ac2fb 100644 --- a/pkgs/test_analyzer_plugin/README.md +++ b/pkgs/test_analyzer_plugin/README.md @@ -6,16 +6,22 @@ usage of the test package. This analyzer plugin provides the following additional analysis: * Report a warning when a `test` or a `group` is declared inside a `test` - declaration. This can _sometimes_ be detected at runtime. This warning is - reported statically. + declaration. This can _sometimes_ be detected at runtime, but it's more + convenient to report this warning statically. * Report a warning when a non-nullable value is matched against `isNotNull` or `isNull`. +* Report a warning when an `expect` expectation of `true`, `false`, `isTrue`, or + `isFalse` is paired with a `.contains` method call on an actual value (maybe + wrapped with a `!`). Instead, the `contains` Matcher (maybe wrapped with the + `isNot()` Matcher) should be used. This Matcher yields meaningful failure + messages. + * Report a warning when an `expect` expectation of `true`, `false`, `isTrue`, - or `isFalse` is paired with a `.isEmpty` or `.isNotEmpty` property access on + or `isFalse` is paired with an `.isEmpty` or `.isNotEmpty` property access on an actual value. Instead, the `isEmpty` or `isNotEmpty` Matcher should be - used. + used. These Matchers yield meaningful failure messages. * Report a lint when the body argument of a `test` or `group` is not last. Lint rules are not enabled by default and must be actively enabled in analysis @@ -23,4 +29,3 @@ This analyzer plugin provides the following additional analysis: * Offer a quick fix in the IDE for the above warning, which moves the violating `test` or `group` declaration below the containing `test` declaration. - diff --git a/pkgs/test_analyzer_plugin/lib/main.dart b/pkgs/test_analyzer_plugin/lib/main.dart index d7dc3e7b4..4bedf4ed2 100644 --- a/pkgs/test_analyzer_plugin/lib/main.dart +++ b/pkgs/test_analyzer_plugin/lib/main.dart @@ -9,6 +9,7 @@ import 'src/fixes.dart'; import 'src/rules/non_nullable_is_not_null_rule.dart'; import 'src/rules/test_body_goes_last_rule.dart'; import 'src/rules/test_in_test_rule.dart'; +import 'src/rules/use_contains_matcher_rule.dart'; import 'src/rules/use_is_empty_matcher.dart'; final plugin = TestPackagePlugin(); @@ -26,6 +27,7 @@ class TestPackagePlugin extends Plugin { ); registry.registerWarningRule(NonNullableIsNotNullRule()); + registry.registerWarningRule(UseContainsMatcherRule()); registry.registerWarningRule(UseIsEmptyMatcherRule()); registry.registerLintRule(TestBodyGoesLastRule()); // Should we register a fix for this rule? The only automatic fix I can @@ -33,10 +35,9 @@ class TestPackagePlugin extends Plugin { // `expect(a, isNotNull);` or `expect(a, isNull);`. // TODO(srawlins): More rules to catch: - // * `expect(7, contains(6))` - should only use `hasLength` with `Iterable` - // or `String`. - // * `expect(7, hasLength(3))` - should only use `hasLength` with `Iterable` - // or `String`. - // * `expect([].contains(7), isFalse)` - should use `contains` matcher. + // * `expect(7, contains(6))` - should only use `hasLength` with `Iterable`, + // `Map`, or `String`. + // * `expect(7, hasLength(3))` - should only use `hasLength` with + // `Iterable`, `Map`, or `String`. } } diff --git a/pkgs/test_analyzer_plugin/lib/src/rules/use_contains_matcher_rule.dart b/pkgs/test_analyzer_plugin/lib/src/rules/use_contains_matcher_rule.dart new file mode 100644 index 000000000..ca35c55fa --- /dev/null +++ b/pkgs/test_analyzer_plugin/lib/src/rules/use_contains_matcher_rule.dart @@ -0,0 +1,107 @@ +// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/token.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/dart/element/type_system.dart'; +import 'package:analyzer/error/error.dart'; + +import '../utilities.dart'; + +class UseContainsMatcherRule extends MultiAnalysisRule { + static const LintCode useIsContainsCode = LintCode( + 'use_contains_matcher', + "Use the 'contains' matcher.", + ); + + static const LintCode useIsNotAndContainsMatchersCode = LintCode( + 'use_is_not_and_contains_matchers', + "Use the 'isNot' and 'contains' matchers.", + ); + + UseContainsMatcherRule() + : super( + name: 'use_is_empty_matcher', + description: "Use the built-in 'contains' matcher.", + ); + + @override + List get diagnosticCodes => [ + useIsContainsCode, + useIsNotAndContainsMatchersCode, + ]; + + @override + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, + ) { + var visitor = _Visitor(this, context.typeSystem); + registry.addMethodInvocation(this, visitor); + } +} + +class _Visitor extends SimpleAstVisitor { + final MultiAnalysisRule rule; + + final TypeSystem typeSystem; + + _Visitor(this.rule, this.typeSystem); + + @override + void visitMethodInvocation(MethodInvocation node) { + if (!node.methodName.isExpect) return; + + var arguments = node.argumentList.arguments; + if (arguments.isEmpty || arguments.length > 2) return; + var [actual, matcher] = arguments; + + bool actualIsContains; + if (actual is MethodInvocation && actual.methodName.name == 'contains') { + actualIsContains = true; + } else if (actual is PrefixExpression && + actual.operator.type == TokenType.BANG) { + var operand = actual.operand.unParenthesized; + if (operand is MethodInvocation && + operand.methodName.name == 'contains') { + actualIsContains = false; + } else { + return; + } + } else { + return; + } + + bool matcherValue; + if (matcher is BooleanLiteral) { + matcherValue = matcher.value; + } else if (matcher is SimpleIdentifier && matcher.isIsFalse) { + matcherValue = false; + } else if (matcher is SimpleIdentifier && matcher.isIsTrue) { + matcherValue = true; + } else { + return; + } + + if (actualIsContains == matcherValue) { + // Either `expect(a.contains(...), isTrue|true)` or + // `expect(!a.contains(...), isFalse|false)`. + rule.reportAtNode( + matcher, + diagnosticCode: UseContainsMatcherRule.useIsContainsCode, + ); + } else { + // Either `expect(a.contains(...), isFalse|false)` or + // `expect(!a.contains(...), isTrue|true)`. + rule.reportAtNode( + matcher, + diagnosticCode: UseContainsMatcherRule.useIsNotAndContainsMatchersCode, + ); + } + } +} diff --git a/pkgs/test_analyzer_plugin/test/rules/use_contains_matcher_test.dart b/pkgs/test_analyzer_plugin/test/rules/use_contains_matcher_test.dart new file mode 100644 index 000000000..2cd35dac9 --- /dev/null +++ b/pkgs/test_analyzer_plugin/test/rules/use_contains_matcher_test.dart @@ -0,0 +1,150 @@ +// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +// ignore_for_file: non_constant_identifier_names + +import 'package:analyzer/utilities/package_config_file_builder.dart'; +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:test_analyzer_plugin/src/rules/use_contains_matcher_rule.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(UseContainsMatcherTest); + }); +} + +@reflectiveTest +class UseContainsMatcherTest extends AnalysisRuleTest { + @override + void setUp() { + rule = UseContainsMatcherRule(); + super.setUp(); + + var matcherPath = '/packages/matcher'; + newFile('$matcherPath/lib/matcher.dart', ''' +void expect(dynamic actual, dynamic matcher) {} + +const isNotNull = 0; +const isNull = 0; + +const isEmpty = 0; +const isFalse = 0; +const isNotEmpty = 0; +const isTrue = 0; + +class Matcher {} +Matcher contains(Object? expected) => throw UnimplementedError(); +'''); + writeTestPackageConfig( + PackageConfigFileBuilder() + ..add(name: 'matcher', rootPath: convertPath(matcherPath)), + ); + } + + void test_contains_false() async { + await assertDiagnostics( + r''' +import 'package:matcher/matcher.dart'; +void f() { + expect(''.contains(''), false); +} +''', + [lint(76, 5, name: 'use_is_not_and_contains_matchers')], + ); + } + + void test_contains_isFalse() async { + await assertDiagnostics( + r''' +import 'package:matcher/matcher.dart'; +void f() { + expect(''.contains(''), isFalse); +} +''', + [lint(76, 7, name: 'use_is_not_and_contains_matchers')], + ); + } + + void test_contains_isTrue() async { + await assertDiagnostics( + r''' +import 'package:matcher/matcher.dart'; +void f() { + expect(''.contains(''), isTrue); +} +''', + [lint(76, 6, name: 'use_contains_matcher')], + ); + } + + void test_contains_true() async { + await assertDiagnostics( + r''' +import 'package:matcher/matcher.dart'; +void f() { + expect(''.contains(''), true); +} +''', + [lint(76, 4, name: 'use_contains_matcher')], + ); + } + + void test_containsMatcher() async { + await assertNoDiagnostics(r''' +import 'package:matcher/matcher.dart'; +void f() { + expect('', contains('')); +} +'''); + } + + void test_notContainsParens_false() async { + await assertDiagnostics( + r''' +import 'package:matcher/matcher.dart'; +void f() { + expect(!(''.contains('')), false); +} +''', + [lint(79, 5, name: 'use_contains_matcher')], + ); + } + + void test_notContains_isFalse() async { + await assertDiagnostics( + r''' +import 'package:matcher/matcher.dart'; +void f(String s) { + expect(!s.contains(''), isFalse); +} +''', + [lint(84, 7, name: 'use_contains_matcher')], + ); + } + + void test_notContains_isTrue() async { + await assertDiagnostics( + r''' +import 'package:matcher/matcher.dart'; +void f(String s) { + expect(!s.contains(''), isTrue); +} +''', + [lint(84, 6, name: 'use_is_not_and_contains_matchers')], + ); + } + + void test_notContains_true() async { + await assertDiagnostics( + r''' +import 'package:matcher/matcher.dart'; +void f(String s) { + expect(!s.contains(''), true); +} +''', + [lint(84, 4, name: 'use_is_not_and_contains_matchers')], + ); + } +} From b9725d03e2dccdf1369aac5e35f536cb7e64d6d3 Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Mon, 24 Nov 2025 17:16:52 -0800 Subject: [PATCH 13/13] Add support for setUp, setUpAll, tearDown, and tearDownAll --- pkgs/test_analyzer_plugin/CHANGELOG.md | 5 +- pkgs/test_analyzer_plugin/README.md | 7 +- .../lib/src/rules/test_in_test_rule.dart | 18 ++-- .../lib/src/utilities.dart | 63 +++++++++++- .../test/rules/test_in_test_test.dart | 99 ++++++++++++++++++- .../test/with_test_package.dart | 12 +++ 6 files changed, 186 insertions(+), 18 deletions(-) diff --git a/pkgs/test_analyzer_plugin/CHANGELOG.md b/pkgs/test_analyzer_plugin/CHANGELOG.md index 071f946f1..3624f43d5 100644 --- a/pkgs/test_analyzer_plugin/CHANGELOG.md +++ b/pkgs/test_analyzer_plugin/CHANGELOG.md @@ -3,8 +3,9 @@ - Initial release - Available rules: - - `test_in_test`: Report a warning when a `test` or a `group` is declared inside a `test` - declaration. + - `test_in_test`: Report a warning when a `test`, `group`, `setUp`, `setUpAll`, + `tearDown`, or `tearDownAll` is declared inside a `test`, `setUp`, `setUpAll`, + `tearDown`, or `tearDownAll` declaration. - `non_nullable_is_not_null`: Report a warning when a non-nullable value is matched against `isNotNull` or `isNull`. diff --git a/pkgs/test_analyzer_plugin/README.md b/pkgs/test_analyzer_plugin/README.md index 9622ac2fb..f60560f15 100644 --- a/pkgs/test_analyzer_plugin/README.md +++ b/pkgs/test_analyzer_plugin/README.md @@ -5,9 +5,10 @@ usage of the test package. This analyzer plugin provides the following additional analysis: -* Report a warning when a `test` or a `group` is declared inside a `test` - declaration. This can _sometimes_ be detected at runtime, but it's more - convenient to report this warning statically. +* Report a warning when a `test`, `group`, `setUp`, `setUpAll`, `tearDown`, or + `tearDownAll` is declared inside a `test`, `setUp`, `setUpAll`, `tearDown`, or + `tearDownAll` declaration. This can _sometimes_ be detected at runtime, but + it's more convenient to report this warning statically. * Report a warning when a non-nullable value is matched against `isNotNull` or `isNull`. diff --git a/pkgs/test_analyzer_plugin/lib/src/rules/test_in_test_rule.dart b/pkgs/test_analyzer_plugin/lib/src/rules/test_in_test_rule.dart index 2b0b99969..db7346c38 100644 --- a/pkgs/test_analyzer_plugin/lib/src/rules/test_in_test_rule.dart +++ b/pkgs/test_analyzer_plugin/lib/src/rules/test_in_test_rule.dart @@ -14,8 +14,8 @@ import '../utilities.dart'; class TestInTestRule extends AnalysisRule { static const LintCode code = LintCode( 'test_in_test', - "Do not declare a 'test' or a 'group' inside a 'test'", - correctionMessage: "Try moving 'test' or 'group' outside of 'test'", + "Do not declare a 'test' or a 'group' inside a '{0}'", + correctionMessage: "Try moving 'test' or 'group' outside of '{0}'", ); TestInTestRule() @@ -46,11 +46,15 @@ class _Visitor extends SimpleAstVisitor { @override void visitMethodInvocation(MethodInvocation node) { - if (!node.methodName.isTest && !node.methodName.isGroup) return; - - var enclosingTestCall = findEnclosingTestCall(node); - if (enclosingTestCall != null) { - rule.reportAtNode(node); + if (!node.methodName.isTestOrGroupOrSetUpOrTearDown) return; + + var enclosingTestOrSetUpOrTearDownCall = + findEnclosingTestOrSetUpOrTearDownCall(node); + if (enclosingTestOrSetUpOrTearDownCall != null) { + rule.reportAtNode( + node.methodName, + arguments: [enclosingTestOrSetUpOrTearDownCall.methodName.name], + ); } } } diff --git a/pkgs/test_analyzer_plugin/lib/src/utilities.dart b/pkgs/test_analyzer_plugin/lib/src/utilities.dart index bb110d294..77fced0f7 100644 --- a/pkgs/test_analyzer_plugin/lib/src/utilities.dart +++ b/pkgs/test_analyzer_plugin/lib/src/utilities.dart @@ -16,14 +16,67 @@ MethodInvocation? findEnclosingTestCall(MethodInvocation node) { return null; } +/// Finds an enclosing call to the 'test', 'setUp', 'setUpAll', 'tearDown', or +/// 'tearDownAll' function if there is one. +MethodInvocation? findEnclosingTestOrSetUpOrTearDownCall( + MethodInvocation node, +) { + var ancestor = node.parent?.thisOrAncestorOfType(); + while (ancestor != null) { + var methodName = ancestor.methodName; + if (methodName.isTest || + methodName.name == 'setUp' || + methodName.name == 'setUpAll' || + methodName.name == 'tearDown' || + methodName.name == 'tearDownAll') { + if (methodName.isFromTestCore) { + return ancestor; + } + } + ancestor = ancestor.parent?.thisOrAncestorOfType(); + } + return null; +} + extension SimpleIdentifierExtension on SimpleIdentifier { + /// Whether this identifier represents the 'test', 'group', 'setUp', + /// 'setUpAll', 'tearDown', or 'tearDownAll' function from the 'test_core' + /// package. + bool get isTestOrGroupOrSetUpOrTearDown { + final element = this.element; + if (element == null) return false; + if (element.name != 'test' && + element.name != 'group' && + element.name != 'setUp' && + element.name != 'setUpAll' && + element.name != 'tearDown' && + element.name != 'tearDownAll') { + return false; + } + return element.library?.uri.path.startsWith('test_core/') ?? false; + } + + /// Whether this identifier represents the 'test', 'setUp', 'setUpAll', + /// 'tearDown', or 'tearDownAll' function from the 'test_core' package. + bool get isTestOrSetUpOrTearDown { + final element = this.element; + if (element == null) return false; + if (element.name != 'test' && + element.name != 'setUp' && + element.name != 'setUpAll' && + element.name != 'tearDown' && + element.name != 'tearDownAll') { + return false; + } + return isFromTestCore; + } + /// Whether this identifier represents the 'test' function from the /// 'test_core' package. bool get isTest { final element = this.element; if (element == null) return false; - if (element.name != 'test') return false; - return element.library?.uri.path.startsWith('test_core/') ?? false; + return element.name == 'test' && isFromTestCore; } /// Whether this identifier represents the 'group' function from the @@ -31,10 +84,12 @@ extension SimpleIdentifierExtension on SimpleIdentifier { bool get isGroup { final element = this.element; if (element == null) return false; - if (element.name != 'group') return false; - return element.library?.uri.path.startsWith('test_core/') ?? false; + return element.name == 'group' && isFromTestCore; } + bool get isFromTestCore => + element?.library?.uri.path.startsWith('test_core/') ?? false; + /// Whether this identifier represents the 'expect' function from the /// 'matcher' package. bool get isExpect { diff --git a/pkgs/test_analyzer_plugin/test/rules/test_in_test_test.dart b/pkgs/test_analyzer_plugin/test/rules/test_in_test_test.dart index a488cfaf6..7a93c911f 100644 --- a/pkgs/test_analyzer_plugin/test/rules/test_in_test_test.dart +++ b/pkgs/test_analyzer_plugin/test/rules/test_in_test_test.dart @@ -49,7 +49,21 @@ void f() { ); } ''', - [lint(83, 19)], + [lint(83, 5)], + ); + } + + void test_groupInSetUp() async { + await assertDiagnostics( + r''' +import 'package:test_core/test_core.dart'; +void f() { + setUp(() { + group('two', () {}); + }); +} +''', + [lint(71, 5)], ); } @@ -78,7 +92,88 @@ void f() { ); } ''', - [lint(83, 18)], + [lint(83, 4)], + ); + } + + void test_testInSetUp() async { + await assertDiagnostics( + r''' +import 'package:test_core/test_core.dart'; +void f() { + setUp(() { + test('two', () {}); + }); +} +''', + [lint(71, 4)], + ); + } + + void test_testInSetUpAll() async { + await assertDiagnostics( + r''' +import 'package:test_core/test_core.dart'; +void f() { + setUpAll(() { + test('two', () {}); + }); +} +''', + [lint(74, 4)], + ); + } + + void test_testInTearDown() async { + await assertDiagnostics( + r''' +import 'package:test_core/test_core.dart'; +void f() { + tearDown(() { + test('two', () {}); + }); +} +''', + [lint(74, 4)], + ); + } + + void test_testInTearDownAll() async { + await assertDiagnostics( + r''' +import 'package:test_core/test_core.dart'; +void f() { + tearDownAll(() { + test('two', () {}); + }); +} +''', + [lint(77, 4)], + ); + } + + void test_setUpInSetUp() async { + await assertDiagnostics( + r''' +import 'package:test_core/test_core.dart'; +void f() { + setUp(() { + setUp(() {}); + }); +} +''', + [lint(71, 5)], ); } + + void test_setUpInGroup() async { + await assertNoDiagnostics(r''' +import 'package:test_core/test_core.dart'; +void f() { + group('', () { + setUp(() {}); + }); +} +'''); + } } diff --git a/pkgs/test_analyzer_plugin/test/with_test_package.dart b/pkgs/test_analyzer_plugin/test/with_test_package.dart index f189a5f2f..c0001777c 100644 --- a/pkgs/test_analyzer_plugin/test/with_test_package.dart +++ b/pkgs/test_analyzer_plugin/test/with_test_package.dart @@ -33,6 +33,18 @@ void test( Object? /*TestLocation?*/ location, bool solo = false, }) {} + +void setUp(dynamic callback()) {} + +void setUpAll(dynamic callback(), { + Object? /*TestLocation?*/ location, +}) {} + +void tearDown(dynamic callback()) {} + +void tearDownAll(dynamic callback(), { + Object? /*TestLocation?*/ location, +}) {} '''); writeTestPackageConfig( PackageConfigFileBuilder()