Skip to content

Commit 0f1e202

Browse files
solid-illiaaihistovIllia Aihistov
solid-illiaaihistov
and
Illia Aihistov
authored
refactor: migrate prefer_conditional_expressions (#303)
* refactor: migrate prefer_conditional_expressions * fix: add parentheses to inverted conditional expressions for complex conditions and update tests * refactor: extract logic from prefer_conditional_expressions_visitor into separate model and visitor classes --------- Co-authored-by: Illia Aihistov <illia.aihistov-us@solid.software>
1 parent e2db387 commit 0f1e202

11 files changed

Lines changed: 465 additions & 289 deletions

‎lib/main.dart‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import 'package:solid_lints/src/lints/named_parameters_ordering/named_parameters
2020
import 'package:solid_lints/src/lints/no_empty_block/no_empty_block_rule.dart';
2121
import 'package:solid_lints/src/lints/no_magic_number/no_magic_number_rule.dart';
2222
import 'package:solid_lints/src/lints/number_of_parameters/number_of_parameters_rule.dart';
23+
import 'package:solid_lints/src/lints/prefer_conditional_expressions/fixes/prefer_conditional_expressions_fix.dart';
24+
import 'package:solid_lints/src/lints/prefer_conditional_expressions/prefer_conditional_expressions_rule.dart';
2325
import 'package:solid_lints/src/lints/prefer_first/fixes/prefer_first_fix.dart';
2426
import 'package:solid_lints/src/lints/prefer_first/prefer_first_rule.dart';
2527
import 'package:solid_lints/src/lints/prefer_last/fixes/prefer_last_fix.dart';
@@ -50,6 +52,9 @@ class SolidLintsPlugin extends Plugin {
5052
AvoidUnnecessaryTypeAssertionsRule();
5153
final doubleLiteralFormatRule = DoubleLiteralFormatRule();
5254
final preferFirstRule = PreferFirstRule();
55+
final preferConditionalExpressionsRule = PreferConditionalExpressionsRule(
56+
analysisOptionsLoader: analysisLoader,
57+
);
5358
final preferLastRule = PreferLastRule();
5459

5560
final lintRules = [
@@ -71,6 +76,7 @@ class SolidLintsPlugin extends Plugin {
7176
UseNearestContextRule(),
7277
NoMagicNumberRule(analysisOptionsLoader: analysisLoader),
7378
preferFirstRule,
79+
preferConditionalExpressionsRule,
7480
preferLastRule,
7581
// TODO: Add more lint rules and use analysisLoader
7682
// for rules that need parameters
@@ -110,6 +116,11 @@ class SolidLintsPlugin extends Plugin {
110116
PreferLastFix.new,
111117
);
112118

119+
registry.registerFixForRule(
120+
preferConditionalExpressionsRule.diagnosticCode,
121+
PreferConditionalExpressionsFix.new,
122+
);
123+
113124
registry.registerFixForRule(
114125
NamedParametersOrderingRule.code,
115126
NamedParametersOrderingFix.new,

‎lib/src/lints/prefer_conditional_expressions/fixes/prefer_conditional_expressions_fix.dart‎

Lines changed: 55 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,56 +1,55 @@
1+
import 'package:analysis_server_plugin/edit/dart/correction_producer.dart';
2+
import 'package:analysis_server_plugin/edit/dart/dart_fix_kind_priority.dart';
13
import 'package:analyzer/dart/ast/ast.dart';
2-
import 'package:analyzer/dart/ast/token.dart';
3-
import 'package:analyzer/diagnostic/diagnostic.dart';
4-
import 'package:analyzer/source/source_range.dart';
5-
import 'package:custom_lint_builder/custom_lint_builder.dart';
6-
import 'package:solid_lints/src/lints/prefer_conditional_expressions/visitors/prefer_conditional_expressions_visitor.dart';
4+
import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
5+
import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
6+
import 'package:solid_lints/src/lints/prefer_conditional_expressions/models/statement_info.dart';
7+
import 'package:solid_lints/src/lints/prefer_conditional_expressions/prefer_conditional_expressions_rule.dart';
78

89
/// A Quick fix for `prefer_conditional_expressions` rule
9-
/// Suggests to remove unnecessary assertions
10-
class PreferConditionalExpressionsFix extends DartFix {
11-
final Expando<StatementInfo> _diagnosticsInfoExpando;
10+
/// Suggests to convert simple if statements to conditional expressions
11+
class PreferConditionalExpressionsFix extends ResolvedCorrectionProducer {
12+
static const _fixComment = "Convert to conditional expression.";
1213

13-
/// A Quick fix for `prefer_conditional_expressions` rule
14-
/// Suggests to remove unnecessary assertions
15-
PreferConditionalExpressionsFix(this._diagnosticsInfoExpando);
14+
/// Creates a new instance of [PreferConditionalExpressionsFix]
15+
PreferConditionalExpressionsFix({required super.context});
1616

1717
@override
18-
void run(
19-
CustomLintResolver resolver,
20-
ChangeReporter reporter,
21-
CustomLintContext context,
22-
Diagnostic diagnostic,
23-
List<Diagnostic> others,
24-
) {
25-
context.registry.addIfStatement((node) {
26-
if (!diagnostic.sourceRange.intersects(node.sourceRange)) return;
18+
FixKind get fixKind => const FixKind(
19+
'solid_lints.fix.${PreferConditionalExpressionsRule.lintName}',
20+
DartFixKindPriority.standard,
21+
_fixComment,
22+
);
2723

28-
final statementInfo = _diagnosticsInfoExpando[diagnostic];
29-
if (statementInfo == null) return;
24+
@override
25+
FixKind get multiFixKind => const FixKind(
26+
'solid_lints.fix.multi.${PreferConditionalExpressionsRule.lintName}',
27+
DartFixKindPriority.standard,
28+
'$_fixComment across files',
29+
);
3030

31-
final correction = _createCorrection(statementInfo);
32-
if (correction == null) return;
31+
@override
32+
CorrectionApplicability get applicability =>
33+
CorrectionApplicability.automatically;
3334

34-
_addReplacement(reporter, statementInfo.statement, correction);
35-
});
36-
}
35+
@override
36+
Future<void> compute(ChangeBuilder builder) async {
37+
final statement = node.thisOrAncestorOfType<IfStatement>();
38+
if (statement == null) return;
3739

38-
void _addReplacement(
39-
ChangeReporter reporter,
40-
IfStatement node,
41-
String correction,
42-
) {
43-
final changeBuilder = reporter.createChangeBuilder(
44-
message: "Convert to conditional expression.",
45-
priority: 1,
46-
);
40+
final statementInfo = StatementInfo.fromIfStatement(statement);
41+
if (statementInfo == null) return;
4742

48-
changeBuilder.addDartFileEdit((builder) {
49-
builder.addSimpleReplacement(
50-
SourceRange(node.offset, node.length),
43+
final correction = _createCorrection(statementInfo);
44+
if (correction == null) return;
45+
46+
await builder.addDartFileEdit(
47+
file,
48+
(builder) => builder.addSimpleReplacement(
49+
statement.sourceRange,
5150
correction,
52-
);
53-
});
51+
),
52+
);
5453
}
5554

5655
String? _createCorrection(StatementInfo info) {
@@ -64,28 +63,15 @@ class PreferConditionalExpressionsFix extends DartFix {
6463
final target = thenStatement.leftHandSide;
6564
final firstExpression = thenStatement.rightHandSide;
6665
final secondExpression = elseStatement.rightHandSide;
67-
68-
final thenStatementOperator = thenStatement.operator.type;
69-
final elseStatementOperator = elseStatement.operator.type;
70-
71-
if (_isAssignmentOperatorNotEq(thenStatementOperator) &&
72-
_isAssignmentOperatorNotEq(elseStatementOperator)) {
73-
final prefix = thenStatement.leftHandSide;
74-
final thenPart =
75-
'$prefix ${thenStatementOperator.stringValue} $firstExpression';
76-
final elsePart =
77-
'$prefix ${elseStatementOperator.stringValue} $secondExpression;';
78-
79-
return '$condition ? $thenPart : $elsePart';
80-
}
66+
final op = thenStatement.operator.lexeme;
8167

8268
final correctionForLiterals = _createCorrectionForLiterals(
8369
condition,
8470
firstExpression,
8571
secondExpression,
8672
);
8773

88-
return '$target = $correctionForLiterals';
74+
return '$target $op $correctionForLiterals';
8975
}
9076

9177
if (thenStatement is ReturnStatement && elseStatement is ReturnStatement) {
@@ -110,14 +96,23 @@ class PreferConditionalExpressionsFix extends DartFix {
11096
) {
11197
if (firstExpression is BooleanLiteral &&
11298
secondExpression is BooleanLiteral) {
99+
if (firstExpression.value == secondExpression.value) {
100+
return '${firstExpression.value};';
101+
}
113102
final isInverted = !firstExpression.value && secondExpression.value;
103+
if (isInverted) {
104+
final useParentheses =
105+
condition is! Identifier &&
106+
condition is! PropertyAccess &&
107+
condition is! MethodInvocation &&
108+
condition is! IndexExpression &&
109+
condition is! ParenthesizedExpression;
110+
return '${useParentheses ? '!($condition)' : '!$condition'};';
111+
}
114112

115-
return '${isInverted ? "!" : ""}$condition;';
113+
return '$condition;';
116114
}
117115

118116
return '$condition ? $firstExpression : $secondExpression;';
119117
}
120-
121-
bool _isAssignmentOperatorNotEq(TokenType token) =>
122-
token.isAssignmentOperator && token != TokenType.EQ;
123118
}

‎lib/src/lints/prefer_conditional_expressions/models/prefer_conditional_expressions_parameters.dart‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,16 @@ class PreferConditionalExpressionsParameters {
2020
required this.ignoreNested,
2121
});
2222

23+
/// Empty [PreferConditionalExpressionsParameters] model.
24+
factory PreferConditionalExpressionsParameters.empty() =>
25+
const PreferConditionalExpressionsParameters(
26+
ignoreNested: false,
27+
);
28+
2329
/// Method for creating from json data
2430
factory PreferConditionalExpressionsParameters.fromJson(
2531
Map<String, Object?> json,
26-
) =>
27-
PreferConditionalExpressionsParameters(
28-
ignoreNested: json[_ignoreNestedConfig] as bool? ?? false,
29-
);
32+
) => PreferConditionalExpressionsParameters(
33+
ignoreNested: json[_ignoreNestedConfig] as bool? ?? false,
34+
);
3035
}

‎lib/src/lints/prefer_conditional_expressions/models/statement_info.dart‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
// MIT License
2+
//
3+
// Copyright (c) 2020-2021 Dart Code Checker team
4+
//
5+
// Permission is hereby granted, free of charge, to any person obtaining a copy
6+
// of this software and associated documentation files (the "Software"), to deal
7+
// in the Software without restriction, including without limitation the rights
8+
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
9+
// copies of the Software, and to permit persons to whom the Software is
10+
// furnished to do so, subject to the following conditions:
11+
//
12+
// The above copyright notice and this permission notice shall be included in
13+
// all
14+
// copies or substantial portions of the Software.
15+
//
16+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
17+
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
18+
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
19+
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
20+
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
21+
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
22+
// SOFTWARE.
23+
24+
import 'package:analyzer/dart/ast/ast.dart';
25+
26+
/// Data class contains info required for fix
27+
class StatementInfo {
28+
/// If statement node
29+
final IfStatement statement;
30+
31+
/// Contents of if block
32+
final AstNode unwrappedThenStatement;
33+
34+
/// Contents of else block
35+
final AstNode unwrappedElseStatement;
36+
37+
/// Creates instance of an [StatementInfo]
38+
const StatementInfo({
39+
required this.statement,
40+
required this.unwrappedThenStatement,
41+
required this.unwrappedElseStatement,
42+
});
43+
44+
/// Factory constructor to create [StatementInfo] from [IfStatement] if it
45+
/// can be simplified.
46+
static StatementInfo? fromIfStatement(IfStatement statement) {
47+
if (statement.parent is IfStatement ||
48+
statement.elseStatement == null ||
49+
statement.elseStatement is IfStatement) {
50+
return null;
51+
}
52+
53+
final thenAssignment = _getAssignmentExpression(statement.thenStatement);
54+
final elseAssignment = _getAssignmentExpression(statement.elseStatement);
55+
56+
if (thenAssignment != null &&
57+
elseAssignment != null &&
58+
thenAssignment.operator.type == elseAssignment.operator.type &&
59+
_haveEqualNames(thenAssignment, elseAssignment)) {
60+
return StatementInfo(
61+
statement: statement,
62+
unwrappedThenStatement: thenAssignment,
63+
unwrappedElseStatement: elseAssignment,
64+
);
65+
}
66+
67+
final thenReturn = _getReturnStatement(statement.thenStatement);
68+
final elseReturn = _getReturnStatement(statement.elseStatement);
69+
70+
if (thenReturn != null &&
71+
elseReturn != null &&
72+
thenReturn.expression != null &&
73+
elseReturn.expression != null) {
74+
return StatementInfo(
75+
statement: statement,
76+
unwrappedThenStatement: thenReturn,
77+
unwrappedElseStatement: elseReturn,
78+
);
79+
}
80+
81+
return null;
82+
}
83+
84+
static AssignmentExpression? _getAssignmentExpression(Statement? statement) {
85+
if (statement is ExpressionStatement &&
86+
statement.expression is AssignmentExpression) {
87+
return statement.expression as AssignmentExpression;
88+
}
89+
90+
if (statement is Block && statement.statements.length == 1) {
91+
return _getAssignmentExpression(statement.statements.first);
92+
}
93+
94+
return null;
95+
}
96+
97+
static bool _haveEqualNames(
98+
AssignmentExpression thenAssignment,
99+
AssignmentExpression elseAssignment,
100+
) =>
101+
thenAssignment.leftHandSide is Identifier &&
102+
elseAssignment.leftHandSide is Identifier &&
103+
(thenAssignment.leftHandSide as Identifier).name ==
104+
(elseAssignment.leftHandSide as Identifier).name;
105+
106+
static ReturnStatement? _getReturnStatement(Statement? statement) {
107+
if (statement is ReturnStatement) {
108+
return statement;
109+
}
110+
111+
if (statement is Block && statement.statements.length == 1) {
112+
return _getReturnStatement(statement.statements.first);
113+
}
114+
115+
return null;
116+
}
117+
}

0 commit comments

Comments
 (0)