Introduce an analyzer plugin for the test package.

pkgs/test_analyzer_plugin/CHANGELOG.md

@@ -0,0 +1,27 @@
+## 0.1.0
+
+- Initial release
+
+- Available rules:
+  - `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`.
+
+  - `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 an `.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`.

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.

pkgs/test_analyzer_plugin/README.md

@@ -0,0 +1,32 @@
+# 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`, `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`.
+
+* 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 an `.isEmpty` or `.isNotEmpty` property access on
+  an actual value. Instead, the `isEmpty` or `isNotEmpty` Matcher should be
+  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
+  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.

pkgs/test_analyzer_plugin/lib/main.dart

@@ -0,0 +1,43 @@
+// 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/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();
+
+class TestPackagePlugin extends Plugin {
+  @override
+  String get name => 'Test package plugin';
+
+  @override
+  void register(PluginRegistry registry) {
+    registry.registerWarningRule(TestInTestRule());
+    registry.registerFixForRule(
+      TestInTestRule.code,
+      MoveBelowEnclosingTestCall.new,
+    );
+
+    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
+    // 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`,
+    //   `Map`, or `String`.
+    // * `expect(7, hasLength(3))` - should only use `hasLength` with
+    //   `Iterable`, `Map`, or `String`.
+  }
+}

pkgs/test_analyzer_plugin/lib/src/fixes.dart

@@ -0,0 +1,61 @@
+// 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 _moveBelowEnclosingTestCallKind = 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 => _moveBelowEnclosingTestCallKind;
+
+  @override
+  Future<void> 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,
+        '$defaultEol$defaultEol$indent$source',
+      );
+    });
+  }
+}

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

@@ -0,0 +1,86 @@
+// 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 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<LintCode> get diagnosticCodes => [
+    nonNullableIsNotNullCode,
+    nonNullableIsNullCode,
+  ];
+
+  @override
+  void registerNodeProcessors(
+    RuleVisitorRegistry registry,
+    RuleContext context,
+  ) {
+    var visitor = _Visitor(this, context.typeSystem);
+    registry.addMethodInvocation(this, visitor);
+  }
+}
+
+class _Visitor extends SimpleAstVisitor<void> {
+  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,
+          );
+        }
+      }
+    }
+  }
+}

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<void> {
+  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;
+      }
+    }
+  }
+}