fix: stop verify, reports and briefs from claiming more than pipe checked #1
3 changed files with 79 additions and 3 deletions
fix(verify): judge only the baselined findings of the rules that ran
`verify --rule X` compared the fresh report against the whole baseline. Every accepted finding of every other rule was absent from the report, because those rules never ran, so verify listed them as RESOLVED - "these fixes are confirmed" for fixes nobody made. Baseline entries are now filtered to the enabled rules before comparison. An entry's rule is its `rule` field or, for a fingerprints-only baseline, the fingerprint's first `|` segment. Entries set aside are counted as `outOfScope` in JSON and named in the text summary, so the filter is visible rather than silent.
commit
cf9e0b7e57
|
|
@ -548,6 +548,7 @@ class VerifyCommand extends Command<int> {
|
|||
final report = verifyAgainstBaseline(
|
||||
report: analysis.report,
|
||||
baseline: entries,
|
||||
rules: selected.isEmpty ? null : selected,
|
||||
);
|
||||
if (results['format'] as String == 'json') {
|
||||
stdout.writeln(_encode(report.toJson()));
|
||||
|
|
|
|||
|
|
@ -11,10 +11,16 @@ class VerificationReport {
|
|||
required this.resolved,
|
||||
required this.stillPresent,
|
||||
required this.newFindings,
|
||||
this.outOfScope = 0,
|
||||
});
|
||||
|
||||
/// Accepted findings of the rules that ran, i.e. the ones this report judges.
|
||||
final int baselineTotal;
|
||||
|
||||
/// Accepted findings of rules that were not run (`--rule`), so they are
|
||||
/// neither resolved nor still present: nothing looked for them.
|
||||
final int outOfScope;
|
||||
|
||||
/// Accepted findings that are no longer reported.
|
||||
final List<BaselineEntry> resolved;
|
||||
|
||||
|
|
@ -28,6 +34,7 @@ class VerificationReport {
|
|||
|
||||
Map<String, Object?> toJson() => {
|
||||
'baselineTotal': baselineTotal,
|
||||
'outOfScope': outOfScope,
|
||||
'resolved': resolved.length,
|
||||
'stillPresent': stillPresent.length,
|
||||
'new': newFindings.length,
|
||||
|
|
@ -40,6 +47,12 @@ class VerificationReport {
|
|||
String describe({int top = 25}) {
|
||||
final buffer = StringBuffer()
|
||||
..writeln('baseline $baselineTotal accepted findings')
|
||||
..write(
|
||||
outOfScope == 0
|
||||
? ''
|
||||
: 'scope $outOfScope accepted finding(s) of rules not run '
|
||||
'(--rule) were not checked\n',
|
||||
)
|
||||
..writeln(
|
||||
'resolved ${resolved.length} no longer reported'
|
||||
'${resolved.isEmpty ? '' : ' — these fixes are confirmed'}',
|
||||
|
|
@ -77,24 +90,40 @@ class VerificationReport {
|
|||
}
|
||||
}
|
||||
|
||||
/// The rule a baseline entry belongs to. Entries always carry it, but a baseline
|
||||
/// written before entries existed has only fingerprints, whose first `|`
|
||||
/// segment is the rule.
|
||||
String baselineEntryRule(BaselineEntry entry) =>
|
||||
entry.rule.isNotEmpty ? entry.rule : entry.fingerprint.split('|').first;
|
||||
|
||||
/// Compare a fresh analysis against the accepted baseline.
|
||||
///
|
||||
/// Self-contained: it derives new findings by comparing fingerprints, so it does
|
||||
/// not depend on the caller having passed the baseline to `runChecks`.
|
||||
///
|
||||
/// When only some rules ran ([rules]), accepted findings of the other rules are
|
||||
/// set aside as out of scope. Comparing them too reported every one of them as
|
||||
/// RESOLVED, because a rule that did not run reports nothing.
|
||||
VerificationReport verifyAgainstBaseline({
|
||||
required FindingReport report,
|
||||
required List<BaselineEntry> baseline,
|
||||
Set<String>? rules,
|
||||
}) {
|
||||
final inScope = [
|
||||
for (final entry in baseline)
|
||||
if (rules == null || rules.contains(baselineEntryRule(entry))) entry,
|
||||
];
|
||||
final present = {for (final finding in report.findings) finding.fingerprint};
|
||||
final accepted = {for (final entry in baseline) entry.fingerprint};
|
||||
return VerificationReport(
|
||||
baselineTotal: baseline.length,
|
||||
baselineTotal: inScope.length,
|
||||
outOfScope: baseline.length - inScope.length,
|
||||
resolved: [
|
||||
for (final entry in baseline)
|
||||
for (final entry in inScope)
|
||||
if (!present.contains(entry.fingerprint)) entry,
|
||||
],
|
||||
stillPresent: [
|
||||
for (final entry in baseline)
|
||||
for (final entry in inScope)
|
||||
if (present.contains(entry.fingerprint)) entry,
|
||||
],
|
||||
newFindings: [
|
||||
|
|
|
|||
|
|
@ -130,4 +130,50 @@ void main() {
|
|||
expect(verification.newFindings.map((f) => f.baselineLabel), ['NewlyDead']);
|
||||
expect(verification.resolved, isEmpty);
|
||||
});
|
||||
|
||||
test('verify --rule judges only the baselined findings of that rule', () {
|
||||
// `verify --rule X` compared against the WHOLE baseline, so every accepted
|
||||
// finding of every other rule was absent from the fresh report - because
|
||||
// those rules never ran - and was reported as RESOLVED.
|
||||
final report = FindingReport(
|
||||
root: '/root',
|
||||
findings: [present],
|
||||
suppressed: const [],
|
||||
);
|
||||
final verification = verifyAgainstBaseline(
|
||||
report: report,
|
||||
baseline: [
|
||||
BaselineEntry(
|
||||
fingerprint: present.fingerprint,
|
||||
rule: present.rule,
|
||||
file: present.file!,
|
||||
line: present.line,
|
||||
label: 'StillBroken',
|
||||
),
|
||||
BaselineEntry(
|
||||
fingerprint: fixed.fingerprint,
|
||||
rule: fixed.rule,
|
||||
file: fixed.file!,
|
||||
line: fixed.line,
|
||||
label: 'loadNow',
|
||||
),
|
||||
// A fingerprints-only baseline carries no rule; it is the fingerprint's
|
||||
// first segment.
|
||||
BaselineEntry(
|
||||
fingerprint: 'dead-handler|lib/c.dart|event=Old',
|
||||
rule: '',
|
||||
file: '',
|
||||
label: 'dead-handler|lib/c.dart|event=Old',
|
||||
),
|
||||
],
|
||||
rules: {'emitted-not-rendered'},
|
||||
);
|
||||
|
||||
expect(verification.resolved, isEmpty);
|
||||
expect(verification.stillPresent.map((e) => e.label), ['StillBroken']);
|
||||
expect(verification.baselineTotal, 1);
|
||||
expect(verification.outOfScope, 2);
|
||||
expect(verification.describe(), isNot(contains('confirmed')));
|
||||
expect(verification.describe(), contains('rules not run'));
|
||||
});
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue