From c0830b5387ddceb9fb4998d2d8ce9c4495c40b68 Mon Sep 17 00:00:00 2001 From: k-nkmr Date: Fri, 20 Sep 2024 13:21:56 +0900 Subject: [PATCH 1/2] feat: Improve prefer_sliver_prefix to prefer_to_include_sliver_in_name --- packages/altive_lints/README.md | 6 +- packages/altive_lints/lib/altive_lints.dart | 4 +- .../lib/src/lints/prefer_sliver_prefix.dart | 76 --------------- .../prefer_to_include_sliver_in_name.dart | 93 +++++++++++++++++++ .../lint_test/lints/prefer_sliver_prefix.dart | 16 ---- .../prefer_to_include_sliver_in_name.dart | 35 +++++++ 6 files changed, 133 insertions(+), 97 deletions(-) delete mode 100644 packages/altive_lints/lib/src/lints/prefer_sliver_prefix.dart create mode 100644 packages/altive_lints/lib/src/lints/prefer_to_include_sliver_in_name.dart delete mode 100644 packages/altive_lints/lint_test/lints/prefer_sliver_prefix.dart create mode 100644 packages/altive_lints/lint_test/lints/prefer_to_include_sliver_in_name.dart diff --git a/packages/altive_lints/README.md b/packages/altive_lints/README.md index c9b7aac..afa44e4 100644 --- a/packages/altive_lints/README.md +++ b/packages/altive_lints/README.md @@ -19,7 +19,7 @@ There are also Altive-made rules by custom_lint. - [avoid\_single\_child](#avoid_single_child) - [prefer\_clock\_now](#prefer_clock_now) - [prefer\_dedicated\_media\_query\_methods](#prefer_dedicated_media_query_methods) - - [prefer\_sliver\_prefix](#prefer_sliver_prefix) + - [prefer\_to\_include\_sliver\_in\_name](#prefer_to_include_sliver_in_name) - [prefer\_space\_between\_elements](#prefer_space_between_elements) - [Lint rules adopted by altive\_lints and why](#lint-rules-adopted-by-altive_lints-and-why) - [public\_member\_api\_docs](#public_member_api_docs) @@ -252,9 +252,9 @@ var size = MediaQuery.sizeOf(context); var padding = MediaQuery.viewInsetsOf(context); ``` -### prefer_sliver_prefix +### prefer_to_include_sliver_in_name -Prefer to prefix the class name of a Widget that returns a Sliver type Widget with “Sliver”. +Prefer to include ‘Sliver’ in the class name or named constructor of a widget that returns a Sliver-type widget. This makes it easy for the user to know at a glance that it is a Sliver type Widget, and improves readability and consistency. diff --git a/packages/altive_lints/lib/altive_lints.dart b/packages/altive_lints/lib/altive_lints.dart index ba7cdae..3eb31a4 100644 --- a/packages/altive_lints/lib/altive_lints.dart +++ b/packages/altive_lints/lib/altive_lints.dart @@ -7,8 +7,8 @@ import 'src/lints/avoid_shrink_wrap_in_list_view.dart'; import 'src/lints/avoid_single_child.dart'; import 'src/lints/prefer_clock_now.dart'; import 'src/lints/prefer_dedicated_media_query_methods.dart'; -import 'src/lints/prefer_sliver_prefix.dart'; import 'src/lints/prefer_space_between_elements.dart'; +import 'src/lints/prefer_to_include_sliver_in_name.dart'; /// Returns the Altive Plugin instance. PluginBase createPlugin() => _AltivePlugin(); @@ -24,6 +24,6 @@ class _AltivePlugin extends PluginBase { const PreferClockNow(), const PreferDedicatedMediaQueryMethods(), const PreferSpaceBetweenElements(), - const PreferSliverPrefix(), + const PreferToIncludeSliverInName(), ]; } diff --git a/packages/altive_lints/lib/src/lints/prefer_sliver_prefix.dart b/packages/altive_lints/lib/src/lints/prefer_sliver_prefix.dart deleted file mode 100644 index 34b13d5..0000000 --- a/packages/altive_lints/lib/src/lints/prefer_sliver_prefix.dart +++ /dev/null @@ -1,76 +0,0 @@ -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'; - -/// A `prefer_sliver_prefix` rule that ensures widgets returning -/// a Sliver-type widget have a "Sliver" prefix in their class names. -/// -/// This naming convention improves code readability and -/// consistency by clearly indicating the widget's functionality and -/// return type through its name. -/// -/// ### Example -/// -/// #### BAD: -/// -/// ```dart -/// class MyCustomList extends StatelessWidget { -/// @override -/// Widget build(BuildContext context) { -/// return SliverList(...); // LINT -/// } -/// } -/// ``` -/// -/// #### GOOD: -/// -/// ```dart -/// class SliverMyCustomList extends StatelessWidget { -/// @override -/// Widget build(BuildContext context) { -/// return SliverList(...); -/// } -/// } -/// ``` -class PreferSliverPrefix extends DartLintRule { - /// Creates a new instance of [PreferSliverPrefix]. - const PreferSliverPrefix() : super(code: _code); - - static const _code = LintCode( - name: 'prefer_sliver_prefix', - problemMessage: 'Widgets returning Sliver should have a "Sliver" prefix.', - correctionMessage: 'Consider adding "Sliver" prefix to the widget name.', - ); - - @override - void run( - CustomLintResolver resolver, - ErrorReporter reporter, - CustomLintContext context, - ) { - context.registry.addClassDeclaration((node) { - final className = node.name.lexeme; - final methodBody = node.members - .whereType() - .firstWhereOrNull((method) => method.name.lexeme == 'build') - ?.body; - - if (methodBody is BlockFunctionBody) { - final returnStatements = - methodBody.block.statements.whereType(); - final returnsSliverWidget = returnStatements.any( - (returnStatement) { - final returnType = returnStatement.expression?.staticType; - final typeName = returnType?.getDisplayString(); - return typeName?.startsWith('Sliver') ?? false; - }, - ); - - if (returnsSliverWidget && !className.startsWith('Sliver')) { - reporter.atNode(node, _code); - } - } - }); - } -} diff --git a/packages/altive_lints/lib/src/lints/prefer_to_include_sliver_in_name.dart b/packages/altive_lints/lib/src/lints/prefer_to_include_sliver_in_name.dart new file mode 100644 index 0000000..b5fcbf1 --- /dev/null +++ b/packages/altive_lints/lib/src/lints/prefer_to_include_sliver_in_name.dart @@ -0,0 +1,93 @@ +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'; + +/// A `prefer_to_include_sliver_in_name` rule that ensures widgets returning +/// a Sliver-type widget include "Sliver" in their class names. +/// +/// This naming convention improves code readability and consistency +/// by clearly indicating the widget's functionality +/// and return type through its name. +/// +/// The rule also applies if "Sliver" is present in the named constructor, +/// allowing flexibility in how the convention is followed. +/// +/// ### Example +/// +/// #### BAD: +/// +/// ```dart +/// class MyCustomList extends StatelessWidget { +/// @override +/// Widget build(BuildContext context) { +/// return SliverList(...); // LINT +/// } +/// } +/// ``` +/// +/// #### GOOD: +/// +/// ```dart +/// class SliverMyCustomList extends StatelessWidget { +/// @override +/// Widget build(BuildContext context) { +/// return SliverList(...); +/// } +/// } +/// ``` +class PreferToIncludeSliverInName extends DartLintRule { + /// Creates a new instance of [PreferToIncludeSliverInName]. + const PreferToIncludeSliverInName() : super(code: _code); + + static const _code = LintCode( + name: 'prefer_to_include_sliver_in_name', + problemMessage: 'Widgets returning Sliver should include "Sliver" ' + 'in the class name or named constructor.', + correctionMessage: + 'Consider adding "Sliver" to the class name or a named constructor.', + ); + + @override + void run( + CustomLintResolver resolver, + ErrorReporter reporter, + CustomLintContext context, + ) { + context.registry.addClassDeclaration((node) { + final methodBody = node.members + .whereType() + .firstWhereOrNull((method) => method.name.lexeme == 'build') + ?.body; + + if (methodBody is! BlockFunctionBody) { + return; + } + + final returnStatements = + methodBody.block.statements.whereType(); + final returnsSliverWidget = returnStatements.any( + (returnStatement) { + final returnType = returnStatement.expression?.staticType; + final typeName = returnType?.getDisplayString(); + return typeName?.startsWith('Sliver') ?? false; + }, + ); + + final className = node.name.lexeme; + final constructorNames = node.members + .whereType() + .map((constructor) => constructor.name?.lexeme) + .whereType(); + final hasSliverInClassOrConstructor = className.contains('Sliver') || + constructorNames.any( + (constructorName) => + constructorName.toLowerCase().contains('sliver'), + ); + + if (returnsSliverWidget && !hasSliverInClassOrConstructor) { + reporter.atNode(node, _code); + } + }); + } +} diff --git a/packages/altive_lints/lint_test/lints/prefer_sliver_prefix.dart b/packages/altive_lints/lint_test/lints/prefer_sliver_prefix.dart deleted file mode 100644 index 94d792f..0000000 --- a/packages/altive_lints/lint_test/lints/prefer_sliver_prefix.dart +++ /dev/null @@ -1,16 +0,0 @@ -import 'package:flutter/material.dart'; - -// expect_lint: prefer_sliver_prefix -class MyWidget extends StatelessWidget { - const MyWidget({super.key}); - - @override - Widget build(BuildContext context) { - return SliverList.builder( - itemCount: 10, - itemBuilder: (context, index) { - return const Placeholder(); - }, - ); - } -} diff --git a/packages/altive_lints/lint_test/lints/prefer_to_include_sliver_in_name.dart b/packages/altive_lints/lint_test/lints/prefer_to_include_sliver_in_name.dart new file mode 100644 index 0000000..ba04ef7 --- /dev/null +++ b/packages/altive_lints/lint_test/lints/prefer_to_include_sliver_in_name.dart @@ -0,0 +1,35 @@ +import 'package:flutter/material.dart'; + +// expect_lint: prefer_to_include_sliver_in_name +class MyWidget extends StatelessWidget { + const MyWidget({super.key}); + + @override + Widget build(BuildContext context) { + return SliverList.builder( + itemCount: 10, + itemBuilder: (context, index) { + return const Placeholder(); + }, + ); + } +} + +// expect_lint: prefer_to_include_sliver_in_name +class MyWidget2 extends StatelessWidget { + const MyWidget2({super.key, this.maxCount = 10}); + + const MyWidget2.small({super.key, this.maxCount = 5}); + + final int maxCount; + + @override + Widget build(BuildContext context) { + return SliverList.builder( + itemCount: maxCount, + itemBuilder: (context, index) { + return const Placeholder(); + }, + ); + } +} From f9f092a65a490bbf7ab4d838db691433333ae33b Mon Sep 17 00:00:00 2001 From: k-nkmr Date: Tue, 24 Sep 2024 15:38:56 +0900 Subject: [PATCH 2/2] chore: use early return --- .../prefer_to_include_sliver_in_name.dart | 26 +++++++++++----- .../prefer_to_include_sliver_in_name.dart | 30 +++++++++++++++++++ 2 files changed, 48 insertions(+), 8 deletions(-) diff --git a/packages/altive_lints/lib/src/lints/prefer_to_include_sliver_in_name.dart b/packages/altive_lints/lib/src/lints/prefer_to_include_sliver_in_name.dart index b5fcbf1..902326d 100644 --- a/packages/altive_lints/lib/src/lints/prefer_to_include_sliver_in_name.dart +++ b/packages/altive_lints/lib/src/lints/prefer_to_include_sliver_in_name.dart @@ -74,20 +74,30 @@ class PreferToIncludeSliverInName extends DartLintRule { }, ); + if (!returnsSliverWidget) { + return; + } + final className = node.name.lexeme; + + if (className.contains('Sliver')) { + return; + } + final constructorNames = node.members .whereType() .map((constructor) => constructor.name?.lexeme) - .whereType(); - final hasSliverInClassOrConstructor = className.contains('Sliver') || - constructorNames.any( - (constructorName) => - constructorName.toLowerCase().contains('sliver'), - ); + .nonNulls; + + final hasSliverInConstructor = constructorNames.any( + (constructorName) => constructorName.toLowerCase().contains('sliver'), + ); - if (returnsSliverWidget && !hasSliverInClassOrConstructor) { - reporter.atNode(node, _code); + if (hasSliverInConstructor) { + return; } + + reporter.atNode(node, _code); }); } } diff --git a/packages/altive_lints/lint_test/lints/prefer_to_include_sliver_in_name.dart b/packages/altive_lints/lint_test/lints/prefer_to_include_sliver_in_name.dart index ba04ef7..6f5c496 100644 --- a/packages/altive_lints/lint_test/lints/prefer_to_include_sliver_in_name.dart +++ b/packages/altive_lints/lint_test/lints/prefer_to_include_sliver_in_name.dart @@ -33,3 +33,33 @@ class MyWidget2 extends StatelessWidget { ); } } + +class MySliverWidget extends StatelessWidget { + const MySliverWidget({super.key}); + + @override + Widget build(BuildContext context) { + return SliverList.builder( + itemCount: 10, + itemBuilder: (context, index) { + return const Placeholder(); + }, + ); + } +} + +class MyWidget3 extends StatelessWidget { + const MyWidget3({super.key}); + + const MyWidget3.sliver({super.key}); + + @override + Widget build(BuildContext context) { + return SliverList.builder( + itemCount: 10, + itemBuilder: (context, index) { + return const Placeholder(); + }, + ); + } +}