Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added prefer condtional expressions rule and fix #59

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,
);
}
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;
}
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()];
}
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,
});
}
2 changes: 2 additions & 0 deletions lib/solid_lints.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -47,6 +48,7 @@ class _SolidLints extends PluginBase {
NoEqualThenElseRule.createRule(configs),
MemberOrderingRule.createRule(configs),
NoMagicNumberRule.createRule(configs),
PreferConditionalExpressionsRule.createRule(configs),
];

// Return only enabled rules
Expand Down
1 change: 1 addition & 0 deletions lint_test/analysis_options.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,4 @@ custom_lint:
- did-update-widget-method
- dispose-method
- no-magic-number
- prefer-conditional-expressions
1 change: 1 addition & 0 deletions lint_test/no_equal_then_else_test.dart
Original file line number Diff line number Diff line change
@@ -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() {
Expand Down
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
Loading