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

Commit 8039dba

Browse files
lsaudonincendial
andauthored
feat: add static code diagnostic prefer-using-list-view (#1088)
Co-authored-by: Dmitry Zhifarsky <[email protected]>
1 parent 54d50d5 commit 8039dba

File tree

11 files changed

+246
-0
lines changed

11 files changed

+246
-0
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
## Unreleased
44

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

78
## 5.1.0
89

lib/presets/flutter_all.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,4 @@ dart_code_metrics:
1313
- prefer-correct-edge-insets-constructor
1414
- prefer-extracting-callbacks
1515
- prefer-single-widget-per-file
16+
- prefer-using-list-view

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ import 'rules_list/prefer_on_push_cd_strategy/prefer_on_push_cd_strategy_rule.da
6565
import 'rules_list/prefer_single_widget_per_file/prefer_single_widget_per_file_rule.dart';
6666
import 'rules_list/prefer_static_class/prefer_static_class_rule.dart';
6767
import 'rules_list/prefer_trailing_comma/prefer_trailing_comma_rule.dart';
68+
import 'rules_list/prefer_using_list_view/prefer_using_list_view_rule.dart';
6869
import 'rules_list/provide_correct_intl_args/provide_correct_intl_args_rule.dart';
6970
import 'rules_list/tag_name/tag_name_rule.dart';
7071

@@ -144,6 +145,7 @@ final _implementedRules = <String, Rule Function(Map<String, Object>)>{
144145
PreferSingleWidgetPerFileRule.ruleId: PreferSingleWidgetPerFileRule.new,
145146
PreferStaticClassRule.ruleId: PreferStaticClassRule.new,
146147
PreferTrailingCommaRule.ruleId: PreferTrailingCommaRule.new,
148+
PreferUsingListViewRule.ruleId: PreferUsingListViewRule.new,
147149
ProvideCorrectIntlArgsRule.ruleId: ProvideCorrectIntlArgsRule.new,
148150
TagNameRule.ruleId: TagNameRule.new,
149151
};
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
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+
import 'package:collection/collection.dart';
6+
7+
import '../../../../../utils/flutter_types_utils.dart';
8+
import '../../../../../utils/node_utils.dart';
9+
import '../../../lint_utils.dart';
10+
import '../../../models/internal_resolved_unit_result.dart';
11+
import '../../../models/issue.dart';
12+
import '../../../models/severity.dart';
13+
import '../../models/flutter_rule.dart';
14+
import '../../rule_utils.dart';
15+
16+
part 'visitor.dart';
17+
18+
class PreferUsingListViewRule extends FlutterRule {
19+
static const String ruleId = 'prefer-using-list-view';
20+
21+
static const _warning =
22+
'Preferred to use ListView instead of the combo SingleChildScrollView and Column.';
23+
24+
PreferUsingListViewRule([Map<String, Object> config = const {}])
25+
: super(
26+
id: ruleId,
27+
severity: readSeverity(config, Severity.performance),
28+
excludes: readExcludes(config),
29+
includes: readIncludes(config),
30+
);
31+
32+
@override
33+
Iterable<Issue> check(InternalResolvedUnitResult source) {
34+
final visitor = _Visitor();
35+
36+
source.unit.visitChildren(visitor);
37+
38+
return visitor.expressions
39+
.map((expression) => createIssue(
40+
rule: this,
41+
location: nodeLocation(
42+
node: expression,
43+
source: source,
44+
),
45+
message: _warning,
46+
))
47+
.toList(growable: false);
48+
}
49+
}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
part of 'prefer_using_list_view_rule.dart';
2+
3+
class _Visitor extends RecursiveAstVisitor<void> {
4+
final _expressions = <Expression>[];
5+
6+
Iterable<Expression> get expressions => _expressions;
7+
8+
@override
9+
void visitInstanceCreationExpression(InstanceCreationExpression node) {
10+
super.visitInstanceCreationExpression(node);
11+
12+
if (isSingleChildScrollViewWidget(node.staticType) &&
13+
_hasColumnChild(node)) {
14+
_expressions.add(node);
15+
}
16+
}
17+
18+
bool _hasColumnChild(InstanceCreationExpression node) {
19+
final child = node.argumentList.arguments.firstWhereOrNull(
20+
(arg) => arg is NamedExpression && arg.name.label.name == 'child',
21+
);
22+
23+
if (child != null && child is NamedExpression) {
24+
final expression = child.expression;
25+
26+
if (expression is InstanceCreationExpression &&
27+
isColumnWidget(expression.staticType)) {
28+
final notChildren = expression.argumentList.arguments.firstWhereOrNull(
29+
(arg) => arg is NamedExpression && arg.name.label.name != 'children',
30+
);
31+
32+
return notChildren == null;
33+
}
34+
}
35+
36+
return false;
37+
}
38+
}

lib/src/utils/flutter_types_utils.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ bool isSubclassOfListenable(DartType? type) =>
3030
bool isListViewWidget(DartType? type) =>
3131
type?.getDisplayString(withNullability: false) == 'ListView';
3232

33+
bool isSingleChildScrollViewWidget(DartType? type) =>
34+
type?.getDisplayString(withNullability: false) == 'SingleChildScrollView';
35+
3336
bool isColumnWidget(DartType? type) =>
3437
type?.getDisplayString(withNullability: false) == 'Column';
3538

test/src/analyzers/lint_analyzer/rules/rules_factory_test.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ void main() {
4848
'format-comment': <String, Object>{},
4949
'prefer-immediate-return': <String, Object>{},
5050
'tag-name': <String, Object>{},
51+
'prefer-using-list-view': <String, Object>{},
5152
}).map((rule) => rule.id),
5253
equals([
5354
'always-remove-listener',
@@ -89,6 +90,7 @@ void main() {
8990
'prefer-on-push-cd-strategy',
9091
'prefer-single-widget-per-file',
9192
'prefer-trailing-comma',
93+
'prefer-using-list-view',
9294
'provide-correct-intl-args',
9395
'tag-name',
9496
]),
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
import 'package:flutter/material.dart';
2+
3+
void main() {
4+
runApp(MyApp());
5+
}
6+
7+
class MyApp extends StatelessWidget {
8+
@override
9+
Widget build(BuildContext context) {
10+
// LINT
11+
return SingleChildScrollView(
12+
child: Column(
13+
children: [
14+
Text('Wow lint rule'),
15+
Text('Wow another lint rule'),
16+
],
17+
),
18+
);
19+
}
20+
}
21+
22+
class MyApp extends StatelessWidget {
23+
@override
24+
Widget build(BuildContext context) {
25+
return SingleChildScrollView(
26+
child: Column(
27+
crossAxisAlignment: CrossAxisAlignment.start,
28+
children: [
29+
Text('Wow lint rule'),
30+
Text('Wow another lint rule'),
31+
],
32+
),
33+
);
34+
}
35+
}
36+
37+
class MyApp extends StatelessWidget {
38+
@override
39+
Widget build(BuildContext context) {
40+
return SingleChildScrollView(
41+
child: Column(
42+
mainAxisAlignment: MainAxisAlignment.start,
43+
children: [
44+
Text('Wow lint rule'),
45+
Text('Wow another lint rule'),
46+
],
47+
),
48+
);
49+
}
50+
}
51+
52+
class MyApp extends StatelessWidget {
53+
@override
54+
Widget build(BuildContext context) {
55+
return MaterialApp(
56+
home: Scaffold(
57+
body: SingleChildScrollView(
58+
child: Text(),
59+
),
60+
),
61+
);
62+
}
63+
}
64+
65+
class SingleChildScrollView {}
66+
67+
class Column {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
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/prefer_using_list_view/prefer_using_list_view_rule.dart';
3+
import 'package:test/test.dart';
4+
5+
import '../../../../../helpers/rule_test_helper.dart';
6+
7+
const _examplePath = 'prefer_using_list_view/examples/example.dart';
8+
9+
void main() {
10+
group(
11+
'PreferUsingListViewRule',
12+
() {
13+
test('initialization', () async {
14+
final unit = await RuleTestHelper.resolveFromFile(_examplePath);
15+
final issues = PreferUsingListViewRule().check(unit);
16+
17+
RuleTestHelper.verifyInitialization(
18+
issues: issues,
19+
ruleId: 'prefer-using-list-view',
20+
severity: Severity.performance,
21+
);
22+
});
23+
24+
test('reports about found issues', () async {
25+
final unit = await RuleTestHelper.resolveFromFile(_examplePath);
26+
final issues = PreferUsingListViewRule().check(unit);
27+
28+
RuleTestHelper.verifyIssues(
29+
issues: issues,
30+
startLines: [11],
31+
startColumns: [12],
32+
messages: [
33+
'Preferred to use ListView instead of the combo SingleChildScrollView and Column.',
34+
],
35+
);
36+
});
37+
},
38+
);
39+
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import RuleDetails from '@site/src/components/RuleDetails';
2+
3+
<RuleDetails version="5.2.0" severity="performance" />
4+
5+
Warns when a `Column` widget with only `children` parameter is wrapped in a `SingleChildScrollView` widget.
6+
7+
Additional resources:
8+
9+
- <https://stackoverflow.com/a/62147092>
10+
11+
### Example {#example}
12+
13+
**❌ Bad:**
14+
15+
```dart
16+
SingleChildScrollView(
17+
child: Column(
18+
children: [
19+
Text('Wow lint rule'),
20+
Text('Wow another lint rule'),
21+
],
22+
),
23+
),
24+
```
25+
26+
**✅ Good:**
27+
28+
```dart
29+
ListView(
30+
children: [
31+
Text('Wow lint rule'),
32+
Text('Wow another lint rule'),
33+
],
34+
),
35+
```

website/docs/rules/index.mdx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -648,6 +648,15 @@ Rules are grouped by category to help you understand their purpose. Each rule ha
648648
Warns when a file contains more than a single widget.
649649
</RuleEntry>
650650

651+
<RuleEntry
652+
name="prefer-using-list-view"
653+
type="flutter"
654+
severity="performance"
655+
version="5.2.0"
656+
>
657+
Warns when a <code>Column</code> widget with only <code>children</code> parameter is wrapped in a <code>SingleChildScrollView</code> widget.
658+
</RuleEntry>
659+
651660
## Intl specific {#intl-specific}
652661

653662
<RuleEntry

0 commit comments

Comments
 (0)