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 7a650dcd..8c95d096 100644 --- a/lib/solid_lints.dart +++ b/lib/solid_lints.dart @@ -19,6 +19,7 @@ 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/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/models/solid_lint_rule.dart'; /// Creates a plugin for our custom linter @@ -47,6 +48,7 @@ class _SolidLints extends PluginBase { NoEqualThenElseRule.createRule(configs), MemberOrderingRule.createRule(configs), NoMagicNumberRule.createRule(configs), + PreferConditionalExpressionsRule.createRule(configs), ]; // Return only enabled rules diff --git a/lint_test/analysis_options.yaml b/lint_test/analysis_options.yaml index a0c905cb..4164f2ef 100644 --- a/lint_test/analysis_options.yaml +++ b/lint_test/analysis_options.yaml @@ -49,3 +49,4 @@ custom_lint: - did-update-widget-method - dispose-method - no-magic-number + - prefer-conditional-expressions diff --git a/lint_test/no_equal_then_else_test.dart b/lint_test/no_equal_then_else_test.dart index 358bf6f2..df35eeb4 100644 --- a/lint_test/no_equal_then_else_test.dart +++ b/lint_test/no_equal_then_else_test.dart @@ -1,6 +1,7 @@ // ignore_for_file: unused_local_variable // ignore_for_file: cyclomatic_complexity // ignore_for_file: no-magic-number +// 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..957617b9 --- /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..fddfa18a --- /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 enabled + 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..8196ddc5 --- /dev/null +++ b/lint_test/prefer_conditional_expressions_test.dart @@ -0,0 +1,47 @@ +// ignore_for_file: unused_local_variable +// 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() { + 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; +}