languages/dart-sdk
languages/dart-sdk
1
0
Fork 0
mirror of https://github.com/dart-lang/sdk synced 2024-09-16 03:07:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

Report HintCode.UNNECESSARY_TYPE_CHECK_TRUE for null safe code.

Browse source 
In legacy code `int v` was implicitly `int|Null v`, so `v is int` could
be useful (although probably should be `v != null` then). In null safe
code, we cannot have `Null`, so we can report a hint.

Change-Id: I60dc0791c6b8346d924fd13b1d2ce3fcceb22d32
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/190360
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This commit is contained in:
Konstantin Shcheglov 2021-09-16 06:43:07 +00:00 committed by commit-bot@chromium.org
parent 63f21ac34c
commit c4f4e80fe9
2 changed files with 236 additions and 78 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

99
pkg/analyzer/lib/src/error/best_practices_verifier.dart
View file

@ -798,71 +798,58 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
/// [HintCode.UNNECESSARY_TYPE_CHECK_TRUE], and
/// [HintCode.UNNECESSARY_TYPE_CHECK_FALSE].
bool _checkAllTypeChecks(IsExpression node) {
Expression expression = node.expression;
TypeAnnotation typeName = node.type;
var rhsType = typeName.type as TypeImpl;
var rhsNameStr = typeName is TypeName ? typeName.name.name : null;
// if x is dynamic
if (rhsType.isDynamic && rhsNameStr == Keyword.DYNAMIC.lexeme) {
if (node.notOperator == null) {
// the is case
_errorReporter.reportErrorForNode(
HintCode.UNNECESSARY_TYPE_CHECK_TRUE, node);
} else {
// the is not case
_errorReporter.reportErrorForNode(
HintCode.UNNECESSARY_TYPE_CHECK_FALSE, node);
}
return true;
var leftNode = node.expression;
var rightNode = node.type;
var rightType = rightNode.type as TypeImpl;
void report() {
_errorReporter.reportErrorForNode(
node.notOperator == null
? HintCode.UNNECESSARY_TYPE_CHECK_TRUE
: HintCode.UNNECESSARY_TYPE_CHECK_FALSE,
node,
);
}
// `is dynamic` or `is! dynamic`
if (rightType.isDynamic) {
var rightTypeStr = rightNode is TypeName ? rightNode.name.name : null;
if (rightTypeStr == Keyword.DYNAMIC.lexeme) {
report();
return true;
}
return false;
}
// `is Null` or `is! Null`
if (rhsType.isDartCoreNull) {
if (expression is NullLiteral) {
if (node.notOperator == null) {
_errorReporter.reportErrorForNode(
HintCode.UNNECESSARY_TYPE_CHECK_TRUE,
node,
);
} else {
_errorReporter.reportErrorForNode(
HintCode.UNNECESSARY_TYPE_CHECK_FALSE,
node,
);
}
if (rightType.isDartCoreNull) {
if (leftNode is NullLiteral) {
report();
} else {
if (node.notOperator == null) {
_errorReporter.reportErrorForNode(
HintCode.TYPE_CHECK_IS_NULL,
node,
);
} else {
_errorReporter.reportErrorForNode(
HintCode.TYPE_CHECK_IS_NOT_NULL,
node,
);
}
_errorReporter.reportErrorForNode(
node.notOperator == null
? HintCode.TYPE_CHECK_IS_NULL
: HintCode.TYPE_CHECK_IS_NOT_NULL,
node,
);
}
return true;
}
// `is Object` or `is! Object`
if (rhsType.isDartCoreObject) {
var nullability = rhsType.nullabilitySuffix;
if (nullability == NullabilitySuffix.star ||
nullability == NullabilitySuffix.question) {
if (node.notOperator == null) {
_errorReporter.reportErrorForNode(
HintCode.UNNECESSARY_TYPE_CHECK_TRUE,
node,
);
} else {
_errorReporter.reportErrorForNode(
HintCode.UNNECESSARY_TYPE_CHECK_FALSE,
node,
);
}
if (_isNonNullableByDefault) {
var leftType = leftNode.typeOrThrow;
if (_typeSystem.isSubtypeOf(leftType, rightType)) {
report();
return true;
}
} else {
// In legacy all types are subtypes of `Object`.
if (rightType.isDartCoreObject) {
report();
return true;
}
}
return false;
}

215
pkg/analyzer/test/src/diagnostics/unnecessary_type_check_test.dart
View file

@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
import 'package:analyzer/src/dart/error/hint_codes.dart';
import 'package:analyzer/src/error/codes.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';
import '../dart/resolution/context_collection_resolution.dart';
@ -19,16 +20,87 @@ main() {
@reflectiveTest
class UnnecessaryTypeCheckFalseTest extends PubPackageResolutionTest
with UnnecessaryTypeCheckFalseTestCases {
@override
test_type_not_object() async {
test_typeNonNullable_isNot_same() async {
await assertErrorsInCode(r'''
void f(int a) {
a is! int;
}
''', [
error(HintCode.UNNECESSARY_TYPE_CHECK_FALSE, 18, 9),
]);
}
test_typeNonNullable_isNot_subtype() async {
await assertNoErrorsInCode(r'''
void f<T>(T a) {
a is! Object;
void f(num a) {
a is! int;
}
''');
}
test_type_not_objectQuestion() async {
test_typeNonNullable_isNot_supertype() async {
await assertErrorsInCode(r'''
void f(int a) {
a is! num;
}
''', [
error(HintCode.UNNECESSARY_TYPE_CHECK_FALSE, 18, 9),
]);
}
test_typeNullable_isNot_same() async {
await assertErrorsInCode(r'''
void f(int? a) {
a is! int?;
}
''', [
error(HintCode.UNNECESSARY_TYPE_CHECK_FALSE, 19, 10),
]);
}
test_typeNullable_isNot_same_nonNullable() async {
await assertNoErrorsInCode(r'''
void f(int? a) {
a is! int;
}
''');
}
test_typeNullable_isNot_subtype() async {
await assertNoErrorsInCode(r'''
void f(num? a) {
a is! int?;
}
''');
}
test_typeNullable_isNot_subtype_nonNullable() async {
await assertNoErrorsInCode(r'''
void f(num? a) {
a is! int;
}
''');
}
test_typeNullable_isNot_supertype() async {
await assertErrorsInCode(r'''
void f(int? a) {
a is! num?;
}
''', [
error(HintCode.UNNECESSARY_TYPE_CHECK_FALSE, 19, 10),
]);
}
test_typeNullable_isNot_supertype_nonNullable() async {
await assertNoErrorsInCode(r'''
void f(int? a) {
a is! num;
}
''');
}
test_typeParameter_isNot_objectQuestion() async {
await assertErrorsInCode(r'''
void f<T>(T a) {
a is! Object?;
@ -40,7 +112,7 @@ void f<T>(T a) {
}
mixin UnnecessaryTypeCheckFalseTestCases on PubPackageResolutionTest {
test_null_not_Null() async {
test_null_isNot_Null() async {
await assertErrorsInCode(r'''
var b = null is! Null;
''', [
@ -48,7 +120,7 @@ var b = null is! Null;
]);
}
test_type_not_dynamic() async {
test_typeParameter_isNot_dynamic() async {
await assertErrorsInCode(r'''
void f<T>(T a) {
a is! dynamic;
@ -58,35 +130,110 @@ void f<T>(T a) {
]);
}
test_type_not_object() async {
test_typeParameter_isNot_object() async {
var expectedErrors = expectedErrorsByNullability(
nullable: [],
legacy: [
error(HintCode.UNNECESSARY_TYPE_CHECK_FALSE, 19, 12),
],
);
await assertErrorsInCode(r'''
void f<T>(T a) {
a is! Object;
}
''', [
error(HintCode.UNNECESSARY_TYPE_CHECK_FALSE, 19, 12),
]);
''', expectedErrors);
}
}
@reflectiveTest
class UnnecessaryTypeCheckFalseWithoutNullSafetyTest
extends PubPackageResolutionTest
with UnnecessaryTypeCheckFalseTestCases, WithoutNullSafetyMixin {}
with WithoutNullSafetyMixin, UnnecessaryTypeCheckFalseTestCases {}
@reflectiveTest
class UnnecessaryTypeCheckTrueTest extends PubPackageResolutionTest
with UnnecessaryTypeCheckTrueTestCases {
@override
test_type_is_object() async {
test_typeNonNullable_is_same() async {
await assertErrorsInCode(r'''
void f(int a) {
a is int;
}
''', [
error(HintCode.UNNECESSARY_TYPE_CHECK_TRUE, 18, 8),
]);
}
test_typeNonNullable_is_subtype() async {
await assertNoErrorsInCode(r'''
void f<T>(T a) {
a is Object;
void f(num a) {
a is int;
}
''');
}
test_type_is_objectQuestion() async {
test_typeNonNullable_is_supertype() async {
await assertErrorsInCode(r'''
void f(int a) {
a is num;
}
''', [
error(HintCode.UNNECESSARY_TYPE_CHECK_TRUE, 18, 8),
]);
}
test_typeNullable_is_same() async {
await assertErrorsInCode(r'''
void f(int? a) {
a is int?;
}
''', [
error(HintCode.UNNECESSARY_TYPE_CHECK_TRUE, 19, 9),
]);
}
test_typeNullable_is_same_nonNullable() async {
await assertNoErrorsInCode(r'''
void f(int? a) {
a is int;
}
''');
}
test_typeNullable_is_subtype() async {
await assertNoErrorsInCode(r'''
void f(num? a) {
a is int?;
}
''');
}
test_typeNullable_is_subtype_nonNullable() async {
await assertNoErrorsInCode(r'''
void f(num? a) {
a is int;
}
''');
}
test_typeNullable_is_supertype() async {
await assertErrorsInCode(r'''
void f(int? a) {
a is num?;
}
''', [
error(HintCode.UNNECESSARY_TYPE_CHECK_TRUE, 19, 9),
]);
}
test_typeNullable_is_supertype_nonNullable() async {
await assertNoErrorsInCode(r'''
void f(int? a) {
a is num;
}
''');
}
test_typeParameter_is_objectQuestion() async {
await assertErrorsInCode(r'''
void f<T>(T a) {
a is Object?;
@ -108,6 +255,26 @@ var b = null is Null;
test_type_is_dynamic() async {
await assertErrorsInCode(r'''
void f(int a) {
a is dynamic;
}
''', [
error(HintCode.UNNECESSARY_TYPE_CHECK_TRUE, 18, 12),
]);
}
test_type_is_unresolved() async {
await assertErrorsInCode(r'''
void f(int a) {
a is Unresolved;
}
''', [
error(CompileTimeErrorCode.TYPE_TEST_WITH_UNDEFINED_NAME, 23, 10),
]);
}
test_typeParameter_is_dynamic() async {
await assertErrorsInCode(r'''
void f<T>(T a) {
a is dynamic;
}
@ -116,18 +283,22 @@ void f<T>(T a) {
]);
}
test_type_is_object() async {
test_typeParameter_is_object() async {
var expectedErrors = expectedErrorsByNullability(
nullable: [],
legacy: [
error(HintCode.UNNECESSARY_TYPE_CHECK_TRUE, 19, 11),
],
);
await assertErrorsInCode(r'''
void f<T>(T a) {
a is Object;
}
''', [
error(HintCode.UNNECESSARY_TYPE_CHECK_TRUE, 19, 11),
]);
''', expectedErrors);
}
}
@reflectiveTest
class UnnecessaryTypeCheckTrueWithoutNullSafetyTest
extends PubPackageResolutionTest
with UnnecessaryTypeCheckTrueTestCases, WithoutNullSafetyMixin {}
with WithoutNullSafetyMixin, UnnecessaryTypeCheckTrueTestCases {}