Skip to content

fix: _isOverride is never reset, so lint rules skip every member after an @override #4863

Description

@mysCod3r

Description

avoid_public_bloc_methods and prefer_void_public_cubit_methods share an _isOverride flag that is only ever written from beginMetadata:

var _isOverride = false;

@override
void beginMetadata(Token token) {
  _isOverride = token.next?.lexeme == 'override';
}

beginMetadata fires only when a declaration has an annotation, so the flag is never cleared. Once any member is annotated with @override, the exemption leaks to every member that follows it in that class.

Two separate defects come out of that one line:

  1. False negative (the main one). Every public method declared after an @override method is skipped, however it is written.
  2. False positive. A method carrying @override plus a second annotation gets linted anyway, because the second beginMetadata call overwrites the flag back to false.

The exemption itself is intended. #4395 added it as "avoid_public_bloc_methods allows @override". It is only the leaking that looks unintended.

Steps To Reproduce

Defect 1, false negative. Lint this file with prefer_void_public_cubit_methods enabled. The two classes are identical apart from the order of their members:

import 'package:flutter_bloc/flutter_bloc.dart';

class CounterCubit extends Cubit<int> {
  CounterCubit() : super(0);

  // Reported, as expected.
  Future<bool> increment() async => true;

  @override
  void emit(int state) => super.emit(state);
}

class CounterCubit2 extends Cubit<int> {
  CounterCubit2() : super(0);

  @override
  void emit(int state) => super.emit(state);

  // Identical method, now silent.
  Future<bool> increment() async => true;
}

Only CounterCubit.increment is reported. CounterCubit2.increment is the same method, but it sits after an @override member, so it produces no diagnostic at all. This happens both in the CLI and in the editor.

Defect 2, false positive. Same rule. isReady is an @override, so it should be exempt, but it is reported because @visibleForTesting clears the flag that @override just set:

import 'package:flutter_bloc/flutter_bloc.dart';
import 'package:meta/meta.dart';

class CounterCubit extends Cubit<int> {
  CounterCubit() : super(0);

  @override
  @visibleForTesting
  Future<bool> isReady() async => true; // reported, although it is an @override
}

Both defects also affect avoid_public_bloc_methods, which carries a copy of the same _isOverride code. Its version of defect 1:

import 'package:flutter_bloc/flutter_bloc.dart';

class CounterBloc extends Bloc<CounterEvent, int> {
  CounterBloc() : super(0);

  @override
  Future<void> close() => super.close();

  // Should be reported, but is not.
  void increment() {}
}

Expected Behavior

The @override exemption should apply to the annotated member only. Moving a method inside a class should not change whether it is reported, and an @override member should stay exempt no matter how many other annotations it carries.

Screenshots

Image

Both classes in one editor window. The Problems panel holds a single entry, prefer_void_public_cubit_methods on line 7, and nothing for the identical method on line 20.

Additional Context

The root cause is the hook, not the comparison. Instrumenting the parser gives this event order for every class member:

beginMetadataStar        <- fires for every member, before its metadata
  beginMetadata @override
endMetadataStar count=N
beginMember              <- fires after metadata
beginMethod

beginMetadataStar runs for every member, including the ones with no annotation, so that is where the flag can be cleared. beginMember is not usable for this: it runs after the metadata has already been read, so clearing there would drop a legitimate @override.

That makes the fix two lines per rule:

@override
void beginMetadataStar(Token token) {
  _isOverride = false;
}

@override
void beginMetadata(Token token) {
  if (token.next?.lexeme == 'override') _isOverride = true;
}

Clearing on beginMetadataStar fixes the false negative. Accumulating instead of assigning in beginMetadata fixes the false positive.

I have this applied locally against master with three regression tests, one per defect plus the avoid_public_bloc_methods case. Each of them fails without the change and passes with it, and the bloc_lint suite goes from 151 to 154 tests with nothing else changing. I will open a PR.

Versions

  • bloc_lint 0.4.2, also reproduced on master (8fcf54d)
  • bloc_tools 0.1.0-dev.24
  • Dart 3.11.5, macOS arm64

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions