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

Commit ced086d

Browse files
author
Konoshenko Vlad
authored
feat: add static code diagnostics avoid-border-all. (#691)
* feat: add static code diagnostics `avoid-border-all`. * doc: added documentation * fix: added avoid-border-all rule to rule factory * fix: test * fix: test * fix: review changes * fix: review changes * fix: review changes * fix: documentation * fix: documentation changed
1 parent ed52f53 commit ced086d

File tree

10 files changed

+316
-17
lines changed

10 files changed

+316
-17
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
## Unreleased
44

5+
* feat: add static code diagnostics `avoid-border-all`.
56
* chore: activate new lint rules.
67
* feat: improve `avoid-returning-widgets` builder functions handling.
78
* fix: correctly handle const maps in `no-magic-number`.

‎lib/src/analyzers/lint_analyzer/rules/rules_factory.dart‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import 'models/rule.dart';
22
import 'rules_list/always_remove_listener/always_remove_listener_rule.dart';
3+
import 'rules_list/avoid_border_all/avoid_border_all_rule.dart';
34
import 'rules_list/avoid_dynamic/avoid_dynamic_rule.dart';
45
import 'rules_list/avoid_global_state/avoid_global_state_rule.dart';
56
import 'rules_list/avoid_ignoring_return_values/avoid_ignoring_return_values_rule.dart';
@@ -45,6 +46,7 @@ import 'rules_list/provide_correct_intl_args/provide_correct_intl_args_rule.dart
4546

4647
final _implementedRules = <String, Rule Function(Map<String, Object>)>{
4748
AlwaysRemoveListenerRule.ruleId: (config) => AlwaysRemoveListenerRule(config),
49+
AvoidBorderAllRule.ruleId: (config) => AvoidBorderAllRule(config),
4850
AvoidDynamicRule.ruleId: (config) => AvoidDynamicRule(config),
4951
AvoidGlobalStateRule.ruleId: (config) => AvoidGlobalStateRule(config),
5052
AvoidIgnoringReturnValuesRule.ruleId: (config) =>

‎lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_border_all/avoid_border_all_rule.dart‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// ignore_for_file: public_member_api_docs
2+
3+
import 'package:analyzer/dart/ast/ast.dart';
4+
import 'package:analyzer/dart/ast/visitor.dart';
5+
6+
import '../../../../../utils/node_utils.dart';
7+
import '../../../lint_utils.dart';
8+
import '../../../models/internal_resolved_unit_result.dart';
9+
import '../../../models/issue.dart';
10+
import '../../../models/replacement.dart';
11+
import '../../../models/severity.dart';
12+
import '../../models/flutter_rule.dart';
13+
import '../../rule_utils.dart';
14+
15+
part 'visitor.dart';
16+
17+
class AvoidBorderAllRule extends FlutterRule {
18+
static const ruleId = 'avoid-border-all';
19+
static const _issueMessage =
20+
'Prefer using const constructor Border.fromBorderSide.';
21+
static const _replaceComment = 'Replace with const Border.fromBorderSide.';
22+
23+
AvoidBorderAllRule([Map<String, Object> config = const {}])
24+
: super(
25+
id: ruleId,
26+
severity: readSeverity(config, Severity.performance),
27+
excludes: readExcludes(config),
28+
);
29+
30+
@override
31+
Iterable<Issue> check(InternalResolvedUnitResult source) {
32+
final visitor = _Visitor();
33+
source.unit.visitChildren(visitor);
34+
35+
return visitor.expressions
36+
.map((expression) => createIssue(
37+
rule: this,
38+
location: nodeLocation(
39+
node: expression,
40+
source: source,
41+
withCommentOrMetadata: true,
42+
),
43+
message: _issueMessage,
44+
replacement: _createReplacement(expression),
45+
))
46+
.toList(growable: false);
47+
}
48+
49+
Replacement? _createReplacement(InstanceCreationExpression expression) {
50+
final value = expression.argumentList.arguments
51+
.map((arg) => '$arg')
52+
.join(', ')
53+
.trim();
54+
55+
return Replacement(
56+
comment: _replaceComment,
57+
replacement: 'const Border.fromBorderSide(BorderSide($value))',
58+
);
59+
}
60+
}

‎lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_border_all/visitor.dart‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
part of 'avoid_border_all_rule.dart';
2+
3+
const _className = 'Border';
4+
const _borderRadiusConstructorName = 'all';
5+
6+
class _Visitor extends RecursiveAstVisitor<void> {
7+
final _expressions = <InstanceCreationExpression>[];
8+
9+
Iterable<InstanceCreationExpression> get expressions => _expressions;
10+
11+
@override
12+
void visitInstanceCreationExpression(InstanceCreationExpression expression) {
13+
super.visitInstanceCreationExpression(expression);
14+
15+
if (expression.staticType?.getDisplayString(withNullability: true) ==
16+
_className &&
17+
expression.constructorName.name?.name == _borderRadiusConstructorName) {
18+
_expressions.add(expression);
19+
}
20+
}
21+
}

‎lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_const_border_radius/prefer_const_border_radius_rule.dart‎

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -48,19 +48,14 @@ class PreferConstBorderRadiusRule extends FlutterRule {
4848
}
4949

5050
Replacement? _createReplacement(InstanceCreationExpression expression) {
51-
final value = _getConstructorArgumentValue(expression);
52-
53-
return value != null
54-
? Replacement(
55-
comment: _replaceComment,
56-
replacement: 'const BorderRadius.all(Radius.circular($value))',
57-
)
58-
: null;
59-
}
60-
61-
String? _getConstructorArgumentValue(InstanceCreationExpression expression) {
62-
final arguments = expression.argumentList.arguments;
63-
64-
return arguments.isNotEmpty ? arguments.first.toString() : null;
51+
final value = expression.argumentList.arguments
52+
.map((arg) => '$arg')
53+
.join(', ')
54+
.trim();
55+
56+
return Replacement(
57+
comment: _replaceComment,
58+
replacement: 'const BorderRadius.all(Radius.circular($value))',
59+
);
6560
}
6661
}

‎test/src/analyzers/lint_analyzer/rules/rules_list/avoid_border_all/avoid_border_all_rule_test.dart‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
import 'package:dart_code_metrics/src/analyzers/lint_analyzer/models/severity.dart';
2+
import 'package:dart_code_metrics/src/analyzers/lint_analyzer/rules/rules_list/avoid_border_all/avoid_border_all_rule.dart';
3+
import 'package:test/test.dart';
4+
5+
import '../../../../../helpers/rule_test_helper.dart';
6+
7+
const _examplePath = 'avoid_border_all/examples/example.dart';
8+
9+
void main() {
10+
group('AvoidBorderAllRule', () {
11+
test('initialization', () async {
12+
final unit = await RuleTestHelper.resolveFromFile(_examplePath);
13+
final issues = AvoidBorderAllRule().check(unit);
14+
15+
RuleTestHelper.verifyInitialization(
16+
issues: issues,
17+
ruleId: 'avoid-border-all',
18+
severity: Severity.performance,
19+
);
20+
});
21+
22+
test('reports about found issues', () async {
23+
final unit = await RuleTestHelper.resolveFromFile(_examplePath);
24+
final issues = AvoidBorderAllRule().check(unit);
25+
26+
RuleTestHelper.verifyIssues(
27+
issues: issues,
28+
startLines: [4, 6, 11, 17],
29+
startColumns: [27, 19, 19, 19],
30+
locationTexts: [
31+
'Border.all()',
32+
'Border.all(\n'
33+
' color: const Color(0),\n'
34+
' )',
35+
'Border.all(\n'
36+
' color: const Color(0),\n'
37+
' width: 1,\n'
38+
' )',
39+
'Border.all(\n'
40+
' color: const Color(0),\n'
41+
' width: 1,\n'
42+
' style: BorderStyle.none,\n'
43+
' )',
44+
],
45+
replacementComments: [
46+
'Replace with const Border.fromBorderSide.',
47+
'Replace with const Border.fromBorderSide.',
48+
'Replace with const Border.fromBorderSide.',
49+
'Replace with const Border.fromBorderSide.',
50+
],
51+
replacements: [
52+
'const Border.fromBorderSide(BorderSide())',
53+
'const Border.fromBorderSide(BorderSide(color: const Color(0)))',
54+
'const Border.fromBorderSide(BorderSide(color: const Color(0), width: 1))',
55+
'const Border.fromBorderSide(BorderSide(color: const Color(0), width: 1, style: BorderStyle.none))',
56+
],
57+
messages: [
58+
'Prefer using const constructor Border.fromBorderSide.',
59+
'Prefer using const constructor Border.fromBorderSide.',
60+
'Prefer using const constructor Border.fromBorderSide.',
61+
'Prefer using const constructor Border.fromBorderSide.',
62+
],
63+
);
64+
});
65+
});
66+
}

‎test/src/analyzers/lint_analyzer/rules/rules_list/avoid_border_all/examples/example.dart‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
class MyWidget extends StatelessWidget {
2+
Widget build(BuildContext _) => Column(children: [
3+
const Container(border: Border.fromBorderSide(BorderSide())),
4+
Container(border: Border.all()), // LINT
5+
Container(
6+
border: Border.all(
7+
color: const Color(0),
8+
),
9+
), // LINT
10+
Container(
11+
border: Border.all(
12+
color: const Color(0),
13+
width: 1,
14+
),
15+
), // LINT
16+
Container(
17+
border: Border.all(
18+
color: const Color(0),
19+
width: 1,
20+
style: BorderStyle.none,
21+
),
22+
), // LINT
23+
]);
24+
}
25+
26+
class Border {
27+
factory Border.all({
28+
Color color = const Color(0xFF000000),
29+
double width = 1.0,
30+
BorderStyle style = BorderStyle.solid,
31+
}) {
32+
final side = BorderSide(color: color, width: width, style: style);
33+
34+
return Border.fromBorderSide(side);
35+
}
36+
37+
const Border.fromBorderSide(BorderSide _);
38+
}
39+
40+
enum BorderStyle {
41+
none,
42+
solid,
43+
}
44+
45+
class BorderSide {
46+
final Color color;
47+
final double width;
48+
final BorderStyle style;
49+
50+
const BorderSide({
51+
this.color = const Color(0xFF000000),
52+
this.width = 1.0,
53+
this.style = BorderStyle.solid,
54+
});
55+
}
56+
57+
class Color {
58+
final int value;
59+
60+
const Color(int value) : value = value & 0xFFFFFFFF;
61+
}
62+
63+
class Container extends Widget {
64+
final Widget? child;
65+
final Border? border;
66+
67+
const Container({this.child, this.border});
68+
}
69+
70+
class Column extends Widget {
71+
final List<Widget> children;
72+
73+
Column({required this.children});
74+
}
75+
76+
class StatelessWidget extends Widget {}
77+
78+
class Widget {
79+
const Widget();
80+
}
81+
82+
class BuildContext {}

‎website/docs/rules/flutter/avoid-border-all.md‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
# Avoid using Border.all constructor
2+
3+
![Has auto-fix](https://img.shields.io/badge/-has%20auto--fix-success)
4+
5+
## Rule id {#rule-id}
6+
7+
avoid-border-all
8+
9+
## Severity {#severity}
10+
11+
Performance
12+
13+
## Description {#description}
14+
15+
`Border.all` constructor calls **const** `Border.fromBorderSide` constructor under the hood. This rule allows to replace `Border.all` with **const** `Border.fromBorderSide`.
16+
17+
### Example {#example}
18+
19+
Bad:
20+
21+
```dart
22+
23+
class BorderWidget extends StatelessWidget {
24+
final Widget child;
25+
26+
const RoundedWidget({
27+
Key? key,
28+
required this.child,
29+
}) : super(key: key);
30+
31+
@override
32+
Widget build(BuildContext context) {
33+
return Container(
34+
//LINT
35+
border: Border.all({
36+
Color color = const Color(0xFF000000),
37+
double width = 1.0,
38+
BorderStyle style = BorderStyle.solid,
39+
}),
40+
child: child,
41+
);
42+
}
43+
}
44+
```
45+
46+
Good:
47+
48+
```dart
49+
50+
class BorderWidget extends StatelessWidget {
51+
final Widget child;
52+
53+
const RoundedWidget({
54+
Key? key,
55+
required this.child,
56+
}) : super(key: key);
57+
58+
@override
59+
Widget build(BuildContext context) {
60+
return Container(
61+
border: const Border.fromBorderSide(BorderSide(
62+
color: const Color(0xFF000000),
63+
width: 1.0,
64+
style: BorderStyle.solid,
65+
)),
66+
child: child,
67+
);
68+
}
69+
}
70+
```

‎website/docs/rules/flutter/prefer-const-border-radius.md‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,7 @@ Performance
1212

1313
## Description {#description}
1414

15-
`BorderRadius.circular` constructor calls const `BorderRadius.all` constructor under the hood. This rule allows to
16-
replace
17-
`BorderRadius.circular(value)` with const `BorderRadius.all(Radius.circular(value))` if radius is a constant value.
15+
`BorderRadius.circular` constructor calls const `BorderRadius.all` constructor under the hood. This rule allows to replace `BorderRadius.circular(value)` with const `BorderRadius.all(Radius.circular(value))` if radius is a constant value.
1816

1917
### Example {#example}
2018

‎website/docs/rules/overview.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,10 @@ Rules configuration is [described here](../getting-started/configuration#configu
141141

142142
Warns when an event listener is added but never removed.
143143

144+
- [avoid-border-all](./flutter/avoid-border-all.md)
145+
146+
Avoid using Border.all constructor.
147+
144148
- [avoid-returning-widgets](./flutter/avoid-returning-widgets.md) &nbsp; [![Configurable](https://img.shields.io/badge/-configurable-informational)](./flutter/avoid-returning-widgets.md#config-example)
145149

146150
Warns when a method or function returns a Widget or subclass of a Widget.

0 commit comments

Comments
 (0)