diff --git a/lib/lints/avoid_unnecessary_type_assertions/avoid_unnecessary_type_assertions_rule.dart b/lib/lints/avoid_unnecessary_type_assertions/avoid_unnecessary_type_assertions_rule.dart index c6774d42..84745b4a 100644 --- a/lib/lints/avoid_unnecessary_type_assertions/avoid_unnecessary_type_assertions_rule.dart +++ b/lib/lints/avoid_unnecessary_type_assertions/avoid_unnecessary_type_assertions_rule.dart @@ -3,7 +3,6 @@ import 'package:analyzer/dart/element/type.dart'; import 'package:analyzer/error/error.dart'; import 'package:analyzer/error/listener.dart'; import 'package:analyzer/source/source_range.dart'; -import 'package:collection/collection.dart'; import 'package:custom_lint_builder/custom_lint_builder.dart'; import 'package:solid_lints/models/rule_config.dart'; import 'package:solid_lints/models/solid_lint_rule.dart'; @@ -75,15 +74,25 @@ class AvoidUnnecessaryTypeAssertions extends SolidLintRule { final objectType = node.expression.staticType; final castedType = node.type.type; + if (objectType == null || castedType == null) { + return false; + } + + final typeCast = TypeCast( + source: objectType, + target: castedType, + isReversed: true, + ); + if (node.notOperator != null && - objectType != null && objectType is! TypeParameterType && objectType is! DynamicType && !objectType.isDartCoreObject && - _isUnnecessaryTypeCheck(objectType, castedType, isReversed: true)) { + typeCast.isUnnecessaryTypeCheck) { return true; } else { - return _isUnnecessaryTypeCheck(objectType, castedType); + final typeCast = TypeCast(source: objectType, target: castedType); + return typeCast.isUnnecessaryTypeCheck; } } @@ -98,74 +107,16 @@ class AvoidUnnecessaryTypeAssertions extends SolidLintRule { when targetType is ParameterizedType && isIterable(realTargetType) && arguments.isNotEmpty) { - return _isUnnecessaryTypeCheck( - targetType.typeArguments.first, - arguments.first.type, - ); - } else { - return false; - } - } - - /// Checks that type checking is unnecessary - /// [objectType] is the source expression type - /// [castedType] is the type against which the expression type is compared - /// [isReversed] true for opposite comparison, i.e 'is!' - /// and false for positive comparison, i.e. 'is' or 'whereType' - bool _isUnnecessaryTypeCheck( - DartType? objectType, - DartType? castedType, { - bool isReversed = false, - }) { - if (objectType == null || castedType == null) { - return false; - } + final objectType = targetType.typeArguments.first; + final castedType = arguments.first.type; - final typeCast = TypeCast( - source: objectType, - target: castedType, - ); - - if (_isNullableCompatibility(typeCast)) { - return false; - } - - final objectCastedType = typeCast.castTypeInHierarchy(); - - if (objectCastedType == null) { - return isReversed; - } - - final objectTypeCast = TypeCast( - source: objectCastedType, - target: castedType, - ); - if (!_areGenericsWithSameTypeArgs(objectTypeCast)) { - return false; - } - - return !isReversed; - } - - bool _isNullableCompatibility(TypeCast typeCast) { - final isObjectTypeNullable = isNullableType(typeCast.source); - final isCastedTypeNullable = isNullableType(typeCast.target); - - // Only one case `Type? is Type` always valid assertion case. - return isObjectTypeNullable && !isCastedTypeNullable; - } - - bool _areGenericsWithSameTypeArgs(TypeCast typeCast) { - if (typeCast - case TypeCast(source: final objectType, target: final castedType) - when objectType is ParameterizedType && - castedType is ParameterizedType) { - if (objectType.typeArguments.length != castedType.typeArguments.length) { + if (castedType == null) { return false; } - return IterableZip([objectType.typeArguments, castedType.typeArguments]) - .every((e) => _isUnnecessaryTypeCheck(e[0], e[1])); + final typeCast = TypeCast(source: objectType, target: castedType); + + return typeCast.isUnnecessaryTypeCheck; } else { return false; } diff --git a/lib/lints/avoid_unnecessary_type_casts/avoid_unnecessary_type_casts_fix.dart b/lib/lints/avoid_unnecessary_type_casts/avoid_unnecessary_type_casts_fix.dart new file mode 100644 index 00000000..4c8c578c --- /dev/null +++ b/lib/lints/avoid_unnecessary_type_casts/avoid_unnecessary_type_casts_fix.dart @@ -0,0 +1,39 @@ +part of 'avoid_unnecessary_type_casts_rule.dart'; + +/// A Quick fix for `avoid-unnecessary-type-assertions` rule +/// Suggests to remove unnecessary assertions +class _UnnecessaryTypeCastsFix extends DartFix { + @override + void run( + CustomLintResolver resolver, + ChangeReporter reporter, + CustomLintContext context, + AnalysisError analysisError, + List others, + ) { + context.registry.addAsExpression((node) { + if (analysisError.sourceRange.intersects(node.sourceRange)) { + _addDeletion(reporter, 'as', node, node.asOperator.offset); + } + }); + } + + void _addDeletion( + ChangeReporter reporter, + String itemToDelete, + Expression node, + int operatorOffset, + ) { + final targetNameLength = operatorOffset - node.offset; + final removedPartLength = node.length - targetNameLength; + + final changeBuilder = reporter.createChangeBuilder( + message: "Remove unnecessary '$itemToDelete'", + priority: 1, + ); + + changeBuilder.addDartFileEdit((builder) { + builder.addDeletion(SourceRange(operatorOffset, removedPartLength)); + }); + } +} diff --git a/lib/lints/avoid_unnecessary_type_casts/avoid_unnecessary_type_casts_rule.dart b/lib/lints/avoid_unnecessary_type_casts/avoid_unnecessary_type_casts_rule.dart new file mode 100644 index 00000000..97a3690c --- /dev/null +++ b/lib/lints/avoid_unnecessary_type_casts/avoid_unnecessary_type_casts_rule.dart @@ -0,0 +1,52 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/error/error.dart'; +import 'package:analyzer/error/listener.dart'; +import 'package:analyzer/source/source_range.dart'; +import 'package:custom_lint_builder/custom_lint_builder.dart'; +import 'package:solid_lints/lints/avoid_unnecessary_type_casts/avoid_unnecessary_type_casts_visitor.dart'; +import 'package:solid_lints/models/rule_config.dart'; +import 'package:solid_lints/models/solid_lint_rule.dart'; + +part 'avoid_unnecessary_type_casts_fix.dart'; + +/// A `avoid-unnecessary-type-casts` rule which +/// warns about unnecessary usage of `as` operator +class AvoidUnnecessaryTypeCastsRule extends SolidLintRule { + /// The [LintCode] of this lint rule that represents + /// the error whether we use bad formatted double literals. + static const lintName = 'avoid-unnecessary-type-casts'; + + AvoidUnnecessaryTypeCastsRule._(super.config); + + /// Creates a new instance of [AvoidUnnecessaryTypeCastsRule] + /// based on the lint configuration. + factory AvoidUnnecessaryTypeCastsRule.createRule(CustomLintConfigs configs) { + final rule = RuleConfig( + configs: configs, + name: lintName, + problemMessage: (_) => "Avoid unnecessary usage of as operator.", + ); + + return AvoidUnnecessaryTypeCastsRule._(rule); + } + + @override + void run( + CustomLintResolver resolver, + ErrorReporter reporter, + CustomLintContext context, + ) { + final visitor = AvoidUnnecessaryTypeCastsVisitor(); + + context.registry.addAsExpression((node) { + visitor.visitAsExpression(node); + + for (final element in visitor.expressions.entries) { + reporter.reportErrorForNode(code, element.key); + } + }); + } + + @override + List getFixes() => [_UnnecessaryTypeCastsFix()]; +} diff --git a/lib/lints/avoid_unnecessary_type_casts/avoid_unnecessary_type_casts_visitor.dart b/lib/lints/avoid_unnecessary_type_casts/avoid_unnecessary_type_casts_visitor.dart new file mode 100644 index 00000000..69d01579 --- /dev/null +++ b/lib/lints/avoid_unnecessary_type_casts/avoid_unnecessary_type_casts_visitor.dart @@ -0,0 +1,56 @@ +// MIT License +// +// Copyright (c) 2020-2021 Dart Code Checker team +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:solid_lints/utils/typecast_utils.dart'; + +/// AST Visitor which finds all as expressions and checks if they are +/// necessary +class AvoidUnnecessaryTypeCastsVisitor extends RecursiveAstVisitor { + final _expressions = {}; + + /// All as expressions + Map get expressions => _expressions; + + @override + void visitAsExpression(AsExpression node) { + super.visitAsExpression(node); + + final objectType = node.expression.staticType; + final castedType = node.type.type; + + if (objectType == null || castedType == null) { + return; + } + + final typeCast = TypeCast( + source: objectType, + target: castedType, + ); + + if (typeCast.isUnnecessaryTypeCheck) { + _expressions[node] = 'as'; + } + } +} diff --git a/lib/solid_lints.dart b/lib/solid_lints.dart index 41088533..5b437b10 100644 --- a/lib/solid_lints.dart +++ b/lib/solid_lints.dart @@ -7,6 +7,7 @@ import 'package:solid_lints/lints/avoid_non_null_assertion/avoid_non_null_assert import 'package:solid_lints/lints/avoid_returning_widgets/avoid_returning_widgets_rule.dart'; import 'package:solid_lints/lints/avoid_unnecessary_setstate/avoid_unnecessary_set_state_rule.dart'; import 'package:solid_lints/lints/avoid_unnecessary_type_assertions/avoid_unnecessary_type_assertions_rule.dart'; +import 'package:solid_lints/lints/avoid_unnecessary_type_casts/avoid_unnecessary_type_casts_rule.dart'; import 'package:solid_lints/lints/cyclomatic_complexity/cyclomatic_complexity_metric.dart'; import 'package:solid_lints/lints/double_literal_format/double_literal_format_rule.dart'; import 'package:solid_lints/lints/function_lines_of_code/function_lines_of_code_metric.dart'; @@ -31,6 +32,7 @@ class _SolidLints extends PluginBase { DoubleLiteralFormatRule.createRule(configs), AvoidUnnecessaryTypeAssertions.createRule(configs), AvoidUnnecessarySetStateRule.createRule(configs), + AvoidUnnecessaryTypeCastsRule.createRule(configs), ]; // Return only enabled rules diff --git a/lib/utils/typecast_utils.dart b/lib/utils/typecast_utils.dart index d329fd17..5de93092 100644 --- a/lib/utils/typecast_utils.dart +++ b/lib/utils/typecast_utils.dart @@ -1,3 +1,4 @@ +import 'package:analyzer/dart/element/nullability_suffix.dart'; import 'package:analyzer/dart/element/type.dart'; import 'package:collection/collection.dart'; @@ -16,9 +17,16 @@ class TypeCast { /// The type being tested for final DartType target; + /// Set to true for opposite comparison, i.e 'is!' + final bool isReversed; + /// Creates a new Typecast object with a given expression's or object's type /// and a tested type - TypeCast({required this.source, required this.target}); + TypeCast({ + required this.source, + required this.target, + this.isReversed = false, + }); /// Returns the first type from source's supertypes /// which is corresponding to target or null @@ -36,4 +44,57 @@ class TypeCast { return null; } + + /// Checks for nullable type casts + /// Only one case `Type? is Type` always valid assertion case. + bool get isNullableCompatibility { + final isObjectTypeNullable = + source.nullabilitySuffix != NullabilitySuffix.none; + final isCastedTypeNullable = + target.nullabilitySuffix != NullabilitySuffix.none; + + return isObjectTypeNullable && !isCastedTypeNullable; + } + + /// Checks that type checking is unnecessary + /// [source] is the source expression type + /// [target] is the type against which the expression type is compared + /// and false for positive comparison, i.e. 'is', 'as' or 'whereType' + bool get isUnnecessaryTypeCheck { + if (isNullableCompatibility) { + return false; + } + + final objectCastedType = castTypeInHierarchy(); + if (objectCastedType == null) { + return isReversed; + } + + final objectTypeCast = TypeCast( + source: objectCastedType, + target: target, + ); + if (!objectTypeCast.areGenericsWithSameTypeArgs) { + return false; + } + + return !isReversed; + } + + /// Checks for type arguments and compares them + bool get areGenericsWithSameTypeArgs { + if (this case TypeCast(source: final objectType, target: final castedType) + when objectType is ParameterizedType && + castedType is ParameterizedType) { + if (objectType.typeArguments.length != castedType.typeArguments.length) { + return false; + } + + return IterableZip([objectType.typeArguments, castedType.typeArguments]) + .map((e) => TypeCast(source: e[0], target: e[1])) + .every((cast) => cast.isUnnecessaryTypeCheck); + } else { + return false; + } + } } diff --git a/lint_test/analysis_options.yaml b/lint_test/analysis_options.yaml index f940145c..dd0e156d 100644 --- a/lint_test/analysis_options.yaml +++ b/lint_test/analysis_options.yaml @@ -17,3 +17,4 @@ custom_lint: - avoid_unnecessary_setstate - double-literal-format - avoid-unnecessary-type-assertions + - avoid-unnecessary-type-casts diff --git a/lint_test/avoid_unnecessary_type_casts_test.dart b/lint_test/avoid_unnecessary_type_casts_test.dart new file mode 100644 index 00000000..302353c1 --- /dev/null +++ b/lint_test/avoid_unnecessary_type_casts_test.dart @@ -0,0 +1,35 @@ +// ignore_for_file: prefer_const_declarations +// ignore_for_file: unnecessary_nullable_for_final_variable_declarations +// ignore_for_file: unnecessary_cast +// ignore_for_file: unused_local_variable + +/// Check the `avoid-unnecessary-type-casts` rule + +void fun() { + final testList = [1.0, 2.0, 3.0]; + + // to check quick-fix => testList + // expect_lint: avoid-unnecessary-type-casts + final result = testList as List; + + final double? nullableD = 2.0; + // casting `Type? is Type` is allowed + final castedD = nullableD as double; + + final testMap = {'A': 'B'}; + + // expect_lint: avoid-unnecessary-type-casts + final castedMapValue = testMap['A'] as String?; + + // casting `Type? is Type` is allowed + final castedNotNullMapValue = testMap['A'] as String; + + final testString = 'String'; + // expect_lint: avoid-unnecessary-type-casts + _testFun(testString as String); +} + +void _testFun(String a) { + // expect_lint: avoid-unnecessary-type-casts + final result = (a as String).length; +}