linter: Move no_runtimeType_toString tests

Also re-order the skip checks, to avoid walking up ancestors on
every method call that isn't named 'toString'.

Change-Id: I69e9b58b1ae26d44498381546d5881c7e66d4935
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/323681
Auto-Submit: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>

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

@@ -33,11 +33,11 @@ class A {
 This lint has some exceptions where performance is not a problem or where real
 type information is more important than performance:
 
-* in assertion
-* in throw expressions
-* in catch clauses
-* in mixin declaration
-* in abstract class
+* in an assertion
+* in a throw expression
+* in a catch clause
+* in a mixin declaration
+* in an abstract class declaration
 
 ''';
 
@@ -71,23 +71,19 @@ class _Visitor extends SimpleAstVisitor<void> {
 
   @override
   void visitInterpolationExpression(InterpolationExpression node) {
-    if (_canSkip(node)) {
-      return;
-    }
-    if (_isRuntimeTypeAccess(node.expression)) {
-      rule.reportLint(node.expression);
-    }
+    if (!_isRuntimeTypeAccess(node.expression)) return;
+    if (_canSkip(node)) return;
+
+    rule.reportLint(node.expression);
   }
 
   @override
   void visitMethodInvocation(MethodInvocation node) {
-    if (_canSkip(node)) {
-      return;
-    }
-    if (node.methodName.name == 'toString' &&
-        _isRuntimeTypeAccess(node.realTarget)) {
-      rule.reportLint(node.methodName);
-    }
+    if (node.methodName.name != 'toString') return;
+    if (!_isRuntimeTypeAccess(node.realTarget)) return;
+    if (_canSkip(node)) return;
+
+    rule.reportLint(node.methodName);
   }
 
   bool _canSkip(AstNode node) =>

pkg/linter/test/rules/all.dart

@@ -90,6 +90,8 @@ import 'no_duplicate_case_values_test.dart' as no_duplicate_case_values;
 import 'no_leading_underscores_for_local_identifiers_test.dart'
     as no_leading_underscores_for_local_identifiers;
 import 'no_logic_in_create_state_test.dart' as no_logic_in_create_state;
+// ignore: library_prefixes
+import 'no_runtimeType_toString_test.dart' as no_runtimeType_toString;
 import 'no_self_assignments_test.dart' as no_self_assignments;
 import 'no_wildcard_variable_uses_test.dart' as no_wildcard_variable_uses;
 import 'non_constant_identifier_names_test.dart'
@@ -294,6 +296,7 @@ void main() {
   no_duplicate_case_values.main();
   no_leading_underscores_for_local_identifiers.main();
   no_logic_in_create_state.main();
+  no_runtimeType_toString.main();
   no_self_assignments.main();
   no_wildcard_variable_uses.main();
   non_constant_identifier_names.main();

pkg/linter/test/rules/no_runtimeType_toString_test.dart

@@ -0,0 +1,183 @@
+// 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: file_names
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import '../rule_test_support.dart';
+
+main() {
+  defineReflectiveSuite(() {
+    defineReflectiveTests(NoRuntimeTypeToStringTest);
+  });
+}
+
+@reflectiveTest
+class NoRuntimeTypeToStringTest extends LintRuleTest {
+  @override
+  String get lintRule => 'no_runtimeType_toString';
+
+  test_extension_onAbstractClass() async {
+    await assertNoDiagnostics(r'''
+abstract class C {}
+extension E on C {
+  String f() => '$runtimeType';
+}
+''');
+  }
+
+  test_extension_onClass() async {
+    await assertDiagnostics(r'''
+class C {}
+extension E on C {
+  String f() => '$runtimeType';
+}
+''', [
+      lint(48, 11),
+    ]);
+  }
+
+  test_inAbstractClass() async {
+    await assertNoDiagnostics(r'''
+abstract class C {
+  void f() {
+    '$runtimeType';
+  }
+}
+''');
+  }
+
+  test_inCatchClause() async {
+    await assertNoDiagnostics(r'''
+class C {
+  void f() {
+    try {} catch (e) {
+      '${runtimeType}';
+    }
+  }
+}
+''');
+  }
+
+  test_inMixin() async {
+    await assertNoDiagnostics(r'''
+mixin M {
+  void f() {
+    '$runtimeType';
+  }
+}
+''');
+  }
+
+  test_interpolation_expression() async {
+    await assertNoDiagnostics(r'''
+int x = 1;
+class C {
+  void f() {
+    '${x.runtimeType}';
+  }
+}
+''');
+  }
+
+  test_interpolation_implicitThis() async {
+    await assertDiagnostics(r'''
+class C {
+  void f() {
+    '$runtimeType';
+  }
+}
+''', [
+      lint(29, 11),
+    ]);
+  }
+
+  test_interpolation_withBraces() async {
+    await assertDiagnostics(r'''
+class C {
+  void f() {
+    '${runtimeType}';
+  }
+}
+''', [
+      lint(30, 11),
+    ]);
+  }
+
+  test_inThrowExpression() async {
+    await assertNoDiagnostics(r'''
+class C {
+  void f() {
+    throw '${runtimeType}';
+  }
+}
+''');
+  }
+
+  test_localVariableNamedRuntimeType() async {
+    await assertNoDiagnostics(r'''
+class C {
+  void f() {
+    var runtimeType = 'C';
+    print('$runtimeType');
+  }
+}
+''');
+  }
+
+  test_noToString() async {
+    await assertNoDiagnostics(r'''
+class C {
+  void f() {
+    runtimeType == runtimeType;
+  }
+}
+''');
+  }
+
+  test_toString_explicitThis() async {
+    await assertDiagnostics(r'''
+class C {
+  void f() {
+    this.runtimeType.toString();
+  }
+}
+''', [
+      lint(44, 8),
+    ]);
+  }
+
+  test_toString_expression() async {
+    await assertNoDiagnostics(r'''
+int x = 1;
+class C {
+  void f() {
+    x.runtimeType.toString();
+  }
+}
+''');
+  }
+
+  test_toString_expression_nullAware() async {
+    await assertNoDiagnostics(r'''
+class C {
+  void f(int? p) {
+    p?.runtimeType.toString();
+  }
+}
+''');
+  }
+
+  test_toString_super() async {
+    await assertDiagnostics(r'''
+class C {
+  void f() {
+    super.runtimeType.toString();
+  }
+}
+''', [
+      lint(45, 8),
+    ]);
+  }
+}

pkg/linter/test_data/rules/no_runtimeType_toString.dart

@@ -1,54 +0,0 @@
-// Copyright (c) 2020, 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.
-
-// test w/ `dart test -N no_runtimeType_toString`
-
-var o;
-
-class A {
-  var field;
-  String f() {
-    final s1 = '$runtimeType'; // LINT
-    final s2 = runtimeType.toString(); // LINT
-    final s3 = this.runtimeType.toString(); // LINT
-    final s4 = '${runtimeType}'; // LINT
-    final s5 = '${o.runtimeType}'; // OK
-    final s6 = o.runtimeType.toString(); // OK
-    final s7 = runtimeType == runtimeType; // OK
-    final s8 = field?.runtimeType?.toString(); // OK
-    try {
-      final s9 = '${runtimeType}'; // LINT
-    } catch (e) {
-      final s10 = '${runtimeType}'; // OK
-    }
-    final s11 = super.runtimeType.toString(); // LINT
-    throw '${runtimeType}'; // OK
-  }
-}
-
-abstract class B {
-  void f() {
-    final s1 = '$runtimeType'; // OK
-  }
-}
-
-mixin C {
-  void f() {
-    final s1 = '$runtimeType'; // OK
-  }
-}
-
-class D {
-  void f() {
-    var runtimeType = 'C';
-    print('$runtimeType'); // OK
-  }
-}
-
-extension on A {
-  String f() => '$runtimeType'; // LINT
-}
-extension on B {
-  String f() => '$runtimeType'; // OK
-}

pkg/linter/tool/machine/rules.json

@@ -1459,7 +1459,7 @@
     "incompatible": [],
     "sets": [],
     "fixStatus": "noFix",
-    "details": "Calling `toString` on a runtime type is a non-trivial operation that can\nnegatively impact performance. It's better to avoid it.\n\n**BAD:**\n```dart\nclass A {\n  String toString() => '$runtimeType()';\n}\n```\n\n**GOOD:**\n```dart\nclass A {\n  String toString() => 'A()';\n}\n```\n\nThis lint has some exceptions where performance is not a problem or where real\ntype information is more important than performance:\n\n* in assertion\n* in throw expressions\n* in catch clauses\n* in mixin declaration\n* in abstract class\n\n",
+    "details": "Calling `toString` on a runtime type is a non-trivial operation that can\nnegatively impact performance. It's better to avoid it.\n\n**BAD:**\n```dart\nclass A {\n  String toString() => '$runtimeType()';\n}\n```\n\n**GOOD:**\n```dart\nclass A {\n  String toString() => 'A()';\n}\n```\n\nThis lint has some exceptions where performance is not a problem or where real\ntype information is more important than performance:\n\n* in an assertion\n* in a throw expression\n* in a catch clause\n* in a mixin declaration\n* in an abstract class declaration\n\n",
     "sinceDartSdk": "2.8.1"
   },
   {