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

feat: add static code diagnostic prefer-using-list-view #1088

Merged
merged 2 commits into from
Dec 5, 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 @@ -3,6 +3,7 @@
## Unreleased

* 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).

## 5.1.0

Expand Down
1 change: 1 addition & 0 deletions lib/presets/flutter_all.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@ dart_code_metrics:
- prefer-correct-edge-insets-constructor
- prefer-extracting-callbacks
- prefer-single-widget-per-file
- prefer-using-list-view
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 @@ -65,6 +65,7 @@ import 'rules_list/prefer_on_push_cd_strategy/prefer_on_push_cd_strategy_rule.da
import 'rules_list/prefer_single_widget_per_file/prefer_single_widget_per_file_rule.dart';
import 'rules_list/prefer_static_class/prefer_static_class_rule.dart';
import 'rules_list/prefer_trailing_comma/prefer_trailing_comma_rule.dart';
import 'rules_list/prefer_using_list_view/prefer_using_list_view_rule.dart';
import 'rules_list/provide_correct_intl_args/provide_correct_intl_args_rule.dart';
import 'rules_list/tag_name/tag_name_rule.dart';

Expand Down Expand Up @@ -144,6 +145,7 @@ final _implementedRules = <String, Rule Function(Map<String, Object>)>{
PreferSingleWidgetPerFileRule.ruleId: PreferSingleWidgetPerFileRule.new,
PreferStaticClassRule.ruleId: PreferStaticClassRule.new,
PreferTrailingCommaRule.ruleId: PreferTrailingCommaRule.new,
PreferUsingListViewRule.ruleId: PreferUsingListViewRule.new,
ProvideCorrectIntlArgsRule.ruleId: ProvideCorrectIntlArgsRule.new,
TagNameRule.ruleId: TagNameRule.new,
};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// ignore_for_file: public_member_api_docs

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

import '../../../../../utils/flutter_types_utils.dart';
import '../../../../../utils/node_utils.dart';
import '../../../lint_utils.dart';
import '../../../models/internal_resolved_unit_result.dart';
import '../../../models/issue.dart';
import '../../../models/severity.dart';
import '../../models/flutter_rule.dart';
import '../../rule_utils.dart';

part 'visitor.dart';

class PreferUsingListViewRule extends FlutterRule {
static const String ruleId = 'prefer-using-list-view';

static const _warning =
'Preferred to use ListView instead of the combo SingleChildScrollView and Column.';

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

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

source.unit.visitChildren(visitor);

return visitor.expressions
.map((expression) => createIssue(
rule: this,
location: nodeLocation(
node: expression,
source: source,
),
message: _warning,
))
.toList(growable: false);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
part of 'prefer_using_list_view_rule.dart';

class _Visitor extends RecursiveAstVisitor<void> {
final _expressions = <Expression>[];

Iterable<Expression> get expressions => _expressions;

@override
void visitInstanceCreationExpression(InstanceCreationExpression node) {
super.visitInstanceCreationExpression(node);

if (isSingleChildScrollViewWidget(node.staticType) &&
_hasColumnChild(node)) {
_expressions.add(node);
}
}

bool _hasColumnChild(InstanceCreationExpression node) {
final child = node.argumentList.arguments.firstWhereOrNull(
(arg) => arg is NamedExpression && arg.name.label.name == 'child',
);

if (child != null && child is NamedExpression) {
final expression = child.expression;

if (expression is InstanceCreationExpression &&
isColumnWidget(expression.staticType)) {
final notChildren = expression.argumentList.arguments.firstWhereOrNull(
(arg) => arg is NamedExpression && arg.name.label.name != 'children',
);

return notChildren == null;
}
}

return false;
}
}
3 changes: 3 additions & 0 deletions lib/src/utils/flutter_types_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ bool isSubclassOfListenable(DartType? type) =>
bool isListViewWidget(DartType? type) =>
type?.getDisplayString(withNullability: false) == 'ListView';

bool isSingleChildScrollViewWidget(DartType? type) =>
type?.getDisplayString(withNullability: false) == 'SingleChildScrollView';

bool isColumnWidget(DartType? type) =>
type?.getDisplayString(withNullability: false) == 'Column';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ void main() {
'format-comment': <String, Object>{},
'prefer-immediate-return': <String, Object>{},
'tag-name': <String, Object>{},
'prefer-using-list-view': <String, Object>{},
}).map((rule) => rule.id),
equals([
'always-remove-listener',
Expand Down Expand Up @@ -89,6 +90,7 @@ void main() {
'prefer-on-push-cd-strategy',
'prefer-single-widget-per-file',
'prefer-trailing-comma',
'prefer-using-list-view',
'provide-correct-intl-args',
'tag-name',
]),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import 'package:flutter/material.dart';

void main() {
runApp(MyApp());
}

class MyApp extends StatelessWidget {
@override
Widget build(BuildContext context) {
// LINT
return SingleChildScrollView(
child: Column(
children: [
Text('Wow lint rule'),
Text('Wow another lint rule'),
],
),
);
}
}

class MyApp extends StatelessWidget {
@override
Widget build(BuildContext context) {
return SingleChildScrollView(
child: Column(
crossAxisAlignment: CrossAxisAlignment.start,
children: [
Text('Wow lint rule'),
Text('Wow another lint rule'),
],
),
);
}
}

class MyApp extends StatelessWidget {
@override
Widget build(BuildContext context) {
return SingleChildScrollView(
child: Column(
mainAxisAlignment: MainAxisAlignment.start,
children: [
Text('Wow lint rule'),
Text('Wow another lint rule'),
],
),
);
}
}

class MyApp extends StatelessWidget {
@override
Widget build(BuildContext context) {
return MaterialApp(
home: Scaffold(
body: SingleChildScrollView(
child: Text(),
),
),
);
}
}

class SingleChildScrollView {}

class Column {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import 'package:dart_code_metrics/src/analyzers/lint_analyzer/models/severity.dart';
import 'package:dart_code_metrics/src/analyzers/lint_analyzer/rules/rules_list/prefer_using_list_view/prefer_using_list_view_rule.dart';
import 'package:test/test.dart';

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

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

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

RuleTestHelper.verifyInitialization(
issues: issues,
ruleId: 'prefer-using-list-view',
severity: Severity.performance,
);
});

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

RuleTestHelper.verifyIssues(
issues: issues,
startLines: [11],
startColumns: [12],
messages: [
'Preferred to use ListView instead of the combo SingleChildScrollView and Column.',
],
);
});
},
);
}
35 changes: 35 additions & 0 deletions website/docs/rules/flutter/prefer-using-list-view.mdx
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import RuleDetails from '@site/src/components/RuleDetails';

<RuleDetails version="5.2.0" severity="performance" />

Warns when a `Column` widget with only `children` parameter is wrapped in a `SingleChildScrollView` widget.

Additional resources:

- <https://stackoverflow.com/a/62147092>

### Example {#example}

**❌ Bad:**

```dart
SingleChildScrollView(
child: Column(
children: [
Text('Wow lint rule'),
Text('Wow another lint rule'),
],
),
),
```

**✅ Good:**

```dart
ListView(
children: [
Text('Wow lint rule'),
Text('Wow another lint rule'),
],
),
```
9 changes: 9 additions & 0 deletions website/docs/rules/index.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,15 @@ Rules are grouped by category to help you understand their purpose. Each rule ha
Warns when a file contains more than a single widget.
</RuleEntry>

<RuleEntry
name="prefer-using-list-view"
type="flutter"
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.
</RuleEntry>

## Intl specific {#intl-specific}

<RuleEntry
Expand Down