diff --git a/docs/src/components/lint-rules/avoid_mutable_events/BadSnippet.mdx b/docs/src/components/lint-rules/avoid_mutable_events/BadSnippet.mdx new file mode 100644 index 00000000000..3c5f160bbc0 --- /dev/null +++ b/docs/src/components/lint-rules/avoid_mutable_events/BadSnippet.mdx @@ -0,0 +1,20 @@ +import { Code } from '@astrojs/starlight/components'; +import { transformerMetaHighlight } from '@shikijs/transformers'; + + diff --git a/docs/src/components/lint-rules/avoid_mutable_events/GoodSnippet.astro b/docs/src/components/lint-rules/avoid_mutable_events/GoodSnippet.astro new file mode 100644 index 00000000000..3a4e2c8c103 --- /dev/null +++ b/docs/src/components/lint-rules/avoid_mutable_events/GoodSnippet.astro @@ -0,0 +1,17 @@ +--- +import { Code } from '@astrojs/starlight/components'; + +const code = ` +import 'package:bloc/bloc.dart'; + +sealed class CounterEvent {} + +final class CounterIncrementPressed extends CounterEvent { + const CounterIncrementPressed(this.amount); + + final int amount; +} +`; +--- + + diff --git a/docs/src/content/docs/lint-rules/avoid_mutable_events.mdx b/docs/src/content/docs/lint-rules/avoid_mutable_events.mdx new file mode 100644 index 00000000000..e49b19a4b88 --- /dev/null +++ b/docs/src/content/docs/lint-rules/avoid_mutable_events.mdx @@ -0,0 +1,50 @@ +--- +title: Avoid Mutable Events +description: The avoid_mutable_events rule. +--- + +import { Badge } from '@astrojs/starlight/components'; +import EnableRuleSnippet from '~/components/lint-rules/EnableRuleSnippet.astro'; +import BadSnippet from '~/components/lint-rules/avoid_mutable_events/BadSnippet.mdx'; +import GoodSnippet from '~/components/lint-rules/avoid_mutable_events/GoodSnippet.astro'; + +
+ + +
+ +Avoid mutable fields and setters on `Bloc` events. + +## Rationale + +Events are not handled the moment they are added. They are queued and processed +asynchronously, and an `EventTransformer` such as `debounce` or `throttle` can +delay them further. If an event is mutated after being added, the handler +observes different data than the caller intended, and the resulting behavior +depends on timing. + +Mutable fields also break value equality, which blocs and tests rely on when +comparing events — for example when extending `Equatable` or when a transformer +compares incoming events. + +Modeling events as immutable value objects keeps them a faithful record of what +happened. + +## Examples + +**Avoid** mutable fields and setters on events. + +**BAD**: + + + +**GOOD**: + + + +## Enable + +To enable the `avoid_mutable_events` rule, add it to your +`analysis_options.yaml` under `bloc` > `rules`: + + diff --git a/packages/bloc_lint/README.md b/packages/bloc_lint/README.md index 636093d53b4..d778f10fb7d 100644 --- a/packages/bloc_lint/README.md +++ b/packages/bloc_lint/README.md @@ -98,6 +98,7 @@ For more information, check out the [official documentation](https://bloclibrary - [avoid_build_context_extensions](https://bloclibrary.dev/lint-rules/avoid_build_context_extensions) - [avoid_flutter_imports](https://bloclibrary.dev/lint-rules/avoid_flutter_imports) +- [avoid_mutable_events](https://bloclibrary.dev/lint-rules/avoid_mutable_events) - [avoid_public_bloc_methods](https://bloclibrary.dev/lint-rules/avoid_public_bloc_methods) - [avoid_public_fields](https://bloclibrary.dev/lint-rules/avoid_public_fields) - [prefer_bloc](https://bloclibrary.dev/lint-rules/prefer_bloc) diff --git a/packages/bloc_lint/lib/all.yaml b/packages/bloc_lint/lib/all.yaml index d7720e9a240..4a71888695f 100644 --- a/packages/bloc_lint/lib/all.yaml +++ b/packages/bloc_lint/lib/all.yaml @@ -2,6 +2,7 @@ bloc: rules: - avoid_build_context_extensions - avoid_flutter_imports + - avoid_mutable_events - avoid_public_bloc_methods - avoid_public_fields - prefer_bloc diff --git a/packages/bloc_lint/lib/bloc_lint.dart b/packages/bloc_lint/lib/bloc_lint.dart index 9db741ce5be..5f0b44c027b 100644 --- a/packages/bloc_lint/lib/bloc_lint.dart +++ b/packages/bloc_lint/lib/bloc_lint.dart @@ -10,6 +10,7 @@ export 'src/rules/rules.dart' show AvoidBuildContextExtensions, AvoidFlutterImports, + AvoidMutableEvents, AvoidPublicBlocMethods, AvoidPublicFields, PreferBloc, diff --git a/packages/bloc_lint/lib/src/linter.dart b/packages/bloc_lint/lib/src/linter.dart index db24ec826d4..74856bbe676 100644 --- a/packages/bloc_lint/lib/src/linter.dart +++ b/packages/bloc_lint/lib/src/linter.dart @@ -23,6 +23,7 @@ import 'package:path/path.dart' as p; final allRules = { AvoidBuildContextExtensions.rule: AvoidBuildContextExtensions.new, AvoidFlutterImports.rule: AvoidFlutterImports.new, + AvoidMutableEvents.rule: AvoidMutableEvents.new, AvoidPublicBlocMethods.rule: AvoidPublicBlocMethods.new, AvoidPublicFields.rule: AvoidPublicFields.new, PreferBloc.rule: PreferBloc.new, diff --git a/packages/bloc_lint/lib/src/rules/avoid_mutable_events.dart b/packages/bloc_lint/lib/src/rules/avoid_mutable_events.dart new file mode 100644 index 00000000000..0c7112a4294 --- /dev/null +++ b/packages/bloc_lint/lib/src/rules/avoid_mutable_events.dart @@ -0,0 +1,122 @@ +import 'package:bloc_lint/bloc_lint.dart'; + +/// {@template avoid_mutable_events} +/// The avoid_mutable_events lint rule. +/// {@endtemplate} +class AvoidMutableEvents extends LintRule { + /// {@macro avoid_mutable_events} + AvoidMutableEvents([Severity? severity]) + : super(name: rule, severity: severity ?? Severity.warning); + + /// The name of the lint rule. + static const rule = 'avoid_mutable_events'; + + @override + Listener? create(LintContext context) => _Listener(context); +} + +class _Listener extends Listener { + _Listener(this.context); + + final LintContext context; + + /// Whether the enclosing class is a bloc event. + bool _isEventClass = false; + + @override + void beginClassDeclaration( + Token begin, + Token? abstractToken, + Token? sealedToken, + Token? baseToken, + Token? interfaceToken, + Token? finalToken, + Token? augmentToken, + Token? mixinToken, + Token name, + ) { + // e.g. `sealed class CounterEvent {}` + _isEventClass = name.lexeme.isEventType; + } + + @override + void handleClassExtends(Token? extendsKeyword, int typeCount) { + final superclass = extendsKeyword?.next; + if (superclass == null) return; + // e.g. `final class CounterIncrementPressed extends CounterEvent {}` + if (superclass.lexeme.isEventType) _isEventClass = true; + } + + @override + void endClassDeclaration(Token beginToken, Token endToken) { + _isEventClass = false; + } + + @override + void endFields( + DeclarationKind kind, + Token? abstractToken, + Token? augmentToken, + Token? externalToken, + Token? staticToken, + Token? covariantToken, + Token? lateToken, + Token? varFinalOrConst, + int count, + Token beginToken, + Token endToken, + ) { + if (!_isEventClass) return; + if (kind != DeclarationKind.Class) return; + + // Static fields are not part of the event instance. + if (staticToken != null) return; + + // `final` and `const` fields cannot be reassigned. + if (varFinalOrConst != null && !varFinalOrConst.isVar) return; + + context.reportTokenRange( + beginToken: beginToken, + endToken: endToken, + message: 'Avoid mutable events.', + hint: 'Prefer marking fields as final.', + ); + } + + @override + void beginMethod( + DeclarationKind declarationKind, + Token? augmentToken, + Token? externalToken, + Token? staticToken, + Token? covariantToken, + Token? varFinalOrConst, + Token? getOrSet, + Token name, + String? enclosingDeclarationName, + ) { + if (!_isEventClass) return; + if (declarationKind != DeclarationKind.Class) return; + if (staticToken != null) return; + if (getOrSet == null || !getOrSet.isSet) return; + + context.reportToken( + token: name, + message: 'Avoid mutable events.', + hint: 'Prefer removing the setter.', + ); + } +} + +extension on String { + /// Whether the type name refers to a bloc event. + bool get isEventType => endsWith('Event'); +} + +extension on Token { + /// Whether the token is the `var` keyword. + bool get isVar => type == Keyword.VAR; + + /// Whether the token is the `set` keyword. + bool get isSet => type == Keyword.SET; +} diff --git a/packages/bloc_lint/lib/src/rules/rules.dart b/packages/bloc_lint/lib/src/rules/rules.dart index cf15d858171..5e417542b1a 100644 --- a/packages/bloc_lint/lib/src/rules/rules.dart +++ b/packages/bloc_lint/lib/src/rules/rules.dart @@ -1,5 +1,6 @@ export 'avoid_build_context_extensions.dart'; export 'avoid_flutter_imports.dart'; +export 'avoid_mutable_events.dart'; export 'avoid_public_bloc_methods.dart'; export 'avoid_public_fields.dart'; export 'prefer_bloc.dart'; diff --git a/packages/bloc_lint/test/src/rules/avoid_mutable_events_test.dart b/packages/bloc_lint/test/src/rules/avoid_mutable_events_test.dart new file mode 100644 index 00000000000..623c8f8d9ea --- /dev/null +++ b/packages/bloc_lint/test/src/rules/avoid_mutable_events_test.dart @@ -0,0 +1,206 @@ +import 'package:bloc_lint/src/rules/rules.dart'; +import 'package:test/test.dart'; + +import '../lint_test_helper.dart'; + +void main() { + group(AvoidMutableEvents, () { + lintTest( + 'lints when an event has a non-final field', + rule: AvoidMutableEvents.new, + path: 'counter_event.dart', + content: ''' +sealed class CounterEvent {} + +final class CounterIncrementPressed extends CounterEvent { + CounterIncrementPressed(this.count); + + int count; + ^^^^^^^^^^ +} +''', + ); + + lintTest( + 'lints when an event has a var field', + rule: AvoidMutableEvents.new, + path: 'counter_event.dart', + content: ''' +sealed class CounterEvent {} + +final class CounterIncrementPressed extends CounterEvent { + var count = 0; + ^^^^^^^^^^^^^^ +} +''', + ); + + lintTest( + 'lints when an event has a late, non-final field', + rule: AvoidMutableEvents.new, + path: 'counter_event.dart', + content: ''' +sealed class CounterEvent {} + +final class CounterIncrementPressed extends CounterEvent { + late int count; + ^^^^^^^^^^^^^^^ +} +''', + ); + + lintTest( + 'lints when the base event class has a non-final field', + rule: AvoidMutableEvents.new, + path: 'counter_event.dart', + content: ''' +abstract class CounterEvent { + int count = 0; + ^^^^^^^^^^^^^^ +} +''', + ); + + lintTest( + 'lints when an event has a setter', + rule: AvoidMutableEvents.new, + path: 'counter_event.dart', + content: ''' +sealed class CounterEvent {} + +final class CounterIncrementPressed extends CounterEvent { + set count(int value) {} + ^^^^^ +} +''', + ); + + lintTest( + 'does not lint when all event fields are final', + rule: AvoidMutableEvents.new, + path: 'counter_event.dart', + content: ''' +sealed class CounterEvent {} + +final class CounterIncrementPressed extends CounterEvent { + const CounterIncrementPressed(this.count); + + final int count; +} +''', + ); + + lintTest( + 'does not lint late final event fields', + rule: AvoidMutableEvents.new, + path: 'counter_event.dart', + content: ''' +sealed class CounterEvent {} + +final class CounterIncrementPressed extends CounterEvent { + late final int count; +} +''', + ); + + lintTest( + 'does not lint static event fields', + rule: AvoidMutableEvents.new, + path: 'counter_event.dart', + content: ''' +sealed class CounterEvent {} + +final class CounterIncrementPressed extends CounterEvent { + static int instances = 0; +} +''', + ); + + lintTest( + 'does not lint const event fields', + rule: AvoidMutableEvents.new, + path: 'counter_event.dart', + content: ''' +sealed class CounterEvent {} + +final class CounterIncrementPressed extends CounterEvent { + static const defaultCount = 0; +} +''', + ); + + lintTest( + 'does not lint getters on events', + rule: AvoidMutableEvents.new, + path: 'counter_event.dart', + content: ''' +sealed class CounterEvent {} + +final class CounterIncrementPressed extends CounterEvent { + const CounterIncrementPressed(this.count); + + final int count; + + bool get isEven => count % 2 == 0; +} +''', + ); + + lintTest( + 'does not lint classes which are not events', + rule: AvoidMutableEvents.new, + path: 'counter.dart', + content: ''' +class Counter { + int count = 0; +} +''', + ); + + lintTest( + 'does not lint state classes', + rule: AvoidMutableEvents.new, + path: 'counter_state.dart', + content: ''' +sealed class CounterState {} + +final class CounterInitial extends CounterState { + int count = 0; +} +''', + ); + + lintTest( + 'does not lint fields declared after the event class', + rule: AvoidMutableEvents.new, + path: 'counter_event.dart', + content: ''' +sealed class CounterEvent {} + +final class CounterIncrementPressed extends CounterEvent { + const CounterIncrementPressed(this.count); + + final int count; +} + +class Counter { + int count = 0; +} +''', + ); + + lintTest( + 'does not lint when the mutable field is ignored', + rule: AvoidMutableEvents.new, + path: 'counter_event.dart', + content: ''' +sealed class CounterEvent {} + +final class CounterIncrementPressed extends CounterEvent { + // ignore: avoid_mutable_events + int count = 0; +} +''', + ); + }); +}