fix(lint): report an unavailable AST once per run, not per (file, rule)
Linting a march whose PCMs are not built produced 102 findings — 34 files times three AST rules — for a single fact: the project has not been built for that configuration. The one actionable sentence was buried. Now recorded per file and reported once, naming the rules that could not run, how many files were affected, one example reason, and what to do about it. Still one error, so the run still fails; --no-ast remains the way to proceed without building. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
6438cb9ebb
commit
7f71030b5b
2 changed files with 25 additions and 3 deletions
|
|
@ -953,6 +953,9 @@ format` applies it to disk, and `crafter-build lint` reports where it would.
|
||||||
return PathInsideRoot(p, cwd) ? p.lexically_relative(cwd) : p;
|
return PathInsideRoot(p, cwd) ? p.lexically_relative(cwd) : p;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
std::map<fs::path, std::string> astFailures;
|
||||||
|
std::set<std::string_view> skippedAstRules;
|
||||||
|
|
||||||
// Any rule reading Decls() needs this configuration's module PCMs, and a
|
// Any rule reading Decls() needs this configuration's module PCMs, and a
|
||||||
// parse without them is fatal rather than degraded. Produce them up front
|
// parse without them is fatal rather than degraded. Produce them up front
|
||||||
// rather than letting each file fail on its own, and cache the assembled
|
// rather than letting each file fail on its own, and cache the assembled
|
||||||
|
|
@ -1020,8 +1023,12 @@ format` applies it to disk, and `crafter-build lint` reports where it would.
|
||||||
// from reporting it clean.
|
// from reporting it clean.
|
||||||
if (ctx.compileCommand.empty()) continue;
|
if (ctx.compileCommand.empty()) continue;
|
||||||
if (!ctx.AstAvailable()) {
|
if (!ctx.AstAvailable()) {
|
||||||
ctx.Report(0, std::format("rule '{}' needs an AST, which is unavailable: {}", rule->name, ctx.AstUnavailableReason()));
|
// Recorded once per file and reported as a single grouped
|
||||||
++summary.errors;
|
// error after the run. One missing PCM would otherwise
|
||||||
|
// produce a finding per (file, rule) and bury the one fact
|
||||||
|
// that matters — which is that a build has to happen first.
|
||||||
|
astFailures.emplace(file, ctx.AstUnavailableReason());
|
||||||
|
skippedAstRules.insert(rule->name);
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -1113,6 +1120,18 @@ format` applies it to disk, and `crafter-build lint` reports where it would.
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (!astFailures.empty()) {
|
||||||
|
std::string rules;
|
||||||
|
for (std::string_view name : skippedAstRules) {
|
||||||
|
if (!rules.empty()) rules += ", ";
|
||||||
|
rules += name;
|
||||||
|
}
|
||||||
|
std::println(std::cerr, "lint: {} rule(s) needing an AST ({}) could not run on {} of {} file(s).", skippedAstRules.size(), rules, astFailures.size(), summary.filesLinted);
|
||||||
|
std::println(std::cerr, " {}: {}", shown(astFailures.begin()->first).string(), astFailures.begin()->second);
|
||||||
|
std::println(std::cerr, " Build the project first so the module PCMs exist, or pass --no-ast to skip these rules.");
|
||||||
|
++summary.errors;
|
||||||
|
}
|
||||||
|
|
||||||
std::sort(summary.findings.begin(), summary.findings.end(),
|
std::sort(summary.findings.begin(), summary.findings.end(),
|
||||||
[](const LintFinding& a, const LintFinding& b) {
|
[](const LintFinding& a, const LintFinding& b) {
|
||||||
return std::tie(a.file, a.line) < std::tie(b.file, b.line);
|
return std::tie(a.file, a.line) < std::tie(b.file, b.line);
|
||||||
|
|
|
||||||
|
|
@ -699,7 +699,10 @@ int main() {
|
||||||
Check(!ran, "ast: rule is skipped when the AST is unavailable");
|
Check(!ran, "ast: rule is skipped when the AST is unavailable");
|
||||||
Check(summary.errors > 0, "ast: unavailable AST counts as an error");
|
Check(summary.errors > 0, "ast: unavailable AST counts as an error");
|
||||||
Check(!summary.Clean(), "ast: unavailable AST is not Clean");
|
Check(!summary.Clean(), "ast: unavailable AST is not Clean");
|
||||||
Check(std::any_of(summary.findings.begin(), summary.findings.end(), [](const LintFinding& f) { return f.message.contains("needs an AST"); }), "ast: a finding explains why");
|
// Explained on stderr as one grouped message rather than a finding per
|
||||||
|
// (file, rule): a single missing PCM would otherwise bury the one fact
|
||||||
|
// that matters. The run still failing is the part that counts, and the
|
||||||
|
// two assertions above cover it.
|
||||||
}
|
}
|
||||||
|
|
||||||
// --no-ast skips those rules deliberately and exits normally.
|
// --no-ast skips those rules deliberately and exits normally.
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue