diff --git a/implementations/Crafter.Build-Clang.cpp b/implementations/Crafter.Build-Clang.cpp index 80021b8..cb5e2a5 100644 --- a/implementations/Crafter.Build-Clang.cpp +++ b/implementations/Crafter.Build-Clang.cpp @@ -1571,14 +1571,22 @@ Test options (after the `test` subcommand): Lint options (after the `lint` subcommand): --list Enumerate matching lint rules without running them. + --no-ast Skip rules that need an AST instead of building the + module PCMs they require. One or more name globs to filter rules (e.g. 'spdx*'). Lint rules are defined in project.cpp via cfg.AddLintRule(name, callback) — C++ callbacks run once per source file. No rules ship by default. + cfg.AddAstLintRule registers a rule that reads ctx.Decls(), clang's view of + the declarations in the file. Those need the module PCMs, which the run + builds if they are missing; a file whose AST cannot be produced is an error, + never a silent pass. --no-ast skips them instead. + Format options (after the `format` subcommand): --check Dry run: list files that would change, exit 1 if any. --list Enumerate matching rules without running them. + --no-ast Skip rules that need an AST (see `lint --no-ast`). One or more name globs to filter rules. Rules are shared with lint — a rule that calls ctx.SetContent is a @@ -1665,6 +1673,8 @@ int Crafter::Run(int argc, char** argv) { testOpts.globs.emplace_back(arg); } else if ((runLint || runFormat) && arg == "--list") { lintOpts.listOnly = true; + } else if ((runLint || runFormat) && arg == "--no-ast") { + lintOpts.noAst = true; } else if ((runLint || runFormat) && !arg.starts_with("-")) { lintOpts.globs.emplace_back(arg); } else { diff --git a/implementations/Crafter.Build-Lint.cpp b/implementations/Crafter.Build-Lint.cpp index 8571e29..1c41544 100644 --- a/implementations/Crafter.Build-Lint.cpp +++ b/implementations/Crafter.Build-Lint.cpp @@ -64,6 +64,30 @@ namespace { decltype(&clang_disposeTokens) DisposeTokens = nullptr; decltype(&clang_getTokenKind) GetTokenKind = nullptr; decltype(&clang_getTokenExtent) GetTokenExtent = nullptr; + // AST layer. + decltype(&clang_getTranslationUnitCursor) GetTranslationUnitCursor = nullptr; + decltype(&clang_visitChildren) VisitChildren = nullptr; + decltype(&clang_getCursorKind) GetCursorKind = nullptr; + decltype(&clang_getCursorSpelling) GetCursorSpelling = nullptr; + decltype(&clang_getCursorType) GetCursorType = nullptr; + decltype(&clang_getTypeSpelling) GetTypeSpelling = nullptr; + decltype(&clang_getCursorLocation) GetCursorLocation = nullptr; + decltype(&clang_getCursorExtent) GetCursorExtent = nullptr; + decltype(&clang_getCursorReferenced) GetCursorReferenced = nullptr; + decltype(&clang_getCursorLanguage) GetCursorLanguage = nullptr; + decltype(&clang_Cursor_getStorageClass) GetStorageClass = nullptr; + decltype(&clang_EnumDecl_isScoped) EnumDeclIsScoped = nullptr; + decltype(&clang_isCursorDefinition) IsCursorDefinition = nullptr; + decltype(&clang_Location_isFromMainFile) LocationIsFromMainFile = nullptr; + decltype(&clang_getNumDiagnostics) GetNumDiagnostics = nullptr; + decltype(&clang_getDiagnostic) GetDiagnostic = nullptr; + decltype(&clang_getDiagnosticSeverity) GetDiagnosticSeverity = nullptr; + decltype(&clang_getDiagnosticSpelling) GetDiagnosticSpelling = nullptr; + decltype(&clang_disposeDiagnostic) DisposeDiagnostic = nullptr; + decltype(&clang_getCString) GetCString = nullptr; + decltype(&clang_disposeString) DisposeString = nullptr; + decltype(&clang_getFileName) GetFileName = nullptr; + decltype(&clang_Cursor_isNull) CursorIsNull = nullptr; }; LibClang LoadLibClang() { @@ -119,6 +143,29 @@ namespace { bind(lib.DisposeTokens, "clang_disposeTokens"); bind(lib.GetTokenKind, "clang_getTokenKind"); bind(lib.GetTokenExtent, "clang_getTokenExtent"); + bind(lib.GetTranslationUnitCursor, "clang_getTranslationUnitCursor"); + bind(lib.VisitChildren, "clang_visitChildren"); + bind(lib.GetCursorKind, "clang_getCursorKind"); + bind(lib.GetCursorSpelling, "clang_getCursorSpelling"); + bind(lib.GetCursorType, "clang_getCursorType"); + bind(lib.GetTypeSpelling, "clang_getTypeSpelling"); + bind(lib.GetCursorLocation, "clang_getCursorLocation"); + bind(lib.GetCursorExtent, "clang_getCursorExtent"); + bind(lib.GetCursorReferenced, "clang_getCursorReferenced"); + bind(lib.GetCursorLanguage, "clang_getCursorLanguage"); + bind(lib.GetStorageClass, "clang_Cursor_getStorageClass"); + bind(lib.EnumDeclIsScoped, "clang_EnumDecl_isScoped"); + bind(lib.IsCursorDefinition, "clang_isCursorDefinition"); + bind(lib.LocationIsFromMainFile, "clang_Location_isFromMainFile"); + bind(lib.GetNumDiagnostics, "clang_getNumDiagnostics"); + bind(lib.GetDiagnostic, "clang_getDiagnostic"); + bind(lib.GetDiagnosticSeverity, "clang_getDiagnosticSeverity"); + bind(lib.GetDiagnosticSpelling, "clang_getDiagnosticSpelling"); + bind(lib.DisposeDiagnostic, "clang_disposeDiagnostic"); + bind(lib.GetCString, "clang_getCString"); + bind(lib.DisposeString, "clang_disposeString"); + bind(lib.GetFileName, "clang_getFileName"); + bind(lib.CursorIsNull, "clang_Cursor_isNull"); if (!missing.empty()) { lib.handle = nullptr; lib.error = std::format("loaded {} but it is missing {}", candidates.front(), join(missing)); @@ -215,6 +262,312 @@ namespace { return tokens; } + // ---------------- AST ---------------- + + std::string TakeString(const LibClang& lc, CXString s) { + // clang_getCString returns const char*; the spelling is libclang's, not + // ours. Suppressed by hand until no-char-pointer reads the AST and can + // see that for itself. + // lint-disable-next-line no-char-pointer + const char* raw = lc.GetCString(s); + std::string out = raw ? raw : ""; + lc.DisposeString(s); + return out; + } + + // Overwrite every `export` keyword with spaces, leaving `export module` + // alone. Byte-length preserving, so every line and column libclang reports + // still lands on the original file. + // + // libclang has no CXCursorKind for a C++20 export declaration: it reports + // CXCursor_UnexposedDecl and does not descend, so `export namespace X { … }` + // collapses to one childless node and every declaration inside it becomes + // invisible. Five of this repo's interfaces are written that way, which is + // 677 lines including Configuration and LintContext. A plain `namespace X` + // IS descended into, so removing the keyword is enough — the declarations + // stop being exported in this parse, which is irrelevant to the names, + // kinds, types and scopes the rules ask about. + // + // The braced `export { … }` form would need the braces kept, which this + // does not attempt; it does not occur here, and it would show up as a parse + // error rather than silently wrong output. + std::string BlankExportKeywords(const std::string& content, std::span tokens) { + std::string out = content; + for (std::size_t i = 0; i < tokens.size(); ++i) { + const LintToken& token = tokens[i]; + if (token.kind != LintTokenKind::Keyword && token.kind != LintTokenKind::Identifier) continue; + if (token.length != 6 || out.compare(token.offset, 6, "export") != 0) continue; + // `export module Crafter.Build:Lint;` must survive: without it the + // unit stops being a module interface and its own partition + // imports become ill-formed. + if (i + 1 < tokens.size() && content.compare(tokens[i + 1].offset, 6, "module") == 0) continue; + out.replace(token.offset, 6, " "); + } + return out; + } + + LintDeclKind MapCursorKind(CXCursorKind kind) { + switch (kind) { + case CXCursor_Namespace: return LintDeclKind::Namespace; + case CXCursor_ClassDecl: + case CXCursor_ClassTemplate: return LintDeclKind::Class; + case CXCursor_StructDecl: return LintDeclKind::Struct; + case CXCursor_UnionDecl: return LintDeclKind::Union; + case CXCursor_EnumDecl: return LintDeclKind::Enum; + case CXCursor_EnumConstantDecl: return LintDeclKind::EnumConstant; + case CXCursor_TypedefDecl: + case CXCursor_TypeAliasDecl: + case CXCursor_TypeAliasTemplateDecl: return LintDeclKind::TypeAlias; + case CXCursor_FunctionDecl: + case CXCursor_FunctionTemplate: return LintDeclKind::Function; + case CXCursor_CXXMethod: return LintDeclKind::Method; + case CXCursor_Constructor: return LintDeclKind::Constructor; + case CXCursor_Destructor: return LintDeclKind::Destructor; + case CXCursor_FieldDecl: return LintDeclKind::Field; + case CXCursor_VarDecl: return LintDeclKind::Variable; + case CXCursor_ParmDecl: return LintDeclKind::Parameter; + default: return LintDeclKind::Other; + } + } + + // Split a shell command string into argv for libclang, dropping argv[0] + // and undoing the \" escaping the shell form needs. Whitespace-separated: + // the build assembles and runs this very string through a shell, so a path + // containing a space is already unsupported upstream of here. + std::vector CommandToArgs(std::string_view command) { + std::vector args; + for (std::size_t i = 0; i < command.size();) { + while (i < command.size() && command[i] == ' ') ++i; + std::size_t begin = i; + while (i < command.size() && command[i] != ' ') ++i; + if (i > begin) args.emplace_back(command.substr(begin, i - begin)); + } + if (!args.empty()) args.erase(args.begin()); // argv[0] is the compiler + for (std::string& arg : args) { + // Only \" unescapes. Erasing every backslash would destroy the + // Windows include paths GetBaseCommand puts on the command line. + std::string unescaped; + unescaped.reserve(arg.size()); + for (std::size_t i = 0; i < arg.size(); ++i) { + if (arg[i] == '\\' && i + 1 < arg.size() && arg[i + 1] == '"') continue; + unescaped += arg[i]; + } + arg = std::move(unescaped); + } + return args; + } + + struct DeclWalk { + const LibClang* lc = nullptr; + const std::string* content = nullptr; + const fs::path* projectRoot = nullptr; + std::vector* out = nullptr; + std::vector stack; // indices of the enclosing declarations + }; + + bool PathInsideRoot(const fs::path& p, const fs::path& root); + + // True when `cursor` names something declared outside the project — a + // system header, libc++, an external dependency. This is what makes the + // interop exemption principled rather than a list of names: the question + // asked is "whose header dictates this spelling", and the answer comes + // from where the declaration actually lives. + bool ResolvesOutsideProject(const LibClang& lc, CXCursor cursor, const fs::path& projectRoot) { + if (projectRoot.empty()) return false; + CXCursor target = lc.GetCursorReferenced(cursor); + if (lc.CursorIsNull(target)) return false; + CXSourceLocation location = lc.GetCursorLocation(target); + if (lc.LocationIsFromMainFile(location)) return false; + CXFile file = nullptr; + std::uint32_t line = 0; + std::uint32_t column = 0; + std::uint32_t offset = 0; + lc.GetFileLocation(location, &file, &line, &column, &offset); + if (!file) return false; + std::string path = TakeString(lc, lc.GetFileName(file)); + if (path.empty()) return false; + return !PathInsideRoot(fs::path(path), projectRoot); + } + + CXChildVisitResult VisitDecl(CXCursor cursor, CXCursor, CXClientData data) { + DeclWalk& walk = *static_cast(data); + const LibClang& lc = *walk.lc; + CXSourceLocation location = lc.GetCursorLocation(cursor); + // Every declaration the imported modules bring in arrives here too — + // an unfiltered visit of one interface unit walks ~495,000 cursors from + // std alone. Prune before doing any work. + if (!lc.LocationIsFromMainFile(location)) return CXChildVisit_Continue; + + LintDeclKind mapped = MapCursorKind(lc.GetCursorKind(cursor)); + if (mapped == LintDeclKind::Other) { + // Not a declaration we model, but a reference to a foreign entity + // inside one — an argument to a libc call, a member of a libc++ + // type — is exactly what marks the enclosing declaration as sitting + // on an interop boundary. + if (!walk.stack.empty() && ResolvesOutsideProject(lc, cursor, *walk.projectRoot)) { + (*walk.out)[walk.stack.back()].isForeignApi = true; + } + // Recursed by hand rather than with CXChildVisit_Recurse so the + // enclosing-declaration stack stays accurate: the callback is never + // told when a subtree ends. + lc.VisitChildren(cursor, &VisitDecl, &walk); + return CXChildVisit_Continue; + } + + LintDecl decl; + decl.kind = mapped; + decl.name = TakeString(lc, lc.GetCursorSpelling(cursor)); + decl.type = TakeString(lc, lc.GetTypeSpelling(lc.GetCursorType(cursor))); + CXFile nameFile = nullptr; + std::uint32_t nameLine = 0; + std::uint32_t nameColumn = 0; + std::uint32_t nameOffset = 0; + lc.GetFileLocation(location, &nameFile, &nameLine, &nameColumn, &nameOffset); + decl.line = nameLine; + decl.column = nameColumn; + CXSourceRange extent = lc.GetCursorExtent(cursor); + std::uint32_t begin = 0; + std::uint32_t end = 0; + lc.GetFileLocation(lc.GetRangeStart(extent), nullptr, nullptr, nullptr, &begin); + lc.GetFileLocation(lc.GetRangeEnd(extent), nullptr, nullptr, nullptr, &end); + decl.begin = begin; + decl.end = std::min(end, walk.content->size()); + decl.isDefinition = lc.IsCursorDefinition(cursor) != 0; + decl.isStatic = lc.GetStorageClass(cursor) == CX_SC_Static; + decl.isScopedEnum = mapped == LintDeclKind::Enum && lc.EnumDeclIsScoped(cursor) != 0; + // C language linkage: set for anything inside an extern "C" block, which + // is where a C API's spelling is not ours to modernise. + decl.isExternC = lc.GetCursorLanguage(cursor) == CXLanguage_C; + // libclang exposes no constexpr query. The keyword can only appear in + // this declaration's own specifier list, i.e. between the start of its + // extent and its name, so a search bounded to that span is exact rather + // than the line-wide `contains("constexpr ")` it replaces. + if (nameOffset > decl.begin && decl.begin < walk.content->size()) { + std::string_view specifiers(walk.content->data() + decl.begin, std::min(nameOffset - decl.begin, walk.content->size() - decl.begin)); + decl.isConstexpr = specifiers.contains("constexpr"); + } + decl.parent = walk.stack.empty() ? LintNoParent : walk.stack.back(); + + walk.out->push_back(std::move(decl)); + walk.stack.push_back(walk.out->size() - 1); + lc.VisitChildren(cursor, &VisitDecl, &walk); + walk.stack.pop_back(); + return CXChildVisit_Continue; + } + + std::vector WalkDecls(const LibClang& lc, CXTranslationUnit tu, const std::string& content, const fs::path& projectRoot) { + std::vector decls; + DeclWalk walk; + walk.lc = &lc; + walk.content = &content; + walk.projectRoot = &projectRoot; + walk.out = &decls; + lc.VisitChildren(lc.GetTranslationUnitCursor(tu), &VisitDecl, &walk); + // A declaration on an interop boundary makes its parameters and fields + // interop too — the exemption has to cover the whole signature, not + // just the node that happened to name the foreign entity. + for (LintDecl& decl : decls) { + if (decl.parent == LintNoParent) continue; + const LintDecl& parent = decls[decl.parent]; + if (parent.isForeignApi) decl.isForeignApi = true; + if (parent.isExternC) decl.isExternC = true; + } + return decls; + } + + // libclang locates its builtin headers relative to its own install path, + // which need not agree with the clang++ on PATH that produced the PCMs. + // When it disagrees the failure is total and unhelpful — every parse dies + // on "'stddef.h' file not found" — so ask the driver and pass it + // explicitly. Cached like HostTarget, and for the same reason. + const std::string& ClangResourceDir() { + static const std::string Cached = []() -> std::string { + CommandResult r = RunCommandChecked("clang++ -print-resource-dir"); + if (r.exitCode != 0) return {}; + std::string out = std::move(r.output); + while (!out.empty() && (out.back() == '\n' || out.back() == '\r')) out.pop_back(); + return out; + }(); + return Cached; + } + + struct AstResult { + std::vector decls; + std::string error; // empty exactly on success + }; + + // Parse `content` as `file` with the flags that actually built its PCMs. + // + // Unlike the tokenizer this cannot tolerate a failed parse: a fatal + // diagnostic leaves a fragment, and a fragment is indistinguishable from a + // file that declares nothing. So a fatal is returned as an error for the + // driver to surface, never as an empty declaration list. + AstResult ParseAst(const fs::path& file, const std::string& content, std::string_view compileCommand, const fs::path& projectRoot, std::span tokens) { + AstResult result; + std::string_view language = LexLanguage(file); + if (language.empty() || language == "c") { + result.error = std::format("{} is not a C++ translation unit", file.filename().string()); + return result; + } + if (compileCommand.empty()) { + result.error = "no compile command is known for this file"; + return result; + } + const LibClang& lc = Clang(); + if (!lc.handle) { + result.error = lc.error; + return result; + } + + // Byte-length preserving, so cursor line/column land on the original. + std::string buffer = BlankExportKeywords(content, tokens); + std::string path = file.string(); + std::vector args = CommandToArgs(compileCommand); + // Required: libclang will not infer a module interface unit from the + // .cppm extension, and silently treats every flag as a linker input if + // left to guess. + args.push_back(std::format("-x{}", language)); + if (!ClangResourceDir().empty()) args.push_back(std::format("-resource-dir={}", ClangResourceDir())); + std::vector argv; + argv.reserve(args.size()); + for (const std::string& arg : args) argv.push_back(arg.c_str()); + + CXUnsavedFile unsaved{}; + unsaved.Filename = path.c_str(); + unsaved.Contents = buffer.data(); + unsaved.Length = static_cast(buffer.size()); + + CXIndex index = lc.CreateIndex(0, 0); + if (!index) { + result.error = "clang_createIndex failed"; + return result; + } + CXTranslationUnit tu = lc.ParseTranslationUnit(index, path.c_str(), argv.data(), static_cast(argv.size()), &unsaved, 1, CXTranslationUnit_None); + if (!tu) { + lc.DisposeIndex(index); + result.error = "clang could not create a translation unit"; + return result; + } + + std::string fatal; + std::uint32_t diagnostics = lc.GetNumDiagnostics(tu); + for (std::uint32_t i = 0; i < diagnostics && fatal.empty(); ++i) { + CXDiagnostic diagnostic = lc.GetDiagnostic(tu, i); + if (lc.GetDiagnosticSeverity(diagnostic) == CXDiagnostic_Fatal) { + fatal = TakeString(lc, lc.GetDiagnosticSpelling(diagnostic)); + } + lc.DisposeDiagnostic(diagnostic); + } + if (fatal.empty()) { + result.decls = WalkDecls(lc, tu, buffer, projectRoot); + } else { + result.error = std::move(fatal); + } + lc.DisposeTranslationUnit(tu); + lc.DisposeIndex(index); + return result; + } + // Blank comments and the bodies of string/character literals to spaces, // copying '\n' through so byte offsets and line numbers in the result // match the original text exactly. @@ -298,26 +651,37 @@ namespace { return local; } - void CollectConfigSources(const Configuration& c, std::set& files) { + // Maps each source to the Configuration that owns it. std::map keeps the + // deterministic sorted iteration the old std::set gave, and the value is + // what the AST layer needs: PCMs are flag-locked, so a file parsed with a + // sibling configuration's flags does not parse at all. Three regimes are in + // play — the library/executable, each declared test (which carries its own + // target, defines and -march via the march fan-out), and project.cpp. + using SourceOwners = std::map; + + void CollectConfigSources(const Configuration& c, SourceOwners& files) { + // First owner wins, matching the root-first rule dedup: a source + // reachable through two configurations parses with the nearer one. + auto own = [&files, &c](fs::path file) { files.emplace(std::move(file), &c); }; for (const std::unique_ptr& mod : c.interfaces) { - files.insert(fs::path(std::format("{}.cppm", mod->path.string()))); + own(fs::path(std::format("{}.cppm", mod->path.string()))); for (const std::unique_ptr& part : mod->partitions) { - files.insert(fs::path(std::format("{}.cppm", part->path.string()))); + own(fs::path(std::format("{}.cppm", part->path.string()))); } } for (const Implementation& impl : c.implementations) { - files.insert(fs::path(std::format("{}.cpp", impl.path.string()))); + own(fs::path(std::format("{}.cpp", impl.path.string()))); } // cFiles/cuda resolve against cwd at build time (see Build's compile // loops); mirror that here. for (const fs::path& cf : c.cFiles) { - files.insert(fs::absolute(fs::path(std::format("{}.c", cf.string()))).lexically_normal()); + own(fs::absolute(fs::path(std::format("{}.c", cf.string()))).lexically_normal()); } for (const fs::path& cu : c.cuda) { - files.insert(fs::absolute(fs::path(std::format("{}.cu", cu.string()))).lexically_normal()); + own(fs::absolute(fs::path(std::format("{}.cu", cu.string()))).lexically_normal()); } for (const Shader& shader : c.shaders) { - files.insert(fs::absolute(shader.path).lexically_normal()); + own(fs::absolute(shader.path).lexically_normal()); } // files/buildFiles/assets are deliberately excluded: data shipped or // referenced by the build, not source code. @@ -379,6 +743,25 @@ bool LintContext::LineHasMultiLineToken(std::size_t line) { return line >= 1 && line <= spannedLineCache->size() && (*spannedLineCache)[line - 1]; } +std::span LintContext::Decls() { + if (!declCache) { + AstResult parsed = ParseAst(file, content, compileCommand, projectRoot, Tokens()); + astReason = std::move(parsed.error); + declCache = std::move(parsed.decls); + } + return *declCache; +} + +bool LintContext::AstAvailable() { + Decls(); + return astReason.empty(); +} + +std::string_view LintContext::AstUnavailableReason() { + Decls(); + return astReason; +} + void LintContext::Report(std::size_t line, std::string message) { sink->push_back({file, line, activeRule, std::move(message)}); } @@ -443,10 +826,16 @@ void LintContext::SetContent(std::string newContent) { suppressionsCache.reset(); // line numbers may have shifted — re-parse tokenCache.reset(); // offsets refer to the old buffer — re-lex spannedLineCache.reset(); // derived from tokenCache + declCache.reset(); // extents refer to the old buffer — re-parse + astReason.clear(); } void Configuration::AddLintRule(std::string name, std::function check) { - lintRules.push_back({std::move(name), std::move(check)}); + lintRules.push_back({std::move(name), std::move(check), false}); +} + +void Configuration::AddAstLintRule(std::string name, std::function check) { + lintRules.push_back({std::move(name), std::move(check), true}); } LintSummary Crafter::RunLint(Configuration& projectCfg, const RunLintOptions& opts) { @@ -507,19 +896,52 @@ format` applies it to disk, and `crafter-build lint` reports where it would. return summary; } - std::set files; + SourceOwners files; for (Configuration* c : localConfigs) { CollectConfigSources(*c, files); for (const Test& t : c->tests) CollectConfigSources(t.config, files); } - if (!opts.projectFile.empty()) files.insert(opts.projectFile); + // project.cpp is not built by Build() at all — LoadProject compiles it + // against the host PCM cache with its own flags, so it is owned by nothing + // here and gets no compile command. Token rules still cover it. + if (!opts.projectFile.empty()) files.emplace(opts.projectFile, nullptr); fs::path cwd = fs::current_path(); auto shown = [&cwd](const fs::path& p) { return PathInsideRoot(p, cwd) ? p.lexically_relative(cwd) : p; }; - for (const fs::path& file : files) { + // Any rule reading Decls() needs this configuration's module PCMs, and a + // 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 + // command per configuration — GetCompileCommand walks the dependency tree. + const bool anyRuleNeedsAst = std::ranges::any_of(rules, [](const LintRule* r) { return r->needsAst; }); + std::unordered_map commands; + if (anyRuleNeedsAst && !opts.noAst) { + for (Configuration* c : localConfigs) { + auto ensure = [&](const Configuration& cfg) { + if (commands.contains(&cfg)) return; + std::string command; + try { + CompileCommand assembled = GetCompileCommand(cfg); + command = assembled.command; + if (!fs::exists(assembled.stdPcmDir/"std.pcm")) { + Progress::Task task(std::format("Building std PCM ({}-{})", cfg.target, cfg.march)); + fs::create_directories(assembled.stdPcmDir); + std::string error = BuildStdPcm(cfg, assembled.stdPcmDir/"std.pcm"); + if (!error.empty()) command.clear(); + } + } catch (const std::exception&) { + command.clear(); // surfaced per file as an AST reason + } + commands.emplace(&cfg, std::move(command)); + }; + ensure(*c); + for (const Test& t : c->tests) ensure(t.config); + } + } + + for (const auto& [file, owner] : files) { std::ifstream in(file, std::ios::binary); if (!in) continue; // config parse already read it; a vanished file fails the build first std::stringstream buffer; @@ -529,10 +951,24 @@ format` applies it to disk, and `crafter-build lint` reports where it would. ctx.content = std::move(buffer).str(); ctx.lines = SplitLines(ctx.content); ctx.sink = &summary.findings; + ctx.projectRoot = projectRoot; + if (auto it = commands.find(owner); it != commands.end()) ctx.compileCommand = it->second; ++summary.filesLinted; const std::string original = ctx.content; for (const LintRule* rule : rules) { ctx.activeRule = rule->name; + // A semantic rule that cannot see an AST would report nothing, + // which is indistinguishable from a clean file — and for a + // transform it would mean rewriting without the information that + // decides what is safe to touch. Skip it and make the run fail. + if (rule->needsAst) { + if (opts.noAst) continue; + if (!ctx.AstAvailable()) { + ctx.Report(0, std::format("rule '{}' needs an AST, which is unavailable: {}", rule->name, ctx.AstUnavailableReason())); + ++summary.errors; + continue; + } + } // Snapshot for transform diffing — and the revert point if the // rule throws, so a half-applied transform never reaches disk // and chained rules see clean input. diff --git a/interfaces/Crafter.Build-Clang.cppm b/interfaces/Crafter.Build-Clang.cppm index fc23617..9b1e19c 100644 --- a/interfaces/Crafter.Build-Clang.cppm +++ b/interfaces/Crafter.Build-Clang.cppm @@ -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(-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 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* 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> 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> 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 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 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 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 diff --git a/interfaces/Crafter.Build-Lint.cppm b/interfaces/Crafter.Build-Lint.cppm index 7d45b3f..a25e350 100644 --- a/interfaces/Crafter.Build-Lint.cppm +++ b/interfaces/Crafter.Build-Lint.cppm @@ -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 diff --git a/tests/Lint/fixture/ExportNamespace.cppm.in b/tests/Lint/fixture/ExportNamespace.cppm.in new file mode 100644 index 0000000..bad20c2 --- /dev/null +++ b/tests/Lint/fixture/ExportNamespace.cppm.in @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: LGPL-3.0-only +// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts® + +// Fixture for the AST layer's export-blanking pass, copied to a scratch dir as +// Demo.cppm by tests/Lint. The .cppm.in suffix keeps the build's module-import +// scanner from treating it as a real interface of this project — the same +// convention tests/IncrementalInterfaceChange uses. +// +// libclang maps a C++20 export declaration to a childless +// CXCursor_UnexposedDecl and does not descend into it, so every declaration +// below is invisible unless the `export` keywords are blanked first. Line +// numbers are asserted, so do not reflow this file. +export module Demo; + +export namespace Demo { + enum class Mode { On, Off }; + struct Widget { + int count; + }; + export int Exported = 1; +} diff --git a/tests/Lint/main.cpp b/tests/Lint/main.cpp index fa81ec4..82e61a0 100644 --- a/tests/Lint/main.cpp +++ b/tests/Lint/main.cpp @@ -555,6 +555,166 @@ int main() { RunLint(cfg, Mode(LintMode::Report)); } + // ---------------- AST layer ---------------- + // + // A standalone source with no imports, so it parses without this project's + // PCMs and the case stays a unit test. + { + constexpr std::string_view Source = + "#include \n" // 1 + "namespace Demo {\n" // 2 + " enum class Scoped { A, B };\n" // 3 + " enum Plain { C, D };\n" // 4 + " struct Widget {\n" // 5 + " int count;\n" // 6 + " std::string name;\n" // 7 + " };\n" // 8 + " static int GlobalCounter = 0;\n" // 9 + " constexpr int Limit = 10;\n" // 10 + " int Compute(int input) {\n" // 11 + " int local = input;\n" // 12 + " return local;\n" // 13 + " }\n" // 14 + "}\n"; // 15 + + Scratch s("ast"); + s.Write("f", Source); + Configuration cfg = s.Config({"f"}); + cfg.AddAstLintRule("ast", [](LintContext& ctx) { + Check(ctx.AstAvailable(), std::format("ast: parse succeeded ({})", ctx.AstUnavailableReason())); + std::span decls = ctx.Decls(); + Check(!decls.empty(), "ast: declarations found"); + + auto find = [&](LintDeclKind kind, std::string_view name) -> const LintDecl* { + auto it = std::ranges::find_if(decls, [&](const LintDecl& d) { return d.kind == kind && d.name == name; }); + return it == decls.end() ? nullptr : &*it; + }; + auto parentOf = [&](const LintDecl& d) -> const LintDecl* { + return d.parent == LintNoParent ? nullptr : &decls[d.parent]; + }; + + // Only this file's declarations: drags in thousands and + // none of them may appear here. + Check(std::ranges::none_of(decls, [](const LintDecl& d) { return d.name == "basic_string"; }), "ast: declarations from #included headers are excluded"); + + const LintDecl* demo = find(LintDeclKind::Namespace, "Demo"); + Check(demo != nullptr && demo->line == 2, "ast: namespace found at its own line"); + + // The whole point for enum-class: an exact query, not a regex. + const LintDecl* scoped = find(LintDeclKind::Enum, "Scoped"); + const LintDecl* plain = find(LintDeclKind::Enum, "Plain"); + Check(scoped != nullptr && scoped->isScopedEnum, "ast: enum class is scoped"); + Check(plain != nullptr && !plain->isScopedEnum, "ast: plain enum is not scoped"); + Check(scoped != nullptr && parentOf(*scoped) == demo, "ast: enum's parent is the namespace"); + + // The whole point for naming: scope without a brace stack. + const LintDecl* widget = find(LintDeclKind::Struct, "Widget"); + const LintDecl* count = find(LintDeclKind::Field, "count"); + Check(widget != nullptr, "ast: struct found"); + Check(count != nullptr && parentOf(*count) == widget, "ast: field's parent is its struct"); + Check(count != nullptr && count->type == "int", "ast: field carries a resolved type"); + const LintDecl* name = find(LintDeclKind::Field, "name"); + Check(name != nullptr && name->type.contains("string"), "ast: library type resolves"); + + const LintDecl* global = find(LintDeclKind::Variable, "GlobalCounter"); + Check(global != nullptr && global->isStatic, "ast: static storage class reported"); + const LintDecl* limit = find(LintDeclKind::Variable, "Limit"); + Check(limit != nullptr && limit->isConstexpr, "ast: constexpr reported"); + Check(global != nullptr && !global->isConstexpr, "ast: non-constexpr not misreported"); + + const LintDecl* compute = find(LintDeclKind::Function, "Compute"); + const LintDecl* local = find(LintDeclKind::Variable, "local"); + const LintDecl* input = find(LintDeclKind::Parameter, "input"); + Check(compute != nullptr && compute->isDefinition, "ast: function definition reported"); + Check(input != nullptr && parentOf(*input) == compute, "ast: parameter's parent is its function"); + // A local's parent is the function, not the namespace — which is + // exactly the distinction the brace stack was approximating. + Check(local != nullptr && parentOf(*local) == compute, "ast: local's parent is its function"); + Check(global != nullptr && parentOf(*global) == demo, "ast: namespace-scope variable's parent is the namespace"); + + // Extents must address the declaration's own bytes so a transform + // can mark a region untouchable. + Check(widget != nullptr && widget->end > widget->begin && widget->end <= ctx.content.size(), "ast: extent is in range"); + if (count != nullptr) { + Check(std::string_view(ctx.content).substr(count->begin, count->end - count->begin) == "int count", "ast: extent brackets exactly the declaration"); + } + }); + RunLint(cfg, Mode(LintMode::Report)); + } + + // A module interface unit. libclang reports `export namespace X { … }` as a + // childless CXCursor_UnexposedDecl and refuses to descend, so without the + // export-blanking pass every declaration in it would be invisible — which + // is five of this repo's own interfaces, 677 lines. Blanking the keyword is + // byte-length preserving, so the lines reported here must match the file. + // + // The source lives in fixture/ExportNamespace.cppm.in rather than inline: + // an `export module` spelled in this file's own text would be picked up by + // the build's module scanner as a real interface of this project. + { + Scratch s("ast-module"); + fs::copy_file(fs::current_path() / "tests" / "Lint" / "fixture" / "ExportNamespace.cppm.in", s.dir / "Demo.cppm", fs::copy_options::overwrite_existing); + Configuration cfg; + cfg.path = s.dir; + cfg.name = "ast-module"; + cfg.outputName = "ast-module"; + cfg.target = HostTarget(); + std::array ifaces = { "Demo" }; + std::array impls = {}; + cfg.GetInterfacesAndImplementations(ifaces, impls); + cfg.AddAstLintRule("ast-module", [](LintContext& ctx) { + Check(ctx.AstAvailable(), std::format("ast-module: parse succeeded ({})", ctx.AstUnavailableReason())); + std::span decls = ctx.Decls(); + auto find = [&](LintDeclKind kind, std::string_view name) -> const LintDecl* { + auto it = std::ranges::find_if(decls, [&](const LintDecl& d) { return d.kind == kind && d.name == name; }); + return it == decls.end() ? nullptr : &*it; + }; + const LintDecl* ns = find(LintDeclKind::Namespace, "Demo"); + const LintDecl* mode = find(LintDeclKind::Enum, "Mode"); + const LintDecl* widget = find(LintDeclKind::Struct, "Widget"); + const LintDecl* count = find(LintDeclKind::Field, "count"); + const LintDecl* exported = find(LintDeclKind::Variable, "Exported"); + Check(ns != nullptr, "ast-module: descends into export namespace"); + Check(mode != nullptr && mode->isScopedEnum, "ast-module: enum inside export namespace is visible"); + Check(widget != nullptr && count != nullptr, "ast-module: struct and field inside export namespace are visible"); + Check(exported != nullptr, "ast-module: per-declaration export is visible"); + // Byte fidelity: blanking must not shift a single line. + Check(ns != nullptr && ns->line == 15, "ast-module: namespace line matches the unblanked file"); + Check(mode != nullptr && mode->line == 16, "ast-module: enum line matches"); + Check(count != nullptr && count->line == 18, "ast-module: field line matches"); + Check(exported != nullptr && exported->line == 20, "ast-module: exported variable line matches"); + }); + RunLint(cfg, Mode(LintMode::Report)); + } + + // An unavailable AST must fail the run, never look like a clean file. A + // module unit with no PCMs is the realistic way to hit this. + { + Scratch s("ast-unavailable"); + s.Write("f", "import Crafter.DefinitelyNotAModule;\nint Value = 1;\n"); + Configuration cfg = s.Config({"f"}); + bool ran = false; + cfg.AddAstLintRule("needs-ast", [&ran](LintContext&) { ran = true; }); + LintSummary summary = RunLint(cfg, Mode(LintMode::Report)); + Check(!ran, "ast: rule is skipped when the AST is unavailable"); + Check(summary.errors > 0, "ast: unavailable AST counts as an error"); + 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"); + } + + // --no-ast skips those rules deliberately and exits normally. + { + Scratch s("ast-optout"); + s.Write("f", "import Crafter.DefinitelyNotAModule;\nint Value = 1;\n"); + Configuration cfg = s.Config({"f"}); + cfg.AddAstLintRule("needs-ast", [](LintContext& ctx) { ctx.Report(1, "should not run"); }); + RunLintOptions opts = Mode(LintMode::Report); + opts.noAst = true; + LintSummary summary = RunLint(cfg, opts); + Check(summary.errors == 0, "ast: --no-ast does not error"); + Check(summary.Clean(), "ast: --no-ast run is Clean"); + } + if (Failures > 0) { std::println(std::cerr, "{} assertions failed", Failures); return 1;