Skip to content
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
5 changes: 3 additions & 2 deletions lib/analysis_options.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,9 @@ solid_lints:
newline_before_return: true
no_empty_block: true
no_equal_then_else: true
# Disabled by default for now. Will be considered for future inclusion.
prefer_early_return: false
prefer_early_return:
maximum_statements: 1
ignore_if_case: true

no_magic_number:
allowed_in_widget_params: true
Expand Down
2 changes: 1 addition & 1 deletion lib/main.dart
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class SolidLintsPlugin extends Plugin {
NoMagicNumberRule(analysisOptionsLoader: analysisLoader),
NumberOfParametersRule(analysisOptionsLoader: analysisLoader),
PreferConditionalExpressionsRule(analysisOptionsLoader: analysisLoader),
PreferEarlyReturnRule(),
PreferEarlyReturnRule(analysisOptionsLoader: analysisLoader),
PreferFirstRule(),
PreferLastRule(),
PreferMatchFileNameRule(analysisOptionsLoader: analysisLoader),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/// A data model class that represents the "prefer early return"
/// input parameters.
class PreferEarlyReturnParameters {
static const _maximumStatementsConfig = 'maximum_statements';
static const _ignoreIfCaseConfig = 'ignore_if_case';

static const _defaultMaximumStatements = 1;
static const _defaultIgnoreIfCase = true;

/// The maximum number of statements allowed inside an `if` block before
/// triggering the lint. If the number of statements does not exceed this
/// threshold, the analysis is skipped.
final int maximumStatements;

/// Whether to ignore `if-case` pattern matching statements.
final bool ignoreIfCase;

/// Constructor for [PreferEarlyReturnParameters] model.
const PreferEarlyReturnParameters({
required this.maximumStatements,
required this.ignoreIfCase,
});

/// Empty [PreferEarlyReturnParameters] model with default values.
factory PreferEarlyReturnParameters.empty() =>
const PreferEarlyReturnParameters(
maximumStatements: _defaultMaximumStatements,
ignoreIfCase: _defaultIgnoreIfCase,
);

/// Method for creating [PreferEarlyReturnParameters] from json data.
factory PreferEarlyReturnParameters.fromJson(
Map<String, Object?> json,
) => PreferEarlyReturnParameters(
maximumStatements:
json[_maximumStatementsConfig] as int? ?? _defaultMaximumStatements,
ignoreIfCase: json[_ignoreIfCaseConfig] as bool? ?? _defaultIgnoreIfCase,
Comment thread
solid-illiaaihistov marked this conversation as resolved.
);
}
69 changes: 54 additions & 15 deletions lib/src/lints/prefer_early_return/prefer_early_return_rule.dart
Original file line number Diff line number Diff line change
@@ -1,12 +1,23 @@
import 'package:analyzer/analysis_rule/analysis_rule.dart';
import 'package:analyzer/analysis_rule/rule_context.dart';
import 'package:analyzer/analysis_rule/rule_visitor_registry.dart';
import 'package:analyzer/error/error.dart';
import 'package:solid_lints/src/lints/prefer_early_return/models/prefer_early_return_parameters.dart';
import 'package:solid_lints/src/lints/prefer_early_return/visitors/prefer_early_return_visitor.dart';
import 'package:solid_lints/src/models/solid_lint_rule.dart';

/// A rule which highlights `if` statements that span the entire body,
/// and suggests replacing them with a reversed boolean check
/// with an early return.
/// A rule which highlights `if` statements that span the entire body of a
/// function or loop, and suggests replacing them with a reversed boolean check
/// with an early return or continue.
///
/// ### Example config:
///
/// ```yaml
/// solid_lints:
/// diagnostics:
/// prefer_early_return:
/// maximum_statements: 1
/// ignore_if_case: true
/// ```
///
/// ### Example
///
Expand All @@ -15,8 +26,16 @@ import 'package:solid_lints/src/lints/prefer_early_return/visitors/prefer_early_
/// ```dart
/// void func() {
/// if (a) { //LINT
/// if (b) { //LINT
/// c;
/// c;
/// d;
/// }
/// }
///
/// void loop() {
/// for (final item in items) {
/// if (item.isValid) { //LINT
/// process(item);
/// save(item);
/// }
/// }
/// }
Expand All @@ -27,26 +46,36 @@ import 'package:solid_lints/src/lints/prefer_early_return/visitors/prefer_early_
/// ```dart
/// void func() {
/// if (!a) return;
/// if (!b) return;
/// c;
/// d;
/// }
///
/// void loop() {
/// for (final item in items) {
/// if (!item.isValid) continue;
/// process(item);
/// save(item);
/// }
/// }
/// ```
class PreferEarlyReturnRule extends AnalysisRule {
class PreferEarlyReturnRule extends SolidLintRule<PreferEarlyReturnParameters> {
/// Lint name
static const String lintName = 'prefer_early_return';

/// Lint code
static const LintCode _code = LintCode(
lintName,
"Use reverse if to reduce nesting",
'Use reverse if to reduce nesting',
);

/// Creates an instance of [PreferEarlyReturnRule]
PreferEarlyReturnRule()
: super(
name: lintName,
description: 'Use reverse if to reduce nesting',
);
PreferEarlyReturnRule({
required super.analysisOptionsLoader,
}) : super.withParameters(
name: lintName,
description: 'Use reverse if to reduce nesting',
parametersParser: PreferEarlyReturnParameters.fromJson,
);

@override
LintCode get diagnosticCode => _code;
Expand All @@ -56,11 +85,21 @@ class PreferEarlyReturnRule extends AnalysisRule {
RuleVisitorRegistry registry,
RuleContext context,
) {
super.registerNodeProcessors(registry, context);

final parameters =
getParametersForContext(context) ?? PreferEarlyReturnParameters.empty();

final visitor = PreferEarlyReturnVisitor(
rule: this,
context: context,
parameters: parameters,
);

registry.addBlockFunctionBody(this, visitor);
registry
..addBlockFunctionBody(this, visitor)
..addForStatement(this, visitor)
..addWhileStatement(this, visitor)
..addDoStatement(this, visitor);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/visitor.dart';

/// AST visitor that checks if a statement contains an early exit
/// (return, throw, or loop break/continue) respecting scope boundaries.
class EarlyReturnExitVisitor extends RecursiveAstVisitor<void> {
final Statement _root;
bool _hasExit = false;

EarlyReturnExitVisitor._(this._root);

/// Checks whether [node] contains any early exit statement.
static bool hasExitIn(Statement node) {
final visitor = EarlyReturnExitVisitor._(node);
node.accept(visitor);
return visitor._hasExit;
}

@override
void visitReturnStatement(ReturnStatement node) => _hasExit = true;

@override
void visitThrowExpression(ThrowExpression node) => _hasExit = true;

@override
void visitRethrowExpression(RethrowExpression node) => _hasExit = true;

@override
void visitBreakStatement(BreakStatement node) => _checkExit(node.target);

@override
void visitContinueStatement(ContinueStatement node) =>
_checkExit(node.target);

@override
void visitFunctionExpression(FunctionExpression node) {}

@override
void visitFunctionDeclaration(FunctionDeclaration node) {}

bool _isDescendantOfRoot(AstNode target) {
AstNode? current = target;
while (current != null) {
if (identical(current, _root)) {
return true;
}
current = current.parent;
}
return false;
}

void _checkExit(AstNode? target) {
if (target case final target? when !_isDescendantOfRoot(target)) {
_hasExit = true;
}
}
}
Original file line number Diff line number Diff line change
@@ -1,84 +1,92 @@
import 'package:analyzer/analysis_rule/rule_context.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/visitor.dart';
import 'package:solid_lints/src/lints/prefer_early_return/models/prefer_early_return_parameters.dart';
import 'package:solid_lints/src/lints/prefer_early_return/prefer_early_return_rule.dart';
import 'package:solid_lints/src/lints/prefer_early_return/visitors/return_statement_visitor.dart';
import 'package:solid_lints/src/lints/prefer_early_return/visitors/throw_expression_visitor.dart';
import 'package:solid_lints/src/lints/prefer_early_return/visitors/early_return_exit_visitor.dart';

/// Visitor for [PreferEarlyReturnRule].
class PreferEarlyReturnVisitor extends RecursiveAstVisitor<void> {
class PreferEarlyReturnVisitor extends SimpleAstVisitor<void> {
/// The rule associated with the visitor.
final PreferEarlyReturnRule rule;

/// The context associated with the visitor.
final RuleContext context;

/// The parameters associated with the rule.
final PreferEarlyReturnParameters parameters;

/// Constructor for [PreferEarlyReturnVisitor].
PreferEarlyReturnVisitor({
required this.rule,
required this.context,
required this.parameters,
});

@override
void visitBlockFunctionBody(BlockFunctionBody node) {
super.visitBlockFunctionBody(node);
void visitBlockFunctionBody(BlockFunctionBody node) =>
_checkStatements(node.block.statements, isLoop: false);

if (node.block.statements.isEmpty) return;
@override
void visitForStatement(ForStatement node) => _checkLoopBody(node.body);

final (ifStatements, nextStatement) = _getIfStatementsAndNextStatement(
node,
);
if (ifStatements.isEmpty) return;
@override
void visitWhileStatement(WhileStatement node) => _checkLoopBody(node.body);

@override
void visitDoStatement(DoStatement node) => _checkLoopBody(node.body);

// limit visitor to only work with functions
// that don't have a return statement or the return statement is empty
final nextStatementIsEmptyReturn =
nextStatement is ReturnStatement && nextStatement.expression == null;
final nextStatementIsNull = nextStatement == null;
void _checkLoopBody(Statement body) {
final List<Statement> statements = switch (body) {
Block(:final statements) => statements,
IfStatement() => [body],
_ => const [],
};

if (!nextStatementIsEmptyReturn && !nextStatementIsNull) return;
if (statements.isNotEmpty) {
_checkStatements(statements, isLoop: true);
}
}

final lastIf = ifStatements.last;
void _checkStatements(
List<Statement> statements, {
required bool isLoop,
}) {
final (leading, lastIf) = switch (statements) {
[...final leading, IfStatement lastIf] => (leading, lastIf),
[...final leading, IfStatement lastIf, ContinueStatement(label: null)]
when isLoop =>
(leading, lastIf),
[...final leading, IfStatement lastIf, ReturnStatement(expression: null)]
when !isLoop =>
(leading, lastIf),
_ => (null, null),
};

if (lastIf case IfStatement(elseStatement: Statement())) return;
if (_hasReturnStatement(lastIf) || _hasThrowExpression(lastIf)) return;
if (lastIf == null || !leading!.every((s) => s is IfStatement)) return;
if (!_shouldReport(lastIf)) return;

context.currentUnit?.diagnosticReporter.atNode(
lastIf,
rule.diagnosticCode,
);
}

// returns a list of if statements at the start of the function
// and the next statement after it
// examples:
// [if, if, if, return] -> ([if, if, if], return)
// [if, if, if, _doSomething, return] -> ([if, if, if], _doSomething)
// [if, if, if] -> ([if, if, if], null)
(List<IfStatement>, Statement?) _getIfStatementsAndNextStatement(
BlockFunctionBody body,
) {
final List<IfStatement> ifStatements = [];
for (final statement in body.block.statements) {
if (statement is IfStatement) {
ifStatements.add(statement);
} else {
return (ifStatements, statement);
}
}
bool _shouldReport(IfStatement ifStatement) {
final IfStatement(:thenStatement, :elseStatement, :caseClause) =
ifStatement;

return (ifStatements, null);
}
if (elseStatement != null ||
(parameters.ignoreIfCase && caseClause != null)) {
return false;
}

bool _hasReturnStatement(Statement node) {
final visitor = ReturnStatementVisitor();
node.accept(visitor);
return visitor.nodes.isNotEmpty;
}
final statementsCount = switch (thenStatement) {
Block(:final statements) => statements.length,
_ => 1,
};

bool _hasThrowExpression(Statement node) {
final visitor = ThrowExpressionVisitor();
node.accept(visitor);
return visitor.nodes.isNotEmpty;
return statementsCount > parameters.maximumStatements &&
!EarlyReturnExitVisitor.hasExitIn(thenStatement);
}
}

This file was deleted.

Loading