merge/audit-and-state-map #7

Merged
trevon merged 15 commits from merge/audit-and-state-map into main 2026-09-25 00:07:04 +00:00
5 changed files with 230 additions and 16 deletions
Showing only changes of commit cb30269f64 - Show all commits

fix(event-rules): follow the event hierarchy and every owning bloc

Two ways the event rules reported handled events as unhandled (error
severity, high confidence):

1. A handler on the event base or an intermediate group. `on<SyncEvent>`
   with a switch inside handles every subclass, but handlers were matched
   to dispatches by exact name, so every dispatched subclass was reported
   by event-without-handler and the base handler itself by dead-handler.
   Variants now record the supertype they were enumerated through, and
   coverage follows it: an event is handled when it or an ancestor is
   registered, and a handler is live when anything at or below it is
   dispatched or constructed. A group handler still does not cover a
   sibling outside the group (UploadAbandoned stays reported).

2. Blocs sharing an event type. Only the first owner of an event type was
   kept, so a second bloc's handlers were ignored: an event only it handles
   was reported unhandled, and its dead handlers were never examined. There
   is now one event union per owning bloc. A dispatch does not record which
   bloc it targets, so an event any sharing bloc handles counts as handled,
   and an unhandled event is reported once per shared union rather than
   once per bloc. Dead handlers are still judged per bloc.

Reproduced first on base_event.dart: 4 false findings before
(SyncStarted, UploadRetryNow unhandled; SyncEvent, UploadRetry dead) and
the shared case (ShareDismissed falsely unhandled, ShareBetaBloc's dead
ShareExpired missed). Each is now a contract control.

No change on nuwayze (102 findings) or tick (2): byte-identical before
and after, since neither uses these patterns.
trvon 2026-09-22 13:30:21 -06:00
Signed by: trevon
GPG key ID: 38686EA797D7A8A7

View file

@ -573,15 +573,53 @@ FindingReport runChecks({
handledByBloc.putIfAbsent(handler.bloc, () => {}).add(handler.event);
}
// A dispatch does not record which bloc it targets, so when several blocs
// share an event type, an event any of them handles is handled.
final ownersByUnion = <String, List<String>>{};
for (final union in flow.eventUnions) {
ownersByUnion.putIfAbsent(union.name, () => []).add(union.owner);
}
final reportedUnhandled = <String>{};
for (final union in flow.eventUnions) {
final variantNames = {for (final v in union.variants) v.name};
final handled = handledByBloc[union.owner] ?? <String>{};
final owners = ownersByUnion[union.name]!;
final handledByAnyOwner = <String>{
for (final owner in owners) ...?handledByBloc[owner],
};
final dispatchedHere = dispatched.keys
.where(variantNames.contains)
.toSet();
// `on<Base>` or `on<Group>` handles every subtype, so coverage follows the
// hierarchy: an event is handled when it or any ancestor up to the union
// base is registered, and a handler is live when anything at or below it
// is dispatched. Exact-name matching reported every dispatched subclass
// of a base handler as unhandled (error severity) and the base handler
// itself as dead.
final parentOf = {
for (final variant in union.variants)
if (variant.parent != null) variant.name: variant.parent!,
};
Iterable<String> selfAndAncestors(String event) sync* {
final seen = <String>{};
String? current = event;
while (current != null && seen.add(current)) {
yield current;
current = parentOf[current];
}
}
bool isHandled(String event) =>
selfAndAncestors(event).any(handledByAnyOwner.contains);
bool covers(String handler, String event) =>
selfAndAncestors(event).contains(handler);
if (enabled.contains(ruleEventWithoutHandler)) {
for (final event in dispatchedHere.difference(handled)) {
for (final event in dispatchedHere.where((e) => !isHandled(e))) {
// Reported once per shared union, not once per owning bloc.
if (!reportedUnhandled.add('${union.name}.$event')) continue;
final sites = dispatched[event]!;
findings.add(
Finding(
@ -589,7 +627,8 @@ FindingReport runChecks({
severity: PipeSeverity.error,
confidence: 'high',
message:
'$event is dispatched but ${union.owner} registers no handler '
'$event is dispatched but ${owners.join(', ')} '
'${owners.length == 1 ? 'registers' : 'register'} no handler '
'for it, so the interaction does nothing.',
file: sites.first.path,
line: sites.first.line,
@ -607,7 +646,9 @@ FindingReport runChecks({
}
if (enabled.contains(ruleDeadHandler)) {
for (final event in handled.difference(dispatchedHere)) {
for (final event in handled.where(
(h) => !dispatchedHere.any((e) => covers(h, e)),
)) {
final registration = flow.handlers.firstWhere(
(handler) => handler.bloc == union.owner && handler.event == event,
);
@ -622,7 +663,8 @@ FindingReport runChecks({
};
final constructedElsewhere = constructedByFile.entries.any(
(entry) =>
!declaredIn.contains(entry.key) && entry.value.contains(event),
!declaredIn.contains(entry.key) &&
entry.value.any((built) => covers(event, built)),
);
if (constructedElsewhere) continue;
findings.add(

View file

@ -26,16 +26,22 @@ enum UnionStyle {
/// Something that can be matched on the state type.
class StateVariant {
StateVariant({required this.name, this.declaredIn, this.note});
StateVariant({required this.name, this.declaredIn, this.note, this.parent});
final String name;
final String? declaredIn;
final String? note;
/// The direct supertype this variant was enumerated through: the union base
/// or an intermediate group. Null for enum constants and freezed redirects.
/// Rules use it so a handler on a base or group type covers its subtypes.
final String? parent;
Map<String, Object?> toJson() => {
'name': name,
if (declaredIn != null) 'declaredIn': declaredIn,
if (note != null) 'note': note,
if (parent != null) 'parent': parent,
};
}
@ -1119,7 +1125,9 @@ class _MentionCollector extends RecursiveAstVisitor<void> {
final current = queue.removeLast();
if (!visited.add(current)) continue;
for (final type in bySupertype[current] ?? const <_TypeInfo>[]) {
variants.add(StateVariant(name: type.name, declaredIn: type.file));
variants.add(
StateVariant(name: type.name, declaredIn: type.file, parent: current),
);
queue.add(type.name);
}
}
@ -1649,13 +1657,13 @@ StateFlow buildStateFlow(Project project, List<FileOutcome> outcomes) {
);
}
// Event unions: the `E` of `Bloc<E, S>` plus its subclass events.
// Event unions: the `E` of `Bloc<E, S>` plus its subclass events. One per
// owning bloc: blocs that share an event type each register their own
// handlers, and keeping only the first owner ignored every other bloc's.
final eventUnions = <EventUnion>[];
final seenEvents = <String>{};
for (final entry in blocEvent.entries) {
final eventType = entry.value;
if (eventType == null) continue;
if (!seenEvents.add(eventType)) continue;
final enumeration = _enumerateVariants(
eventType,
byName,

View file

@ -0,0 +1,127 @@
import 'bloc.dart';
// --- a handler registered on the event BASE --------------------------------
//
// `on<SyncEvent>` with a switch inside handles every subclass. Matching handlers
// to dispatches by exact name would report every dispatched subclass as
// unhandled (error severity) and the base handler itself as dead. This is the
// regression guard for handler coverage following the event hierarchy.
sealed class SyncEvent {
const SyncEvent();
}
class SyncStarted extends SyncEvent {
const SyncStarted();
}
class SyncCancelled extends SyncEvent {
const SyncCancelled();
}
/// An intermediate group: `on<RetryEvent>` covers [RetryNow] only.
abstract class RetryEvent extends SyncEvent {
const RetryEvent();
}
class RetryNow extends RetryEvent {
const RetryNow();
}
class SyncState {
const SyncState();
}
class SyncBloc extends Bloc<SyncEvent, SyncState> {
SyncBloc() : super(const SyncState());
void register() {
on<SyncEvent>((event, emit) {
switch (event) {
case SyncStarted():
case SyncCancelled():
case RetryEvent():
emit(const SyncState());
}
});
}
void start() => add(const SyncStarted());
}
// A second bloc whose only handler is on an intermediate group, so the rule must
// credit dispatches of the group's subclasses to it, and still report a sibling
// outside the group.
sealed class UploadEvent {
const UploadEvent();
}
abstract class UploadRetry extends UploadEvent {
const UploadRetry();
}
class UploadRetryNow extends UploadRetry {
const UploadRetryNow();
}
/// Dispatched, and outside the handled `UploadRetry` group: truly unhandled.
class UploadAbandoned extends UploadEvent {
const UploadAbandoned();
}
class UploadBloc extends Bloc<UploadEvent, SyncState> {
UploadBloc() : super(const SyncState());
void register() {
on<UploadRetry>((event, emit) => emit(const SyncState()));
}
void retry() => add(const UploadRetryNow());
void abandon() => add(const UploadAbandoned());
}
// --- two blocs sharing one event type ---------------------------------------
//
// Keeping only the first owner of an event type ignored the second bloc's
// handlers: an event only the second bloc handles was reported unhandled (error
// severity), and a dead handler in the second bloc was never seen.
sealed class ShareEvent {
const ShareEvent();
}
class SharePressed extends ShareEvent {
const SharePressed();
}
class ShareDismissed extends ShareEvent {
const ShareDismissed();
}
/// Handled only by [ShareBetaBloc], and never dispatched: dead there.
class ShareExpired extends ShareEvent {
const ShareExpired();
}
class ShareAlphaBloc extends Bloc<ShareEvent, SyncState> {
ShareAlphaBloc() : super(const SyncState());
void register() {
on<SharePressed>((event, emit) => emit(const SyncState()));
}
void press() => add(const SharePressed());
}
class ShareBetaBloc extends Bloc<ShareEvent, SyncState> {
ShareBetaBloc() : super(const SyncState());
void register() {
on<ShareDismissed>((event, emit) => emit(const SyncState()));
on<ShareExpired>((event, emit) => emit(const SyncState()));
}
void dismiss() => add(const ShareDismissed());
}

View file

@ -49,8 +49,21 @@ const Map<String, List<_Id>> _negatives = {
// event rules actually consult the handler and dispatch information.
// NeverDispatched (never handled) and UnhandledTap (never handled) were
// vacuous.
ruleEventWithoutHandler: [('event', 'HandledTap')],
ruleDeadHandler: [('event', 'HandledTap')],
// SyncStarted/UploadRetryNow are handled through `on<SyncEvent>` and
// `on<UploadRetry>`; those base and group handlers are live because a
// subtype is dispatched (base_event.dart).
ruleEventWithoutHandler: [
('event', 'HandledTap'),
('event', 'SyncStarted'),
('event', 'UploadRetryNow'),
// Handled by ShareBetaBloc, which shares ShareEvent with ShareAlphaBloc.
('event', 'ShareDismissed'),
],
ruleDeadHandler: [
('event', 'HandledTap'),
('event', 'SyncEvent'),
('event', 'UploadRetry'),
],
// handledProvider covers all AsyncValue cases.
ruleRiverpodAsyncGap: [('provider', 'handledProvider')],
// `bump` notifies; `setQuiet` mutates but a notifying method calls it.
@ -96,6 +109,27 @@ void main() {
}
}
test('dead-handler: every bloc sharing an event type is checked', () {
// Only the first owner of ShareEvent used to be kept, so ShareBetaBloc's
// handlers were never examined.
final owners = {
for (final finding in findings)
if (finding.rule == ruleDeadHandler &&
finding.data['event'] == 'ShareExpired')
finding.data['owner'],
};
expect(owners, {'ShareBetaBloc'});
});
test('event-without-handler: a group handler does not cover a sibling', () {
// UploadAbandoned sits outside the handled UploadRetry group, so following
// the hierarchy must not over-credit it.
expect(
valuesOf(ruleEventWithoutHandler, 'event'),
contains('UploadAbandoned'),
);
});
for (final entry in _positives.entries) {
test('${entry.key}: fires on the positive control', () {
final (key, value) = entry.value;

View file

@ -103,11 +103,14 @@ void main() {
final hits = report.findings
.where((finding) => finding.rule == ruleEventWithoutHandler)
.toList();
// ItemBloc registers handlers for both of its events, so the only report
// must be the deliberate gap fixture.
expect(hits, hasLength(1));
expect(hits.single.data['event'], 'UnhandledTap');
expect(hits.single.data['owner'], 'GapBloc');
// ItemBloc registers handlers for both of its events, and SyncBloc/UploadBloc
// cover their other events through base and group handlers, so the only
// reports must be the two deliberate gaps.
expect(
{for (final hit in hits) '${hit.data['owner']}.${hit.data['event']}'},
{'GapBloc.UnhandledTap', 'UploadBloc.UploadAbandoned'},
);
expect(hits, hasLength(2));
});
test('every finding carries evidence', () {