Skip to content
This repository was archived by the owner on Jul 16, 2023. It is now read-only.

feat: add avoid-unnecessary-conditionals rule #1094

Merged
merged 1 commit into from
Dec 8, 2022
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

* fix: remove recursive traversal for [`ban-name`](https://dartcodemetrics.dev/docs/rules/common/ban-name) rule.
* feat: add static code diagnostic [`prefer-using-list-view`](https://dartcodemetrics.dev/docs/rules/flutter/prefer-using-list-view).
* feat: add static code diagnostic [`avoid-unnecessary-conditionals`](https://dartcodemetrics.dev/docs/rules/common/avoid-unnecessary-conditionals).

## 5.1.0

Expand Down
2 changes: 2 additions & 0 deletions lib/src/analyzers/lint_analyzer/rules/rules_factory.dart
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import 'rules_list/avoid_returning_widgets/avoid_returning_widgets_rule.dart';
import 'rules_list/avoid_shrink_wrap_in_lists/avoid_shrink_wrap_in_lists_rule.dart';
import 'rules_list/avoid_throw_in_catch_block/avoid_throw_in_catch_block_rule.dart';
import 'rules_list/avoid_top_level_members_in_tests/avoid_top_level_members_in_tests_rule.dart';
import 'rules_list/avoid_unnecessary_conditionals/avoid_unnecessary_conditionals_rule.dart';
import 'rules_list/avoid_unnecessary_setstate/avoid_unnecessary_setstate_rule.dart';
import 'rules_list/avoid_unnecessary_type_assertions/avoid_unnecessary_type_assertions_rule.dart';
import 'rules_list/avoid_unnecessary_type_casts/avoid_unnecessary_type_casts_rule.dart';
Expand Down Expand Up @@ -96,6 +97,7 @@ final _implementedRules = <String, Rule Function(Map<String, Object>)>{
AvoidShrinkWrapInListsRule.ruleId: AvoidShrinkWrapInListsRule.new,
AvoidThrowInCatchBlockRule.ruleId: AvoidThrowInCatchBlockRule.new,
AvoidTopLevelMembersInTestsRule.ruleId: AvoidTopLevelMembersInTestsRule.new,
AvoidUnnecessaryConditionalsRule.ruleId: AvoidUnnecessaryConditionalsRule.new,
AvoidUnnecessarySetStateRule.ruleId: AvoidUnnecessarySetStateRule.new,
AvoidUnnecessaryTypeAssertionsRule.ruleId:
AvoidUnnecessaryTypeAssertionsRule.new,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// ignore_for_file: public_member_api_docs

import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/visitor.dart';

import '../../../../../utils/node_utils.dart';
import '../../../lint_utils.dart';
import '../../../models/internal_resolved_unit_result.dart';
import '../../../models/issue.dart';
import '../../../models/replacement.dart';
import '../../../models/severity.dart';
import '../../models/common_rule.dart';
import '../../rule_utils.dart';

part 'visitor.dart';

class AvoidUnnecessaryConditionalsRule extends CommonRule {
static const String ruleId = 'avoid-unnecessary-conditionals';

static const _warning = 'Avoid unnecessary conditional expressions.';
static const _correctionMessage =
'Remove unnecessary conditional expression.';

AvoidUnnecessaryConditionalsRule([Map<String, Object> config = const {}])
: super(
id: ruleId,
severity: readSeverity(config, Severity.warning),
excludes: readExcludes(config),
includes: readIncludes(config),
);

@override
Iterable<Issue> check(InternalResolvedUnitResult source) {
final visitor = _Visitor();

source.unit.visitChildren(visitor);

return visitor.conditionalsInfo
.map((info) => createIssue(
rule: this,
location: nodeLocation(node: info.expression, source: source),
message: _warning,
replacement: _createReplacement(info),
))
.toList(growable: false);
}

Replacement? _createReplacement(_ConditionalInfo info) {
final condition = info.expression.condition;
final correction = '${info.isInverted ? "!" : ""}$condition';

return Replacement(comment: _correctionMessage, replacement: correction);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
part of 'avoid_unnecessary_conditionals_rule.dart';

class _Visitor extends RecursiveAstVisitor<void> {
final _conditionalsInfo = <_ConditionalInfo>[];

Iterable<_ConditionalInfo> get conditionalsInfo => _conditionalsInfo;

_Visitor();

@override
void visitConditionalExpression(ConditionalExpression node) {
super.visitConditionalExpression(node);

final thenExpression = node.thenExpression;
final elseExpression = node.elseExpression;

if (thenExpression is BooleanLiteral && elseExpression is BooleanLiteral) {
final isInverted = !thenExpression.value && elseExpression.value;

_conditionalsInfo.add(
_ConditionalInfo(expression: node, isInverted: isInverted),
);
}
}
}

class _ConditionalInfo {
final ConditionalExpression expression;
final bool isInverted;

const _ConditionalInfo({
required this.expression,
required this.isInverted,
});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import 'package:dart_code_metrics/src/analyzers/lint_analyzer/models/severity.dart';
import 'package:dart_code_metrics/src/analyzers/lint_analyzer/rules/rules_list/avoid_unnecessary_conditionals/avoid_unnecessary_conditionals_rule.dart';
import 'package:test/test.dart';

import '../../../../../helpers/rule_test_helper.dart';

const _examplePath = 'avoid_unnecessary_conditionals/examples/example.dart';

void main() {
group('AvoidUnnecessaryConditionalsRule', () {
test('initialization', () async {
final unit = await RuleTestHelper.resolveFromFile(_examplePath);
final issues = AvoidUnnecessaryConditionalsRule().check(unit);

RuleTestHelper.verifyInitialization(
issues: issues,
ruleId: 'avoid-unnecessary-conditionals',
severity: Severity.warning,
);
});

test('reports about found issues', () async {
final unit = await RuleTestHelper.resolveFromFile(_examplePath);
final issues = AvoidUnnecessaryConditionalsRule().check(unit);

RuleTestHelper.verifyIssues(
issues: issues,
startLines: [7, 13, 16, 18],
startColumns: [19, 10, 15, 15],
locationTexts: [
'str.isEmpty ? true : false',
'str.isEmpty ? false : true',
'foo ? false : true',
'foo ? true : false',
],
messages: [
'Avoid unnecessary conditional expressions.',
'Avoid unnecessary conditional expressions.',
'Avoid unnecessary conditional expressions.',
'Avoid unnecessary conditional expressions.',
],
replacements: [
'str.isEmpty',
'!str.isEmpty',
'!foo',
'foo',
],
replacementComments: [
'Remove unnecessary conditional expression.',
'Remove unnecessary conditional expression.',
'Remove unnecessary conditional expression.',
'Remove unnecessary conditional expression.',
],
);
});
});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
void main() {
const str = '';

final assignment = str.isEmpty ? otherFunction() : bar();
final boolean = str.isEmpty ? bar() : baz();

final another = str.isEmpty ? true : false; // LINT
}

bool foo() {
const str = '';

return str.isEmpty ? false : true; // LINT
}

bool bar() => foo ? false : true; // LINT

bool baz() => foo ? true : false; // LINT

String otherFunction() => 'str';
23 changes: 23 additions & 0 deletions website/docs/rules/common/avoid-unnecessary-conditionals.mdx
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import RuleDetails from '@site/src/components/RuleDetails';

<RuleDetails version="5.2.0" severity="warning" hasFix />

Checks for unnessesary conditional expressions.

### Example {#example}

**❌ Bad:**

```dart
bool baz() => foo ? true : false; // LINT

bool bar() => foo ? false : true; // LINT
```

**✅ Good:**

```dart
bool baz() => foo;

bool bar() => !foo;
```
21 changes: 16 additions & 5 deletions website/docs/rules/index.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ Rules are grouped by category to help you understand their purpose. Each rule ha
hasConfig
hasFix
>
Enforces named argument order in function and constructor invocations
to be the same as corresponding named parameter declaration order.
Enforces named argument order in function and constructor invocations to be
the same as corresponding named parameter declaration order.
</RuleEntry>

<RuleEntry
Expand Down Expand Up @@ -182,6 +182,16 @@ Rules are grouped by category to help you understand their purpose. Each rule ha
inside a test file.
</RuleEntry>

<RuleEntry
name="avoid-unnecessary-conditionals"
type="common"
severity="warning"
version="5.2.0"
hasFix
>
Checks for unnessesary conditional expressions.
</RuleEntry>

<RuleEntry
name="avoid-unnecessary-type-assertions"
type="common"
Expand Down Expand Up @@ -487,8 +497,8 @@ Rules are grouped by category to help you understand their purpose. Each rule ha
version="5.1.0"
hasConfig
>
Suggests to use static class member instead of global constants, variables
and functions.
Suggests to use static class member instead of global constants, variables and
functions.
</RuleEntry>

<RuleEntry
Expand Down Expand Up @@ -654,7 +664,8 @@ Rules are grouped by category to help you understand their purpose. Each rule ha
severity="performance"
version="5.2.0"
>
Warns when a <code>Column</code> widget with only <code>children</code> parameter is wrapped in a <code>SingleChildScrollView</code> widget.
Warns when a <code>Column</code> widget with only <code>children</code>{' '}
parameter is wrapped in a <code>SingleChildScrollView</code> widget.
</RuleEntry>

## Intl specific {#intl-specific}
Expand Down