diff --git a/packages/leancode_lint/README.md b/packages/leancode_lint/README.md index c64f1312..a59854a2 100644 --- a/packages/leancode_lint/README.md +++ b/packages/leancode_lint/README.md @@ -218,6 +218,59 @@ None. +
+`bloc_related_class_naming` + +### `bloc_related_class_naming` + +**DO** follow the naming convention for Bloc/Cubit related classes. + +For `ExampleBloc`: +- Event class: `ExampleEvent` +- State class: `ExampleState` +- Presentation Event class: `ExamplePresentationEvent` + +For `ExampleCubit`: +- State class: `ExampleState` +- Presentation Event class: `ExamplePresentationEvent` + +> [!NOTE] +> This lint only checks classes defined in the same library (including parts) as the Bloc/Cubit. +> Presentation events are only checked if the `bloc_presentation` package is used. + +**BAD:** + +```dart +class MyBloc extends Bloc {} +``` + +**GOOD:** + +```dart +class MyBloc extends Bloc {} +``` + +#### Configuration + +Configured via `LeanCodeLintConfig.blocRelatedClassNaming`: + +```dart +import 'package:leancode_lint/plugin.dart'; + +final plugin = LeanCodeLintPlugin( + name: 'my_lints', + config: LeanCodeLintConfig( + blocRelatedClassNaming: BlocRelatedClassNamingConfig( + stateSuffix: 'State', + eventSuffix: 'Event', + presentationEventSuffix: 'PresentationEvent', + ), + ), +); +``` + +
+
catch_parameter_names diff --git a/packages/leancode_lint/lib/config.dart b/packages/leancode_lint/lib/config.dart index be289832..7604d154 100644 --- a/packages/leancode_lint/lib/config.dart +++ b/packages/leancode_lint/lib/config.dart @@ -3,6 +3,7 @@ final class LeanCodeLintConfig { this.applicationPrefix, this.designSystemItemReplacements = const {}, this.catchParameterNames = const .new(), + this.blocRelatedClassNaming = const .new(), }); /// Used by some rules (e.g. `prefix_widgets_returning_slivers`) to match @@ -19,6 +20,30 @@ final class LeanCodeLintConfig { /// Configuration for the `catch_parameter_names` rule. final CatchParameterNamesConfig catchParameterNames; + + /// Configuration for the `bloc_related_class_naming` rule. + final BlocRelatedClassNamingConfig blocRelatedClassNaming; +} + +/// Configuration for the `bloc_related_class_naming` rule. +/// +/// Each suffix is appended to the BLoC/Cubit subject name (the part before +/// `Bloc` or `Cubit`) to form the expected class name. +/// +/// For example, for `FooBloc` the default expected names are: +/// - state → `FooState` +/// - event → `FooEvent` +/// - presentation event → `FooPresentationEvent` +class BlocRelatedClassNamingConfig { + const BlocRelatedClassNamingConfig({ + this.stateSuffix = 'State', + this.eventSuffix = 'Event', + this.presentationEventSuffix = 'PresentationEvent', + }); + + final String stateSuffix; + final String eventSuffix; + final String presentationEventSuffix; } class CatchParameterNamesConfig { diff --git a/packages/leancode_lint/lib/plugin.dart b/packages/leancode_lint/lib/plugin.dart index 7f153c5c..734b2bf0 100644 --- a/packages/leancode_lint/lib/plugin.dart +++ b/packages/leancode_lint/lib/plugin.dart @@ -7,6 +7,7 @@ import 'package:leancode_lint/src/assists/convert_record_into_nominal_type.dart' import 'package:leancode_lint/src/lints/add_cubit_suffix_for_cubits.dart'; import 'package:leancode_lint/src/lints/avoid_conditional_hooks.dart'; import 'package:leancode_lint/src/lints/avoid_single_child_in_multi_child_widget.dart'; +import 'package:leancode_lint/src/lints/bloc_related_class_naming.dart'; import 'package:leancode_lint/src/lints/catch_parameter_names.dart'; import 'package:leancode_lint/src/lints/constructor_parameters_and_fields_should_have_the_same_order.dart'; import 'package:leancode_lint/src/lints/hook_widget_does_not_use_hooks.dart'; @@ -40,13 +41,22 @@ final class LeanCodeLintPlugin extends Plugin { ).forEach(registry.registerWarningRule); registry ..registerWarningRule(StartCommentsWithSpace()) - ..registerWarningRule(PrefixWidgetsReturningSlivers(config: config)) + ..registerWarningRule( + PrefixWidgetsReturningSlivers( + applicationPrefix: config.applicationPrefix, + ), + ) ..registerFixForRule( StartCommentsWithSpace.code, AddStartingSpaceToComment.new, ) ..registerWarningRule(AddCubitSuffixForYourCubits()) - ..registerWarningRule(CatchParameterNames(config: config)) + ..registerWarningRule( + BlocRelatedClassNaming(config: config.blocRelatedClassNaming), + ) + ..registerWarningRule( + CatchParameterNames(config: config.catchParameterNames), + ) ..registerWarningRule(AvoidConditionalHooks()) ..registerWarningRule(HookWidgetDoesNotUseHooks()) ..registerFixForRule( diff --git a/packages/leancode_lint/lib/src/bloc_utils.dart b/packages/leancode_lint/lib/src/bloc_utils.dart new file mode 100644 index 00000000..edbee114 --- /dev/null +++ b/packages/leancode_lint/lib/src/bloc_utils.dart @@ -0,0 +1,93 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/element/element.dart'; +import 'package:leancode_lint/src/type_checker.dart'; + +const blocChecker = TypeChecker.fromName('Bloc', packageName: 'bloc'); + +const cubitChecker = TypeChecker.fromName('Cubit', packageName: 'bloc'); + +const blocPresentationMixinChecker = TypeChecker.fromName( + 'BlocPresentationMixin', + packageName: 'bloc_presentation', +); + +String? getBlocSubject(String className, {required BlocType blocType}) => + switch (blocType) { + .bloc when className.endsWith('Bloc') => className.substring( + 0, + className.length - 4, + ), + .cubit when className.endsWith('Cubit') => className.substring( + 0, + className.length - 5, + ), + _ => null, + }; + +enum BlocType { bloc, cubit } + +BlocType? determineBlocType(Element? element) { + if (element == null) { + return null; + } + + if (blocChecker.isAssignableFrom(element)) { + return .bloc; + } else if (cubitChecker.isAssignableFrom(element)) { + return .cubit; + } + + return null; +} + +class BlocInfo { + const BlocInfo({ + required this.type, + this.stateType, + this.eventType, + this.presentationEventType, + }); + + final BlocType type; + final TypeAnnotation? stateType; + final TypeAnnotation? eventType; + final TypeAnnotation? presentationEventType; +} + +BlocInfo? getBlocInfo(ClassDeclaration node) { + final extendsClause = node.extendsClause; + final superclass = extendsClause?.superclass; + final superclassElement = superclass?.element; + + final type = determineBlocType(superclassElement); + if (type == null) { + return null; + } + + final typeArguments = superclass?.typeArguments?.arguments; + + final (eventType, stateType) = switch (typeArguments) { + [final state] when type == .cubit => (null, state), + [final event, final state] when type == .bloc => (event, state), + _ => (null, null), + }; + + TypeAnnotation? presentationEventType; + if (node.withClause case final withClause?) { + for (final mixin in withClause.mixinTypes) { + if (mixin.element case final mixinElement? + when blocPresentationMixinChecker.isExactly(mixinElement)) { + if (mixin.typeArguments?.arguments case [_, final presentationEvent]) { + presentationEventType = presentationEvent; + } + } + } + } + + return .new( + type: type, + stateType: stateType, + eventType: eventType, + presentationEventType: presentationEventType, + ); +} diff --git a/packages/leancode_lint/lib/src/lints/bloc_related_class_naming.dart b/packages/leancode_lint/lib/src/lints/bloc_related_class_naming.dart new file mode 100644 index 00000000..9a1b94d4 --- /dev/null +++ b/packages/leancode_lint/lib/src/lints/bloc_related_class_naming.dart @@ -0,0 +1,106 @@ +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/error/error.dart'; +import 'package:leancode_lint/config.dart'; +import 'package:leancode_lint/src/bloc_utils.dart'; +import 'package:leancode_lint/src/utils.dart'; + +/// Enforces consistent naming of state, event, and presentation event classes +/// related to a BLoC or Cubit. +/// +/// Given a BLoC or Cubit named `FooBloc` or `FooCubit`, the associated classes +/// should be named: +/// - state → `FooState` +/// - event → `FooEvent` +/// - presentation event → `FooPresentationEvent` +/// +/// The suffixes are configurable via [BlocRelatedClassNamingConfig]. +class BlocRelatedClassNaming extends AnalysisRule { + BlocRelatedClassNaming({this.config = const .new()}) + : super(name: code.lowerCaseName, description: code.problemMessage); + + final BlocRelatedClassNamingConfig config; + + static const code = LintCode( + 'bloc_related_class_naming', + "The name of {0}'s {1} should be {2}.", + severity: .WARNING, + ); + + @override + LintCode get diagnosticCode => code; + + @override + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, + ) { + registry.addClassDeclaration(this, _Visitor(this, context, config)); + } +} + +class _Visitor extends SimpleAstVisitor { + _Visitor(this.rule, this.context, this.config); + + final AnalysisRule rule; + final RuleContext context; + final BlocRelatedClassNamingConfig config; + + @override + void visitClassDeclaration(ClassDeclaration node) { + final blocInfo = getBlocInfo(node); + if (blocInfo == null) { + return; + } + + final classElement = node.declaredFragment?.element; + final className = node.namePart.typeName.lexeme; + final subject = getBlocSubject(className, blocType: blocInfo.type); + + if (subject == null) { + return; + } + + void checkName(TypeAnnotation type, String classType, String suffix) { + final expectedName = '$subject$suffix'; + + if (type case NamedType( + :final name, + :final element?, + :final CompilationUnit root, + ) when name.lexeme != expectedName) { + if (element.library != classElement?.library) { + return; + } + + final declaration = root.declarations + .whereType() + .firstWhereOrNull((d) => d.declaredFragment?.element == element); + + rule.reportAtToken( + declaration?.namePart.typeName ?? name, + arguments: [className, classType, expectedName], + ); + } + } + + if (blocInfo.stateType case final stateType?) { + checkName(stateType, 'state', config.stateSuffix); + } + + if (blocInfo.eventType case final eventType?) { + checkName(eventType, 'event', config.eventSuffix); + } + + if (blocInfo.presentationEventType case final presentationEventType?) { + checkName( + presentationEventType, + 'presentation event', + config.presentationEventSuffix, + ); + } + } +} diff --git a/packages/leancode_lint/lib/src/lints/catch_parameter_names.dart b/packages/leancode_lint/lib/src/lints/catch_parameter_names.dart index f71e10fb..f56ae5ad 100644 --- a/packages/leancode_lint/lib/src/lints/catch_parameter_names.dart +++ b/packages/leancode_lint/lib/src/lints/catch_parameter_names.dart @@ -20,7 +20,7 @@ class CatchParameterNames extends AnalysisRule { CatchParameterNames({required this.config}) : super(name: code.lowerCaseName, description: code.problemMessage); - final LeanCodeLintConfig config; + final CatchParameterNamesConfig config; static const code = LintCode( 'catch_parameter_names', @@ -46,7 +46,7 @@ class _Visitor extends SimpleAstVisitor { final AnalysisRule rule; final RuleContext context; - final LeanCodeLintConfig config; + final CatchParameterNamesConfig config; @override void visitCatchClause(CatchClause node) { @@ -81,8 +81,8 @@ enum _CatchClauseParameter { exception, stackTrace; - String preferredName(LeanCodeLintConfig config) => switch (this) { - exception => config.catchParameterNames.exception, - stackTrace => config.catchParameterNames.stackTrace, + String preferredName(CatchParameterNamesConfig config) => switch (this) { + exception => config.exception, + stackTrace => config.stackTrace, }; } diff --git a/packages/leancode_lint/lib/src/lints/prefix_widgets_returning_slivers.dart b/packages/leancode_lint/lib/src/lints/prefix_widgets_returning_slivers.dart index 473e429f..ada7c261 100644 --- a/packages/leancode_lint/lib/src/lints/prefix_widgets_returning_slivers.dart +++ b/packages/leancode_lint/lib/src/lints/prefix_widgets_returning_slivers.dart @@ -4,17 +4,16 @@ import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; import 'package:analyzer/error/error.dart'; -import 'package:leancode_lint/config.dart'; import 'package:leancode_lint/src/helpers.dart'; /// Displays warning for widgets which return slivers but do not have the /// `Sliver`/`_Sliver` (or `${AppPrefix}Sliver`/`_${AppPrefix}Sliver` if /// `AppPrefix` is specified in the config) prefix in their name. class PrefixWidgetsReturningSlivers extends AnalysisRule { - PrefixWidgetsReturningSlivers({required this.config}) + PrefixWidgetsReturningSlivers({required this.applicationPrefix}) : super(name: code.lowerCaseName, description: code.problemMessage); - final LeanCodeLintConfig config; + final String? applicationPrefix; static const code = LintCode( 'prefix_widgets_returning_slivers', @@ -31,16 +30,19 @@ class PrefixWidgetsReturningSlivers extends AnalysisRule { RuleVisitorRegistry registry, RuleContext context, ) { - registry.addClassDeclaration(this, _Visitor(this, context, config)); + registry.addClassDeclaration( + this, + _Visitor(this, context, applicationPrefix), + ); } } class _Visitor extends SimpleAstVisitor { - _Visitor(this.rule, this.context, this.config); + _Visitor(this.rule, this.context, this.applicationPrefix); final AnalysisRule rule; final RuleContext context; - final LeanCodeLintConfig config; + final String? applicationPrefix; @override void visitClassDeclaration(ClassDeclaration node) { @@ -69,7 +71,7 @@ class _Visitor extends SimpleAstVisitor { if (isSliver) { rule.reportAtToken( name, - arguments: [_getSuggestedClassName(config, name.lexeme)], + arguments: [_getSuggestedClassName(applicationPrefix, name.lexeme)], ); } } @@ -77,7 +79,7 @@ class _Visitor extends SimpleAstVisitor { late final possiblePrefixes = [ 'Sliver', '_Sliver', - if (config.applicationPrefix case final applicationPrefix?) ...[ + if (applicationPrefix != null) ...[ '${applicationPrefix}Sliver', '_${applicationPrefix}Sliver', ], @@ -93,7 +95,7 @@ class _Visitor extends SimpleAstVisitor { ); static String _getSuggestedClassName( - LeanCodeLintConfig config, + String? applicationPrefix, String className, ) { var name = className; @@ -103,8 +105,7 @@ class _Visitor extends SimpleAstVisitor { suggested.write('_'); name = name.substring(1); } - if (config.applicationPrefix case final applicationPrefix? - when name.startsWith(applicationPrefix)) { + if (applicationPrefix != null && name.startsWith(applicationPrefix)) { suggested.write(applicationPrefix); name = name.substring(applicationPrefix.length); } diff --git a/packages/leancode_lint/test/mock_libraries.dart b/packages/leancode_lint/test/mock_libraries.dart index a5142e63..67b98f0a 100644 --- a/packages/leancode_lint/test/mock_libraries.dart +++ b/packages/leancode_lint/test/mock_libraries.dart @@ -1,6 +1,7 @@ import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; part 'mock_libraries/bloc.dart'; +part 'mock_libraries/bloc_presentation.dart'; part 'mock_libraries/equatable.dart'; part 'mock_libraries/flutter.dart'; part 'mock_libraries/flutter_bloc.dart'; diff --git a/packages/leancode_lint/test/mock_libraries/bloc.dart b/packages/leancode_lint/test/mock_libraries/bloc.dart index 83a1bc94..db9f874c 100644 --- a/packages/leancode_lint/test/mock_libraries/bloc.dart +++ b/packages/leancode_lint/test/mock_libraries/bloc.dart @@ -4,9 +4,18 @@ mixin MockBloc on AnalysisRuleTest { @override void setUp() { newPackage('bloc').addFile('lib/bloc.dart', ''' +class BlocBase { + BlocBase(this.state); + State state; +} + abstract class Cubit extends BlocBase { Cubit(State initialState) : super(initialState); } + +abstract class Bloc extends BlocBase { + Bloc(State initialState) : super(initialState); +} '''); super.setUp(); } diff --git a/packages/leancode_lint/test/mock_libraries/bloc_presentation.dart b/packages/leancode_lint/test/mock_libraries/bloc_presentation.dart new file mode 100644 index 00000000..3f301a35 --- /dev/null +++ b/packages/leancode_lint/test/mock_libraries/bloc_presentation.dart @@ -0,0 +1,11 @@ +part of '../mock_libraries.dart'; + +mixin MockBlocPresentation on AnalysisRuleTest { + @override + void setUp() { + newPackage('bloc_presentation').addFile('lib/bloc_presentation.dart', ''' +mixin BlocPresentationMixin {} +'''); + super.setUp(); + } +} diff --git a/packages/leancode_lint/test/test_cases/bloc_related_class_naming_test.dart b/packages/leancode_lint/test/test_cases/bloc_related_class_naming_test.dart new file mode 100644 index 00000000..41676918 --- /dev/null +++ b/packages/leancode_lint/test/test_cases/bloc_related_class_naming_test.dart @@ -0,0 +1,177 @@ +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:leancode_lint/config.dart'; +import 'package:leancode_lint/src/lints/bloc_related_class_naming.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +import '../assert_ranges.dart'; +import '../mock_libraries.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(BlocRelatedClassNamingTest); + defineReflectiveTests(BlocRelatedClassNamingCustomSuffixesTest); + }); +} + +@reflectiveTest +class BlocRelatedClassNamingTest extends AnalysisRuleTest + with MockBloc, MockBlocPresentation, MockFlutterBloc { + @override + void setUp() { + rule = BlocRelatedClassNaming(); + + newPackage('external_lib').addFile('lib/external_lib.dart', ''' +class ExternalWrongEvent {} +class ExternalWrongState {} +'''); + + super.setUp(); + } + + Future test_bloc() async { + await assertDiagnosticsInRanges(''' +import 'package:bloc/bloc.dart'; +import 'package:bloc_presentation/bloc_presentation.dart'; + +class GoodEvent {} +class GoodState {} +class GoodPresentationEvent {} + +class GoodBloc extends Bloc + with BlocPresentationMixin { + GoodBloc() : super(GoodState()); +} + +class /*[0*/WrongEvent/*0]*/ {} +class /*[1*/WrongState/*1]*/ {} +class /*[2*/WrongPresentationEvent/*2]*/ {} + +class MyBloc extends Bloc + with BlocPresentationMixin { + MyBloc() : super(WrongState()); +} +'''); + } + + Future test_cubit() async { + await assertDiagnosticsInRanges(''' +import 'package:bloc/bloc.dart'; +import 'package:bloc_presentation/bloc_presentation.dart'; + +class GoodState {} +class GoodPresentationEvent {} + +class GoodCubit extends Cubit + with BlocPresentationMixin { + GoodCubit() : super(GoodState()); +} + +class /*[0*/WrongState/*0]*/ {} +class /*[1*/WrongPresentationEvent/*1]*/ {} + +class MyCubit extends Cubit + with BlocPresentationMixin { + MyCubit() : super(WrongState()); +} +'''); + } + + Future test_external_classes_ignored() async { + await assertNoDiagnostics(''' +import 'package:bloc/bloc.dart'; +import 'package:external_lib/external_lib.dart'; + +class MyBloc extends Bloc { + MyBloc() : super(ExternalWrongState()); +} +'''); + } + + Future test_part_file_definition() async { + newFile('$testPackageLibPath/part.dart', ''' +part of 'test.dart'; + +class WrongState {} + +class GoodState {} +'''); + + await assertDiagnosticsInRanges(''' +import 'package:bloc/bloc.dart'; + +part 'part.dart'; + +class MyCubit extends Cubit { + MyCubit() : super(WrongState()); +} + +class GoodCubit extends Cubit { + GoodCubit() : super(GoodState()); +} +'''); + } +} + +@reflectiveTest +class BlocRelatedClassNamingCustomSuffixesTest extends AnalysisRuleTest + with MockBloc, MockBlocPresentation, MockFlutterBloc { + @override + void setUp() { + rule = BlocRelatedClassNaming( + config: const BlocRelatedClassNamingConfig( + stateSuffix: 'Foobar', + eventSuffix: 'Cmd', + presentationEventSuffix: 'Output', + ), + ); + super.setUp(); + } + + Future test_bloc_custom_suffixes() async { + await assertDiagnosticsInRanges(''' +import 'package:bloc/bloc.dart'; +import 'package:bloc_presentation/bloc_presentation.dart'; + +class GoodCmd {} +class GoodFoobar {} +class GoodOutput {} + +class GoodBloc extends Bloc + with BlocPresentationMixin { + GoodBloc() : super(GoodFoobar()); +} + +class /*[0*/WrongCmd/*0]*/ {} +class /*[1*/WrongFoobar/*1]*/ {} +class /*[2*/WrongOutput/*2]*/ {} + +class MyBloc extends Bloc + with BlocPresentationMixin { + MyBloc() : super(WrongFoobar()); +} +'''); + } + + Future test_cubit_custom_suffixes() async { + await assertDiagnosticsInRanges(''' +import 'package:bloc/bloc.dart'; +import 'package:bloc_presentation/bloc_presentation.dart'; + +class GoodFoobar {} +class GoodOutput {} + +class GoodCubit extends Cubit + with BlocPresentationMixin { + GoodCubit() : super(GoodFoobar()); +} + +class /*[0*/WrongFoobar/*0]*/ {} +class /*[1*/WrongOutput/*1]*/ {} + +class MyCubit extends Cubit + with BlocPresentationMixin { + MyCubit() : super(WrongFoobar()); +} +'''); + } +} diff --git a/packages/leancode_lint/test/test_cases/prefix_widgets_returning_slivers_test.dart b/packages/leancode_lint/test/test_cases/prefix_widgets_returning_slivers_test.dart index 55ea1856..05060b28 100644 --- a/packages/leancode_lint/test/test_cases/prefix_widgets_returning_slivers_test.dart +++ b/packages/leancode_lint/test/test_cases/prefix_widgets_returning_slivers_test.dart @@ -16,9 +16,7 @@ class PrefixWidgetsReturningSliversTest extends AnalysisRuleTest with MockFlutter { @override void setUp() { - rule = PrefixWidgetsReturningSlivers( - config: const .new(applicationPrefix: 'Lncd'), - ); + rule = PrefixWidgetsReturningSlivers(applicationPrefix: 'Lncd'); super.setUp(); }