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

Commit 73b31fe

Browse files
feat: add avoid-shrink-wrap-in-lists (#918)
Co-authored-by: Dmitry Krutskikh <dmitry.krutskikh@gmail.com>
1 parent 0be027b commit 73b31fe

File tree

8 files changed

+239
-3
lines changed

8 files changed

+239
-3
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
* feat: add static code diagnostic [`avoid-duplicate-exports`](https://dartcodemetrics.dev/docs/rules/common/avoid-duplicate-exports).
6+
* feat: add static code diagnostic [`avoid-shrink-wrap-in-lists`](https://dartcodemetrics.dev/docs/rules/flutter/avoid-shrink-wrap-in-lists).
67
* feat: add static code diagnostic [`avoid-top-level-members-in-tests`](https://dartcodemetrics.dev/docs/rules/common/avoid-top-level-members-in-tests).
78
* feat: add static code diagnostic [`prefer-correct-edge-insets-constructor-rule`](https://dartcodemetrics.dev/docs/rules/flutter/prefer-correct-edge-insets-constructor).
89
* feat: add static code diagnostic [`prefer-enums-by-name`](https://dartcodemetrics.dev/docs/rules/common/prefer-enums-by-name).

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import 'rules_list/avoid_non_ascii_symbols/avoid_non_ascii_symbols_rule.dart';
1515
import 'rules_list/avoid_non_null_assertion/avoid_non_null_assertion_rule.dart';
1616
import 'rules_list/avoid_preserve_whitespace_false/avoid_preserve_whitespace_false_rule.dart';
1717
import 'rules_list/avoid_returning_widgets/avoid_returning_widgets_rule.dart';
18+
import 'rules_list/avoid_shrink_wrap_in_lists/avoid_shrink_wrap_in_lists_rule.dart';
1819
import 'rules_list/avoid_throw_in_catch_block/avoid_throw_in_catch_block_rule.dart';
1920
import 'rules_list/avoid_top_level_members_in_tests/avoid_top_level_members_in_tests_rule.dart';
2021
import 'rules_list/avoid_unnecessary_setstate/avoid_unnecessary_setstate_rule.dart';
@@ -77,6 +78,7 @@ final _implementedRules = <String, Rule Function(Map<String, Object>)>{
7778
AvoidNonNullAssertionRule.ruleId: AvoidNonNullAssertionRule.new,
7879
AvoidPreserveWhitespaceFalseRule.ruleId: AvoidPreserveWhitespaceFalseRule.new,
7980
AvoidReturningWidgetsRule.ruleId: AvoidReturningWidgetsRule.new,
81+
AvoidShrinkWrapInListsRule.ruleId: AvoidShrinkWrapInListsRule.new,
8082
AvoidThrowInCatchBlockRule.ruleId: AvoidThrowInCatchBlockRule.new,
8183
AvoidTopLevelMembersInTestsRule.ruleId: AvoidTopLevelMembersInTestsRule.new,
8284
AvoidUnnecessarySetStateRule.ruleId: AvoidUnnecessarySetStateRule.new,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
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 AvoidShrinkWrapInListsRule extends FlutterRule {
19+
static const String ruleId = 'avoid-shrink-wrap-in-lists';
20+
21+
static const _warning =
22+
'Avoid using ListView with shrinkWrap, since it might degrade the performance. Consider using slivers instead.';
23+
24+
AvoidShrinkWrapInListsRule([Map<String, Object> config = const {}])
25+
: super(
26+
id: ruleId,
27+
severity: readSeverity(config, Severity.performance),
28+
excludes: readExcludes(config),
29+
);
30+
31+
@override
32+
Iterable<Issue> check(InternalResolvedUnitResult source) {
33+
final visitor = _Visitor();
34+
35+
source.unit.visitChildren(visitor);
36+
37+
return visitor.expressions
38+
.map((expression) => createIssue(
39+
rule: this,
40+
location: nodeLocation(
41+
node: expression,
42+
source: source,
43+
),
44+
message: _warning,
45+
))
46+
.toList(growable: false);
47+
}
48+
}
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
part of 'avoid_shrink_wrap_in_lists_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 (isListViewWidget(node.staticType) &&
13+
_hasShrinkWrap(node) &&
14+
_hasParentList(node)) {
15+
_expressions.add(node);
16+
}
17+
}
18+
19+
bool _hasShrinkWrap(InstanceCreationExpression node) =>
20+
node.argumentList.arguments.firstWhereOrNull(
21+
(arg) => arg is NamedExpression && arg.name.label.name == 'shrinkWrap',
22+
) !=
23+
null;
24+
25+
bool _hasParentList(InstanceCreationExpression node) =>
26+
node.thisOrAncestorMatching((parent) =>
27+
parent != node &&
28+
parent is InstanceCreationExpression &&
29+
(isListViewWidget(parent.staticType) ||
30+
isColumnWidget(parent.staticType) ||
31+
isRowWidget(parent.staticType))) !=
32+
null;
33+
}

lib/src/utils/flutter_types_utils.dart

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,24 @@ bool isWidgetStateOrSubclass(DartType? type) =>
1818
bool isSubclassOfListenable(DartType? type) =>
1919
type is InterfaceType && type.allSupertypes.any(_isListenable);
2020

21+
bool isListViewWidget(DartType? type) =>
22+
type?.getDisplayString(withNullability: false) == 'ListView';
23+
24+
bool isColumnWidget(DartType? type) =>
25+
type?.getDisplayString(withNullability: false) == 'Column';
26+
27+
bool isRowWidget(DartType? type) =>
28+
type?.getDisplayString(withNullability: false) == 'Row';
29+
2130
bool isPaddingWidget(DartType? type) =>
2231
type?.getDisplayString(withNullability: false) == 'Padding';
2332

24-
bool _isWidget(DartType? type) =>
25-
type?.getDisplayString(withNullability: false) == 'Widget';
26-
2733
bool isBuildContext(DartType? type) =>
2834
type?.getDisplayString(withNullability: false) == 'BuildContext';
2935

36+
bool _isWidget(DartType? type) =>
37+
type?.getDisplayString(withNullability: false) == 'Widget';
38+
3039
bool _isSubclassOfWidget(DartType? type) =>
3140
type is InterfaceType && type.allSupertypes.any(_isWidget);
3241

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
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_shrink_wrap_in_lists/avoid_shrink_wrap_in_lists_rule.dart';
3+
import 'package:test/test.dart';
4+
5+
import '../../../../../helpers/rule_test_helper.dart';
6+
7+
const _examplePath = 'avoid_shrink_wrap_in_lists/examples/example.dart';
8+
9+
void main() {
10+
group('AvoidShrinkWrapInListsRule', () {
11+
test('initialization', () async {
12+
final unit = await RuleTestHelper.resolveFromFile(_examplePath);
13+
final issues = AvoidShrinkWrapInListsRule().check(unit);
14+
15+
RuleTestHelper.verifyInitialization(
16+
issues: issues,
17+
ruleId: 'avoid-shrink-wrap-in-lists',
18+
severity: Severity.performance,
19+
);
20+
});
21+
22+
test('reports about found issues', () async {
23+
final unit = await RuleTestHelper.resolveFromFile(_examplePath);
24+
final issues = AvoidShrinkWrapInListsRule().check(unit);
25+
26+
RuleTestHelper.verifyIssues(
27+
issues: issues,
28+
startLines: [10, 17, 24],
29+
startColumns: [30, 27, 32],
30+
locationTexts: [
31+
'ListView(shrinkWrap: true, children: [])',
32+
'ListView(shrinkWrap: true, children: [])',
33+
'ListView(shrinkWrap: true, children: [])',
34+
],
35+
messages: [
36+
'Avoid using ListView with shrinkWrap, since it might degrade the performance. Consider using slivers instead.',
37+
'Avoid using ListView with shrinkWrap, since it might degrade the performance. Consider using slivers instead.',
38+
'Avoid using ListView with shrinkWrap, since it might degrade the performance. Consider using slivers instead.',
39+
],
40+
);
41+
});
42+
});
43+
}
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
class CoolWidget {
2+
Widget build() {
3+
return ListView(child: Widget());
4+
}
5+
}
6+
7+
class WidgetWithColumn {
8+
Widget build() {
9+
// LINT
10+
return Column(children: [ListView(shrinkWrap: true, children: [])]);
11+
}
12+
}
13+
14+
class WidgetWithRow {
15+
Widget build() {
16+
// LINT
17+
return Row(children: [ListView(shrinkWrap: true, children: [])]);
18+
}
19+
}
20+
21+
class WidgetWithList {
22+
Widget build() {
23+
// LINT
24+
return ListView(children: [ListView(shrinkWrap: true, children: [])]);
25+
}
26+
}
27+
28+
class ListView extends Widget {
29+
final bool shrinkWrap;
30+
final List<Widget> children;
31+
32+
const ListView({required this.shrinkWrap, required this.children});
33+
}
34+
35+
class Column extends Widget {
36+
final List<Widget> children;
37+
38+
const Column({required this.children});
39+
}
40+
41+
class Row extends Widget {
42+
final List<Widget> children;
43+
44+
const Row({required this.children});
45+
}
46+
47+
class Widget {}
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
# Avoid shrink wrap in lists
2+
3+
## Rule id {#rule-id}
4+
5+
avoid-shrink-wrap-in-lists
6+
7+
## Severity {#severity}
8+
9+
Performance
10+
11+
## Description {#description}
12+
13+
Warns when a `ListView` widget with `shrinkWrap` parameter is wrapped in a `Column`, `Row` or another `ListView` widget.
14+
15+
According to the Flutter documentation, using `shrinkWrap` in lists is expensive performance-wise and should be avoided, since using slivers is significantly cheaper and achieves the same or even better results.
16+
17+
Additional resources:
18+
19+
- <https://github.com/dart-lang/linter/issues/3496>
20+
21+
### Example {#example}
22+
23+
Bad:
24+
25+
```dart
26+
Column(
27+
children: [
28+
Expanded(
29+
// LINT
30+
child: ListView(
31+
shrinkWrap: true,
32+
physics: const NeverScrollableScrollPhysics(),
33+
children: [],
34+
),
35+
),
36+
],
37+
),
38+
```
39+
40+
Good:
41+
42+
```dart
43+
CustomScrollView(
44+
slivers: [
45+
SliverList(
46+
delegate: SliverChildBuilderDelegate(
47+
(context, index) => Container(),
48+
childCount: someObject.length,
49+
),
50+
),
51+
],
52+
),
53+
```

0 commit comments

Comments
 (0)