Skip to content

Commit 83eb816

Browse files
FMorschelCommit Queue
authored and
Commit Queue
committed
[linter] Adds new switch_on_type lint
Fixes: #59546 Change-Id: Ib78714e03c1487376c961214910d10c24ec9f8ff Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/413600 Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]> Reviewed-by: Samuel Rawlins <[email protected]> Auto-Submit: Felipe Morschel <[email protected]>
1 parent 46348af commit 83eb816

File tree

9 files changed

+1141
-0
lines changed

9 files changed

+1141
-0
lines changed

pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2394,6 +2394,8 @@ LintCode.strict_top_level_inference_split_to_types:
23942394
Conceivably for variable declarations we can search some space (like the
23952395
library or the compilation unit) for what values are assigned, and suggest
23962396
the LUB of those value(s)' type(s).
2397+
LintCode.switch_on_type:
2398+
status: noFix
23972399
LintCode.test_types_in_equals:
23982400
status: noFix
23992401
LintCode.throw_in_finally:

pkg/linter/example/all.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ linter:
172172
- specify_nonobvious_local_variable_types
173173
- specify_nonobvious_property_types
174174
- strict_top_level_inference
175+
- switch_on_type
175176
- test_types_in_equals
176177
- throw_in_finally
177178
- tighten_type_of_initializing_formals

pkg/linter/lib/src/lint_codes.g.dart

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1542,6 +1542,12 @@ class LinterLintCode extends LintCode {
15421542
uniqueName: 'strict_top_level_inference_split_to_types',
15431543
);
15441544

1545+
static const LintCode switch_on_type = LinterLintCode(
1546+
LintNames.switch_on_type,
1547+
"Avoid switch statements on a 'Type'.",
1548+
correctionMessage: "Try using pattern matching on a variable instead.",
1549+
);
1550+
15451551
static const LintCode test_types_in_equals = LinterLintCode(
15461552
LintNames.test_types_in_equals,
15471553
"Missing type test for '{0}' in '=='.",

pkg/linter/lib/src/lint_names.g.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,8 @@ abstract final class LintNames {
461461

462462
static const String super_goes_last = 'super_goes_last';
463463

464+
static const String switch_on_type = 'switch_on_type';
465+
464466
static const String test_types_in_equals = 'test_types_in_equals';
465467

466468
static const String throw_in_finally = 'throw_in_finally';

pkg/linter/lib/src/rules.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ import 'rules/specify_nonobvious_local_variable_types.dart';
190190
import 'rules/specify_nonobvious_property_types.dart';
191191
import 'rules/strict_top_level_inference.dart';
192192
import 'rules/super_goes_last.dart';
193+
import 'rules/switch_on_type.dart';
193194
import 'rules/test_types_in_equals.dart';
194195
import 'rules/throw_in_finally.dart';
195196
import 'rules/tighten_type_of_initializing_formals.dart';
@@ -442,6 +443,7 @@ void registerLintRules() {
442443
..registerLintRule(SpecifyNonObviousLocalVariableTypes())
443444
..registerLintRule(SpecifyNonObviousPropertyTypes())
444445
..registerLintRule(StrictTopLevelInference())
446+
..registerLintRule(SwitchOnType())
445447
..registerLintRule(TestTypesInEquals())
446448
..registerLintRule(ThrowInFinally())
447449
..registerLintRule(TightenTypeOfInitializingFormals())
Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:analyzer/dart/analysis/features.dart';
6+
import 'package:analyzer/dart/ast/ast.dart';
7+
import 'package:analyzer/dart/ast/token.dart';
8+
import 'package:analyzer/dart/ast/visitor.dart';
9+
import 'package:analyzer/dart/element/element.dart';
10+
import 'package:analyzer/dart/element/type.dart';
11+
import 'package:analyzer/error/error.dart';
12+
13+
import '../analyzer.dart';
14+
import '../extensions.dart';
15+
16+
const _desc = "Avoid switch statements on a 'Type'.";
17+
18+
const _objectToStringName = 'toString';
19+
20+
class SwitchOnType extends LintRule {
21+
SwitchOnType() : super(name: LintNames.switch_on_type, description: _desc);
22+
23+
@override
24+
DiagnosticCode get diagnosticCode => LinterLintCode.switch_on_type;
25+
26+
@override
27+
LintCode get lintCode => LinterLintCode.switch_on_type;
28+
29+
@override
30+
void registerNodeProcessors(
31+
NodeLintRegistry registry,
32+
LinterContext context,
33+
) {
34+
if (!context.isEnabled(Feature.patterns)) return;
35+
var visitor = _Visitor(this, context);
36+
registry.addSwitchExpression(this, visitor);
37+
registry.addSwitchStatement(this, visitor);
38+
}
39+
}
40+
41+
class _Visitor extends SimpleAstVisitor<void> {
42+
final LintRule rule;
43+
44+
final LinterContext context;
45+
46+
/// The node where the lint will be reported.
47+
late AstNode node;
48+
49+
_Visitor(this.rule, this.context);
50+
51+
/// A reference to the [Type] type.
52+
///
53+
/// This is used to check if the type of the expression is assignable to
54+
/// [Type].
55+
///
56+
/// This shortens the code and avoids multiple calls to
57+
/// `context.typeProvider.typeType`.
58+
InterfaceType get _typeType => context.typeProvider.typeType;
59+
60+
@override
61+
void visitSwitchExpression(SwitchExpression node) {
62+
this.node = node.expression;
63+
_processExpression(node.expression);
64+
}
65+
66+
@override
67+
void visitSwitchStatement(SwitchStatement node) {
68+
this.node = node.expression;
69+
_processExpression(node.expression);
70+
}
71+
72+
/// Returns `true` if the [type] is assignable to [Type].
73+
bool _isAssignableToType(DartType? type) {
74+
if (type == null) return false;
75+
return context.typeSystem.isAssignableTo(type, _typeType);
76+
}
77+
78+
/// Processes the [expression] of a [SwitchStatement] or [SwitchExpression].
79+
///
80+
/// Returns `true` if the lint was reportred and `false` otherwise.
81+
bool _processExpression(Expression expression) {
82+
if (expression case StringInterpolation(:var elements)) {
83+
return _processInterpolation(elements);
84+
}
85+
if (expression case ConditionalExpression(
86+
:var thenExpression,
87+
:var elseExpression,
88+
)) {
89+
return _processExpression(thenExpression) ||
90+
_processExpression(elseExpression);
91+
}
92+
if (expression case SwitchExpression(:var cases)) {
93+
for (var caseClause in cases) {
94+
if (_processExpression(caseClause.expression)) {
95+
return true;
96+
}
97+
}
98+
return false;
99+
}
100+
if (expression case BinaryExpression(
101+
:var leftOperand,
102+
:var rightOperand,
103+
:var operator,
104+
) when operator.lexeme == TokenType.PLUS.lexeme) {
105+
return _processExpression(leftOperand) ||
106+
_processExpression(rightOperand);
107+
}
108+
var type = switch (expression) {
109+
PrefixedIdentifier(:var identifier) => identifier.staticType,
110+
PropertyAccess(:var propertyName) => propertyName.staticType,
111+
SimpleIdentifier(:var staticType) => staticType,
112+
MethodInvocation(:var methodName, :var realTarget?) =>
113+
methodName.element.isToStringMethod ? realTarget.staticType : null,
114+
_ => null,
115+
};
116+
return _reportIfAssignableToType(type);
117+
}
118+
119+
/// Processes the [elements] of an [InterpolationExpression].
120+
///
121+
/// Returns `true` if the lint was reported and `false` otherwise.
122+
bool _processInterpolation(NodeList<InterpolationElement> elements) {
123+
for (var element in elements) {
124+
switch (element) {
125+
case InterpolationExpression(:var expression):
126+
var reported = _processExpression(expression);
127+
128+
// This return is necessary to avoid multiple reporting of the lint
129+
if (reported) {
130+
return true;
131+
}
132+
case InterpolationString():
133+
break;
134+
}
135+
}
136+
return false;
137+
}
138+
139+
/// Reports the lint if the [type] is assignable to [Type].
140+
///
141+
/// Returns `true` if the lint was reported and `false` otherwise.
142+
bool _reportIfAssignableToType(DartType? type) {
143+
var reported = false;
144+
if (_isAssignableToType(type)) {
145+
rule.reportAtNode(node);
146+
reported = true;
147+
}
148+
return reported;
149+
}
150+
}
151+
152+
extension on Element? {
153+
/// Returns `true` if this element is the `toString` method.
154+
bool get isToStringMethod {
155+
var self = this;
156+
return self is MethodElement && self.name3 == _objectToStringName;
157+
}
158+
}

pkg/linter/messages.yaml

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11344,6 +11344,82 @@ LintCode:
1134411344
: _children = children,
1134511345
super(style) {
1134611346
```
11347+
switch_on_type:
11348+
problemMessage: "Avoid switch statements on a 'Type'."
11349+
correctionMessage: "Try using pattern matching on a variable instead."
11350+
state:
11351+
stable: "3.0"
11352+
categories: [unintentional, style, languageFeatureUsage, errorProne]
11353+
hasPublishedDocs: false
11354+
documentation: |-
11355+
#### Description
11356+
11357+
The analyzer produces this diagnostic when a switch statement or switch
11358+
expression is used on either the value of a `Type` or a `toString` call
11359+
on a `Type`.
11360+
11361+
#### Example
11362+
11363+
The following code produces this diagnostic because the switch statement
11364+
is used on a `Type`:
11365+
11366+
```dart
11367+
void f(Object o) {
11368+
switch ([!o.runtimeType!]) {
11369+
case const (int):
11370+
print('int');
11371+
case const (String):
11372+
print('String');
11373+
}
11374+
}
11375+
```
11376+
11377+
#### Common fixes
11378+
11379+
Use pattern matching on the variable instead:
11380+
11381+
```dart
11382+
void f(Object o) {
11383+
switch (o) {
11384+
case int():
11385+
print('int');
11386+
case String():
11387+
print('String');
11388+
}
11389+
}
11390+
```
11391+
deprecatedDetails: |-
11392+
**AVOID** using switch on `Type`.
11393+
11394+
Switching on `Type` is not type-safe and can lead to bugs if the
11395+
class hierarchy changes. Prefer to use pattern matching on the variable
11396+
instead.
11397+
11398+
**BAD:**
11399+
```dart
11400+
void f(Object o) {
11401+
switch (o.runtimeType) {
11402+
case int:
11403+
print('int');
11404+
case String:
11405+
print('String');
11406+
}
11407+
}
11408+
```
11409+
11410+
**GOOD:**
11411+
```dart
11412+
void f(Object o) {
11413+
switch(o) {
11414+
case int():
11415+
print('int');
11416+
case String _:
11417+
print('String');
11418+
default:
11419+
print('other');
11420+
}
11421+
}
11422+
```
1134711423
test_types_in_equals:
1134811424
problemMessage: "Missing type test for '{0}' in '=='."
1134911425
correctionMessage: "Try testing the type of '{0}'."

pkg/linter/test/rules/all.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,7 @@ import 'specify_nonobvious_local_variable_types_test.dart'
242242
import 'specify_nonobvious_property_types_test.dart'
243243
as specify_nonobvious_property_types;
244244
import 'strict_top_level_inference_test.dart' as strict_top_level_inference;
245+
import 'switch_on_type_test.dart' as switch_on_type;
245246
import 'test_types_in_equals_test.dart' as test_types_in_equals;
246247
import 'throw_in_finally_test.dart' as throw_in_finally;
247248
import 'tighten_type_of_initializing_formals_test.dart'
@@ -502,6 +503,7 @@ void main() {
502503
specify_nonobvious_local_variable_types.main();
503504
specify_nonobvious_property_types.main();
504505
strict_top_level_inference.main();
506+
switch_on_type.main();
505507
test_types_in_equals.main();
506508
throw_in_finally.main();
507509
tighten_type_of_initializing_formals.main();

0 commit comments

Comments
 (0)