merge/audit-and-state-map #7
4 changed files with 144 additions and 16 deletions
fix(state-flow): credit freezed factory calls to their redirect target
freezed declares variants as redirecting factories on the base (`const factory DocState.loading() = DocLoading;`) and blocs emit through them. `emit(const DocState.loading())` was recorded as emitting `DocState`, which is never a variant, so no freezed emission matched and every variant the UI renders was reported by rendered-not-emitted as a dead branch. Factory redirects are now also kept by constructor name, and an instance creation of `Type.ctor` (or an unnamed redirecting factory) is credited to the class it redirects to, for emissions, `state =` assignments and dispatches alike. Reproduced on freezed_style.dart before the fix: DocLoading, DocReady and DocStale all reported as never emitted; after, only DocStale, which really is never emitted and stays as the positive control. No change on nuwayze, tick, dex or word_alive.
commit
6e2d47f7db
|
|
@ -478,6 +478,7 @@ class _TypeInfo {
|
|||
required this.isSealed,
|
||||
required this.enumConstants,
|
||||
required this.factoryRedirects,
|
||||
this.namedRedirects = const {},
|
||||
this.loadingFields = const [],
|
||||
this.loadingDefaultsTrue = false,
|
||||
this.fields = const [],
|
||||
|
|
@ -493,6 +494,10 @@ class _TypeInfo {
|
|||
final List<String> enumConstants;
|
||||
final List<String> factoryRedirects;
|
||||
|
||||
/// Factory constructor name (`''` for the unnamed one) to its redirect
|
||||
/// target, so `DocState.loading()` can be credited to the variant it builds.
|
||||
final Map<String, String> namedRedirects;
|
||||
|
||||
/// Loading-style boolean fields declared on the type.
|
||||
final List<String> loadingFields;
|
||||
|
||||
|
|
@ -609,8 +614,14 @@ class _TypeCollector extends RecursiveAstVisitor<void> {
|
|||
|
||||
/// Read freezed-style `factory X.y(...) = Z;` redirect targets from source.
|
||||
/// Parsed textually because the AST accessor is experimental.
|
||||
static List<String> _factoryRedirects(NodeList<ClassMember> members) {
|
||||
final redirects = <String>[];
|
||||
static List<String> _factoryRedirects(NodeList<ClassMember> members) =>
|
||||
_namedFactoryRedirects(members).values.toList();
|
||||
|
||||
/// Same parse, keyed by constructor name (`''` for the unnamed factory).
|
||||
static Map<String, String> _namedFactoryRedirects(
|
||||
NodeList<ClassMember> members,
|
||||
) {
|
||||
final redirects = <String, String>{};
|
||||
for (final member in members) {
|
||||
if (member is! ConstructorDeclaration) continue;
|
||||
if (member.factoryKeyword == null) continue;
|
||||
|
|
@ -619,7 +630,9 @@ class _TypeCollector extends RecursiveAstVisitor<void> {
|
|||
r'=\s*([A-Za-z_$][A-Za-z0-9_$.]*)',
|
||||
).firstMatch(declaration);
|
||||
final target = match?.group(1);
|
||||
if (target != null) redirects.add(_baseName(target));
|
||||
if (target != null) {
|
||||
redirects[member.name?.lexeme ?? ''] = _baseName(target);
|
||||
}
|
||||
}
|
||||
return redirects;
|
||||
}
|
||||
|
|
@ -644,6 +657,7 @@ class _TypeCollector extends RecursiveAstVisitor<void> {
|
|||
isSealed: node.sealedKeyword != null,
|
||||
enumConstants: const [],
|
||||
factoryRedirects: _factoryRedirects(node.body.members),
|
||||
namedRedirects: _namedFactoryRedirects(node.body.members),
|
||||
loadingFields: loadingFields,
|
||||
loadingDefaultsTrue: loadingDefaultsTrue,
|
||||
fields: _declaredFields(node.body.members),
|
||||
|
|
@ -674,9 +688,16 @@ class _TypeCollector extends RecursiveAstVisitor<void> {
|
|||
}
|
||||
|
||||
class _EmissionCollector extends RecursiveAstVisitor<void> {
|
||||
_EmissionCollector(this.project, this.path, this.lineInfo, this.sites);
|
||||
_EmissionCollector(
|
||||
this.project,
|
||||
this.path,
|
||||
this.lineInfo,
|
||||
this.sites,
|
||||
this.redirects,
|
||||
);
|
||||
|
||||
final Project project;
|
||||
final Map<String, String> redirects;
|
||||
final String path;
|
||||
final LineInfo lineInfo;
|
||||
final List<EmissionSite> sites;
|
||||
|
|
@ -731,11 +752,11 @@ class _EmissionCollector extends RecursiveAstVisitor<void> {
|
|||
|
||||
/// Arguments arrive as [Argument] wrappers; positional arguments are also
|
||||
/// [Expression]s, which is how we get at the emitted value.
|
||||
static _Extraction _fromArguments(List<Argument> arguments) {
|
||||
_Extraction _fromArguments(List<Argument> arguments) {
|
||||
if (arguments.isEmpty) return _Extraction(const [], 'unknown');
|
||||
final argument = arguments.first;
|
||||
return argument is Expression
|
||||
? _extractVariants(argument)
|
||||
? _extractVariants(argument, redirects)
|
||||
: _Extraction(const [], 'unknown');
|
||||
}
|
||||
|
||||
|
|
@ -775,7 +796,7 @@ class _EmissionCollector extends RecursiveAstVisitor<void> {
|
|||
@override
|
||||
void visitAssignmentExpression(AssignmentExpression node) {
|
||||
if (node.leftHandSide.toSource().trim() == 'state') {
|
||||
final extraction = _extractVariants(node.rightHandSide);
|
||||
final extraction = _extractVariants(node.rightHandSide, redirects);
|
||||
sites.add(
|
||||
EmissionSite(
|
||||
path: project.relative(path),
|
||||
|
|
@ -805,12 +826,15 @@ class _Extraction {
|
|||
}
|
||||
|
||||
class _VariantFinder extends RecursiveAstVisitor<void> {
|
||||
_VariantFinder(this.redirects);
|
||||
|
||||
final Map<String, String> redirects;
|
||||
final List<String> found = [];
|
||||
var sawCopyWith = false;
|
||||
|
||||
@override
|
||||
void visitInstanceCreationExpression(InstanceCreationExpression node) {
|
||||
final name = node.constructorName.type.name.lexeme;
|
||||
final name = _createdVariant(node, redirects);
|
||||
if (!found.contains(name)) found.add(name);
|
||||
super.visitInstanceCreationExpression(node);
|
||||
}
|
||||
|
|
@ -822,15 +846,32 @@ class _VariantFinder extends RecursiveAstVisitor<void> {
|
|||
}
|
||||
}
|
||||
|
||||
_Extraction _extractVariants(Expression expression) {
|
||||
/// The variant an instance creation builds. A freezed-style factory
|
||||
/// (`const factory DocState.loading() = DocLoading;`) builds its redirect
|
||||
/// target, not the base type it is called on, so `DocState.loading()` is
|
||||
/// credited to `DocLoading`. Recording `DocState` meant no freezed emission ever
|
||||
/// matched its variant.
|
||||
String _createdVariant(
|
||||
InstanceCreationExpression node,
|
||||
Map<String, String> redirects,
|
||||
) {
|
||||
final type = node.constructorName.type.name.lexeme;
|
||||
final constructor = node.constructorName.name?.name ?? '';
|
||||
return redirects['$type.$constructor'] ?? type;
|
||||
}
|
||||
|
||||
_Extraction _extractVariants(
|
||||
Expression expression,
|
||||
Map<String, String> redirects,
|
||||
) {
|
||||
if (expression is InstanceCreationExpression) {
|
||||
return _Extraction([expression.constructorName.type.name.lexeme], 'exact');
|
||||
return _Extraction([_createdVariant(expression, redirects)], 'exact');
|
||||
}
|
||||
if (expression is MethodInvocation &&
|
||||
expression.methodName.name == 'copyWith') {
|
||||
return _Extraction(const [], 'copyWith');
|
||||
}
|
||||
final finder = _VariantFinder();
|
||||
final finder = _VariantFinder(redirects);
|
||||
expression.accept(finder);
|
||||
if (finder.sawCopyWith && finder.found.isEmpty) {
|
||||
return _Extraction(const [], 'copyWith');
|
||||
|
|
@ -1197,9 +1238,16 @@ class _HandlerCollector extends RecursiveAstVisitor<void> {
|
|||
|
||||
/// Records `add(Event())` dispatch sites.
|
||||
class _DispatchCollector extends RecursiveAstVisitor<void> {
|
||||
_DispatchCollector(this.project, this.path, this.lineInfo, this.dispatches);
|
||||
_DispatchCollector(
|
||||
this.project,
|
||||
this.path,
|
||||
this.lineInfo,
|
||||
this.dispatches,
|
||||
this.redirects,
|
||||
);
|
||||
|
||||
final Project project;
|
||||
final Map<String, String> redirects;
|
||||
final String path;
|
||||
final LineInfo lineInfo;
|
||||
final List<EventDispatch> dispatches;
|
||||
|
|
@ -1211,7 +1259,7 @@ class _DispatchCollector extends RecursiveAstVisitor<void> {
|
|||
if (arguments.isNotEmpty) {
|
||||
final argument = arguments.first;
|
||||
if (argument is Expression) {
|
||||
final extraction = _extractVariants(argument);
|
||||
final extraction = _extractVariants(argument, redirects);
|
||||
for (final variant in extraction.variants) {
|
||||
dispatches.add(
|
||||
EventDispatch(
|
||||
|
|
@ -1520,6 +1568,12 @@ StateFlow buildStateFlow(Project project, List<FileOutcome> outcomes) {
|
|||
for (final type in types)
|
||||
if (type.isEnum) type.name: type,
|
||||
};
|
||||
// `Type.constructor` -> the class a redirecting factory actually builds.
|
||||
final redirects = <String, String>{
|
||||
for (final type in types)
|
||||
for (final entry in type.namedRedirects.entries)
|
||||
'${type.name}.${entry.key}': entry.value,
|
||||
};
|
||||
|
||||
// Bloc/cubit owners and their state types.
|
||||
final blocState = <String, String>{};
|
||||
|
|
@ -1693,10 +1747,14 @@ StateFlow buildStateFlow(Project project, List<FileOutcome> outcomes) {
|
|||
final result = entry.value;
|
||||
final unit = result.unit;
|
||||
final lineInfo = result.lineInfo;
|
||||
unit.accept(_EmissionCollector(project, entry.key, lineInfo, emissions));
|
||||
unit.accept(
|
||||
_EmissionCollector(project, entry.key, lineInfo, emissions, redirects),
|
||||
);
|
||||
unit.accept(_RenderCollector(project, entry.key, lineInfo, renders));
|
||||
unit.accept(_HandlerCollector(project, entry.key, lineInfo, handlers));
|
||||
unit.accept(_DispatchCollector(project, entry.key, lineInfo, dispatches));
|
||||
unit.accept(
|
||||
_DispatchCollector(project, entry.key, lineInfo, dispatches, redirects),
|
||||
);
|
||||
unit.accept(
|
||||
_RiverpodCollector(
|
||||
project,
|
||||
|
|
|
|||
41
test/fixtures/stuck_loading/lib/freezed_style.dart
vendored
Normal file
41
test/fixtures/stuck_loading/lib/freezed_style.dart
vendored
Normal file
|
|
@ -0,0 +1,41 @@
|
|||
import 'bloc.dart';
|
||||
|
||||
// --- a freezed-style union --------------------------------------------------
|
||||
//
|
||||
// freezed declares variants as redirecting factories on the base, and the bloc
|
||||
// emits through them: `emit(const DocState.loading())` builds a DocLoading.
|
||||
// Recording the base type (`DocState`) meant no emission ever matched a variant,
|
||||
// so every variant the UI renders was reported as never emitted. The variant
|
||||
// classes are written by hand here, as the generated file would declare them,
|
||||
// because this fixture has no dependencies and no codegen.
|
||||
|
||||
class DocEvent {}
|
||||
|
||||
sealed class DocState {
|
||||
const DocState();
|
||||
|
||||
const factory DocState.loading() = DocLoading;
|
||||
const factory DocState.ready() = DocReady;
|
||||
|
||||
/// Declared, rendered, and never emitted: the positive control that proves
|
||||
/// redirect resolution did not simply suppress the rule.
|
||||
const factory DocState.stale() = DocStale;
|
||||
}
|
||||
|
||||
class DocLoading extends DocState {
|
||||
const DocLoading();
|
||||
}
|
||||
|
||||
class DocReady extends DocState {
|
||||
const DocReady();
|
||||
}
|
||||
|
||||
class DocStale extends DocState {
|
||||
const DocStale();
|
||||
}
|
||||
|
||||
class DocBloc extends Bloc<DocEvent, DocState> {
|
||||
DocBloc() : super(const DocState.loading());
|
||||
|
||||
void load() => emit(const DocState.ready());
|
||||
}
|
||||
17
test/fixtures/stuck_loading/lib/freezed_ui.dart
vendored
Normal file
17
test/fixtures/stuck_loading/lib/freezed_ui.dart
vendored
Normal file
|
|
@ -0,0 +1,17 @@
|
|||
// UI for freezed_style.dart, kept in its own file because type tests in a file
|
||||
// that declares a bloc do not count as UI.
|
||||
|
||||
import 'bloc.dart';
|
||||
import 'freezed_style.dart';
|
||||
|
||||
class DocView {
|
||||
const DocView();
|
||||
|
||||
Object build() => BlocBuilder<DocBloc, DocState>(
|
||||
builder: (context, state) => switch (state) {
|
||||
DocLoading() => 'loading',
|
||||
DocReady() => 'ready',
|
||||
DocStale() => 'stale',
|
||||
},
|
||||
);
|
||||
}
|
||||
|
|
@ -32,7 +32,13 @@ const Map<String, List<_Id>> _negatives = {
|
|||
// PatternState HAS render sites and every variant it renders is emitted.
|
||||
// ItemState would be vacuous: it has no render sites, so it could never be
|
||||
// reported regardless of the emission logic.
|
||||
ruleRenderedNotEmitted: [('union', 'PatternState')],
|
||||
// DocLoading/DocReady are emitted through freezed-style redirecting factories
|
||||
// (`DocState.loading()`), which must be credited to the redirect target.
|
||||
ruleRenderedNotEmitted: [
|
||||
('union', 'PatternState'),
|
||||
('variant', 'DocLoading'),
|
||||
('variant', 'DocReady'),
|
||||
],
|
||||
// GuardedCubit emits a loading variant and awaits inside a try whose catch
|
||||
// resolves it, so reporting it would be a false positive. FlagCubit would be
|
||||
// vacuous: it has no variant union and is skipped outright.
|
||||
|
|
@ -127,6 +133,12 @@ void main() {
|
|||
expect(owners, {'ShareBetaBloc'});
|
||||
});
|
||||
|
||||
test('rendered-not-emitted: redirect resolution does not mute the rule', () {
|
||||
// DocStale is rendered and never emitted, so crediting factory calls to
|
||||
// their redirect targets must leave it reported.
|
||||
expect(valuesOf(ruleRenderedNotEmitted, 'variant'), contains('DocStale'));
|
||||
});
|
||||
|
||||
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.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue