From 377272db24ec9ee8da98b254912592bbd1855e40 Mon Sep 17 00:00:00 2001 From: Denis Bogatirov Date: Fri, 15 Sep 2023 19:36:05 +0300 Subject: [PATCH 1/8] Add prefer-conditional-expressions rule and fix --- ...er_conditional_expressions_parameters.dart | 21 +++ .../prefer_conditional_expressions_fix.dart | 117 +++++++++++++ .../prefer_conditional_expressions_rule.dart | 62 +++++++ ...refer_conditional_expressions_visitor.dart | 158 ++++++++++++++++++ lib/solid_lints.dart | 2 + 5 files changed, 360 insertions(+) create mode 100644 lib/lints/prefer_conditional_expressions/models/prefer_conditional_expressions_parameters.dart create mode 100644 lib/lints/prefer_conditional_expressions/prefer_conditional_expressions_fix.dart create mode 100644 lib/lints/prefer_conditional_expressions/prefer_conditional_expressions_rule.dart create mode 100644 lib/lints/prefer_conditional_expressions/prefer_conditional_expressions_visitor.dart diff --git a/lib/lints/prefer_conditional_expressions/models/prefer_conditional_expressions_parameters.dart b/lib/lints/prefer_conditional_expressions/models/prefer_conditional_expressions_parameters.dart new file mode 100644 index 00000000..3b4f50c7 --- /dev/null +++ b/lib/lints/prefer_conditional_expressions/models/prefer_conditional_expressions_parameters.dart @@ -0,0 +1,21 @@ +/// A data model class that represents the "prefer conditional expressions" +/// input parameters. +class PreferConditionalExpressionsParameters { + static const _ignoreNestedConfig = 'ignore-nested'; + + /// Should rule ignore nested if statements + final bool ignoreNested; + + /// Constructor for [PreferConditionalExpressionsParameters] model + const PreferConditionalExpressionsParameters({ + required this.ignoreNested, + }); + + /// Method for creating from json data + factory PreferConditionalExpressionsParameters.fromJson( + Map json, + ) => + PreferConditionalExpressionsParameters( + ignoreNested: json[_ignoreNestedConfig] as bool? ?? false, + ); +} diff --git a/lib/lints/prefer_conditional_expressions/prefer_conditional_expressions_fix.dart b/lib/lints/prefer_conditional_expressions/prefer_conditional_expressions_fix.dart new file mode 100644 index 00000000..98373bc5 --- /dev/null +++ b/lib/lints/prefer_conditional_expressions/prefer_conditional_expressions_fix.dart @@ -0,0 +1,117 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/token.dart'; +import 'package:analyzer/error/error.dart'; +import 'package:analyzer/source/source_range.dart'; +import 'package:custom_lint_builder/custom_lint_builder.dart'; +import 'package:solid_lints/lints/prefer_conditional_expressions/prefer_conditional_expressions_visitor.dart'; + +/// A Quick fix for `avoid-unnecessary-type-assertions` rule +/// Suggests to remove unnecessary assertions +class PreferConditionalExpressionsFix extends DartFix { + @override + void run( + CustomLintResolver resolver, + ChangeReporter reporter, + CustomLintContext context, + AnalysisError analysisError, + List others, + ) { + context.registry.addIfStatement((node) { + if (analysisError.sourceRange.intersects(node.sourceRange)) { + final statementInfo = analysisError.data as StatementInfo?; + if (statementInfo == null) return; + + final correction = _createCorrection(statementInfo); + if (correction == null) return; + + _addReplacement(reporter, statementInfo.statement, correction); + } + }); + } + + void _addReplacement( + ChangeReporter reporter, + IfStatement node, + String correction, + ) { + final changeBuilder = reporter.createChangeBuilder( + message: "Convert to conditional expression.", + priority: 1, + ); + + changeBuilder.addDartFileEdit((builder) { + builder.addSimpleReplacement( + SourceRange(node.offset, node.length), + correction, + ); + }); + } + + String? _createCorrection(StatementInfo info) { + final thenStatement = info.unwrappedThenStatement; + final elseStatement = info.unwrappedElseStatement; + + final condition = info.statement.expression; + + if (thenStatement is AssignmentExpression && + elseStatement is AssignmentExpression) { + final target = thenStatement.leftHandSide; + final firstExpression = thenStatement.rightHandSide; + final secondExpression = elseStatement.rightHandSide; + + final thenStatementOperator = thenStatement.operator.type; + final elseStatementOperator = elseStatement.operator.type; + + if (_isAssignmentOperatorNotEq(thenStatementOperator) && + _isAssignmentOperatorNotEq(elseStatementOperator)) { + final prefix = thenStatement.leftHandSide; + final thenPart = + '$prefix ${thenStatementOperator.stringValue} $firstExpression'; + final elsePart = + '$prefix ${elseStatementOperator.stringValue} $secondExpression;'; + + return '$condition ? $thenPart : $elsePart'; + } + + final correctionForLiterals = _createCorrectionForLiterals( + condition, + firstExpression, + secondExpression, + ); + + return '$target = $correctionForLiterals'; + } + + if (thenStatement is ReturnStatement && elseStatement is ReturnStatement) { + final firstExpression = thenStatement.expression; + final secondExpression = elseStatement.expression; + final correction = _createCorrectionForLiterals( + condition, + firstExpression, + secondExpression, + ); + + return 'return $correction'; + } + + return null; + } + + String _createCorrectionForLiterals( + Expression condition, + Expression? firstExpression, + Expression? secondExpression, + ) { + if (firstExpression is BooleanLiteral && + secondExpression is BooleanLiteral) { + final isInverted = !firstExpression.value && secondExpression.value; + + return '${isInverted ? "!" : ""}$condition;'; + } + + return '$condition ? $firstExpression : $secondExpression;'; + } + + bool _isAssignmentOperatorNotEq(TokenType token) => + token.isAssignmentOperator && token != TokenType.EQ; +} diff --git a/lib/lints/prefer_conditional_expressions/prefer_conditional_expressions_rule.dart b/lib/lints/prefer_conditional_expressions/prefer_conditional_expressions_rule.dart new file mode 100644 index 00000000..3d06698f --- /dev/null +++ b/lib/lints/prefer_conditional_expressions/prefer_conditional_expressions_rule.dart @@ -0,0 +1,62 @@ +import 'package:analyzer/error/listener.dart'; +import 'package:custom_lint_builder/custom_lint_builder.dart'; +import 'package:solid_lints/lints/prefer_conditional_expressions/models/prefer_conditional_expressions_parameters.dart'; +import 'package:solid_lints/lints/prefer_conditional_expressions/prefer_conditional_expressions_fix.dart'; +import 'package:solid_lints/lints/prefer_conditional_expressions/prefer_conditional_expressions_visitor.dart'; +import 'package:solid_lints/models/rule_config.dart'; +import 'package:solid_lints/models/solid_lint_rule.dart'; + +// Inspired by TSLint (https://palantir.github.io/tslint/rules/prefer-conditional-expression/) + +/// A `prefer-conditional-expressions` rule which warns about +/// simple if statements that can be replaced with conditional expressions +class PreferConditionalExpressionsRule + extends SolidLintRule { + /// The [LintCode] of this lint rule that represents the error if number of + /// parameters reaches the maximum value. + static const lintName = 'prefer-conditional-expressions'; + + PreferConditionalExpressionsRule._(super.config); + + /// Creates a new instance of [PreferConditionalExpressionsRule] + /// based on the lint configuration. + factory PreferConditionalExpressionsRule.createRule( + CustomLintConfigs configs, + ) { + final config = RuleConfig( + configs: configs, + name: lintName, + paramsParser: PreferConditionalExpressionsParameters.fromJson, + problemMessage: (value) => 'Prefer conditional expression.', + ); + + return PreferConditionalExpressionsRule._(config); + } + + @override + void run( + CustomLintResolver resolver, + ErrorReporter reporter, + CustomLintContext context, + ) { + context.registry.addCompilationUnit((node) { + final visitor = PreferConditionalExpressionsVisitor( + ignoreNested: config.parameters.ignoreNested, + ); + node.accept(visitor); + + for (final element in visitor.statementsInfo) { + reporter.reportErrorForNode( + code, + element.statement, + null, + null, + element, + ); + } + }); + } + + @override + List getFixes() => [PreferConditionalExpressionsFix()]; +} diff --git a/lib/lints/prefer_conditional_expressions/prefer_conditional_expressions_visitor.dart b/lib/lints/prefer_conditional_expressions/prefer_conditional_expressions_visitor.dart new file mode 100644 index 00000000..c4df3ff4 --- /dev/null +++ b/lib/lints/prefer_conditional_expressions/prefer_conditional_expressions_visitor.dart @@ -0,0 +1,158 @@ +// 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'; + +/// The AST visitor that will collect all if statements that can be simplified +/// into conditional expressions. +class PreferConditionalExpressionsVisitor extends RecursiveAstVisitor { + final _statementsInfo = []; + + final bool _ignoreNested; + + /// List of statement info that represents all simple if statements + Iterable get statementsInfo => _statementsInfo; + + /// Creates instance of [PreferConditionalExpressionsVisitor] + PreferConditionalExpressionsVisitor({ + required bool ignoreNested, + }) : _ignoreNested = ignoreNested; + + @override + void visitIfStatement(IfStatement node) { + super.visitIfStatement(node); + + if (_ignoreNested) { + final visitor = _ConditionalsVisitor(); + node.visitChildren(visitor); + + if (visitor.hasInnerConditionals) { + return; + } + } + + if (node.parent is! IfStatement && + node.elseStatement != null && + node.elseStatement is! IfStatement) { + _checkBothAssignment(node); + _checkBothReturn(node); + } + } + + void _checkBothAssignment(IfStatement statement) { + final thenAssignment = _getAssignmentExpression(statement.thenStatement); + final elseAssignment = _getAssignmentExpression(statement.elseStatement); + + if (thenAssignment != null && + elseAssignment != null && + _haveEqualNames(thenAssignment, elseAssignment)) { + _statementsInfo.add( + StatementInfo( + statement: statement, + unwrappedThenStatement: thenAssignment, + unwrappedElseStatement: elseAssignment, + ), + ); + } + } + + AssignmentExpression? _getAssignmentExpression(Statement? statement) { + if (statement is ExpressionStatement && + statement.expression is AssignmentExpression) { + return statement.expression as AssignmentExpression; + } + + if (statement is Block && statement.statements.length == 1) { + return _getAssignmentExpression(statement.statements.first); + } + + return null; + } + + bool _haveEqualNames( + AssignmentExpression thenAssignment, + AssignmentExpression elseAssignment, + ) => + thenAssignment.leftHandSide is Identifier && + elseAssignment.leftHandSide is Identifier && + (thenAssignment.leftHandSide as Identifier).name == + (elseAssignment.leftHandSide as Identifier).name; + + void _checkBothReturn(IfStatement statement) { + final thenReturn = _getReturnStatement(statement.thenStatement); + final elseReturn = _getReturnStatement(statement.elseStatement); + + if (thenReturn != null && elseReturn != null) { + _statementsInfo.add( + StatementInfo( + statement: statement, + unwrappedThenStatement: thenReturn, + unwrappedElseStatement: elseReturn, + ), + ); + } + } + + ReturnStatement? _getReturnStatement(Statement? statement) { + if (statement is ReturnStatement) { + return statement; + } + + if (statement is Block && statement.statements.length == 1) { + return _getReturnStatement(statement.statements.first); + } + + return null; + } +} + +class _ConditionalsVisitor extends RecursiveAstVisitor { + bool hasInnerConditionals = false; + + @override + void visitConditionalExpression(ConditionalExpression node) { + hasInnerConditionals = true; + + super.visitConditionalExpression(node); + } +} + +/// Data class contains info required for fix +class StatementInfo { + /// If statement node + final IfStatement statement; + + /// Contents of if block + final AstNode unwrappedThenStatement; + + /// Contents of else block + final AstNode unwrappedElseStatement; + + /// Creates instance of an [StatementInfo] + const StatementInfo({ + required this.statement, + required this.unwrappedThenStatement, + required this.unwrappedElseStatement, + }); +} diff --git a/lib/solid_lints.dart b/lib/solid_lints.dart index eb09549c..910ef74e 100644 --- a/lib/solid_lints.dart +++ b/lib/solid_lints.dart @@ -16,6 +16,7 @@ import 'package:solid_lints/lints/newline_before_return/newline_before_return_ru import 'package:solid_lints/lints/no_empty_block/no_empty_block_rule.dart'; import 'package:solid_lints/lints/no_equal_then_else/no_equal_then_else_rule.dart'; import 'package:solid_lints/lints/number_of_parameters/number_of_parameters_metric.dart'; +import 'package:solid_lints/lints/prefer_conditional_expressions/prefer_conditional_expressions_rule.dart'; import 'package:solid_lints/models/solid_lint_rule.dart'; /// Creates a plugin for our custom linter @@ -41,6 +42,7 @@ class _SolidLints extends PluginBase { NewlineBeforeReturnRule.createRule(configs), NoEmptyBlockRule.createRule(configs), NoEqualThenElseRule.createRule(configs), + PreferConditionalExpressionsRule.createRule(configs), ]; // Return only enabled rules From 319c2c4d8477d0e175cc9cd20c67c4e1a797f9f6 Mon Sep 17 00:00:00 2001 From: Denis Bogatirov Date: Fri, 15 Sep 2023 19:36:43 +0300 Subject: [PATCH 2/8] Add tests for prefer-conditional-expressions rule --- lint_test/analysis_options.yaml | 1 + lint_test/no_equal_then_else_test.dart | 1 + .../analysis_options.yaml | 8 ++++ ...tional_expressions_ignore_nested_test.dart | 15 ++++++ .../pubspec.yaml | 15 ++++++ .../prefer_conditional_expressions_test.dart | 46 +++++++++++++++++++ 6 files changed, 86 insertions(+) create mode 100644 lint_test/prefer_conditional_expressions_ignore_nested_test/analysis_options.yaml create mode 100644 lint_test/prefer_conditional_expressions_ignore_nested_test/prefer_conditional_expressions_ignore_nested_test.dart create mode 100644 lint_test/prefer_conditional_expressions_ignore_nested_test/pubspec.yaml create mode 100644 lint_test/prefer_conditional_expressions_test.dart diff --git a/lint_test/analysis_options.yaml b/lint_test/analysis_options.yaml index 89028572..d59878ba 100644 --- a/lint_test/analysis_options.yaml +++ b/lint_test/analysis_options.yaml @@ -22,3 +22,4 @@ custom_lint: - newline-before-return - no-empty-block - no-equal-then-else + - prefer-conditional-expressions diff --git a/lint_test/no_equal_then_else_test.dart b/lint_test/no_equal_then_else_test.dart index 74930dfd..c7491463 100644 --- a/lint_test/no_equal_then_else_test.dart +++ b/lint_test/no_equal_then_else_test.dart @@ -1,5 +1,6 @@ // ignore_for_file: unused_local_variable // ignore_for_file: cyclomatic_complexity +// ignore_for_file: prefer-conditional-expressions /// Check the `no-equal-then-else` rule void fun() { diff --git a/lint_test/prefer_conditional_expressions_ignore_nested_test/analysis_options.yaml b/lint_test/prefer_conditional_expressions_ignore_nested_test/analysis_options.yaml new file mode 100644 index 00000000..5248b53b --- /dev/null +++ b/lint_test/prefer_conditional_expressions_ignore_nested_test/analysis_options.yaml @@ -0,0 +1,8 @@ +analyzer: + plugins: + - custom_lint + +custom_lint: + rules: + - prefer-conditional-expressions: + ignore-nested: true diff --git a/lint_test/prefer_conditional_expressions_ignore_nested_test/prefer_conditional_expressions_ignore_nested_test.dart b/lint_test/prefer_conditional_expressions_ignore_nested_test/prefer_conditional_expressions_ignore_nested_test.dart new file mode 100644 index 00000000..8c60e7ae --- /dev/null +++ b/lint_test/prefer_conditional_expressions_ignore_nested_test/prefer_conditional_expressions_ignore_nested_test.dart @@ -0,0 +1,15 @@ +// ignore_for_file: unused_local_variable +// ignore_for_file: cyclomatic_complexity +// ignore_for_file: no-equal-then-else + +/// Check the `prefer-conditional-expressions` rule ignore-nested option +void fun() { + int _result = 0; + + // Allowed because ignore-nested flag is set tot true + if (1 > 0) { + _result = 1 > 2 ? 2 : 1; + } else { + _result = 0; + } +} diff --git a/lint_test/prefer_conditional_expressions_ignore_nested_test/pubspec.yaml b/lint_test/prefer_conditional_expressions_ignore_nested_test/pubspec.yaml new file mode 100644 index 00000000..188938a9 --- /dev/null +++ b/lint_test/prefer_conditional_expressions_ignore_nested_test/pubspec.yaml @@ -0,0 +1,15 @@ +name: prefer_conditional_expressions_ignore_nested_test +description: A starting point for Dart libraries or applications. +publish_to: none + +environment: + sdk: '>=3.0.0 <4.0.0' + +dependencies: + flutter: + sdk: flutter + +dev_dependencies: + solid_lints: + path: ../../ + test: ^1.20.1 diff --git a/lint_test/prefer_conditional_expressions_test.dart b/lint_test/prefer_conditional_expressions_test.dart new file mode 100644 index 00000000..08645241 --- /dev/null +++ b/lint_test/prefer_conditional_expressions_test.dart @@ -0,0 +1,46 @@ +// ignore_for_file: unused_local_variable +// ignore_for_file: cyclomatic_complexity +// ignore_for_file: no-equal-then-else +// ignore_for_file: dead_code + +/// Check the `prefer-conditional-expressions` rule +void fun() { + int _result = 0; + + // expect_lint: prefer-conditional-expressions + if (true) { + _result = 1; + } else { + _result = 2; + } + + // expect_lint: prefer-conditional-expressions + if (1 > 0) + _result = 1; + else + _result = 2; + + // expect_lint: prefer-conditional-expressions + if (1 > 0) { + _result = 1 > 2 ? 2 : 1; + } else { + _result = 0; + } +} + +int someFun() { + // expect_lint: prefer-conditional-expressions + if (1 == 1) { + return 0; + } else { + return 1; + } +} + +int anotherFun() { + // expect_lint: prefer-conditional-expressions + if (1 > 0) + return 1; + else + return 2; +} From 7b237f358fea7dc427d7e2846d812a91431f1f3d Mon Sep 17 00:00:00 2001 From: Denis Bogatirov Date: Fri, 15 Sep 2023 19:50:26 +0300 Subject: [PATCH 3/8] fix nested test plugin path --- .../analysis_options.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lint_test/prefer_conditional_expressions_ignore_nested_test/analysis_options.yaml b/lint_test/prefer_conditional_expressions_ignore_nested_test/analysis_options.yaml index 5248b53b..957617b9 100644 --- a/lint_test/prefer_conditional_expressions_ignore_nested_test/analysis_options.yaml +++ b/lint_test/prefer_conditional_expressions_ignore_nested_test/analysis_options.yaml @@ -1,6 +1,6 @@ analyzer: plugins: - - custom_lint + - ../custom_lint custom_lint: rules: From 3a67e1b8afe4452fc3d7c4be1d6939eaa125599a Mon Sep 17 00:00:00 2001 From: Denis Bogatirov Date: Fri, 15 Sep 2023 19:54:27 +0300 Subject: [PATCH 4/8] Fix tests after merge --- lint_test/prefer_conditional_expressions_test.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/lint_test/prefer_conditional_expressions_test.dart b/lint_test/prefer_conditional_expressions_test.dart index 08645241..8196ddc5 100644 --- a/lint_test/prefer_conditional_expressions_test.dart +++ b/lint_test/prefer_conditional_expressions_test.dart @@ -2,6 +2,7 @@ // ignore_for_file: cyclomatic_complexity // ignore_for_file: no-equal-then-else // ignore_for_file: dead_code +// ignore_for_file: no-magic-number /// Check the `prefer-conditional-expressions` rule void fun() { From c19fb0e9ec0d23bab43c83f6dfde3ba721a6f2d2 Mon Sep 17 00:00:00 2001 From: Denis Bogatirov Date: Wed, 20 Sep 2023 11:56:39 +0300 Subject: [PATCH 5/8] Add prefer-first rule and fix --- lib/lints/prefer_first/prefer_first_fix.dart | 67 +++++++++++++++++++ lib/lints/prefer_first/prefer_first_rule.dart | 48 +++++++++++++ .../prefer_first/prefer_first_visitor.dart | 40 +++++++++++ lib/solid_lints.dart | 2 + lib/utils/types_utils.dart | 7 ++ 5 files changed, 164 insertions(+) create mode 100644 lib/lints/prefer_first/prefer_first_fix.dart create mode 100644 lib/lints/prefer_first/prefer_first_rule.dart create mode 100644 lib/lints/prefer_first/prefer_first_visitor.dart diff --git a/lib/lints/prefer_first/prefer_first_fix.dart b/lib/lints/prefer_first/prefer_first_fix.dart new file mode 100644 index 00000000..c1557c97 --- /dev/null +++ b/lib/lints/prefer_first/prefer_first_fix.dart @@ -0,0 +1,67 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/error/error.dart'; +import 'package:analyzer/source/source_range.dart'; +import 'package:custom_lint_builder/custom_lint_builder.dart'; + +/// A Quick fix for `prefer-first` rule +/// Suggests to replace iterable access expressions +class PreferFirstFix extends DartFix { + static const _replaceComment = "Replace with 'first'."; + + @override + void run( + CustomLintResolver resolver, + ChangeReporter reporter, + CustomLintContext context, + AnalysisError analysisError, + List others, + ) { + context.registry.addMethodInvocation((node) { + if (analysisError.sourceRange.intersects(node.sourceRange)) { + final correction = _createCorrection(node); + + _addReplacement(reporter, node, correction); + } + }); + + context.registry.addIndexExpression((node) { + if (analysisError.sourceRange.intersects(node.sourceRange)) { + final correction = _createCorrection(node); + + _addReplacement(reporter, node, correction); + } + }); + } + + String _createCorrection(Expression expression) { + if (expression is MethodInvocation) { + return expression.isCascaded + ? '..first' + : '${expression.target ?? ''}.first'; + } else if (expression is IndexExpression) { + return expression.isCascaded + ? '..first' + : '${expression.target ?? ''}.first'; + } else { + return '.first'; + } + } + + void _addReplacement( + ChangeReporter reporter, + Expression node, + String correction, + ) { + final changeBuilder = reporter.createChangeBuilder( + message: _replaceComment, + priority: 1, + ); + + changeBuilder.addDartFileEdit((builder) { + builder.addSimpleReplacement( + SourceRange(node.offset, node.length), + correction, + ); + }); + } +} diff --git a/lib/lints/prefer_first/prefer_first_rule.dart b/lib/lints/prefer_first/prefer_first_rule.dart new file mode 100644 index 00000000..c9e33f26 --- /dev/null +++ b/lib/lints/prefer_first/prefer_first_rule.dart @@ -0,0 +1,48 @@ +import 'package:analyzer/error/listener.dart'; +import 'package:custom_lint_builder/custom_lint_builder.dart'; +import 'package:solid_lints/lints/prefer_first/prefer_first_fix.dart'; +import 'package:solid_lints/lints/prefer_first/prefer_first_visitor.dart'; +import 'package:solid_lints/models/rule_config.dart'; +import 'package:solid_lints/models/solid_lint_rule.dart'; + +/// A `prefer-first` rule which warns about +/// usage of iterable[0] or iterable.elementAt(0) +class PreferFirstRule extends SolidLintRule { + /// The [LintCode] of this lint rule that represents the error if number of + /// parameters reaches the maximum value. + static const lintName = 'prefer-first'; + + PreferFirstRule._(super.config); + + /// Creates a new instance of [PreferFirstRule] + /// based on the lint configuration. + factory PreferFirstRule.createRule(CustomLintConfigs configs) { + final config = RuleConfig( + configs: configs, + name: lintName, + problemMessage: (value) => + 'Use first instead of accessing the element at zero index.', + ); + + return PreferFirstRule._(config); + } + + @override + void run( + CustomLintResolver resolver, + ErrorReporter reporter, + CustomLintContext context, + ) { + context.registry.addCompilationUnit((node) { + final visitor = PreferFirstVisitor(); + node.accept(visitor); + + for (final element in visitor.expressions) { + reporter.reportErrorForNode(code, element); + } + }); + } + + @override + List getFixes() => [PreferFirstFix()]; +} diff --git a/lib/lints/prefer_first/prefer_first_visitor.dart b/lib/lints/prefer_first/prefer_first_visitor.dart new file mode 100644 index 00000000..3abdf1e5 --- /dev/null +++ b/lib/lints/prefer_first/prefer_first_visitor.dart @@ -0,0 +1,40 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:solid_lints/utils/types_utils.dart'; + +/// The AST visitor that will collect all Iterable access expressions +/// which can be replaced with .first +class PreferFirstVisitor extends RecursiveAstVisitor { + final _expressions = []; + + /// List of all Iterable access expressions + Iterable get expressions => _expressions; + + @override + void visitMethodInvocation(MethodInvocation node) { + super.visitMethodInvocation(node); + final isIterable = isIterableOrSubclass(node.realTarget?.staticType); + final isElementAt = node.methodName.name == 'elementAt'; + + if (isIterable && isElementAt) { + final arg = node.argumentList.arguments.first; + + if (arg is IntegerLiteral && arg.value == 0) { + _expressions.add(node); + } + } + } + + @override + void visitIndexExpression(IndexExpression node) { + super.visitIndexExpression(node); + + if (isListOrSubclass(node.realTarget.staticType)) { + final index = node.index; + + if (index is IntegerLiteral && index.value == 0) { + _expressions.add(node); + } + } + } +} diff --git a/lib/solid_lints.dart b/lib/solid_lints.dart index 8c95d096..b6711421 100644 --- a/lib/solid_lints.dart +++ b/lib/solid_lints.dart @@ -20,6 +20,7 @@ import 'package:solid_lints/lints/no_equal_then_else/no_equal_then_else_rule.dar import 'package:solid_lints/lints/no_magic_number/no_magic_number_rule.dart'; import 'package:solid_lints/lints/number_of_parameters/number_of_parameters_metric.dart'; import 'package:solid_lints/lints/prefer_conditional_expressions/prefer_conditional_expressions_rule.dart'; +import 'package:solid_lints/lints/prefer_first/prefer_first_rule.dart'; import 'package:solid_lints/models/solid_lint_rule.dart'; /// Creates a plugin for our custom linter @@ -49,6 +50,7 @@ class _SolidLints extends PluginBase { MemberOrderingRule.createRule(configs), NoMagicNumberRule.createRule(configs), PreferConditionalExpressionsRule.createRule(configs), + PreferFirstRule.createRule(configs), ]; // Return only enabled rules diff --git a/lib/utils/types_utils.dart b/lib/utils/types_utils.dart index d2584308..d73ce35f 100644 --- a/lib/utils/types_utils.dart +++ b/lib/utils/types_utils.dart @@ -24,6 +24,7 @@ import 'package:analyzer/dart/element/nullability_suffix.dart'; import 'package:analyzer/dart/element/type.dart'; +import 'package:collection/collection.dart'; bool hasWidgetType(DartType type) => (isWidgetOrSubclass(type) || @@ -162,3 +163,9 @@ bool _isFutureInheritedProvider(DartType type) => type.isDartAsyncFuture && type is InterfaceType && _isSubclassOfInheritedProvider(type.typeArguments.firstOrNull); + +bool isIterableOrSubclass(DartType? type) => + _checkSelfOrSupertypes(type, (t) => t?.isDartCoreIterable ?? false); + +bool isListOrSubclass(DartType? type) => + _checkSelfOrSupertypes(type, (t) => t?.isDartCoreList ?? false); From cfd1e367acc3e399d3bf1ad5d931853181f9a4ae Mon Sep 17 00:00:00 2001 From: Denis Bogatirov Date: Wed, 20 Sep 2023 11:56:55 +0300 Subject: [PATCH 6/8] Add tests for prefer-first rule --- lint_test/analysis_options.yaml | 1 + lint_test/prefer_first_test.dart | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+) create mode 100644 lint_test/prefer_first_test.dart diff --git a/lint_test/analysis_options.yaml b/lint_test/analysis_options.yaml index 4164f2ef..406ed6a2 100644 --- a/lint_test/analysis_options.yaml +++ b/lint_test/analysis_options.yaml @@ -50,3 +50,4 @@ custom_lint: - dispose-method - no-magic-number - prefer-conditional-expressions + - prefer-first diff --git a/lint_test/prefer_first_test.dart b/lint_test/prefer_first_test.dart new file mode 100644 index 00000000..49d6bf54 --- /dev/null +++ b/lint_test/prefer_first_test.dart @@ -0,0 +1,21 @@ +/// Check the `prefer-first` rule +void fun() { + const zero = 0; + final list = [0, 1, 2, 3]; + final set = {0, 1, 2, 3}; + final map = {0: 0, 1: 1, 2: 2, 3: 3}; + + // expect_lint: prefer-first + list[0]; + list[zero]; + // expect_lint: prefer-first + list.elementAt(0); + list.elementAt(zero); + // expect_lint: prefer-first + set.elementAt(0); + + // expect_lint: prefer-first + map.keys.elementAt(0); + // expect_lint: prefer-first + map.values.elementAt(0); +} From 1fda0b11917fa43badb67e92a2b2fb0a2d61ed7b Mon Sep 17 00:00:00 2001 From: Denis Bogatirov Date: Wed, 20 Sep 2023 14:06:08 +0300 Subject: [PATCH 7/8] Add prefer-last rule & fix --- lib/lints/prefer_last/prefer_last_fix.dart | 67 +++++++++++++++++ lib/lints/prefer_last/prefer_last_rule.dart | 48 ++++++++++++ .../prefer_last/prefer_last_visitor.dart | 74 +++++++++++++++++++ lib/solid_lints.dart | 2 + 4 files changed, 191 insertions(+) create mode 100644 lib/lints/prefer_last/prefer_last_fix.dart create mode 100644 lib/lints/prefer_last/prefer_last_rule.dart create mode 100644 lib/lints/prefer_last/prefer_last_visitor.dart diff --git a/lib/lints/prefer_last/prefer_last_fix.dart b/lib/lints/prefer_last/prefer_last_fix.dart new file mode 100644 index 00000000..9e276350 --- /dev/null +++ b/lib/lints/prefer_last/prefer_last_fix.dart @@ -0,0 +1,67 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/error/error.dart'; +import 'package:analyzer/source/source_range.dart'; +import 'package:custom_lint_builder/custom_lint_builder.dart'; + +/// A Quick fix for `prefer-last` rule +/// Suggests to replace iterable access expressions +class PreferLastFix extends DartFix { + static const _replaceComment = "Replace with 'last'."; + + @override + void run( + CustomLintResolver resolver, + ChangeReporter reporter, + CustomLintContext context, + AnalysisError analysisError, + List others, + ) { + context.registry.addMethodInvocation((node) { + if (analysisError.sourceRange.intersects(node.sourceRange)) { + final correction = _createCorrection(node); + + _addReplacement(reporter, node, correction); + } + }); + + context.registry.addIndexExpression((node) { + if (analysisError.sourceRange.intersects(node.sourceRange)) { + final correction = _createCorrection(node); + + _addReplacement(reporter, node, correction); + } + }); + } + + String _createCorrection(Expression expression) { + if (expression is MethodInvocation) { + return expression.isCascaded + ? '..last' + : '${expression.target ?? ''}.last'; + } else if (expression is IndexExpression) { + return expression.isCascaded + ? '..last' + : '${expression.target ?? ''}.last'; + } else { + return '.last'; + } + } + + void _addReplacement( + ChangeReporter reporter, + Expression node, + String correction, + ) { + final changeBuilder = reporter.createChangeBuilder( + message: _replaceComment, + priority: 1, + ); + + changeBuilder.addDartFileEdit((builder) { + builder.addSimpleReplacement( + SourceRange(node.offset, node.length), + correction, + ); + }); + } +} diff --git a/lib/lints/prefer_last/prefer_last_rule.dart b/lib/lints/prefer_last/prefer_last_rule.dart new file mode 100644 index 00000000..f965144a --- /dev/null +++ b/lib/lints/prefer_last/prefer_last_rule.dart @@ -0,0 +1,48 @@ +import 'package:analyzer/error/listener.dart'; +import 'package:custom_lint_builder/custom_lint_builder.dart'; +import 'package:solid_lints/lints/prefer_last/prefer_last_fix.dart'; +import 'package:solid_lints/lints/prefer_last/prefer_last_visitor.dart'; +import 'package:solid_lints/models/rule_config.dart'; +import 'package:solid_lints/models/solid_lint_rule.dart'; + +/// A `prefer-last` rule which warns about +/// usage of iterable[length-1] or iterable.elementAt(length-1) +class PreferLastRule extends SolidLintRule { + /// The [LintCode] of this lint rule that represents the error if iterable + /// access can be simplified. + static const lintName = 'prefer-last'; + + PreferLastRule._(super.config); + + /// Creates a new instance of [PreferLastRule] + /// based on the lint configuration. + factory PreferLastRule.createRule(CustomLintConfigs configs) { + final config = RuleConfig( + configs: configs, + name: lintName, + problemMessage: (value) => + 'Use last instead of accessing the last element by index.', + ); + + return PreferLastRule._(config); + } + + @override + void run( + CustomLintResolver resolver, + ErrorReporter reporter, + CustomLintContext context, + ) { + context.registry.addCompilationUnit((node) { + final visitor = PreferLastVisitor(); + node.accept(visitor); + + for (final element in visitor.expressions) { + reporter.reportErrorForNode(code, element); + } + }); + } + + @override + List getFixes() => [PreferLastFix()]; +} diff --git a/lib/lints/prefer_last/prefer_last_visitor.dart b/lib/lints/prefer_last/prefer_last_visitor.dart new file mode 100644 index 00000000..aa76127f --- /dev/null +++ b/lib/lints/prefer_last/prefer_last_visitor.dart @@ -0,0 +1,74 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/token.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:solid_lints/utils/types_utils.dart'; + +/// The AST visitor that will collect all Iterable access expressions +/// which can be replaced with .last +class PreferLastVisitor extends RecursiveAstVisitor { + final _expressions = []; + + /// List of all Iterable access expressions + Iterable get expressions => _expressions; + + @override + void visitMethodInvocation(MethodInvocation node) { + super.visitMethodInvocation(node); + + final target = node.realTarget; + + if (isIterableOrSubclass(target?.staticType) && + node.methodName.name == 'elementAt') { + final arg = node.argumentList.arguments.first; + + if (arg is BinaryExpression && + _isLastElementAccess(arg, target.toString())) { + _expressions.add(node); + } + } + } + + @override + void visitIndexExpression(IndexExpression node) { + super.visitIndexExpression(node); + + final target = node.realTarget; + + if (isListOrSubclass(target.staticType)) { + final index = node.index; + + if (index is BinaryExpression && + _isLastElementAccess(index, target.toString())) { + _expressions.add(node); + } + } + } + + bool _isLastElementAccess(BinaryExpression expression, String targetName) { + final left = expression.leftOperand; + final right = expression.rightOperand; + final leftName = _getLeftOperandName(left); + + if (right is! IntegerLiteral) return false; + if (right.value != 1) return false; + if (expression.operator.type != TokenType.MINUS) return false; + + return leftName == '$targetName.length'; + } + + String? _getLeftOperandName(Expression expression) { + if (expression is PrefixedIdentifier) { + return expression.name; + } + + /// Access target like map.keys.length is being reported as PropertyAccess + /// expression this case will handle such cases + if (expression is PropertyAccess) { + if (expression.operator.type != TokenType.PERIOD) return null; + + return expression.toString(); + } + + return null; + } +} diff --git a/lib/solid_lints.dart b/lib/solid_lints.dart index b6711421..d633409c 100644 --- a/lib/solid_lints.dart +++ b/lib/solid_lints.dart @@ -21,6 +21,7 @@ import 'package:solid_lints/lints/no_magic_number/no_magic_number_rule.dart'; import 'package:solid_lints/lints/number_of_parameters/number_of_parameters_metric.dart'; import 'package:solid_lints/lints/prefer_conditional_expressions/prefer_conditional_expressions_rule.dart'; import 'package:solid_lints/lints/prefer_first/prefer_first_rule.dart'; +import 'package:solid_lints/lints/prefer_last/prefer_last_rule.dart'; import 'package:solid_lints/models/solid_lint_rule.dart'; /// Creates a plugin for our custom linter @@ -51,6 +52,7 @@ class _SolidLints extends PluginBase { NoMagicNumberRule.createRule(configs), PreferConditionalExpressionsRule.createRule(configs), PreferFirstRule.createRule(configs), + PreferLastRule.createRule(configs), ]; // Return only enabled rules From 3e2f9fc25445232bc4601a3da11ac83577381055 Mon Sep 17 00:00:00 2001 From: Denis Bogatirov Date: Wed, 20 Sep 2023 14:06:24 +0300 Subject: [PATCH 8/8] Add tests for prefer-last rule --- lint_test/analysis_options.yaml | 1 + lint_test/prefer_last_test.dart | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 lint_test/prefer_last_test.dart diff --git a/lint_test/analysis_options.yaml b/lint_test/analysis_options.yaml index 406ed6a2..10cd3967 100644 --- a/lint_test/analysis_options.yaml +++ b/lint_test/analysis_options.yaml @@ -51,3 +51,4 @@ custom_lint: - no-magic-number - prefer-conditional-expressions - prefer-first + - prefer-last diff --git a/lint_test/prefer_last_test.dart b/lint_test/prefer_last_test.dart new file mode 100644 index 00000000..0007641d --- /dev/null +++ b/lint_test/prefer_last_test.dart @@ -0,0 +1,25 @@ +/// Check the `prefer-first` rule +void fun() { + final list = [0, 1, 2, 3]; + final length = list.length - 1; + final set = {0, 1, 2, 3}; + final map = {0: 0, 1: 1, 2: 2, 3: 3}; + + // expect_lint: prefer-last + list[list.length - 1]; + + list[length - 1]; + + // expect_lint: prefer-last + list.elementAt(list.length - 1); + list.elementAt(length - 1); + + // expect_lint: prefer-last + set.elementAt(set.length - 1); + + // expect_lint: prefer-last + map.keys.elementAt(map.keys.length - 1); + + // expect_lint: prefer-last + map.values.elementAt(map.values.length - 1); +}