From a6edca6db74a5bb302f1334d6a4f77341c542aaa Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Tue, 24 Oct 2023 19:55:39 +0000 Subject: [PATCH] Only report unnecessary_cast when the cast type is exactly the static type Fixes https://github.com/dart-lang/sdk/issues/44411 Fixes https://github.com/dart-lang/sdk/issues/48984 Fixes https://github.com/dart-lang/sdk/issues/49269 Fixes https://github.com/dart-lang/sdk/issues/49550 Fixes https://github.com/dart-lang/sdk/issues/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 Commit-Queue: Samuel Rawlins --- .../src/error/best_practices_verifier.dart | 32 +---------------- .../cast_from_null_always_fails_test.dart | 12 +++---- .../diagnostics/unnecessary_cast_test.dart | 34 +++++++------------ .../test/src/task/strong/checker_test.dart | 16 --------- .../src/task/strong/inferred_type_test.dart | 10 ++---- 5 files changed, 19 insertions(+), 85 deletions(-) diff --git a/pkg/analyzer/lib/src/error/best_practices_verifier.dart b/pkg/analyzer/lib/src/error/best_practices_verifier.dart index 5b7fc97f715b..f7bde2d73906 100644 --- a/pkg/analyzer/lib/src/error/best_practices_verifier.dart +++ b/pkg/analyzer/lib/src/error/best_practices_verifier.dart @@ -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'; @@ -1677,39 +1676,10 @@ class BestPracticesVerifier extends RecursiveAstVisitor { } // 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; } diff --git a/pkg/analyzer/test/src/diagnostics/cast_from_null_always_fails_test.dart b/pkg/analyzer/test/src/diagnostics/cast_from_null_always_fails_test.dart index 5a4df9ec2b4f..82481574e720 100644 --- a/pkg/analyzer/test/src/diagnostics/cast_from_null_always_fails_test.dart +++ b/pkg/analyzer/test/src/diagnostics/cast_from_null_always_fails_test.dart @@ -83,13 +83,11 @@ void f(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 { @@ -102,15 +100,13 @@ void f(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 { diff --git a/pkg/analyzer/test/src/diagnostics/unnecessary_cast_test.dart b/pkg/analyzer/test/src/diagnostics/unnecessary_cast_test.dart index 80840677ef91..344b00cff13b 100644 --- a/pkg/analyzer/test/src/diagnostics/unnecessary_cast_test.dart +++ b/pkg/analyzer/test/src/diagnostics/unnecessary_cast_test.dart @@ -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; } '''); } @@ -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 { @@ -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 { @@ -191,23 +186,19 @@ void f(num a) { } test_typeParameter_hasBound_same() async { - await assertErrorsInCode(r''' + await assertNoErrorsInCode(r''' void f(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 a) { a as num; } -''', [ - error(WarningCode.UNNECESSARY_CAST, 31, 8), - ]); +'''); } test_typeParameter_hasBound_unrelated() async { @@ -245,7 +236,6 @@ void f() { } ''', [ error(HintCode.IMPORT_OF_LEGACY_LIBRARY_INTO_NULL_SAFE, 7, 8), - error(WarningCode.UNNECESSARY_CAST, 39, 8), ]); } diff --git a/pkg/analyzer/test/src/task/strong/checker_test.dart b/pkg/analyzer/test/src/task/strong/checker_test.dart index 89378585d99d..147a6a6d934e 100644 --- a/pkg/analyzer/test/src/task/strong/checker_test.dart +++ b/pkg/analyzer/test/src/task/strong/checker_test.dart @@ -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), @@ -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), @@ -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), ]); } @@ -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), ]); } @@ -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), ]); } diff --git a/pkg/analyzer/test/src/task/strong/inferred_type_test.dart b/pkg/analyzer/test/src/task/strong/inferred_type_test.dart index 912eaeab6552..8aada95350d1 100644 --- a/pkg/analyzer/test/src/task/strong/inferred_type_test.dart +++ b/pkg/analyzer/test/src/task/strong/inferred_type_test.dart @@ -2547,7 +2547,6 @@ $declared foo() => new $declared.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(); @@ -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), ]); } @@ -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),