-
Notifications
You must be signed in to change notification settings - Fork 17
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Added prefer condtional expressions rule and fix (#59)
* Add prefer-conditional-expressions rule and fix * Add tests for prefer-conditional-expressions rule * fix nested test plugin path * Fix tests after merge * Update lint_test/prefer_conditional_expressions_ignore_nested_test/prefer_conditional_expressions_ignore_nested_test.dart --------- Co-authored-by: Denis Bogatirov <denis.bohatyrov@computools.com> Co-authored-by: Yurii Prykhodko <144313329+yurii-prykhodko-solid@users.noreply.github.com>
- Loading branch information
1 parent
5690c93
commit 1f74e1b
Showing
11 changed files
with
447 additions
and
0 deletions.
There are no files selected for viewing
21 changes: 21 additions & 0 deletions
21
...ints/prefer_conditional_expressions/models/prefer_conditional_expressions_parameters.dart
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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<String, Object?> json, | ||
) => | ||
PreferConditionalExpressionsParameters( | ||
ignoreNested: json[_ignoreNestedConfig] as bool? ?? false, | ||
); | ||
} |
117 changes: 117 additions & 0 deletions
117
lib/lints/prefer_conditional_expressions/prefer_conditional_expressions_fix.dart
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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<AnalysisError> 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; | ||
} |
62 changes: 62 additions & 0 deletions
62
lib/lints/prefer_conditional_expressions/prefer_conditional_expressions_rule.dart
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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<PreferConditionalExpressionsParameters> { | ||
/// 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<Fix> getFixes() => [PreferConditionalExpressionsFix()]; | ||
} |
158 changes: 158 additions & 0 deletions
158
lib/lints/prefer_conditional_expressions/prefer_conditional_expressions_visitor.dart
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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<void> { | ||
final _statementsInfo = <StatementInfo>[]; | ||
|
||
final bool _ignoreNested; | ||
|
||
/// List of statement info that represents all simple if statements | ||
Iterable<StatementInfo> 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<void> { | ||
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, | ||
}); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,3 +49,4 @@ custom_lint: | |
- did-update-widget-method | ||
- dispose-method | ||
- no-magic-number | ||
- prefer-conditional-expressions |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
8 changes: 8 additions & 0 deletions
8
lint_test/prefer_conditional_expressions_ignore_nested_test/analysis_options.yaml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
analyzer: | ||
plugins: | ||
- ../custom_lint | ||
|
||
custom_lint: | ||
rules: | ||
- prefer-conditional-expressions: | ||
ignore-nested: true |
Oops, something went wrong.