merge/audit-and-state-map #7
4 changed files with 691 additions and 485 deletions
refactor(rules): judge loading and construction facts, not syntax trees
First step of the state-map seam: rules should read only a language-neutral model, so a second front end (C++) can feed them. The two loading rules built findings inside tree visitors, and dead-handler and the unresolved-reference check walked every unit from runChecks. Their recognisers now live in the Dart front end (dart_facts.dart) and emit neutral facts (facts.dart) carried on StateFlow: - SwallowedFailure: a busy variant is emitted, then an awaited failure is swallowed (loading-not-reset). The "does this owner hold rendered state" gate moved to the rule, since it is judgement, not recognition. - UnsettledFlag: a loading flag left set across unguarded awaited work (loading-flag-not-reset); the rule derives confidence from setsFlag and guardedTrySeen. - constructedByFile and UnresolvedSummary, with project-relative paths. Every fact serialises to JSON and appears in `pipe state`. rules.dart no longer imports any analyzer AST library; only the provider rules still take resolved units, which is the next step. No behaviour change, checked byte for byte: `check --format json` is identical on the fixture and on resolved mirrors of nuwayze, tick, dex and word_alive. 128 tests pass.
commit
56dcfc85a6
409
lib/src/dart_facts.dart
Normal file
409
lib/src/dart_facts.dart
Normal file
|
|
@ -0,0 +1,409 @@
|
||||||
|
/// The Dart front end's pattern recognisers for the loading rules.
|
||||||
|
///
|
||||||
|
/// These walk syntax trees and emit the language-neutral facts in facts.dart;
|
||||||
|
/// the rules in rules.dart judge the facts and never see a tree.
|
||||||
|
library;
|
||||||
|
|
||||||
|
import 'package:analyzer/dart/ast/ast.dart';
|
||||||
|
import 'package:analyzer/dart/ast/visitor.dart';
|
||||||
|
import 'package:analyzer/source/line_info.dart';
|
||||||
|
|
||||||
|
import 'facts.dart';
|
||||||
|
import 'state_flow.dart';
|
||||||
|
|
||||||
|
/// Type names constructed in a unit, so a handler is not called dead when its
|
||||||
|
/// event is built somewhere and dispatched through a variable.
|
||||||
|
class ConstructionCollector extends RecursiveAstVisitor<void> {
|
||||||
|
final Set<String> constructed = {};
|
||||||
|
|
||||||
|
@override
|
||||||
|
void visitInstanceCreationExpression(InstanceCreationExpression node) {
|
||||||
|
constructed.add(node.constructorName.type.name.lexeme);
|
||||||
|
super.visitInstanceCreationExpression(node);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
class _TryCollector extends RecursiveAstVisitor<void> {
|
||||||
|
final List<TryStatement> tries = [];
|
||||||
|
|
||||||
|
@override
|
||||||
|
void visitTryStatement(TryStatement node) {
|
||||||
|
tries.add(node);
|
||||||
|
super.visitTryStatement(node);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Nested functions are analysed on their own.
|
||||||
|
@override
|
||||||
|
void visitFunctionExpression(FunctionExpression node) {}
|
||||||
|
|
||||||
|
@override
|
||||||
|
void visitFunctionDeclaration(FunctionDeclaration node) {}
|
||||||
|
}
|
||||||
|
|
||||||
|
class _CalledNames extends RecursiveAstVisitor<void> {
|
||||||
|
final Set<String> names = {};
|
||||||
|
|
||||||
|
@override
|
||||||
|
void visitMethodInvocation(MethodInvocation node) {
|
||||||
|
final target = node.target;
|
||||||
|
if (target == null || target is ThisExpression) {
|
||||||
|
names.add(node.methodName.name);
|
||||||
|
}
|
||||||
|
super.visitMethodInvocation(node);
|
||||||
|
}
|
||||||
|
|
||||||
|
@override
|
||||||
|
void visitFunctionExpressionInvocation(FunctionExpressionInvocation node) {
|
||||||
|
final function = node.function;
|
||||||
|
if (function is SimpleIdentifier) names.add(function.name);
|
||||||
|
super.visitFunctionExpressionInvocation(node);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// `on<E>` for a bloc handler closure, so two handlers in one class get
|
||||||
|
/// distinct identities. As a bare `<closure>` they shared a fingerprint, and
|
||||||
|
/// the baseline dropped all but one of them.
|
||||||
|
String _closureMemberName(FunctionExpression node) {
|
||||||
|
final list = node.parent;
|
||||||
|
final call = list?.parent;
|
||||||
|
if (list is ArgumentList && call is MethodInvocation) {
|
||||||
|
final types = call.typeArguments?.arguments;
|
||||||
|
if (call.methodName.name == 'on' && types != null && types.length == 1) {
|
||||||
|
return 'on<${types.single.toSource()}>';
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return '<closure>';
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Finds awaited work on a loading-flag path that no catch clause resolves.
|
||||||
|
class UnsettledFlagCollector extends RecursiveAstVisitor<void> {
|
||||||
|
UnsettledFlagCollector(
|
||||||
|
this.relativePath,
|
||||||
|
this.lineInfo,
|
||||||
|
this.flagStatesByOwner,
|
||||||
|
this.facts,
|
||||||
|
);
|
||||||
|
|
||||||
|
final String relativePath;
|
||||||
|
final LineInfo lineInfo;
|
||||||
|
final Map<String, FlagState> flagStatesByOwner;
|
||||||
|
final List<UnsettledFlag> facts;
|
||||||
|
final List<String> _classStack = [];
|
||||||
|
|
||||||
|
/// Methods invoked from a constructor body: for those, a default-true loading
|
||||||
|
/// flag is in effect even though the method never sets it itself.
|
||||||
|
final Map<String, Set<String>> _constructorCalls = {};
|
||||||
|
|
||||||
|
static final RegExp _stateAssign = RegExp(r'(^|[^.\w])state\s*=(?!=)');
|
||||||
|
static final RegExp _throwing = RegExp(r'\b(rethrow|throw)\b');
|
||||||
|
static final RegExp _errorish = RegExp(
|
||||||
|
r'(Error|Failure|Fail|Cancel|Retry|Recover|Reset|Clear)',
|
||||||
|
caseSensitive: false,
|
||||||
|
);
|
||||||
|
|
||||||
|
static bool _surfacesFailure(String source) {
|
||||||
|
if (source.contains('emit(') || source.contains('notifyListeners(')) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
if (_stateAssign.hasMatch(source)) return true;
|
||||||
|
if (_throwing.hasMatch(source)) return true;
|
||||||
|
return source.contains('add(') && _errorish.hasMatch(source);
|
||||||
|
}
|
||||||
|
|
||||||
|
@override
|
||||||
|
void visitClassDeclaration(ClassDeclaration node) {
|
||||||
|
final name = node.namePart.typeName.lexeme;
|
||||||
|
final calls = <String>{};
|
||||||
|
for (final member in node.body.members) {
|
||||||
|
if (member is! ConstructorDeclaration) continue;
|
||||||
|
final collector = _CalledNames();
|
||||||
|
member.body.accept(collector);
|
||||||
|
calls.addAll(collector.names);
|
||||||
|
}
|
||||||
|
if (calls.isNotEmpty) _constructorCalls[name] = calls;
|
||||||
|
_classStack.add(name);
|
||||||
|
super.visitClassDeclaration(node);
|
||||||
|
_classStack.removeLast();
|
||||||
|
}
|
||||||
|
|
||||||
|
@override
|
||||||
|
void visitMixinDeclaration(MixinDeclaration node) {
|
||||||
|
_classStack.add(node.name.lexeme);
|
||||||
|
super.visitMixinDeclaration(node);
|
||||||
|
_classStack.removeLast();
|
||||||
|
}
|
||||||
|
|
||||||
|
@override
|
||||||
|
void visitMethodDeclaration(MethodDeclaration node) {
|
||||||
|
_analyze(node.body, node.name.lexeme);
|
||||||
|
super.visitMethodDeclaration(node);
|
||||||
|
}
|
||||||
|
|
||||||
|
@override
|
||||||
|
void visitFunctionExpression(FunctionExpression node) {
|
||||||
|
_analyze(node.body, _closureMemberName(node));
|
||||||
|
super.visitFunctionExpression(node);
|
||||||
|
}
|
||||||
|
|
||||||
|
void _analyze(FunctionBody? body, String member) {
|
||||||
|
if (body == null) return;
|
||||||
|
final owner = _classStack.isEmpty ? '<top>' : _classStack.last;
|
||||||
|
final flag = flagStatesByOwner[owner];
|
||||||
|
if (flag == null) return;
|
||||||
|
// Lifecycle teardown never renders a loading state.
|
||||||
|
if (member == 'close' || member == 'dispose') return;
|
||||||
|
|
||||||
|
// Only methods that actually put the loading flag into the true state can
|
||||||
|
// leave the UI spinning. A method that merely runs in a class whose state
|
||||||
|
// has such a flag (a repository write, a teardown call) cannot.
|
||||||
|
//
|
||||||
|
// Every probe here stops at nested functions, which are analysed on their
|
||||||
|
// own. The flag probe and the await probe used to read closure bodies while
|
||||||
|
// the try collector skipped them, so a method whose closure set the flag
|
||||||
|
// and guarded its own await was reported at high confidence.
|
||||||
|
final flagProbe = _SetsFlagTrue(flag.fields.toSet());
|
||||||
|
body.accept(flagProbe);
|
||||||
|
final setsFlagTrue = flagProbe.found;
|
||||||
|
final inheritsInitialFlag =
|
||||||
|
flag.defaultsTrue &&
|
||||||
|
(_constructorCalls[owner]?.contains(member) ?? false);
|
||||||
|
if (!setsFlagTrue && !inheritsInitialFlag) return;
|
||||||
|
|
||||||
|
// Guarded when an awaited try has a catch that surfaces the failure.
|
||||||
|
final tries = _TryCollector();
|
||||||
|
body.accept(tries);
|
||||||
|
var awaitedTryGuarded = false;
|
||||||
|
var awaitedTrySeen = false;
|
||||||
|
for (final statement in tries.tries) {
|
||||||
|
final awaitedInTry = _HasAwait(shallow: true);
|
||||||
|
statement.body.accept(awaitedInTry);
|
||||||
|
if (!awaitedInTry.found) continue;
|
||||||
|
awaitedTrySeen = true;
|
||||||
|
for (final catchClause in statement.catchClauses) {
|
||||||
|
if (_surfacesFailure(catchClause.body.toSource())) {
|
||||||
|
awaitedTryGuarded = true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if (awaitedTryGuarded) return;
|
||||||
|
|
||||||
|
// Nothing awaited at all means there is no failure path to leave dangling.
|
||||||
|
final awaitProbe = _HasAwait(shallow: true);
|
||||||
|
body.accept(awaitProbe);
|
||||||
|
if (!awaitProbe.found) return;
|
||||||
|
|
||||||
|
facts.add(
|
||||||
|
UnsettledFlag(
|
||||||
|
owner: owner,
|
||||||
|
member: member,
|
||||||
|
path: relativePath,
|
||||||
|
line: lineInfo.getLocation(body.offset).lineNumber,
|
||||||
|
stateType: flag.name,
|
||||||
|
fields: flag.fields,
|
||||||
|
defaultsTrue: flag.defaultsTrue,
|
||||||
|
setsFlag: setsFlagTrue,
|
||||||
|
guardedTrySeen: awaitedTrySeen,
|
||||||
|
),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Scans a single function body for in-flight loading emits and top-level
|
||||||
|
/// awaited try statements.
|
||||||
|
class _BodyScan extends RecursiveAstVisitor<void> {
|
||||||
|
final List<int> loadingEmits = [];
|
||||||
|
final List<TryStatement> topLevelTries = [];
|
||||||
|
|
||||||
|
static final RegExp _loading = RegExp(
|
||||||
|
r'(Loading|Processing|Submitting|Saving|Creating|Updating|InProgress)',
|
||||||
|
);
|
||||||
|
|
||||||
|
int _tryDepth = 0;
|
||||||
|
|
||||||
|
void _maybeLoading(AstNode node, List<Argument> arguments) {
|
||||||
|
if (arguments.isEmpty) return;
|
||||||
|
final first = arguments.first;
|
||||||
|
if (first is! Expression) return;
|
||||||
|
if (_loading.hasMatch(first.toSource())) loadingEmits.add(node.offset);
|
||||||
|
}
|
||||||
|
|
||||||
|
@override
|
||||||
|
void visitMethodInvocation(MethodInvocation node) {
|
||||||
|
if (node.methodName.name == 'emit') {
|
||||||
|
_maybeLoading(node, node.argumentList.arguments);
|
||||||
|
}
|
||||||
|
super.visitMethodInvocation(node);
|
||||||
|
}
|
||||||
|
|
||||||
|
@override
|
||||||
|
void visitFunctionExpressionInvocation(FunctionExpressionInvocation node) {
|
||||||
|
final function = node.function;
|
||||||
|
if (function is SimpleIdentifier && function.name == 'emit') {
|
||||||
|
_maybeLoading(node, node.argumentList.arguments);
|
||||||
|
}
|
||||||
|
super.visitFunctionExpressionInvocation(node);
|
||||||
|
}
|
||||||
|
|
||||||
|
@override
|
||||||
|
void visitTryStatement(TryStatement node) {
|
||||||
|
final isTopLevel = _tryDepth == 0;
|
||||||
|
_tryDepth++;
|
||||||
|
if (isTopLevel) topLevelTries.add(node);
|
||||||
|
super.visitTryStatement(node);
|
||||||
|
_tryDepth--;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Nested functions are analysed in their own right by _analyze; descending
|
||||||
|
// into them here would report the same catch twice.
|
||||||
|
@override
|
||||||
|
void visitFunctionExpression(FunctionExpression node) {}
|
||||||
|
|
||||||
|
@override
|
||||||
|
void visitFunctionDeclaration(FunctionDeclaration node) {}
|
||||||
|
}
|
||||||
|
|
||||||
|
class _HasAwait extends RecursiveAstVisitor<void> {
|
||||||
|
_HasAwait({this.shallow = false});
|
||||||
|
|
||||||
|
/// Stop at nested functions: an await inside a closure is not awaited by the
|
||||||
|
/// body that declares the closure.
|
||||||
|
final bool shallow;
|
||||||
|
|
||||||
|
bool found = false;
|
||||||
|
|
||||||
|
@override
|
||||||
|
void visitAwaitExpression(AwaitExpression node) {
|
||||||
|
found = true;
|
||||||
|
super.visitAwaitExpression(node);
|
||||||
|
}
|
||||||
|
|
||||||
|
@override
|
||||||
|
void visitFunctionExpression(FunctionExpression node) {
|
||||||
|
if (!shallow) super.visitFunctionExpression(node);
|
||||||
|
}
|
||||||
|
|
||||||
|
@override
|
||||||
|
void visitFunctionDeclaration(FunctionDeclaration node) {
|
||||||
|
if (!shallow) super.visitFunctionDeclaration(node);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A `field: true` named argument for one of the loading fields, outside any
|
||||||
|
/// nested function.
|
||||||
|
class _SetsFlagTrue extends RecursiveAstVisitor<void> {
|
||||||
|
_SetsFlagTrue(this.fields);
|
||||||
|
|
||||||
|
final Set<String> fields;
|
||||||
|
bool found = false;
|
||||||
|
|
||||||
|
@override
|
||||||
|
void visitNamedArgument(NamedArgument node) {
|
||||||
|
final value = node.argumentExpression;
|
||||||
|
if (fields.contains(node.name.lexeme) &&
|
||||||
|
value is BooleanLiteral &&
|
||||||
|
value.value) {
|
||||||
|
found = true;
|
||||||
|
}
|
||||||
|
super.visitNamedArgument(node);
|
||||||
|
}
|
||||||
|
|
||||||
|
@override
|
||||||
|
void visitFunctionExpression(FunctionExpression node) {}
|
||||||
|
|
||||||
|
@override
|
||||||
|
void visitFunctionDeclaration(FunctionDeclaration node) {}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Finds members that emit an in-progress variant and then await work whose
|
||||||
|
/// catch writes no state. Whether the owner holds rendered state is the rule's
|
||||||
|
/// judgement, not the front end's, so every owner is scanned.
|
||||||
|
class SwallowedFailureCollector extends RecursiveAstVisitor<void> {
|
||||||
|
SwallowedFailureCollector(this.relativePath, this.lineInfo, this.facts);
|
||||||
|
|
||||||
|
final String relativePath;
|
||||||
|
final LineInfo lineInfo;
|
||||||
|
final List<SwallowedFailure> facts;
|
||||||
|
final List<String> _classStack = [];
|
||||||
|
|
||||||
|
static final RegExp _stateAssign = RegExp(r'(^|[^.\w])state\s*=(?!=)');
|
||||||
|
static final RegExp _throwing = RegExp(r'\b(rethrow|throw)\b');
|
||||||
|
static final RegExp _errorish = RegExp(
|
||||||
|
r'(Error|Failure|Fail|Cancel|Retry|Recover|Reset|Clear)',
|
||||||
|
caseSensitive: false,
|
||||||
|
);
|
||||||
|
|
||||||
|
@override
|
||||||
|
void visitClassDeclaration(ClassDeclaration node) {
|
||||||
|
_classStack.add(node.namePart.typeName.lexeme);
|
||||||
|
super.visitClassDeclaration(node);
|
||||||
|
_classStack.removeLast();
|
||||||
|
}
|
||||||
|
|
||||||
|
@override
|
||||||
|
void visitMixinDeclaration(MixinDeclaration node) {
|
||||||
|
_classStack.add(node.name.lexeme);
|
||||||
|
super.visitMixinDeclaration(node);
|
||||||
|
_classStack.removeLast();
|
||||||
|
}
|
||||||
|
|
||||||
|
@override
|
||||||
|
void visitMethodDeclaration(MethodDeclaration node) {
|
||||||
|
_analyze(node.body, node.name.lexeme);
|
||||||
|
super.visitMethodDeclaration(node);
|
||||||
|
}
|
||||||
|
|
||||||
|
@override
|
||||||
|
void visitFunctionExpression(FunctionExpression node) {
|
||||||
|
_analyze(node.body, _closureMemberName(node));
|
||||||
|
super.visitFunctionExpression(node);
|
||||||
|
}
|
||||||
|
|
||||||
|
void _analyze(FunctionBody? body, String member) {
|
||||||
|
if (body == null) return;
|
||||||
|
final owner = _classStack.isEmpty ? '<top>' : _classStack.last;
|
||||||
|
|
||||||
|
final scan = _BodyScan();
|
||||||
|
body.accept(scan);
|
||||||
|
if (scan.loadingEmits.isEmpty) return;
|
||||||
|
|
||||||
|
// Position within the member, not the line: two swallowed catches in one
|
||||||
|
// method need distinct fingerprints, and a line number changed whenever
|
||||||
|
// anything above the method moved.
|
||||||
|
var swallowed = 0;
|
||||||
|
for (final statement in scan.topLevelTries) {
|
||||||
|
if (statement.catchClauses.isEmpty) continue;
|
||||||
|
final probe = _HasAwait();
|
||||||
|
statement.body.accept(probe);
|
||||||
|
if (!probe.found) continue;
|
||||||
|
|
||||||
|
for (final catchClause in statement.catchClauses) {
|
||||||
|
final source = catchClause.body.toSource();
|
||||||
|
final emits =
|
||||||
|
source.contains('emit(') || source.contains('notifyListeners(');
|
||||||
|
final assignsState = _stateAssign.hasMatch(source);
|
||||||
|
final rethrows = _throwing.hasMatch(source);
|
||||||
|
final dispatchesFailure =
|
||||||
|
source.contains('add(') && _errorish.hasMatch(source);
|
||||||
|
if (emits || assignsState || rethrows || dispatchesFailure) continue;
|
||||||
|
swallowed++;
|
||||||
|
|
||||||
|
facts.add(
|
||||||
|
SwallowedFailure(
|
||||||
|
owner: owner,
|
||||||
|
member: member,
|
||||||
|
path: relativePath,
|
||||||
|
line: lineInfo.getLocation(catchClause.offset).lineNumber,
|
||||||
|
ordinal: swallowed,
|
||||||
|
handler: _oneLine(source),
|
||||||
|
),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
static String _oneLine(String source) {
|
||||||
|
final collapsed = source.replaceAll(RegExp(r'\s+'), ' ').trim();
|
||||||
|
final body = collapsed.isEmpty ? '<empty catch>' : collapsed;
|
||||||
|
return body.length <= 120 ? body : '${body.substring(0, 117)}...';
|
||||||
|
}
|
||||||
|
}
|
||||||
153
lib/src/facts.dart
Normal file
153
lib/src/facts.dart
Normal file
|
|
@ -0,0 +1,153 @@
|
||||||
|
/// Language-neutral facts a front end extracts for the rules.
|
||||||
|
///
|
||||||
|
/// The rules judge these; they never read a syntax tree. A front end for
|
||||||
|
/// another language (a clang-based C++ extractor, say) produces the same facts
|
||||||
|
/// and every rule, baseline, verify and suggest path works unchanged. Each fact
|
||||||
|
/// round-trips through JSON so a front end can run as a separate process.
|
||||||
|
library;
|
||||||
|
|
||||||
|
/// A member puts its owner into an in-progress state, then awaits work whose
|
||||||
|
/// failure handler writes no state, rethrows, or dispatches a failure: the
|
||||||
|
/// busy state can outlive the failure (a spinner that never stops).
|
||||||
|
///
|
||||||
|
/// Variant style: the busy state is an emitted variant (`emit(Loading())`).
|
||||||
|
class SwallowedFailure {
|
||||||
|
SwallowedFailure({
|
||||||
|
required this.owner,
|
||||||
|
required this.member,
|
||||||
|
required this.path,
|
||||||
|
required this.line,
|
||||||
|
required this.ordinal,
|
||||||
|
required this.handler,
|
||||||
|
});
|
||||||
|
|
||||||
|
factory SwallowedFailure.fromJson(Map<String, Object?> json) =>
|
||||||
|
SwallowedFailure(
|
||||||
|
owner: json['owner']! as String,
|
||||||
|
member: json['member']! as String,
|
||||||
|
path: json['path']! as String,
|
||||||
|
line: json['line']! as int,
|
||||||
|
ordinal: json['ordinal']! as int,
|
||||||
|
handler: json['handler']! as String,
|
||||||
|
);
|
||||||
|
|
||||||
|
final String owner;
|
||||||
|
final String member;
|
||||||
|
|
||||||
|
/// Project-relative path and line of the failure handler (the catch).
|
||||||
|
final String path;
|
||||||
|
final int line;
|
||||||
|
|
||||||
|
/// 1-based position among the member's swallowed handlers: identity that
|
||||||
|
/// survives code moving, unlike the line.
|
||||||
|
final int ordinal;
|
||||||
|
|
||||||
|
/// The handler's source on one line, as evidence.
|
||||||
|
final String handler;
|
||||||
|
|
||||||
|
Map<String, Object?> toJson() => {
|
||||||
|
'owner': owner,
|
||||||
|
'member': member,
|
||||||
|
'path': path,
|
||||||
|
'line': line,
|
||||||
|
'ordinal': ordinal,
|
||||||
|
'handler': handler,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A member leaves a loading FLAG set across awaited work that no failure
|
||||||
|
/// handler resolves.
|
||||||
|
///
|
||||||
|
/// Flag style: the state is one class with a `bool loading`-like field, so
|
||||||
|
/// there is no variant to match on.
|
||||||
|
class UnsettledFlag {
|
||||||
|
UnsettledFlag({
|
||||||
|
required this.owner,
|
||||||
|
required this.member,
|
||||||
|
required this.path,
|
||||||
|
required this.line,
|
||||||
|
required this.stateType,
|
||||||
|
required this.fields,
|
||||||
|
required this.defaultsTrue,
|
||||||
|
required this.setsFlag,
|
||||||
|
required this.guardedTrySeen,
|
||||||
|
});
|
||||||
|
|
||||||
|
factory UnsettledFlag.fromJson(Map<String, Object?> json) => UnsettledFlag(
|
||||||
|
owner: json['owner']! as String,
|
||||||
|
member: json['member']! as String,
|
||||||
|
path: json['path']! as String,
|
||||||
|
line: json['line']! as int,
|
||||||
|
stateType: json['stateType']! as String,
|
||||||
|
fields: [...(json['fields']! as List).cast<String>()],
|
||||||
|
defaultsTrue: json['defaultsTrue']! as bool,
|
||||||
|
setsFlag: json['setsFlag']! as bool,
|
||||||
|
guardedTrySeen: json['guardedTrySeen']! as bool,
|
||||||
|
);
|
||||||
|
|
||||||
|
final String owner;
|
||||||
|
final String member;
|
||||||
|
final String path;
|
||||||
|
final int line;
|
||||||
|
final String stateType;
|
||||||
|
final List<String> fields;
|
||||||
|
final bool defaultsTrue;
|
||||||
|
|
||||||
|
/// The member itself sets the flag (as opposed to inheriting a default-true
|
||||||
|
/// flag by running from the constructor).
|
||||||
|
final bool setsFlag;
|
||||||
|
|
||||||
|
/// Some awaited work sat inside a try, though none of its catches resolved
|
||||||
|
/// the flag. Weaker evidence than no try at all.
|
||||||
|
final bool guardedTrySeen;
|
||||||
|
|
||||||
|
Map<String, Object?> toJson() => {
|
||||||
|
'owner': owner,
|
||||||
|
'member': member,
|
||||||
|
'path': path,
|
||||||
|
'line': line,
|
||||||
|
'stateType': stateType,
|
||||||
|
'fields': fields,
|
||||||
|
'defaultsTrue': defaultsTrue,
|
||||||
|
'setsFlag': setsFlag,
|
||||||
|
'guardedTrySeen': guardedTrySeen,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Files whose references did not resolve. Evidence in them can be missing,
|
||||||
|
/// so findings naming anything they mention are weaker.
|
||||||
|
class UnresolvedSummary {
|
||||||
|
UnresolvedSummary({
|
||||||
|
required this.files,
|
||||||
|
required this.analysed,
|
||||||
|
this.first,
|
||||||
|
this.names = const {},
|
||||||
|
});
|
||||||
|
|
||||||
|
factory UnresolvedSummary.fromJson(Map<String, Object?> json) =>
|
||||||
|
UnresolvedSummary(
|
||||||
|
files: [...(json['files']! as List).cast<String>()],
|
||||||
|
analysed: json['analysed']! as int,
|
||||||
|
first: json['first'] as String?,
|
||||||
|
names: {...(json['names']! as List).cast<String>()},
|
||||||
|
);
|
||||||
|
|
||||||
|
/// Project-relative paths.
|
||||||
|
final List<String> files;
|
||||||
|
final int analysed;
|
||||||
|
|
||||||
|
/// The first unresolved reference as `path:line: message`.
|
||||||
|
final String? first;
|
||||||
|
|
||||||
|
/// Every name those files mention.
|
||||||
|
final Set<String> names;
|
||||||
|
|
||||||
|
bool get any => files.isNotEmpty;
|
||||||
|
|
||||||
|
Map<String, Object?> toJson() => {
|
||||||
|
'files': files,
|
||||||
|
'analysed': analysed,
|
||||||
|
if (first != null) 'first': first,
|
||||||
|
'names': names.toList()..sort(),
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
@ -1,9 +1,5 @@
|
||||||
import 'dart:convert';
|
import 'dart:convert';
|
||||||
|
|
||||||
import 'package:analyzer/dart/ast/ast.dart';
|
|
||||||
import 'package:analyzer/dart/ast/visitor.dart';
|
|
||||||
import 'package:analyzer/source/line_info.dart';
|
|
||||||
|
|
||||||
import 'analysis.dart';
|
import 'analysis.dart';
|
||||||
import 'errors.dart';
|
import 'errors.dart';
|
||||||
import 'project.dart';
|
import 'project.dart';
|
||||||
|
|
@ -360,16 +356,7 @@ FindingReport runChecks({
|
||||||
|
|
||||||
// Constructed type names per file, used to avoid reporting a handler as dead
|
// Constructed type names per file, used to avoid reporting a handler as dead
|
||||||
// when its event is built somewhere and dispatched through a variable.
|
// when its event is built somewhere and dispatched through a variable.
|
||||||
final constructedByFile = <String, Set<String>>{};
|
final constructedByFile = flow.constructedByFile;
|
||||||
if (enabled.contains(ruleDeadHandler)) {
|
|
||||||
for (final outcome in outcomes) {
|
|
||||||
final unit = outcome.unit;
|
|
||||||
if (unit == null) continue;
|
|
||||||
final collector = _ConstructionCollector();
|
|
||||||
unit.unit.accept(collector);
|
|
||||||
constructedByFile[project.relative(outcome.path)] = collector.constructed;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
String site(EmissionSite emission) =>
|
String site(EmissionSite emission) =>
|
||||||
'${emission.path}:${emission.line}: ${emission.snippet}';
|
'${emission.path}:${emission.line}: ${emission.snippet}';
|
||||||
|
|
@ -734,16 +721,30 @@ FindingReport runChecks({
|
||||||
for (final emission in flow.emissions)
|
for (final emission in flow.emissions)
|
||||||
if (emission.kind == 'notifyListeners') emission.owner,
|
if (emission.kind == 'notifyListeners') emission.owner,
|
||||||
};
|
};
|
||||||
for (final outcome in outcomes) {
|
for (final fact in flow.swallowedFailures) {
|
||||||
final unit = outcome.unit;
|
if (!stateOwners.contains(fact.owner)) continue;
|
||||||
if (unit == null) continue;
|
findings.add(
|
||||||
unit.unit.accept(
|
Finding(
|
||||||
_LoadingResetCollector(
|
rule: ruleLoadingNotReset,
|
||||||
project,
|
severity: PipeSeverity.warning,
|
||||||
outcome.path,
|
confidence: 'high',
|
||||||
unit.lineInfo,
|
message:
|
||||||
stateOwners,
|
'${fact.owner}.${fact.member} emits an in-progress state and then '
|
||||||
findings,
|
'awaits an operation whose catch clause never emits a terminal '
|
||||||
|
'state, so the UI can be left showing a spinner forever.',
|
||||||
|
file: fact.path,
|
||||||
|
line: fact.line,
|
||||||
|
evidence: ['${fact.path}:${fact.line}: ${fact.handler}'],
|
||||||
|
remediation:
|
||||||
|
'Emit a failure/timeout state in the catch (or rethrow) so the '
|
||||||
|
'loading state is always resolved.',
|
||||||
|
data: {
|
||||||
|
'owner': fact.owner,
|
||||||
|
'member': fact.member,
|
||||||
|
// A method can hold more than one swallowed catch, so which one
|
||||||
|
// is part of this rule's identity.
|
||||||
|
'catch': fact.ordinal,
|
||||||
|
},
|
||||||
),
|
),
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
@ -754,23 +755,38 @@ FindingReport runChecks({
|
||||||
// on. An awaited call on the loading path that no catch resolves can leave the
|
// on. An awaited call on the loading path that no catch resolves can leave the
|
||||||
// UI spinning forever, and this week's variant-based rule cannot see it.
|
// UI spinning forever, and this week's variant-based rule cannot see it.
|
||||||
if (enabled.contains(ruleLoadingFlagNotReset)) {
|
if (enabled.contains(ruleLoadingFlagNotReset)) {
|
||||||
final flagStatesByOwner = <String, FlagState>{
|
for (final fact in flow.unsettledFlags) {
|
||||||
for (final state in flow.flagStates) state.owner: state,
|
final fieldList = fact.fields.join(' / ');
|
||||||
};
|
findings.add(
|
||||||
if (flagStatesByOwner.isNotEmpty) {
|
Finding(
|
||||||
for (final outcome in outcomes) {
|
rule: ruleLoadingFlagNotReset,
|
||||||
final unit = outcome.unit;
|
severity: PipeSeverity.warning,
|
||||||
if (unit == null) continue;
|
confidence: fact.setsFlag && !fact.guardedTrySeen ? 'high' : 'medium',
|
||||||
unit.unit.accept(
|
message:
|
||||||
_FlagLoadingCollector(
|
'${fact.owner}.${fact.member} awaits work after putting '
|
||||||
project,
|
'${fact.stateType}`$fieldList` into its loading state, and no '
|
||||||
outcome.path,
|
'catch clause surfaces a failure, so the UI can keep showing its '
|
||||||
unit.lineInfo,
|
'loading state after an error.',
|
||||||
flagStatesByOwner,
|
file: fact.path,
|
||||||
findings,
|
line: fact.line,
|
||||||
),
|
evidence: [
|
||||||
);
|
'${fact.path}:${fact.line}: ${fact.owner}.${fact.member} awaits '
|
||||||
}
|
'with ${fact.stateType}.$fieldList in flight and no catch that '
|
||||||
|
'emits a failure state',
|
||||||
|
],
|
||||||
|
remediation:
|
||||||
|
'Wrap the awaited call in try/catch and emit a state with '
|
||||||
|
'`$fieldList` reset (or rethrow), so the loading state always '
|
||||||
|
'resolves.',
|
||||||
|
data: {
|
||||||
|
'owner': fact.owner,
|
||||||
|
'member': fact.member,
|
||||||
|
'stateType': fact.stateType,
|
||||||
|
'fields': fact.fields,
|
||||||
|
'defaultsTrue': fact.defaultsTrue,
|
||||||
|
},
|
||||||
|
),
|
||||||
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -846,7 +862,7 @@ FindingReport runChecks({
|
||||||
// evidence is name-based, so only a finding whose names an unresolved file
|
// evidence is name-based, so only a finding whose names an unresolved file
|
||||||
// mentions can have lost evidence there; lowering every finding for one
|
// mentions can have lost evidence there; lowering every finding for one
|
||||||
// stray error elsewhere would bury the signal.
|
// stray error elsewhere would bury the signal.
|
||||||
final unresolved = findUnresolvedReferences(outcomes);
|
final unresolved = flow.unresolved;
|
||||||
if (unresolved.any) {
|
if (unresolved.any) {
|
||||||
var lowered = 0;
|
var lowered = 0;
|
||||||
for (var i = 0; i < findings.length; i++) {
|
for (var i = 0; i < findings.length; i++) {
|
||||||
|
|
@ -854,11 +870,10 @@ FindingReport runChecks({
|
||||||
findings[i] = findings[i].lowered('unresolved-references');
|
findings[i] = findings[i].lowered('unresolved-references');
|
||||||
lowered++;
|
lowered++;
|
||||||
}
|
}
|
||||||
final first = unresolved.first!;
|
|
||||||
notes.add(
|
notes.add(
|
||||||
'${unresolved.files.length} of ${unresolved.analysed} analysed file(s) '
|
'${unresolved.files.length} of ${unresolved.analysed} analysed file(s) '
|
||||||
'have unresolved references (first: '
|
'have unresolved references (first: ${unresolved.first}). An '
|
||||||
'${first.replaceFirst('${project.root}/', '')}). An unresolved `Foo()` '
|
'unresolved `Foo()` '
|
||||||
'is not seen as a construction, so evidence in those files can be '
|
'is not seen as a construction, so evidence in those files can be '
|
||||||
'missed: $lowered finding(s) naming something they mention had their '
|
'missed: $lowered finding(s) naming something they mention had their '
|
||||||
'confidence lowered one step. Run `pipe index` and resolve the project '
|
'confidence lowered one step. Run `pipe index` and resolve the project '
|
||||||
|
|
@ -1020,53 +1035,6 @@ Set<String> decodeBaseline(String content) {
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
class _ConstructionCollector extends RecursiveAstVisitor<void> {
|
|
||||||
final Set<String> constructed = {};
|
|
||||||
|
|
||||||
@override
|
|
||||||
void visitInstanceCreationExpression(InstanceCreationExpression node) {
|
|
||||||
constructed.add(node.constructorName.type.name.lexeme);
|
|
||||||
super.visitInstanceCreationExpression(node);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
class _TryCollector extends RecursiveAstVisitor<void> {
|
|
||||||
final List<TryStatement> tries = [];
|
|
||||||
|
|
||||||
@override
|
|
||||||
void visitTryStatement(TryStatement node) {
|
|
||||||
tries.add(node);
|
|
||||||
super.visitTryStatement(node);
|
|
||||||
}
|
|
||||||
|
|
||||||
// Nested functions are analysed on their own.
|
|
||||||
@override
|
|
||||||
void visitFunctionExpression(FunctionExpression node) {}
|
|
||||||
|
|
||||||
@override
|
|
||||||
void visitFunctionDeclaration(FunctionDeclaration node) {}
|
|
||||||
}
|
|
||||||
|
|
||||||
class _CalledNames extends RecursiveAstVisitor<void> {
|
|
||||||
final Set<String> names = {};
|
|
||||||
|
|
||||||
@override
|
|
||||||
void visitMethodInvocation(MethodInvocation node) {
|
|
||||||
final target = node.target;
|
|
||||||
if (target == null || target is ThisExpression) {
|
|
||||||
names.add(node.methodName.name);
|
|
||||||
}
|
|
||||||
super.visitMethodInvocation(node);
|
|
||||||
}
|
|
||||||
|
|
||||||
@override
|
|
||||||
void visitFunctionExpressionInvocation(FunctionExpressionInvocation node) {
|
|
||||||
final function = node.function;
|
|
||||||
if (function is SimpleIdentifier) names.add(function.name);
|
|
||||||
super.visitFunctionExpressionInvocation(node);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
/// The names a finding makes a claim about: the values of its identity keys.
|
/// The names a finding makes a claim about: the values of its identity keys.
|
||||||
Set<String> _identityNames(Finding finding) {
|
Set<String> _identityNames(Finding finding) {
|
||||||
const keys = [
|
const keys = [
|
||||||
|
|
@ -1091,395 +1059,3 @@ Set<String> _identityNames(Finding finding) {
|
||||||
}
|
}
|
||||||
return names;
|
return names;
|
||||||
}
|
}
|
||||||
|
|
||||||
/// `on<E>` for a bloc handler closure, so two handlers in one class get
|
|
||||||
/// distinct identities. As a bare `<closure>` they shared a fingerprint, and
|
|
||||||
/// the baseline dropped all but one of them.
|
|
||||||
String _closureMemberName(FunctionExpression node) {
|
|
||||||
final list = node.parent;
|
|
||||||
final call = list?.parent;
|
|
||||||
if (list is ArgumentList && call is MethodInvocation) {
|
|
||||||
final types = call.typeArguments?.arguments;
|
|
||||||
if (call.methodName.name == 'on' && types != null && types.length == 1) {
|
|
||||||
return 'on<${types.single.toSource()}>';
|
|
||||||
}
|
|
||||||
}
|
|
||||||
return '<closure>';
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Finds awaited work on a loading-flag path that no catch clause resolves.
|
|
||||||
class _FlagLoadingCollector extends RecursiveAstVisitor<void> {
|
|
||||||
_FlagLoadingCollector(
|
|
||||||
this.project,
|
|
||||||
this.path,
|
|
||||||
this.lineInfo,
|
|
||||||
this.flagStatesByOwner,
|
|
||||||
this.findings,
|
|
||||||
);
|
|
||||||
|
|
||||||
final Project project;
|
|
||||||
final String path;
|
|
||||||
final LineInfo lineInfo;
|
|
||||||
final Map<String, FlagState> flagStatesByOwner;
|
|
||||||
final List<Finding> findings;
|
|
||||||
final List<String> _classStack = [];
|
|
||||||
|
|
||||||
/// Methods invoked from a constructor body: for those, a default-true loading
|
|
||||||
/// flag is in effect even though the method never sets it itself.
|
|
||||||
final Map<String, Set<String>> _constructorCalls = {};
|
|
||||||
|
|
||||||
static final RegExp _stateAssign = RegExp(r'(^|[^.\w])state\s*=(?!=)');
|
|
||||||
static final RegExp _throwing = RegExp(r'\b(rethrow|throw)\b');
|
|
||||||
static final RegExp _errorish = RegExp(
|
|
||||||
r'(Error|Failure|Fail|Cancel|Retry|Recover|Reset|Clear)',
|
|
||||||
caseSensitive: false,
|
|
||||||
);
|
|
||||||
|
|
||||||
static bool _surfacesFailure(String source) {
|
|
||||||
if (source.contains('emit(') || source.contains('notifyListeners(')) {
|
|
||||||
return true;
|
|
||||||
}
|
|
||||||
if (_stateAssign.hasMatch(source)) return true;
|
|
||||||
if (_throwing.hasMatch(source)) return true;
|
|
||||||
return source.contains('add(') && _errorish.hasMatch(source);
|
|
||||||
}
|
|
||||||
|
|
||||||
@override
|
|
||||||
void visitClassDeclaration(ClassDeclaration node) {
|
|
||||||
final name = node.namePart.typeName.lexeme;
|
|
||||||
final calls = <String>{};
|
|
||||||
for (final member in node.body.members) {
|
|
||||||
if (member is! ConstructorDeclaration) continue;
|
|
||||||
final collector = _CalledNames();
|
|
||||||
member.body.accept(collector);
|
|
||||||
calls.addAll(collector.names);
|
|
||||||
}
|
|
||||||
if (calls.isNotEmpty) _constructorCalls[name] = calls;
|
|
||||||
_classStack.add(name);
|
|
||||||
super.visitClassDeclaration(node);
|
|
||||||
_classStack.removeLast();
|
|
||||||
}
|
|
||||||
|
|
||||||
@override
|
|
||||||
void visitMixinDeclaration(MixinDeclaration node) {
|
|
||||||
_classStack.add(node.name.lexeme);
|
|
||||||
super.visitMixinDeclaration(node);
|
|
||||||
_classStack.removeLast();
|
|
||||||
}
|
|
||||||
|
|
||||||
@override
|
|
||||||
void visitMethodDeclaration(MethodDeclaration node) {
|
|
||||||
_analyze(node.body, node.name.lexeme);
|
|
||||||
super.visitMethodDeclaration(node);
|
|
||||||
}
|
|
||||||
|
|
||||||
@override
|
|
||||||
void visitFunctionExpression(FunctionExpression node) {
|
|
||||||
_analyze(node.body, _closureMemberName(node));
|
|
||||||
super.visitFunctionExpression(node);
|
|
||||||
}
|
|
||||||
|
|
||||||
void _analyze(FunctionBody? body, String member) {
|
|
||||||
if (body == null) return;
|
|
||||||
final owner = _classStack.isEmpty ? '<top>' : _classStack.last;
|
|
||||||
final flag = flagStatesByOwner[owner];
|
|
||||||
if (flag == null) return;
|
|
||||||
// Lifecycle teardown never renders a loading state.
|
|
||||||
if (member == 'close' || member == 'dispose') return;
|
|
||||||
|
|
||||||
// Only methods that actually put the loading flag into the true state can
|
|
||||||
// leave the UI spinning. A method that merely runs in a class whose state
|
|
||||||
// has such a flag (a repository write, a teardown call) cannot.
|
|
||||||
//
|
|
||||||
// Every probe here stops at nested functions, which are analysed on their
|
|
||||||
// own. The flag probe and the await probe used to read closure bodies while
|
|
||||||
// the try collector skipped them, so a method whose closure set the flag
|
|
||||||
// and guarded its own await was reported at high confidence.
|
|
||||||
final flagProbe = _SetsFlagTrue(flag.fields.toSet());
|
|
||||||
body.accept(flagProbe);
|
|
||||||
final setsFlagTrue = flagProbe.found;
|
|
||||||
final inheritsInitialFlag =
|
|
||||||
flag.defaultsTrue &&
|
|
||||||
(_constructorCalls[owner]?.contains(member) ?? false);
|
|
||||||
if (!setsFlagTrue && !inheritsInitialFlag) return;
|
|
||||||
|
|
||||||
// Guarded when an awaited try has a catch that surfaces the failure.
|
|
||||||
final tries = _TryCollector();
|
|
||||||
body.accept(tries);
|
|
||||||
var awaitedTryGuarded = false;
|
|
||||||
var awaitedTrySeen = false;
|
|
||||||
for (final statement in tries.tries) {
|
|
||||||
final awaitedInTry = _HasAwait(shallow: true);
|
|
||||||
statement.body.accept(awaitedInTry);
|
|
||||||
if (!awaitedInTry.found) continue;
|
|
||||||
awaitedTrySeen = true;
|
|
||||||
for (final catchClause in statement.catchClauses) {
|
|
||||||
if (_surfacesFailure(catchClause.body.toSource())) {
|
|
||||||
awaitedTryGuarded = true;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
if (awaitedTryGuarded) return;
|
|
||||||
|
|
||||||
// Nothing awaited at all means there is no failure path to leave dangling.
|
|
||||||
final awaitProbe = _HasAwait(shallow: true);
|
|
||||||
body.accept(awaitProbe);
|
|
||||||
if (!awaitProbe.found) return;
|
|
||||||
|
|
||||||
final fieldList = flag.fields.join(' / ');
|
|
||||||
findings.add(
|
|
||||||
Finding(
|
|
||||||
rule: ruleLoadingFlagNotReset,
|
|
||||||
severity: PipeSeverity.warning,
|
|
||||||
confidence: setsFlagTrue && !awaitedTrySeen ? 'high' : 'medium',
|
|
||||||
message:
|
|
||||||
'$owner.$member awaits work after putting ${flag.name}`$fieldList` '
|
|
||||||
'into its loading state, and no catch clause surfaces a failure, so '
|
|
||||||
'the UI can keep showing its loading state after an error.',
|
|
||||||
file: project.relative(path),
|
|
||||||
line: lineInfo.getLocation(body.offset).lineNumber,
|
|
||||||
evidence: [
|
|
||||||
'${project.relative(path)}:'
|
|
||||||
'${lineInfo.getLocation(body.offset).lineNumber}: '
|
|
||||||
'$owner.$member awaits with ${flag.name}.$fieldList in flight and '
|
|
||||||
'no catch that emits a failure state',
|
|
||||||
],
|
|
||||||
remediation:
|
|
||||||
'Wrap the awaited call in try/catch and emit a state with '
|
|
||||||
'`$fieldList` reset (or rethrow), so the loading state always '
|
|
||||||
'resolves.',
|
|
||||||
data: {
|
|
||||||
'owner': owner,
|
|
||||||
'member': member,
|
|
||||||
'stateType': flag.name,
|
|
||||||
'fields': flag.fields,
|
|
||||||
'defaultsTrue': flag.defaultsTrue,
|
|
||||||
},
|
|
||||||
),
|
|
||||||
);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Scans a single function body for in-flight loading emits and top-level
|
|
||||||
/// awaited try statements.
|
|
||||||
class _BodyScan extends RecursiveAstVisitor<void> {
|
|
||||||
final List<int> loadingEmits = [];
|
|
||||||
final List<TryStatement> topLevelTries = [];
|
|
||||||
|
|
||||||
static final RegExp _loading = RegExp(
|
|
||||||
r'(Loading|Processing|Submitting|Saving|Creating|Updating|InProgress)',
|
|
||||||
);
|
|
||||||
|
|
||||||
int _tryDepth = 0;
|
|
||||||
|
|
||||||
void _maybeLoading(AstNode node, List<Argument> arguments) {
|
|
||||||
if (arguments.isEmpty) return;
|
|
||||||
final first = arguments.first;
|
|
||||||
if (first is! Expression) return;
|
|
||||||
if (_loading.hasMatch(first.toSource())) loadingEmits.add(node.offset);
|
|
||||||
}
|
|
||||||
|
|
||||||
@override
|
|
||||||
void visitMethodInvocation(MethodInvocation node) {
|
|
||||||
if (node.methodName.name == 'emit') {
|
|
||||||
_maybeLoading(node, node.argumentList.arguments);
|
|
||||||
}
|
|
||||||
super.visitMethodInvocation(node);
|
|
||||||
}
|
|
||||||
|
|
||||||
@override
|
|
||||||
void visitFunctionExpressionInvocation(FunctionExpressionInvocation node) {
|
|
||||||
final function = node.function;
|
|
||||||
if (function is SimpleIdentifier && function.name == 'emit') {
|
|
||||||
_maybeLoading(node, node.argumentList.arguments);
|
|
||||||
}
|
|
||||||
super.visitFunctionExpressionInvocation(node);
|
|
||||||
}
|
|
||||||
|
|
||||||
@override
|
|
||||||
void visitTryStatement(TryStatement node) {
|
|
||||||
final isTopLevel = _tryDepth == 0;
|
|
||||||
_tryDepth++;
|
|
||||||
if (isTopLevel) topLevelTries.add(node);
|
|
||||||
super.visitTryStatement(node);
|
|
||||||
_tryDepth--;
|
|
||||||
}
|
|
||||||
|
|
||||||
// Nested functions are analysed in their own right by _analyze; descending
|
|
||||||
// into them here would report the same catch twice.
|
|
||||||
@override
|
|
||||||
void visitFunctionExpression(FunctionExpression node) {}
|
|
||||||
|
|
||||||
@override
|
|
||||||
void visitFunctionDeclaration(FunctionDeclaration node) {}
|
|
||||||
}
|
|
||||||
|
|
||||||
class _HasAwait extends RecursiveAstVisitor<void> {
|
|
||||||
_HasAwait({this.shallow = false});
|
|
||||||
|
|
||||||
/// Stop at nested functions: an await inside a closure is not awaited by the
|
|
||||||
/// body that declares the closure.
|
|
||||||
final bool shallow;
|
|
||||||
|
|
||||||
bool found = false;
|
|
||||||
|
|
||||||
@override
|
|
||||||
void visitAwaitExpression(AwaitExpression node) {
|
|
||||||
found = true;
|
|
||||||
super.visitAwaitExpression(node);
|
|
||||||
}
|
|
||||||
|
|
||||||
@override
|
|
||||||
void visitFunctionExpression(FunctionExpression node) {
|
|
||||||
if (!shallow) super.visitFunctionExpression(node);
|
|
||||||
}
|
|
||||||
|
|
||||||
@override
|
|
||||||
void visitFunctionDeclaration(FunctionDeclaration node) {
|
|
||||||
if (!shallow) super.visitFunctionDeclaration(node);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
/// A `field: true` named argument for one of the loading fields, outside any
|
|
||||||
/// nested function.
|
|
||||||
class _SetsFlagTrue extends RecursiveAstVisitor<void> {
|
|
||||||
_SetsFlagTrue(this.fields);
|
|
||||||
|
|
||||||
final Set<String> fields;
|
|
||||||
bool found = false;
|
|
||||||
|
|
||||||
@override
|
|
||||||
void visitNamedArgument(NamedArgument node) {
|
|
||||||
final value = node.argumentExpression;
|
|
||||||
if (fields.contains(node.name.lexeme) &&
|
|
||||||
value is BooleanLiteral &&
|
|
||||||
value.value) {
|
|
||||||
found = true;
|
|
||||||
}
|
|
||||||
super.visitNamedArgument(node);
|
|
||||||
}
|
|
||||||
|
|
||||||
@override
|
|
||||||
void visitFunctionExpression(FunctionExpression node) {}
|
|
||||||
|
|
||||||
@override
|
|
||||||
void visitFunctionDeclaration(FunctionDeclaration node) {}
|
|
||||||
}
|
|
||||||
|
|
||||||
class _LoadingResetCollector extends RecursiveAstVisitor<void> {
|
|
||||||
_LoadingResetCollector(
|
|
||||||
this.project,
|
|
||||||
this.path,
|
|
||||||
this.lineInfo,
|
|
||||||
this.stateOwners,
|
|
||||||
this.findings,
|
|
||||||
);
|
|
||||||
|
|
||||||
final Project project;
|
|
||||||
final String path;
|
|
||||||
final LineInfo lineInfo;
|
|
||||||
final Set<String> stateOwners;
|
|
||||||
final List<Finding> findings;
|
|
||||||
final List<String> _classStack = [];
|
|
||||||
|
|
||||||
static final RegExp _stateAssign = RegExp(r'(^|[^.\w])state\s*=(?!=)');
|
|
||||||
static final RegExp _throwing = RegExp(r'\b(rethrow|throw)\b');
|
|
||||||
static final RegExp _errorish = RegExp(
|
|
||||||
r'(Error|Failure|Fail|Cancel|Retry|Recover|Reset|Clear)',
|
|
||||||
caseSensitive: false,
|
|
||||||
);
|
|
||||||
|
|
||||||
@override
|
|
||||||
void visitClassDeclaration(ClassDeclaration node) {
|
|
||||||
_classStack.add(node.namePart.typeName.lexeme);
|
|
||||||
super.visitClassDeclaration(node);
|
|
||||||
_classStack.removeLast();
|
|
||||||
}
|
|
||||||
|
|
||||||
@override
|
|
||||||
void visitMixinDeclaration(MixinDeclaration node) {
|
|
||||||
_classStack.add(node.name.lexeme);
|
|
||||||
super.visitMixinDeclaration(node);
|
|
||||||
_classStack.removeLast();
|
|
||||||
}
|
|
||||||
|
|
||||||
@override
|
|
||||||
void visitMethodDeclaration(MethodDeclaration node) {
|
|
||||||
_analyze(node.body, node.name.lexeme);
|
|
||||||
super.visitMethodDeclaration(node);
|
|
||||||
}
|
|
||||||
|
|
||||||
@override
|
|
||||||
void visitFunctionExpression(FunctionExpression node) {
|
|
||||||
_analyze(node.body, _closureMemberName(node));
|
|
||||||
super.visitFunctionExpression(node);
|
|
||||||
}
|
|
||||||
|
|
||||||
void _analyze(FunctionBody? body, String member) {
|
|
||||||
if (body == null) return;
|
|
||||||
final owner = _classStack.isEmpty ? '<top>' : _classStack.last;
|
|
||||||
if (!stateOwners.contains(owner)) return;
|
|
||||||
|
|
||||||
final scan = _BodyScan();
|
|
||||||
body.accept(scan);
|
|
||||||
if (scan.loadingEmits.isEmpty) return;
|
|
||||||
|
|
||||||
// Position within the member, not the line: two swallowed catches in one
|
|
||||||
// method need distinct fingerprints, and a line number changed whenever
|
|
||||||
// anything above the method moved.
|
|
||||||
var swallowed = 0;
|
|
||||||
for (final statement in scan.topLevelTries) {
|
|
||||||
if (statement.catchClauses.isEmpty) continue;
|
|
||||||
final probe = _HasAwait();
|
|
||||||
statement.body.accept(probe);
|
|
||||||
if (!probe.found) continue;
|
|
||||||
|
|
||||||
for (final catchClause in statement.catchClauses) {
|
|
||||||
final source = catchClause.body.toSource();
|
|
||||||
final emits =
|
|
||||||
source.contains('emit(') || source.contains('notifyListeners(');
|
|
||||||
final assignsState = _stateAssign.hasMatch(source);
|
|
||||||
final rethrows = _throwing.hasMatch(source);
|
|
||||||
final dispatchesFailure =
|
|
||||||
source.contains('add(') && _errorish.hasMatch(source);
|
|
||||||
if (emits || assignsState || rethrows || dispatchesFailure) continue;
|
|
||||||
swallowed++;
|
|
||||||
|
|
||||||
findings.add(
|
|
||||||
Finding(
|
|
||||||
rule: ruleLoadingNotReset,
|
|
||||||
severity: PipeSeverity.warning,
|
|
||||||
confidence: 'high',
|
|
||||||
message:
|
|
||||||
'$owner.$member emits an in-progress state and then awaits an '
|
|
||||||
'operation whose catch clause never emits a terminal state, so '
|
|
||||||
'the UI can be left showing a spinner forever.',
|
|
||||||
file: project.relative(path),
|
|
||||||
line: lineInfo.getLocation(catchClause.offset).lineNumber,
|
|
||||||
evidence: [
|
|
||||||
'${project.relative(path)}:'
|
|
||||||
'${lineInfo.getLocation(catchClause.offset).lineNumber}: '
|
|
||||||
'${_oneLine(source)}',
|
|
||||||
],
|
|
||||||
remediation:
|
|
||||||
'Emit a failure/timeout state in the catch (or rethrow) so the '
|
|
||||||
'loading state is always resolved.',
|
|
||||||
data: {
|
|
||||||
'owner': owner,
|
|
||||||
'member': member,
|
|
||||||
// A method can hold more than one swallowed catch, so which one
|
|
||||||
// is part of this rule's identity.
|
|
||||||
'catch': swallowed,
|
|
||||||
},
|
|
||||||
),
|
|
||||||
);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
static String _oneLine(String source) {
|
|
||||||
final collapsed = source.replaceAll(RegExp(r'\s+'), ' ').trim();
|
|
||||||
final body = collapsed.isEmpty ? '<empty catch>' : collapsed;
|
|
||||||
return body.length <= 120 ? body : '${body.substring(0, 117)}...';
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
|
||||||
|
|
@ -4,6 +4,8 @@ import 'package:analyzer/dart/ast/visitor.dart';
|
||||||
import 'package:analyzer/source/line_info.dart';
|
import 'package:analyzer/source/line_info.dart';
|
||||||
|
|
||||||
import 'analysis.dart';
|
import 'analysis.dart';
|
||||||
|
import 'dart_facts.dart';
|
||||||
|
import 'facts.dart';
|
||||||
import 'project.dart';
|
import 'project.dart';
|
||||||
|
|
||||||
/// How a state union is expressed in source.
|
/// How a state union is expressed in source.
|
||||||
|
|
@ -354,7 +356,12 @@ class StateFlow {
|
||||||
required this.ownerReferences,
|
required this.ownerReferences,
|
||||||
required this.flagStates,
|
required this.flagStates,
|
||||||
required this.riverpod,
|
required this.riverpod,
|
||||||
});
|
this.constructedByFile = const {},
|
||||||
|
this.swallowedFailures = const [],
|
||||||
|
this.unsettledFlags = const [],
|
||||||
|
UnresolvedSummary? unresolved,
|
||||||
|
}) : unresolved =
|
||||||
|
unresolved ?? UnresolvedSummary(files: const [], analysed: 0);
|
||||||
|
|
||||||
final String root;
|
final String root;
|
||||||
final List<StateUnion> unions;
|
final List<StateUnion> unions;
|
||||||
|
|
@ -376,6 +383,19 @@ class StateFlow {
|
||||||
/// Riverpod providers and their consumption sites.
|
/// Riverpod providers and their consumption sites.
|
||||||
final RiverpodModel riverpod;
|
final RiverpodModel riverpod;
|
||||||
|
|
||||||
|
/// Project-relative file -> type names constructed in it.
|
||||||
|
final Map<String, Set<String>> constructedByFile;
|
||||||
|
|
||||||
|
/// Busy variant emitted, then an awaited failure swallowed (loading-not-reset).
|
||||||
|
final List<SwallowedFailure> swallowedFailures;
|
||||||
|
|
||||||
|
/// Loading flag left set across unguarded awaited work
|
||||||
|
/// (loading-flag-not-reset).
|
||||||
|
final List<UnsettledFlag> unsettledFlags;
|
||||||
|
|
||||||
|
/// Files whose references did not resolve, and the names they mention.
|
||||||
|
final UnresolvedSummary unresolved;
|
||||||
|
|
||||||
Map<String, Object?> toJson() => {
|
Map<String, Object?> toJson() => {
|
||||||
'root': root,
|
'root': root,
|
||||||
'totals': {
|
'totals': {
|
||||||
|
|
@ -400,6 +420,13 @@ class StateFlow {
|
||||||
'ownerReferences': ownerReferences,
|
'ownerReferences': ownerReferences,
|
||||||
'flagStates': [for (final state in flagStates) state.toJson()],
|
'flagStates': [for (final state in flagStates) state.toJson()],
|
||||||
'riverpod': riverpod.toJson(),
|
'riverpod': riverpod.toJson(),
|
||||||
|
'constructedByFile': {
|
||||||
|
for (final entry in constructedByFile.entries)
|
||||||
|
entry.key: entry.value.toList()..sort(),
|
||||||
|
},
|
||||||
|
'swallowedFailures': [for (final fact in swallowedFailures) fact.toJson()],
|
||||||
|
'unsettledFlags': [for (final fact in unsettledFlags) fact.toJson()],
|
||||||
|
'unresolved': unresolved.toJson(),
|
||||||
};
|
};
|
||||||
|
|
||||||
/// Human summary, ordered by the most suspicious signal first.
|
/// Human summary, ordered by the most suspicious signal first.
|
||||||
|
|
@ -1788,10 +1815,33 @@ StateFlow buildStateFlow(Project project, List<FileOutcome> outcomes) {
|
||||||
final dispatches = <EventDispatch>[];
|
final dispatches = <EventDispatch>[];
|
||||||
final riverpodProviders = <RiverpodProvider>[];
|
final riverpodProviders = <RiverpodProvider>[];
|
||||||
final riverpodSites = <RiverpodSite>[];
|
final riverpodSites = <RiverpodSite>[];
|
||||||
|
final constructedByFile = <String, Set<String>>{};
|
||||||
|
final swallowedFailures = <SwallowedFailure>[];
|
||||||
|
final unsettledFlags = <UnsettledFlag>[];
|
||||||
|
final flagStatesByOwner = <String, FlagState>{
|
||||||
|
for (final state in flagStates) state.owner: state,
|
||||||
|
};
|
||||||
for (final entry in unitsByPath.entries) {
|
for (final entry in unitsByPath.entries) {
|
||||||
final result = entry.value;
|
final result = entry.value;
|
||||||
final unit = result.unit;
|
final unit = result.unit;
|
||||||
final lineInfo = result.lineInfo;
|
final lineInfo = result.lineInfo;
|
||||||
|
final relative = project.relative(entry.key);
|
||||||
|
final constructions = ConstructionCollector();
|
||||||
|
unit.accept(constructions);
|
||||||
|
constructedByFile[relative] = constructions.constructed;
|
||||||
|
unit.accept(
|
||||||
|
SwallowedFailureCollector(relative, lineInfo, swallowedFailures),
|
||||||
|
);
|
||||||
|
if (flagStatesByOwner.isNotEmpty) {
|
||||||
|
unit.accept(
|
||||||
|
UnsettledFlagCollector(
|
||||||
|
relative,
|
||||||
|
lineInfo,
|
||||||
|
flagStatesByOwner,
|
||||||
|
unsettledFlags,
|
||||||
|
),
|
||||||
|
);
|
||||||
|
}
|
||||||
unit.accept(
|
unit.accept(
|
||||||
_EmissionCollector(project, entry.key, lineInfo, emissions, redirects),
|
_EmissionCollector(project, entry.key, lineInfo, emissions, redirects),
|
||||||
);
|
);
|
||||||
|
|
@ -1886,5 +1936,23 @@ StateFlow buildStateFlow(Project project, List<FileOutcome> outcomes) {
|
||||||
ownerReferences: ownerReferences,
|
ownerReferences: ownerReferences,
|
||||||
flagStates: flagStates,
|
flagStates: flagStates,
|
||||||
riverpod: RiverpodModel(providers: riverpodProviders, sites: riverpodSites),
|
riverpod: RiverpodModel(providers: riverpodProviders, sites: riverpodSites),
|
||||||
|
constructedByFile: constructedByFile,
|
||||||
|
swallowedFailures: swallowedFailures,
|
||||||
|
unsettledFlags: unsettledFlags,
|
||||||
|
unresolved: _unresolvedSummary(project, outcomes),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
UnresolvedSummary _unresolvedSummary(
|
||||||
|
Project project,
|
||||||
|
List<FileOutcome> outcomes,
|
||||||
|
) {
|
||||||
|
final found = findUnresolvedReferences(outcomes);
|
||||||
|
final first = found.first;
|
||||||
|
return UnresolvedSummary(
|
||||||
|
files: [for (final file in found.files) project.relative(file)],
|
||||||
|
analysed: found.analysed,
|
||||||
|
first: first?.replaceFirst('${project.root}/', ''),
|
||||||
|
names: found.names,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue