feat(lint): AST layer over libclang cursors

Adds LintContext::Decls() — clang's view of the declarations written in the
file — plus AddAstLintRule to register a rule that reads it. No rule uses it
yet; the three that will are migrated separately.

The declarations come back as a flat vector with parent indices rather than an
opaque cursor handle: no lifetimes cross the project-DLL boundary, no callback
hops back into project.so per node, and "is this at namespace scope or inside a
function?" becomes an index lookup instead of a hand-rolled brace stack.

Three things had to be solved for this to work at all on this codebase.

libclang cannot see through `export`. A C++20 export declaration has no
CXCursorKind, so `export namespace Crafter { … }` arrives as a childless
CXCursor_UnexposedDecl and clang_visitChildren does not descend. Five of the
eleven interfaces here are written that way — 677 lines, including
Configuration and LintContext, yielding zero usable cursors. A plain
`namespace` IS descended into, so the fix is to blank the keyword before
parsing, byte-length preserving so every line and column still lands on the
original file. `export module` is left alone or the unit stops being a module
interface. Verified end-to-end against a fixture whose asserted line numbers
match the unblanked file.

PCMs are flag-locked, so each file has to parse with the flags that built it.
CollectConfigSources now records which Configuration owns each source instead
of flattening to a set, because three regimes coexist: the library, each test
(carrying its own target, defines and -march), and project.cpp, which Build
never touches and which therefore gets no command at all.

libclang resolves its builtin headers relative to its own install path, which
need not match the clang++ that wrote the PCMs. When it doesn't, every parse
dies on "'stddef.h' file not found", so -resource-dir is passed explicitly
from `clang++ -print-resource-dir`.

Two flags on each declaration replace what would otherwise become more
substring denylists: isExternC, and isForeignApi for a declaration that binds
to an entity declared outside the project root — resolved through
clang_getCursorReferenced and the same inside-the-root test the dependency
walk already uses. Parameters and fields inherit it, so an exemption covers a
whole signature rather than the one node that named the foreign entity.

Failure is never silent. A fatal diagnostic leaves a fragment that is
indistinguishable from a file declaring nothing, so it is reported as an error
instead: the rule is skipped, a finding explains why, and summary.errors makes
the run fail. --no-ast opts out deliberately and exits normally.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jorijn van der Graaf 2026-07-30 23:30:41 +02:00
commit 29888fc5ba
6 changed files with 734 additions and 11 deletions

View file

@ -149,6 +149,57 @@ export namespace Crafter {
std::size_t column = 0; // 1-based, of the token's first byte
};
// What a LintDecl declares.
enum class LintDeclKind {
Namespace,
Class,
Struct,
Union,
Enum,
EnumConstant,
TypeAlias,
Function,
Method,
Constructor,
Destructor,
Field,
Variable, // local or namespace-scope variable
Parameter,
Other,
};
inline constexpr std::size_t LintNoParent = static_cast<std::size_t>(-1);
// One declaration from LintContext::Decls(), for declarations written in
// the file under lint (never ones pulled in from a header or module).
//
// The vector is flattened depth-first with parents before children, and
// `parent` indexes back into it — that is how a rule asks "is this at
// namespace scope, or inside a function?" without re-deriving a brace
// stack from the text.
struct LintDecl {
LintDeclKind kind = LintDeclKind::Other;
std::string name; // unqualified spelling; empty when anonymous
std::string type; // clang's resolved spelling: "char *", "std::int32_t"
std::size_t line = 0;
std::size_t column = 0;
std::size_t begin = 0; // byte offsets of the whole declaration, for
std::size_t end = 0; // marking a region a transform must not touch
std::size_t parent = LintNoParent;
bool isDefinition = false;
bool isStatic = false;
bool isConstexpr = false;
bool isScopedEnum = false; // `enum class` rather than plain `enum`
// ---- foreign-API boundary ----
// Set when this declaration's spelling is dictated by somebody else's
// header, so the type-modernising rules must leave its bytes alone.
// Replaces the hand-maintained substring denylists, which could only
// ever grow: a new external library needs no new entry here.
bool isExternC = false; // declared with C language linkage
bool isForeignApi = false; // its type or its body binds to an entity
// declared outside the project root
};
// Per-file view handed to each LintRule's check callback. Every member
// function is out-of-line and CRAFTER_API (defined in Crafter.Build:Lint's
// implementation unit) because rule lambdas execute from the user's
@ -223,6 +274,27 @@ export namespace Crafter {
// inside an ordinary literal.
CRAFTER_API bool LineHasMultiLineToken(std::size_t line);
// Declarations written in this file, flattened depth-first with
// parents before children (see LintDecl). Empty when the AST is not
// available — check AstAvailable() first, and do not read an empty
// result as "this file declares nothing".
//
// Requires the module PCMs, unlike Tokens(): a module unit's
// `import std;` cannot resolve without them, and clang treats that as
// fatal rather than recovering. RunLint builds them on demand for
// rules registered through AddAstLintRule and fails the run if it
// cannot, so a semantic rule never silently reports clean.
//
// One further asymmetry worth knowing: an AST is ONE configuration's
// slice. Declarations inside a preprocessor branch that is inactive
// for the host are absent here, though Tokens() still sees them. Rules
// that must cover every platform belong on tokens.
CRAFTER_API std::span<const LintDecl> Decls();
CRAFTER_API bool AstAvailable();
// Why AstAvailable() is false — a missing PCM, a parse error, a file
// that is not a translation unit. Empty when the AST is available.
CRAFTER_API std::string_view AstUnavailableReason();
// Driver wiring — set by RunLint before each check call. Not for rules.
std::string activeRule;
std::vector<LintFinding>* sink = nullptr;
@ -232,6 +304,13 @@ export namespace Crafter {
// Per-line flag, 0-based, for LineHasMultiLineToken. Derived from
// tokenCache and invalidated with it.
std::optional<std::vector<bool>> spannedLineCache;
// Flags this file parses with, from GetCompileCommand of the
// Configuration that owns it. Empty disables Decls().
std::string compileCommand;
// Declarations resolving outside this are foreign API (LintDecl).
fs::path projectRoot;
std::optional<std::vector<LintDecl>> declCache;
std::string astReason;
};
// A named lint rule: `check` runs once per (rule, file) over the project's
@ -241,6 +320,10 @@ export namespace Crafter {
struct LintRule {
std::string name;
std::function<void(LintContext&)> check;
// Registered via AddAstLintRule: this rule reads Decls(), so the run
// has to produce the module PCMs first, and must fail rather than let
// the rule quietly find nothing.
bool needsAst = false;
};
// The host target triple, detected once per process by running
@ -357,6 +440,14 @@ export namespace Crafter {
// ctx.SetContent are transforms (see LintContext::SetContent).
// Defined in Crafter.Build:Lint.
CRAFTER_API void AddLintRule(std::string name, std::function<void(LintContext&)> check);
// Same, for a rule that reads LintContext::Decls(). Declared
// separately rather than as a flag on AddLintRule so existing
// registrations keep compiling. Such a rule makes the run build the
// module PCMs if they are missing, and a file whose AST could not be
// produced becomes an error instead of a silent pass. Prefer
// report-only: a transform running after the first one forces a
// re-parse of everything it changed.
CRAFTER_API void AddAstLintRule(std::string name, std::function<void(LintContext&)> check);
// Suffix that uniquely identifies this Configuration's compile state.
// target+march+mtune are spelled out for readability; the rest
// (type, debug, sysroot, defines, compileFlags) collapse into a short

View file

@ -24,6 +24,11 @@ export namespace Crafter {
// Enumerate matching rule names without running them.
bool listOnly = false;
LintMode mode = LintMode::Report;
// Skip rules registered with AddAstLintRule instead of building the
// module PCMs they need. The escape hatch for a fresh clone where the
// token-based rules are wanted immediately; the run then exits normally
// rather than erroring, so it must be asked for explicitly.
bool noAst = false;
// Absolute path of the loaded project.cpp. It is linted too, and its
// parent directory is the project root that decides which dependency
// Configurations contribute rules/files (GitProject / cache-dir deps