Skip to content
This repository has been archived by the owner on Jul 16, 2023. It is now read-only.

Commit

Permalink
fix: fixed issue with type check in `avoid-unnecessary-type-assertion…
Browse files Browse the repository at this point in the history 
…s` (#555)
  • Loading branch information
Konoshenko Vlad authored Nov 16, 2021
1 parent c871a40 commit 79d8277
Show file tree
Hide file tree
Showing 7 changed files with 254 additions and 105 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## Unreleased

* fix: fixed issue with type check in `avoid-unnecessary-type-assertions`
* feat: introduce file metrics
* feat: add static code diagnostics `avoid-unnecessary-type-assertions`
* refactor: cleanup anti-patterns, metrics and rules documentation
Expand Down
2 changes: 2 additions & 0 deletions .../rules_list/avoid_unnecessary_type_assertions/avoid_unnecessary_type_assertions_rule.dart
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/visitor.dart';
import 'package:analyzer/dart/element/nullability_suffix.dart';
import 'package:analyzer/dart/element/type.dart';
import 'package:collection/collection.dart';

import '../../../../../utils/node_utils.dart';
import '../../../lint_utils.dart';
Expand Down
129 changes: 72 additions & 57 deletions ...c/analyzers/lint_analyzer/rules/rules_list/avoid_unnecessary_type_assertions/visitor.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,21 @@ class _Visitor extends RecursiveAstVisitor<void> {

const methodName = 'whereType';

final isWhereTypeFunction = node.methodName.name == methodName;
if (isIterableOrSubclass(node.realTarget?.staticType) &&
isWhereTypeFunction &&
node.target?.staticType is InterfaceType) {
final interfaceType = node.target?.staticType as InterfaceType;
final isTypeHasGeneric = interfaceType.typeArguments.isNotEmpty;

final isCastedHasGeneric =
final isTargetIterable = isIterableOrSubclass(node.realTarget?.staticType);
final isWhereTypeInvocation = node.methodName.name == methodName;
final targetType = node.target?.staticType;

if (isTargetIterable &&
isWhereTypeInvocation &&
targetType is ParameterizedType) {
final isTargetTypeHasGeneric = targetType.typeArguments.isNotEmpty;
final isWhereTypeHasGeneric =
node.typeArguments?.arguments.isNotEmpty ?? false;
if (isTypeHasGeneric &&
isCastedHasGeneric &&

if (isTargetTypeHasGeneric &&
isWhereTypeHasGeneric &&
_isUselessTypeCheck(
interfaceType.typeArguments.first,
targetType.typeArguments.first,
node.typeArguments?.arguments.first.type,
)) {
_expressions[node] = methodName;
Expand All @@ -40,61 +42,74 @@ class _Visitor extends RecursiveAstVisitor<void> {
}
}

bool _isUselessTypeCheck(
DartType? objectType,
DartType? castedType,
) {
bool _isUselessTypeCheck(DartType? objectType, DartType? castedType) {
if (objectType == null || castedType == null) {
return false;
}

// Checked type name
final typeName = objectType.getDisplayString(withNullability: true);
// Casted type name with nullability
final castedNameNull = castedType.getDisplayString(withNullability: true);
// Casted type name without nullability
final castedName = castedType.getDisplayString(withNullability: false);
// Validation checks
final isTypeSame = '$typeName?' == castedNameNull || typeName == castedName;
final isTypeInheritor = _isInheritorType(objectType, castedNameNull);

final isTypeWithGeneric = objectType is InterfaceType &&
castedType is InterfaceType &&
_isTypeWithGeneric(objectType, castedType);

return isTypeSame || isTypeInheritor || isTypeWithGeneric;
if (_checkNullableCompatibility(objectType, castedType)) {
return false;
}

final objectCastedType =
_foundCastedTypeInObjectTypeHierarchy(objectType, castedType);
if (objectCastedType == null) {
return false;
}

if (!_checkGenerics(objectCastedType, castedType)) {
return false;
}

return true;
}

bool _isTypeWithGeneric(InterfaceType objectType, InterfaceType castedType) {
final objectTypeArguments = objectType.typeArguments;
final castedTypeArguments = castedType.typeArguments;
final isHasGeneric = objectTypeArguments.isNotEmpty;
final isCount = objectTypeArguments.length == castedTypeArguments.length;

if (isHasGeneric && isCount) {
if (castedType.element.name == objectType.element.name) {
for (var i = 0; i < objectTypeArguments.length; i++) {
final isCheckUseless = _isUselessTypeCheck(
objectTypeArguments[i],
castedTypeArguments[i],
);
if (!isCheckUseless) {
return false;
}
}

return true;
}
bool _checkNullableCompatibility(DartType objectType, DartType castedType) {
final isObjectTypeNullable =
objectType.nullabilitySuffix != NullabilitySuffix.none;
final isCastedTypeNullable =
castedType.nullabilitySuffix != NullabilitySuffix.none;

// Only one case `Type? is Type` always valid assertion case
return isObjectTypeNullable && !isCastedTypeNullable;
}

DartType? _foundCastedTypeInObjectTypeHierarchy(
DartType objectType,
DartType castedType,
) {
if (objectType.element == castedType.element) {
return objectType;
}

if (objectType is InterfaceType) {
return objectType.allSupertypes
.firstWhereOrNull((value) => value.element == castedType.element);
}

return false;
return null;
}

bool _isInheritorType(DartType objectType, String castedNameNull) =>
objectType is InterfaceType &&
objectType.allSupertypes
.any((value) => _isInheritor(value, castedNameNull));
bool _checkGenerics(DartType objectType, DartType castedType) {
if (objectType is! ParameterizedType || castedType is! ParameterizedType) {
return false;
}

bool _isInheritor(DartType? type, String typeName) =>
type?.getDisplayString(withNullability: false) == typeName;
if (objectType.typeArguments.length != castedType.typeArguments.length) {
return false;
}

for (var argumentIndex = 0;
argumentIndex < objectType.typeArguments.length;
argumentIndex++) {
if (!_isUselessTypeCheck(
objectType.typeArguments[argumentIndex],
castedType.typeArguments[argumentIndex],
)) {
return false;
}
}

return true;
}
}
100 changes: 85 additions & 15 deletions ...s_list/avoid_unnecessary_type_assertions/avoid_unnecessary_type_assertions_rule_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,58 +29,113 @@ void main() {
issues: issues,
startOffsets: [
120,
173,
228,
539,
584,
630,
672,
718,
1020,
1053,
1372,
1420,
1506,
1553,
1620,
1744,
1954,
2021,
2099,
2204,
2263,
],
startLines: [
6,
7,
8,
21,
22,
23,
24,
25,
38,
39,
59,
60,
62,
64,
65,
67,
77,
79,
82,
84,
85,
],
startColumns: [
20,
21,
21,
20,
20,
20,
20,
20,
20,
5,
18,
18,
18,
18,
18,
18,
18,
18,
18,
18,
18,
],
endOffsets: [
143,
198,
252,
555,
601,
643,
689,
731,
1039,
1076,
1393,
1442,
1525,
1585,
1653,
1774,
1965,
2034,
2121,
2228,
2291,
],
locationTexts: [
'regularString is String',
'nullableString is String?',
'regularString is String?',
'animal is Animal',
'cat is HomeAnimal',
'cat is Animal',
'dog is HomeAnimal',
'dog is Animal',
'myList is List<int>',
'myList.whereType<int>()',
'nonNullableCat is Cat',
'nonNullableCat is Cat?',
'nullableCat is Cat?',
'nonNullableCats.whereType<Cat>()',
'nonNullableCats.whereType<Cat?>()',
'nullableCats.whereType<Cat?>()',
'cat is Cat?',
'cat is Animal',
'dogs.whereType<Dog?>()',
'dogs.whereType<Animal>()',
'animals.whereType<Animal?>()',
],
messages: [
'Avoid unnecessary "is" assertion.',
Expand All @@ -91,6 +146,17 @@ void main() {
'Avoid unnecessary "is" assertion.',
'Avoid unnecessary "is" assertion.',
'Avoid unnecessary "is" assertion.',
'Avoid unnecessary "is" assertion.',
'Avoid unnecessary "is" assertion.',
'Avoid unnecessary "is" assertion.',
'Avoid unnecessary "is" assertion.',
'Avoid unnecessary "whereType" assertion.',
'Avoid unnecessary "whereType" assertion.',
'Avoid unnecessary "whereType" assertion.',
'Avoid unnecessary "is" assertion.',
'Avoid unnecessary "is" assertion.',
'Avoid unnecessary "whereType" assertion.',
'Avoid unnecessary "whereType" assertion.',
'Avoid unnecessary "whereType" assertion.',
],
);
Expand All @@ -102,26 +168,30 @@ void main() {

RuleTestHelper.verifyIssues(
issues: issues,
startOffsets: [121, 235, 279, 454, 486, 514, 566],
startLines: [10, 16, 19, 26, 27, 28, 29],
startColumns: [14, 14, 5, 5, 5, 5, 21],
endOffsets: [127, 253, 310, 473, 508, 537, 578],
startOffsets: [118, 290, 497, 572, 647, 832, 946, 1030, 1060],
startLines: [6, 12, 21, 24, 27, 38, 44, 49, 50],
startColumns: [3, 3, 3, 3, 3, 14, 14, 3, 3],
endOffsets: [149, 313, 519, 593, 669, 838, 964, 1049, 1082],
locationTexts: [
"['1', '2'].whereType<String?>()",
'[1, 2].whereType<int>()',
'a.whereType<String?>()',
'b.whereType<String>()',
'b.whereType<String?>()',
'b is A',
'regular is String?',
"['1', '2'].whereType<String?>()",
'myList is List<int>',
'myList is List<Object>',
'myList.whereType<int>()',
'a is dynamic',
],
messages: [
'Avoid unnecessary "is" assertion.',
'Avoid unnecessary "is" assertion.',
'Avoid unnecessary "whereType" assertion.',
'Avoid unnecessary "whereType" assertion.',
'Avoid unnecessary "whereType" assertion.',
'Avoid unnecessary "whereType" assertion.',
'Avoid unnecessary "whereType" assertion.',
'Avoid unnecessary "is" assertion.',
'Avoid unnecessary "is" assertion.',
'Avoid unnecessary "whereType" assertion.',
'Avoid unnecessary "is" assertion.',
'Avoid unnecessary "is" assertion.',
],
);
Expand Down
Loading

0 comments on commit 79d8277

Please sign in to comment.