From b36d165bb48408f16ad977683f702fb747f56bb7 Mon Sep 17 00:00:00 2001 From: Ryota Kobayashi <45661924+naipaka@users.noreply.github.com> Date: Mon, 29 Jul 2024 14:29:08 +0900 Subject: [PATCH] feat: Add `avoid_shrink_wrap_in_list_view` rule (#28) * feat: Add `avoid_shrink_wrap_in_list_view` rule * chore: Add doc comment * chore: Add examples of correct usage comment Co-authored-by: Ryunosuke Muramatsu --------- Co-authored-by: Ryunosuke Muramatsu --- .vscode/settings.json | 1 + packages/altive_lints/lib/altive_lints.dart | 2 + .../lints/avoid_shrink_wrap_in_list_view.dart | 87 +++++++++++ .../lib/src/utils/types_utils.dart | 147 ++++++++++++++++++ .../lints/avoid_shrink_wrap_in_list_view.dart | 26 ++++ packages/altive_lints/lint_test/pubspec.yaml | 2 + packages/altive_lints/pubspec.yaml | 1 + 7 files changed, 266 insertions(+) create mode 100644 packages/altive_lints/lib/src/lints/avoid_shrink_wrap_in_list_view.dart create mode 100644 packages/altive_lints/lib/src/utils/types_utils.dart create mode 100644 packages/altive_lints/lint_test/lints/avoid_shrink_wrap_in_list_view.dart diff --git a/.vscode/settings.json b/.vscode/settings.json index b4c68ee..9c316d5 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -12,6 +12,7 @@ "pubspec", "redeclares", "subosito", + "Supertypes", "tearoffs", "todos", "unawaited", diff --git a/packages/altive_lints/lib/altive_lints.dart b/packages/altive_lints/lib/altive_lints.dart index d426b12..c52c538 100644 --- a/packages/altive_lints/lib/altive_lints.dart +++ b/packages/altive_lints/lib/altive_lints.dart @@ -1,6 +1,7 @@ import 'package:custom_lint_builder/custom_lint_builder.dart'; import 'src/lints/avoid_hardcoded_japanese.dart'; +import 'src/lints/avoid_shrink_wrap_in_list_view.dart'; PluginBase createPlugin() => _AltivePlugin(); @@ -8,5 +9,6 @@ class _AltivePlugin extends PluginBase { @override List getLintRules(CustomLintConfigs configs) => [ const AvoidHardcodedJapanese(), + const AvoidShrinkWrapInListView(), ]; } diff --git a/packages/altive_lints/lib/src/lints/avoid_shrink_wrap_in_list_view.dart b/packages/altive_lints/lib/src/lints/avoid_shrink_wrap_in_list_view.dart new file mode 100644 index 0000000..b45039b --- /dev/null +++ b/packages/altive_lints/lib/src/lints/avoid_shrink_wrap_in_list_view.dart @@ -0,0 +1,87 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/error/listener.dart'; +import 'package:collection/collection.dart'; +import 'package:custom_lint_builder/custom_lint_builder.dart'; + +import '../utils/types_utils.dart'; + +/// An `avoid_shrink_wrap_in_list_view` rule that discourages +/// using `shrinkWrap` with `ListView`. +/// +/// This property causes performance issues by requiring +/// the list to fully layout its content upfront. +/// Instead of `shrinkWrap`, consider using slivers +/// for better performance with large lists. +/// +/// ### Example +/// +/// #### BAD: +/// +/// ```dart +/// ListView( +/// shrinkWrap: true, // LINT +/// children: [ +/// Text('Hello'), +/// Text('World'), +/// ], +/// ); +/// ``` +/// +/// #### GOOD: +/// +/// ```dart +/// CustomScrollView( +/// slivers: [ +/// SliverList.list( +/// children: [ +/// Text('Hello'), +/// Text('World'), +/// ], +/// ), +/// ], +/// ); +/// ``` +class AvoidShrinkWrapInListView extends DartLintRule { + const AvoidShrinkWrapInListView() : super(code: _code); + + static const _code = LintCode( + name: 'avoid_shrink_wrap_in_list_view', + problemMessage: 'Avoid using ListView with shrinkWrap, ' + 'since it might degrade the performance.\n' + 'Consider using slivers instead.' + 'Or, it is originally intended to be used for shrinking ' + 'when there is room for height in a dialog, for example.', + ); + + @override + void run( + CustomLintResolver resolver, + ErrorReporter reporter, + CustomLintContext context, + ) { + context.registry.addInstanceCreationExpression((node) { + if (isListViewWidget(node.staticType) && + _hasShrinkWrap(node) && + _hasParentList(node)) { + reporter.reportErrorForNode(_code, node); + } + }); + } + + bool _hasShrinkWrap(InstanceCreationExpression node) => + node.argumentList.arguments.firstWhereOrNull( + (arg) => arg is NamedExpression && arg.name.label.name == 'shrinkWrap', + ) != + null; + + bool _hasParentList(InstanceCreationExpression node) => + node.thisOrAncestorMatching( + (parent) => + parent != node && + parent is InstanceCreationExpression && + (isListViewWidget(parent.staticType) || + isColumnWidget(parent.staticType) || + isRowWidget(parent.staticType)), + ) != + null; +} diff --git a/packages/altive_lints/lib/src/utils/types_utils.dart b/packages/altive_lints/lib/src/utils/types_utils.dart new file mode 100644 index 0000000..c9b659b --- /dev/null +++ b/packages/altive_lints/lib/src/utils/types_utils.dart @@ -0,0 +1,147 @@ +import 'package:analyzer/dart/element/nullability_suffix.dart'; +import 'package:analyzer/dart/element/type.dart'; +import 'package:collection/collection.dart'; + +bool hasWidgetType(DartType type) => + (isWidgetOrSubclass(type) || + _isIterable(type) || + _isList(type) || + _isFuture(type)) && + !(_isMultiProvider(type) || + _isSubclassOfInheritedProvider(type) || + _isIterableInheritedProvider(type) || + _isListInheritedProvider(type) || + _isFutureInheritedProvider(type)); + +bool isIterable(DartType? type) => + _checkSelfOrSupertypes(type, (t) => t?.isDartCoreIterable ?? false); + +bool isNullableType(DartType? type) => + type?.nullabilitySuffix == NullabilitySuffix.question; + +bool isWidgetOrSubclass(DartType? type) => + _isWidget(type) || _isSubclassOfWidget(type); + +bool isRenderObjectOrSubclass(DartType? type) => + _isRenderObject(type) || _isSubclassOfRenderObject(type); + +bool isRenderObjectWidgetOrSubclass(DartType? type) => + _isRenderObjectWidget(type) || _isSubclassOfRenderObjectWidget(type); + +bool isRenderObjectElementOrSubclass(DartType? type) => + _isRenderObjectElement(type) || _isSubclassOfRenderObjectElement(type); + +bool isWidgetStateOrSubclass(DartType? type) => + _isWidgetState(type) || _isSubclassOfWidgetState(type); + +bool isSubclassOfListenable(DartType? type) => + type is InterfaceType && type.allSupertypes.any(_isListenable); + +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'; + +bool isRowWidget(DartType? type) => + type?.getDisplayString(withNullability: false) == 'Row'; + +bool isPaddingWidget(DartType? type) => + type?.getDisplayString(withNullability: false) == 'Padding'; + +bool isBuildContext(DartType? type) => + type?.getDisplayString(withNullability: false) == 'BuildContext'; + +bool isGameWidget(DartType? type) => + type?.getDisplayString(withNullability: false) == 'GameWidget'; + +bool _checkSelfOrSupertypes( + DartType? type, + bool Function(DartType?) predicate, +) => + predicate(type) || + (type is InterfaceType && type.allSupertypes.any(predicate)); + +bool _isWidget(DartType? type) => + type?.getDisplayString(withNullability: false) == 'Widget'; + +bool _isSubclassOfWidget(DartType? type) => + type is InterfaceType && type.allSupertypes.any(_isWidget); + +// ignore: deprecated_member_use +bool _isWidgetState(DartType? type) => type?.element2?.displayName == 'State'; + +bool _isSubclassOfWidgetState(DartType? type) => + type is InterfaceType && type.allSupertypes.any(_isWidgetState); + +bool _isIterable(DartType type) => + type.isDartCoreIterable && + type is InterfaceType && + isWidgetOrSubclass(type.typeArguments.firstOrNull); + +bool _isList(DartType type) => + type.isDartCoreList && + type is InterfaceType && + isWidgetOrSubclass(type.typeArguments.firstOrNull); + +bool _isFuture(DartType type) => + type.isDartAsyncFuture && + type is InterfaceType && + isWidgetOrSubclass(type.typeArguments.firstOrNull); + +bool _isListenable(DartType type) => + type.getDisplayString(withNullability: false) == 'Listenable'; + +bool _isRenderObject(DartType? type) => + type?.getDisplayString(withNullability: false) == 'RenderObject'; + +bool _isSubclassOfRenderObject(DartType? type) => + type is InterfaceType && type.allSupertypes.any(_isRenderObject); + +bool _isRenderObjectWidget(DartType? type) => + type?.getDisplayString(withNullability: false) == 'RenderObjectWidget'; + +bool _isSubclassOfRenderObjectWidget(DartType? type) => + type is InterfaceType && type.allSupertypes.any(_isRenderObjectWidget); + +bool _isRenderObjectElement(DartType? type) => + type?.getDisplayString(withNullability: false) == 'RenderObjectElement'; + +bool _isSubclassOfRenderObjectElement(DartType? type) => + type is InterfaceType && type.allSupertypes.any(_isRenderObjectElement); + +bool _isMultiProvider(DartType? type) => + type?.getDisplayString(withNullability: false) == 'MultiProvider'; + +bool _isSubclassOfInheritedProvider(DartType? type) => + type is InterfaceType && type.allSupertypes.any(_isInheritedProvider); + +bool _isInheritedProvider(DartType? type) => + type != null && + type + .getDisplayString(withNullability: false) + .startsWith('InheritedProvider<'); + +bool _isIterableInheritedProvider(DartType type) => + type.isDartCoreIterable && + type is InterfaceType && + _isSubclassOfInheritedProvider(type.typeArguments.firstOrNull); + +bool _isListInheritedProvider(DartType type) => + type.isDartCoreList && + type is InterfaceType && + _isSubclassOfInheritedProvider(type.typeArguments.firstOrNull); + +bool _isFutureInheritedProvider(DartType type) => + type.isDartAsyncFuture && + type is InterfaceType && + _isSubclassOfInheritedProvider(type.typeArguments.firstOrNull); + +bool isIterableOrSubclass(DartType? type) => + _checkSelfOrSupertypes(type, (t) => t?.isDartCoreIterable ?? false); + +bool isListOrSubclass(DartType? type) => + _checkSelfOrSupertypes(type, (t) => t?.isDartCoreList ?? false); diff --git a/packages/altive_lints/lint_test/lints/avoid_shrink_wrap_in_list_view.dart b/packages/altive_lints/lint_test/lints/avoid_shrink_wrap_in_list_view.dart new file mode 100644 index 0000000..492f296 --- /dev/null +++ b/packages/altive_lints/lint_test/lints/avoid_shrink_wrap_in_list_view.dart @@ -0,0 +1,26 @@ +import 'package:flutter/material.dart'; + +class MyWidget extends StatelessWidget { + const MyWidget({super.key}); + + @override + Widget build(BuildContext context) { + return Scaffold( + body: ListView( + children: [ + Expanded( + // expect_lint: avoid_shrink_wrap_in_list_view + child: ListView( + shrinkWrap: true, + physics: const NeverScrollableScrollPhysics(), + children: const [ + Text('Hello'), + Text('World'), + ], + ), + ), + ], + ), + ); + } +} diff --git a/packages/altive_lints/lint_test/pubspec.yaml b/packages/altive_lints/lint_test/pubspec.yaml index 912d204..2b0e19e 100644 --- a/packages/altive_lints/lint_test/pubspec.yaml +++ b/packages/altive_lints/lint_test/pubspec.yaml @@ -9,3 +9,5 @@ dev_dependencies: altive_lints: path: ../ custom_lint: ^0.6.4 + flutter: + sdk: flutter diff --git a/packages/altive_lints/pubspec.yaml b/packages/altive_lints/pubspec.yaml index 76ede4d..c52f91d 100644 --- a/packages/altive_lints/pubspec.yaml +++ b/packages/altive_lints/pubspec.yaml @@ -16,4 +16,5 @@ environment: dependencies: analyzer: ^6.4.1 + collection: ^1.18.0 custom_lint_builder: ^0.6.4