Add another rule: non_nullable_is_not_null_rule

Author: srawlins

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.
   }
 }

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<LintCode> get diagnosticCodes => [nonNullableIsNotNullCode];
+
+  @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.dart renamed to 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<void> {
     }
     var enclosingTestCall = findEnclosingTestCall(node);
     if (enclosingTestCall != null) {
-      rule.reportLint(node);
+      rule.reportAtNode(node);
     }
   }
 }

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;
   }
 }

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

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')],
+    );
+  }
+}