feat: Custom lint rules

[zbarbuto]

It might be cool to add some custom lint rules to enforce consistency and/or good practices when using Bloc.

Some random suggestions and ideas (I haven't thought too long on these, some might be outright wrong, just starting discussion):

• State classes should end with State suffix
• Enforce immutable state classes
• Ensure state/event classes are sealed
• Prefer context.select() over BlocBuilder unless providing a buildWhen argument
• Avoid adding public methods on Bloc classes (i.e. not cubits)
• Avoid public properties on Blocs and Cubits besides state
• Avoid BuildContexts as arguments to cubit methods / bloc events
• Prefer context.read<T>() over BlocProvider.of<T>(context) (or vice/versa) for consistency
• Ensure the event superclass or all its subclasses are handled by on<T>s in constructor
• Avoid using the result of a cubit call (to prefer listening for state changes instead)
• Detect accidentally providing a bloc with Provider<MyBloc> or RepositoryProvider<MyBloc> instead of BlocProvider<MyBloc>

Rules could be opt-in but it would also be good to have a "recommended" set which are accepted as general best-practices.

Related work:

• riverpod has some pretty extensive lint rules and custom fixers to go along with them.
• The Angular NgRx state management package's ESLint rules which catch common mistakes
• DCM offers several bloc-centric lints under their paid plan

[tenhobi]

Hi, a bit off topic here: some of these lints/rules are already in the DCM, which I highly recommend. https://dcm.dev

[zbarbuto]

I did come across DCM shortly after posting this. Good for inspiration, but my preference would still be if the lints were first-party to the library itself for various reasons as well as open source to allow contributions.

Also, all the bloc-based lints appear to be behind the Pro+ plan, which wouldn't be for everyone.

Have added it to the related work section.

[sanba-anass]

ich I highly recommend. https://dcm.dev

nice try w promote

[vmichalak]

Because i really like this idea, i've start a project named bloc_lint to purpose something.
Feel free to contribute 🤗

[vmichalak]

I've upload a first version of the package on pub.dev if you want to test it : https://pub.dev/packages/bloc_lint

[felangel]

Hi @vmichalak 👋
Thanks for taking the initiative! Is your intention to maintain that package or did you want it to be part of the bloc monorepo? It sounded like folks wanted first party support based on the discussion.

[vmichalak]

The two options sounds great to me !
If you want to implement it to the main monorepo i can manage to do it :)

[felangel]

The two options sounds great to me ! If you want to implement it to the main monorepo i can manage to do it :)

It’s totally up to you since you did the work if you prefer to maintain it as a separate repo/package that’s totally fine!

If you do want to move it into the bloc mono repo and have time to open a PR and add me as a publisher to the package that would be great! There are some things that we’d likely want to discuss for a v0.2.0 (e.g whether we want to keep the custom_lint dependency, lint rules and their names, etc).

[vmichalak]

This version is really a first implementation to verify the faisability. It's possible to make a better implementation for sure.

I gonna check later today to add it to the main repo, if it's possible i would like to continue to work on it even if it's merge to the main monorepo 🤗

[felangel]

This version is really an first implementation to verify the faisability. It's possible to make a better implementation for sure.

I gonna check later today to add it to the main repo, if it's possible i would like to continue to work on it even if it's merge to the main monorepo 🤗

Yup makes sense!

if it's possible i would like to continue to work on it even if it's merge to the main monorepo 🤗

Of course! I added you as a collaborator to the bloc repo and welcome all contributions! 💙

[zbarbuto]

Because i really like this idea, i've start a project named bloc_lint to purpose something. Feel free to contribute 🤗

Awesome start! I'll be looking to make some contributions this week, for sure! Also appreciate the initiative.

It sounded like folks wanted first party support based on the discussion.

Yeah, my preference is definitely to have it part of the official repo to sort of signify that it's best practices as endorsed by the package itself.

[felangel]

Dumping the list I have so far (will come back and elaborate on each one shortly)

• prefer_immutable_state
• prefer_immutable_event
• prefer_multi_bloc_provider
• prefer_multi_bloc_listener
• prefer_multi_repository_provider
• prefer_context_extensions
• prefer_emit_for_each
• prefer_emit_on_each
• prefer_bloc
• prefer_cubit
• prefer_void_methods_on_cubit
• avoid_context_extensions
• avoid_duplicate_event_handlers
• avoid_missing_event_handlers
• avoid_depending_on_flutter
• avoid_public_methods_on_bloc
• avoid_public_members
• avoid_returning_existing_instance_from_bloc_provider_create
• avoid_returning_new_instance_from_bloc_provider_value
• avoid_using_provider_with_bloc_base

Also, I'm currently doing research to determine the best approach to building a performant linter. I'm evaluating several options including:

• custom_lint
• custom analyzer_plugin
• fully custom solution

My goal is to pick a solution that is fast, memory efficient, easy to configure, and has minimal dependencies/setup needed. Will report back when I know more 👍

[zbarbuto]

@felangel I had a query the other day that might relate to lint rules: Should context extensions avoid being conditional? Or am I getting confused with react hooks.

i.e. is this acceptable or should be avoided (and therefore linted)?

build() {
  if(condition) {
    final foo = context.select((FooBloc bloc) => bloc.state.foo));
    return Text(foo);
  }

  return Text('Bar');
}

[felangel]

@felangel I had a query the other day that might relate to lint rules: Should context extensions avoid being conditional? Or am I getting confused with react hooks.

i.e. is this acceptable or should be avoided (and therefore linted)?

build() {
  if(condition) {
    final foo = context.select((FooBloc bloc) => bloc.state.foo));
    return Text(foo);
  }

  return Text('Bar');
}

Extensions should be okay to use conditionally unlike hooks.

[zbarbuto]

Extensions should be okay to use conditionally unlike hooks.

Good to know.

Another potential suggestion:

prefer_void_cubit_methods
prefer_void_bloc_methods

To avoid things like

class FrogCubit extends Cubit {
  getFrog() async {
    final frog = await frogService.getFrog();
    emit(frog);
    return frog;
  }
}

// in Widget tree
try {
  final frog = await context.read<FrogCubit>.getFrog();
  // Use frog
} catch() {
   showNotification();
} 

Obviously FutureOr<void> is fine.

[felangel]

Extensions should be okay to use conditionally unlike hooks.

Good to know.

Another potential suggestion:

prefer_void_cubit_methods prefer_void_bloc_methods

To avoid things like

class FrogCubit extends Cubit {
  getFrog() async {
    final frog = await frogService.getFrog();
    emit(frog);
    return frog;
  }
}

// in Widget tree
try {
  final frog = await context.read<FrogCubit>.getFrog();
  // Use frog
} catch() {
   showNotification();
} 

Obviously FutureOr<void> is fine.

Good call, I updated the list 👍

[vmichalak]

• prefer_immutable_state
• prefer_immutable_event
• prefer_multi_bloc_provider
• prefer_multi_bloc_listener
• prefer_multi_repository_provider
• prefer_context_extensions
• prefer_emit_for_each
• prefer_emit_on_each
• prefer_bloc
• prefer_cubit
• prefer_void_methods_on_cubit
• avoid_context_extensions
• avoid_duplicate_event_handlers
• avoid_missing_event_handlers
• avoid_depending_on_flutter
• avoid_public_methods_on_bloc
• avoid_public_members
• avoid_returning_existing_instance_from_bloc_provider_create
• avoid_returning_new_instance_from_bloc_provider_value
• avoid_using_provider_with_bloc_base

Since the final objective stay to purpose this as a plugin to the analyzer (perhaps with analyzer plugin v2 in a long long future ?) i think it's important to always have a reference to bloc or cubit in the lint rule name.

Also we have to prioritize the rules from the most important to the least to organize the implementation in the next step.

[tenhobi]

Also, I'm currently doing research to determine the best approach to building a performant linter. I'm evaluating several options including: ...

@felangel just note dart-lang/sdk#50981 and flutter/flutter#121836, aka it seems more than 1 plugin is disabled for Dart now. Sticking with custom_lint over own plugin is therefore advised, since a lot of packages already use custom_lint for some lints.

Another option is to use DCM which has a lot of Bloc related rules already, plus hundreds more for other stuff. But I understand that it might not be option for everybody.

[felangel]

Also, I'm currently doing research to determine the best approach to building a performant linter. I'm evaluating several options including: ...

@felangel just note dart-lang/sdk#50981 and flutter/flutter#121836, aka it seems more than 1 plugin is disabled for Dart now. Sticking with custom_lint over own plugin is therefore advised, since a lot of packages already use custom_lint for some lints.

Another option is to use DCM which has a lot of Bloc related rules already, plus hundreds more for other stuff. But I understand that it might not be option for everybody.

Thanks for the context! The problem with custom_lint is the performance in large codebases and the need to add an additional dependency. The problem with DCM is it's not free to use and only includes a subset of the lint rules we're discussing.

Will update the issue when I have a better sense of how to proceed 👍

[incendial]

The problem with custom_lint is the performance in large codebases and the need to add an additional dependency.

This is the analyzer plugins problem in general (since custom_lint is a plugin). So the only way to make it fast would be to do what we did for DCM (a standalone analysis process) and I'm not sure you want to go this road for just a dozen of rules.

We are actually going to add bloc rules to the free plan with the next release, but I doubt it will affect your plans.

and only includes a subset of the lint rules we're discussing.

And I'll look into that 🙂