Skip to content

Commit

Permalink
Only report unnecessary_cast when the cast type is exactly the static…
Browse files Browse the repository at this point in the history 
… type

Fixes #44411
Fixes #48984
Fixes #49269
Fixes #49550
Fixes #52086

The "prefer type annotations on local variables over upcasting" that this used to cover can be covered with a new lint rule: https://github.com/dart-lang/linter/issues/3786

Change-Id: I6a112732269e918a7faacc399bccebe13de8462d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/265420
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
  • Loading branch information
@srawlins
srawlins authored and Commit Queue committed Oct 24, 2023
1 parent 7ba47c6 commit a6edca6
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 85 deletions.
32 changes: 1 addition & 31 deletions pkg/analyzer/lib/src/error/best_practices_verifier.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import 'package:analyzer/dart/ast/token.dart';
import 'package:analyzer/dart/ast/visitor.dart';
import 'package:analyzer/dart/constant/value.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/dart/element/nullability_suffix.dart';
import 'package:analyzer/dart/element/type.dart';
import 'package:analyzer/error/error.dart';
import 'package:analyzer/error/listener.dart';
Expand Down Expand Up @@ -1677,39 +1676,10 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
}

// The cast is necessary.
if (!typeSystem.isSubtypeOf(leftType, rightType)) {
if (leftType != rightType) {
return false;
}

// Casting from `T*` to `T?` is a way to force `T?`.
if (leftType.nullabilitySuffix == NullabilitySuffix.star &&
rightType.nullabilitySuffix == NullabilitySuffix.question) {
return false;
}

// For `condition ? then : else` the result type is `LUB`.
// Casts might be used to consider only a portion of the inheritance tree.
var parent = node.parent;
if (parent is ConditionalExpression) {
var other = node == parent.thenExpression
? parent.elseExpression
: parent.thenExpression;

var currentType = typeSystem.leastUpperBound(
node.typeOrThrow,
other.typeOrThrow,
);

var typeWithoutCast = typeSystem.leastUpperBound(
node.expression.typeOrThrow,
other.typeOrThrow,
);

if (typeWithoutCast != currentType) {
return false;
}
}

return true;
}

Expand Down
12 changes: 4 additions & 8 deletions pkg/analyzer/test/src/diagnostics/cast_from_null_always_fails_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,11 @@ void f<T extends Object>(Null n) {
}

test_Null_nullable() async {
await assertErrorsInCode('''
await assertNoErrorsInCode('''
void f(Null n) {
n as int?;
}
''', [
error(WarningCode.UNNECESSARY_CAST, 19, 9),
]);
''');
}

test_Null_nullableTypeVariable() async {
Expand All @@ -102,15 +100,13 @@ void f<T>(Null n) {

test_Null_preNullSafety() async {
noSoundNullSafety = false;
await assertErrorsInCode('''
await assertNoErrorsInCode('''
// @dart=2.9
void f(Null n) {
n as int;
}
''', [
error(WarningCode.UNNECESSARY_CAST, 33, 8),
]);
''');
}

test_nullable_nonNullable() async {
Expand Down
34 changes: 12 additions & 22 deletions pkg/analyzer/test/src/diagnostics/unnecessary_cast_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ class A {}
class B extends A {}
dynamic f(bool c, B x, B y) {
return c ? x as A : y;
var r = c ? x as A : y;
return r;
}
''');
}
Expand Down Expand Up @@ -127,23 +128,19 @@ void f(num Function() a) {
}

test_function_toSupertype_viaParameter() async {
await assertErrorsInCode(r'''
await assertNoErrorsInCode(r'''
void f(void Function(num) a) {
(a as void Function(int))(3);
}
''', [
error(WarningCode.UNNECESSARY_CAST, 34, 23),
]);
''');
}

test_function_toSupertype_viaReturnType() async {
await assertErrorsInCode(r'''
await assertNoErrorsInCode(r'''
void f(int Function() a) {
(a as num Function())();
}
''', [
error(WarningCode.UNNECESSARY_CAST, 30, 19),
]);
''');
}

test_function_toUnrelated() async {
Expand Down Expand Up @@ -171,13 +168,11 @@ void f() {
}

test_type_supertype() async {
await assertErrorsInCode(r'''
await assertNoErrorsInCode(r'''
void f(int a) {
a as Object;
}
''', [
error(WarningCode.UNNECESSARY_CAST, 18, 11),
]);
''');
}

test_type_type() async {
Expand All @@ -191,23 +186,19 @@ void f(num a) {
}

test_typeParameter_hasBound_same() async {
await assertErrorsInCode(r'''
await assertNoErrorsInCode(r'''
void f<T extends num>(T a) {
a as num;
}
''', [
error(WarningCode.UNNECESSARY_CAST, 31, 8),
]);
''');
}

test_typeParameter_hasBound_subtype() async {
await assertErrorsInCode(r'''
await assertNoErrorsInCode(r'''
void f<T extends int>(T a) {
a as num;
}
''', [
error(WarningCode.UNNECESSARY_CAST, 31, 8),
]);
''');
}

test_typeParameter_hasBound_unrelated() async {
Expand Down Expand Up @@ -245,7 +236,6 @@ void f() {
}
''', [
error(HintCode.IMPORT_OF_LEGACY_LIBRARY_INTO_NULL_SAFE, 7, 8),
error(WarningCode.UNNECESSARY_CAST, 39, 8),
]);
}

Expand Down
16 changes: 0 additions & 16 deletions pkg/analyzer/test/src/task/strong/checker_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -634,13 +634,11 @@ void main() {
error(CompileTimeErrorCode.INVALID_ASSIGNMENT, 1256, 4),
error(CompileTimeErrorCode.ARGUMENT_TYPE_NOT_ASSIGNABLE, 1331, 6),
error(CompileTimeErrorCode.ARGUMENT_TYPE_NOT_ASSIGNABLE, 1359, 4),
error(WarningCode.UNNECESSARY_CAST, 1526, 11),
error(HintCode.UNUSED_LOCAL_VARIABLE, 1591, 1),
error(CompileTimeErrorCode.INVALID_ASSIGNMENT, 1616, 6),
error(CompileTimeErrorCode.INVALID_CAST_FUNCTION, 1644, 4),
error(CompileTimeErrorCode.ARGUMENT_TYPE_NOT_ASSIGNABLE, 1735, 6),
error(CompileTimeErrorCode.INVALID_CAST_FUNCTION, 1763, 4),
error(WarningCode.UNNECESSARY_CAST, 1960, 11),
error(HintCode.UNUSED_LOCAL_VARIABLE, 2047, 1),
error(CompileTimeErrorCode.INVALID_ASSIGNMENT, 2088, 2),
error(CompileTimeErrorCode.INVALID_CAST_FUNCTION, 2100, 4),
Expand All @@ -650,7 +648,6 @@ void main() {
error(CompileTimeErrorCode.ARGUMENT_TYPE_NOT_ASSIGNABLE, 2255, 4),
error(CompileTimeErrorCode.ARGUMENT_TYPE_NOT_ASSIGNABLE, 2380, 10),
error(CompileTimeErrorCode.INVALID_CAST_FUNCTION_EXPR, 2400, 22),
error(WarningCode.UNNECESSARY_CAST, 2410, 11),
error(CompileTimeErrorCode.ARGUMENT_TYPE_NOT_ASSIGNABLE, 2450, 4),
error(HintCode.UNUSED_LOCAL_VARIABLE, 2495, 1),
error(CompileTimeErrorCode.INVALID_CAST_FUNCTION, 2520, 6),
Expand All @@ -661,7 +658,6 @@ void main() {
error(CompileTimeErrorCode.INVALID_CAST_FUNCTION, 2741, 4),
error(CompileTimeErrorCode.INVALID_CAST_FUNCTION_EXPR, 2914, 10),
error(CompileTimeErrorCode.INVALID_CAST_FUNCTION_EXPR, 2952, 22),
error(WarningCode.UNNECESSARY_CAST, 2962, 11),
]);
}

Expand Down Expand Up @@ -1340,30 +1336,20 @@ void main() {
error(HintCode.UNUSED_LOCAL_VARIABLE, 192, 1),
error(CompileTimeErrorCode.INVALID_ASSIGNMENT, 222, 7),
error(CompileTimeErrorCode.INVALID_ASSIGNMENT, 254, 3),
error(WarningCode.UNNECESSARY_CAST, 268, 13),
error(WarningCode.UNNECESSARY_CAST, 292, 15),
error(HintCode.UNUSED_LOCAL_VARIABLE, 328, 1),
error(CompileTimeErrorCode.INVALID_ASSIGNMENT, 340, 7),
error(CompileTimeErrorCode.INVALID_ASSIGNMENT, 376, 3),
error(WarningCode.UNNECESSARY_CAST, 404, 13),
error(WarningCode.UNNECESSARY_CAST, 428, 15),
error(HintCode.UNUSED_LOCAL_VARIABLE, 462, 1),
error(CompileTimeErrorCode.INVALID_ASSIGNMENT, 492, 7),
error(CompileTimeErrorCode.INVALID_ASSIGNMENT, 510, 3),
error(CompileTimeErrorCode.INVALID_ASSIGNMENT, 524, 3),
error(WarningCode.UNNECESSARY_CAST, 538, 13),
error(WarningCode.UNNECESSARY_CAST, 562, 15),
error(CompileTimeErrorCode.INVALID_ASSIGNMENT, 562, 15),
error(HintCode.UNUSED_LOCAL_VARIABLE, 596, 1),
error(CompileTimeErrorCode.INVALID_ASSIGNMENT, 608, 7),
error(CompileTimeErrorCode.INVALID_ASSIGNMENT, 644, 3),
error(CompileTimeErrorCode.INVALID_ASSIGNMENT, 658, 3),
error(WarningCode.UNNECESSARY_CAST, 672, 13),
error(CompileTimeErrorCode.INVALID_ASSIGNMENT, 696, 15),
error(WarningCode.UNNECESSARY_CAST, 696, 15),
error(HintCode.UNUSED_LOCAL_VARIABLE, 737, 1),
error(WarningCode.UNNECESSARY_CAST, 813, 13),
error(WarningCode.UNNECESSARY_CAST, 837, 15),
]);
}

Expand Down Expand Up @@ -2054,9 +2040,7 @@ void main() {
error(HintCode.UNUSED_LOCAL_VARIABLE, 365, 1),
error(HintCode.UNUSED_LOCAL_VARIABLE, 665, 1),
error(WarningCode.UNNECESSARY_CAST, 674, 10),
error(WarningCode.UNNECESSARY_CAST, 710, 10),
error(WarningCode.UNNECESSARY_CAST, 747, 11),
error(WarningCode.UNNECESSARY_CAST, 804, 11),
]);
}

Expand Down
10 changes: 2 additions & 8 deletions pkg/analyzer/test/src/task/strong/inferred_type_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2547,7 +2547,6 @@ $declared foo() => new $declared<int>.value(1);
error(HintCode.UNUSED_LOCAL_VARIABLE, 309, 2),
error(CompileTimeErrorCode.INVALID_ASSIGNMENT, 314, 1),
error(HintCode.UNUSED_LOCAL_VARIABLE, 475, 2),
error(WarningCode.UNNECESSARY_CAST, 480, 47),
],
);
await disposeAnalysisContextCollection();
Expand Down Expand Up @@ -3068,7 +3067,6 @@ main() {
contextMessages: [message('/home/test/lib/test.dart', 12, 1)]),
error(CompileTimeErrorCode.INVALID_OVERRIDE, 94, 1,
contextMessages: [message('/home/test/lib/test.dart', 33, 1)]),
error(WarningCode.UNNECESSARY_CAST, 132, 12),
]);
}

Expand Down Expand Up @@ -3601,12 +3599,8 @@ test1() {
error(CompileTimeErrorCode.UNDEFINED_IDENTIFIER, 171, 1),
error(CompileTimeErrorCode.UNDEFINED_IDENTIFIER, 201, 1),
error(CompileTimeErrorCode.UNDEFINED_OPERATOR, 572, 1),
error(
isNullSafetyEnabled
? WarningCode.CAST_FROM_NULL_ALWAYS_FAILS
: WarningCode.UNNECESSARY_CAST,
591,
9),
if (isNullSafetyEnabled)
error(WarningCode.CAST_FROM_NULL_ALWAYS_FAILS, 591, 9),
error(CompileTimeErrorCode.INVALID_ASSIGNMENT, 619, 4),
error(CompileTimeErrorCode.INVALID_ASSIGNMENT, 647, 4),
error(CompileTimeErrorCode.INVALID_ASSIGNMENT, 687, 2),
Expand Down

0 comments on commit a6edca6

Please sign in to comment.