diff --git a/.forgejo/workflows/ci.yaml b/.forgejo/workflows/ci.yaml index 0c86de3..ce8eedf 100644 --- a/.forgejo/workflows/ci.yaml +++ b/.forgejo/workflows/ci.yaml @@ -35,7 +35,7 @@ jobs: pacman -Sy --noconfirm --needed archlinux-keyring pacman -Syu --noconfirm --needed \ base-devel git zip tar jq \ - clang lld libc++ cmake \ + clang llvm lld libc++ cmake \ mingw-w64-gcc \ wasi-libc wasi-libc++ wasi-libc++abi wasi-compiler-rt \ nodejs diff --git a/implementations/Crafter.Build-Clang.cpp b/implementations/Crafter.Build-Clang.cpp index 3599f3c..4e7a8f9 100644 --- a/implementations/Crafter.Build-Clang.cpp +++ b/implementations/Crafter.Build-Clang.cpp @@ -204,7 +204,7 @@ void Configuration::GetInterfacesAndImplementations(std::span interfac fileCopy.replace_extension(""); Implementation& implementation = this->implementations.emplace_back(std::move(fileCopy)); if (std::regex_search(fileContent, match, std::regex(R"(module ([a-zA-Z0-9_\.\-]+)(:[a-zA-Z0-9_\.\-]+)?\s*;)"))) { - bool isPartitionImpl = match[2].length() > 0; + const bool isPartitionImpl = match[2].length() > 0; for(const std::unique_ptr& interface : this->interfaces) { if(interface->name == match[1]) { if (!isPartitionImpl) { @@ -252,6 +252,161 @@ void Configuration::GetInterfacesAndImplementations(std::span interfac } } +fs::path Crafter::StdPcmDir(const Configuration& config) { + // The std PCM is cached per target+march, but a wasm variant pass compiles + // it with extra codegen flags (e.g. -mrelaxed-simd) — its BMI must not be + // shared with the baseline's, or the consuming TUs see a target-feature + // mismatch. Suffix the cache dir with the variant flags when present. + std::string stdPcmKey = std::format("{}-{}", config.target, config.march); + for (const std::string& f : config.wasmVariantFlags) { + stdPcmKey += "+"; + for (char c : f) stdPcmKey += (c == '/' || c == '\\') ? '_' : c; + } + return GetCacheDir()/stdPcmKey; +} + +CompileCommand Crafter::GetCompileCommand(const Configuration& config) { + CompileCommand out; + out.stdPcmDir = StdPcmDir(config); + out.pcmDir = config.PcmDir(); + + std::string editedTarget = config.target; + std::replace(editedTarget.begin(), editedTarget.end(), '-', '_'); + + // wasm32 targets reject -march and silently ignore -mtune (clang errors on + // the former). Skip both for any wasm32-* triple. + const bool isWasm = config.target.starts_with("wasm32"); + std::string archFlags = isWasm + ? std::string() + : std::format(" -march={} -mtune={}", config.march, config.mtune); + out.command = std::format("{} --target={}{} -std=c++26 -D CRAFTER_BUILD_CONFIGURATION_TARGET=\\\"{}\\\" -D CRAFTER_BUILD_CONFIGURATION_TARGET_{} -fprebuilt-module-path={} -fprebuilt-module-path={}", GetBaseCommand(config), config.target, archFlags, editedTarget, editedTarget, out.stdPcmDir.string(), out.pcmDir.string()); + + if (!config.sysroot.empty()) { + out.command += std::format(" --sysroot={}", config.sysroot); + } + if (isWasm) { + // -mllvm is consumed by codegen but not the link driver, which is the + // same command line; quiet the unused-flag warning rather than split + // compile and link commands. + out.command += " -fno-exceptions -msimd128 -fno-c++-static-destructors -mllvm -wasm-enable-sjlj -D_WASI_EMULATED_SIGNAL -Wno-unused-command-line-argument"; + // Active variant pass (see the variant driver at the end of Build): + // extra codegen flags (e.g. -mrelaxed-simd) applied to every TU. Empty + // for the baseline build. Part of VariantId, so these objects/PCMs + // land in their own dir. + for (const std::string& f : config.wasmVariantFlags) { + out.command += std::format(" {}", f); + } + } + if (config.target == "x86_64-w64-mingw32") { + // mingw libstdc++ defines TLS via __emutls_v.* (emulated TLS); without + // -femulated-tls clang generates native-TLS references that don't + // match. Symptom: undefined std::__once_callable / __once_call at + // link time. Also -Wno-unused… because -femulated-tls is a codegen + // flag the link driver doesn't consume. + out.command += " -femulated-tls -Wno-unused-command-line-argument"; + } + + if(config.type == ConfigurationType::LibraryDynamic) { + #ifdef CRAFTER_BUILD_CONFIGURATION_TARGET_x86_64_pc_linux_gnu + out.command += " -fPIC -D CRAFTER_BUILD_CONFIGURATION_TYPE_SHARED_LIBRARY"; + #endif + #if defined(CRAFTER_BUILD_CONFIGURATION_TARGET_x86_64_pc_windows_msvc) || defined(CRAFTER_BUILD_CONFIGURATION_TARGET_x86_64_w64_mingw32) + out.command += " -D CRAFTER_BUILD_CONFIGURATION_TYPE_SHARED_LIBRARY"; + #endif + } else if(config.type == ConfigurationType::Executable) { + out.command += " -D CRAFTER_BUILD_CONFIGURATION_TYPE_EXECUTABLE"; + // On Windows targets the API uses __declspec(dllimport) when consuming + // a DLL. Set the macro for executables so CRAFTER_API resolves to + // dllimport in their PCM cache (separate from the lib's PCM cache, + // which gets dllexport). Harmless if the exe doesn't actually link a + // crafter DLL — CRAFTER_API only matters at API call sites. + if (config.target == "x86_64-w64-mingw32" || config.target == "x86_64-pc-windows-msvc") { + out.command += " -D CRAFTER_BUILD_DLL_IMPORT"; + } + } else { + out.command += " -D CRAFTER_BUILD_CONFIGURATION_TYPE_LIBRARY"; + } + + // -I propagation that's valid for both C and C++ compiles. Module-only + // bits (-fprebuilt-module-path) stay on `command` only. + { + std::unordered_set seen; + std::function addFlags = [&](Configuration* dep) { + if (!seen.insert(dep).second) return; + for (const auto& entry : fs::recursive_directory_iterator(dep->path)) { + if (entry.is_directory() && entry.path().filename() == "include") { + out.includeFlags += std::format(" -I{}", entry.path().string()); + } + } + out.includeFlags += std::format(" -I{}", dep->path.string()); + out.command += std::format(" -fprebuilt-module-path={}", dep->PcmDir().string()); + for (Configuration* sub : dep->dependencies) { + addFlags(sub); + } + }; + for (Configuration* dep : config.dependencies) { + addFlags(dep); + } + } + out.command += out.includeFlags; + + // External dependency includes, gathered transitively. Kept off `command` + // so it stays exactly what it was before this was extracted. + { + std::unordered_set seen; + std::function addExternal = [&](const Configuration* cfg) { + if (!seen.insert(cfg).second) return; + for (const ExternalDependency& dep : cfg->externalDependencies) { + for (const std::string& flag : ExternalIncludeFlags(dep, cfg->target)) { + out.externalIncludeFlags += std::format(" {}", flag); + } + } + for (const Configuration* sub : cfg->dependencies) addExternal(sub); + }; + addExternal(&config); + } + + // Defines belong on both C and C++ compiles so vendored C dependencies + // can see configuration-level macros consistently with module sources. + for(const Define& define : config.defines) { + if(define.value.empty()) { + out.defineFlags += std::format(" -D {}", define.name); + } else { + out.defineFlags += std::format(" -D {}={}", define.name, define.value); + } + } + out.command += out.defineFlags; + + // Track caller-provided compileFlags separately so the .c compile can + // pick them up too (vendored C deps usually need -I from this set). + for(const std::string& flag : config.compileFlags) { + out.userFlags += std::format(" {}", flag); + } + out.command += out.userFlags; + + // Behaviour-neutral release performance for the binaries we emit: ThinLTO + // (cross-TU inlining) plus dead-section GC and safe identical-code folding. + // Default-on in Release, never Debug (keeps builds fast and debuggable). + // Excluded for wasm32 — its -mllvm/sjlj codegen flags don't compose with + // LTO here — and for nvcc .cu objects below (they can't emit LLVM bitcode, + // so they link in as ordinary objects alongside the bitcode ones). ThinLTO + // rather than monolithic -flto so link time and memory scale with arbitrary + // user project sizes. Compile-side flags (-flto, -ffunction/-fdata-sections) + // ride on `command`, which is reused as the link base; the link-only -Wl + // flags go on linkExtras so they don't warn on each -c compile. + out.useLto = !config.debug && !isWasm; + out.ltoCompileFlags = out.useLto ? " -flto=thin -ffunction-sections -fdata-sections" : ""; + out.ltoLinkFlags = out.useLto ? " -flto=thin -Wl,--gc-sections -Wl,--icf=safe" : ""; + + if(config.debug) { + out.command += " -g -D CRAFTER_BUILD_CONFIGURATION_DEBUG"; + } else { + out.command += " -O3"; + } + out.command += out.ltoCompileFlags; + return out; +} + BuildResult Crafter::Build(Configuration& config, std::unordered_map>& depResults, std::mutex& depMutex) { // Reset per-build cached state on every Module/ModulePartition so that // successive Build() calls on the same Configuration re-evaluate mtimes @@ -499,16 +654,7 @@ BuildResult Crafter::Build(Configuration& config, std::unordered_map libSet; @@ -601,30 +700,6 @@ BuildResult Crafter::Build(Configuration& config, std::unordered_map repack(false); - // -I propagation that's valid for both C and C++ compiles. Module-only - // bits (-fprebuilt-module-path) stay on `command` only. - std::string includeFlags; - { - std::unordered_set seen; - std::function addFlags = [&](Configuration* dep) { - if (!seen.insert(dep).second) return; - for (const auto& entry : fs::recursive_directory_iterator(dep->path)) { - if (entry.is_directory() && entry.path().filename() == "include") { - includeFlags += std::format(" -I{}", entry.path().string()); - } - } - includeFlags += std::format(" -I{}", dep->path.string()); - command += std::format(" -fprebuilt-module-path={}", dep->PcmDir().string()); - for (Configuration* sub : dep->dependencies) { - addFlags(sub); - } - }; - for (Configuration* dep : config.dependencies) { - addFlags(dep); - } - } - command += includeFlags; - for(Configuration* dep : config.dependencies) { depThreads.emplace_back([&, dep](){ try { @@ -681,50 +756,9 @@ BuildResult Crafter::Build(Configuration& config, std::unordered_map 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 @@ -1595,7 +1637,8 @@ Exit status: )", argv0); } -int Crafter::Run(int argc, char** argv) { +// lint-disable-next-line no-char-pointer +std::int32_t Crafter::Run(std::int32_t argc, char** argv) { try { std::string_view argv0 = argc > 0 ? argv[0] : "crafter-build"; fs::path projectFile = "./project.cpp"; @@ -1611,7 +1654,7 @@ int Crafter::Run(int argc, char** argv) { RunLintOptions lintOpts; Progress::Verbosity verbosity = Progress::Verbosity::Default; - for (int i = 1; i < argc; ++i) { + for (std::int32_t i = 1; i < argc; ++i) { std::string_view arg = argv[i]; if (arg == "-h" || arg == "--help" || (!runTests && !runLint && !runFormat && arg == "help")) { PrintHelp(argv0); @@ -1647,6 +1690,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 { @@ -1732,7 +1777,7 @@ int Crafter::Run(int argc, char** argv) { // server (browser build with index.html). std::system on the // .wasm path goes nowhere useful — replace with detection. if (config.target.starts_with("wasm32")) { - bool browserBuild = fs::exists(absDir / "index.html"); + const bool browserBuild = fs::exists(absDir / "index.html"); auto have = [](std::string_view exe) { #ifdef _WIN32 std::string probe = std::format("where {} > NUL 2>&1", exe); diff --git a/implementations/Crafter.Build-External.cpp b/implementations/Crafter.Build-External.cpp index fb2ed44..d75aaf6 100644 --- a/implementations/Crafter.Build-External.cpp +++ b/implementations/Crafter.Build-External.cpp @@ -206,6 +206,25 @@ std::string BuildCMake(const fs::path& cmakeBuildDir) { } // namespace +fs::path Crafter::ExternalCloneDir(const ExternalDependency& dep, std::string_view target) { + std::string name = dep.name.empty() ? DeriveName(dep.source) : dep.name; + if (name.empty()) return {}; + std::string keyMaterial = std::format("{}|{}|{}|{}|{}", dep.source.url, dep.source.branch, dep.source.commit, JoinOptions(dep.options), target); + std::size_t key = std::hash{}(keyMaterial); + return GetCacheDir() / "external" / std::format("{}-{:016x}", name, key); +} + +std::vector Crafter::ExternalIncludeFlags(const ExternalDependency& dep, std::string_view target) { + std::vector flags; + fs::path cloneDir = ExternalCloneDir(dep, target); + if (cloneDir.empty()) return flags; + for (const fs::path& include : dep.includeDirs) { + fs::path full = include.empty() ? cloneDir : cloneDir / include; + flags.push_back(std::format("-I{}", fs::absolute(full).string())); + } + return flags; +} + ExternalBuildResult Crafter::BuildExternal(const ExternalDependency& dep, std::string_view target, std::atomic& cancelled) { ExternalBuildResult result; @@ -227,9 +246,7 @@ ExternalBuildResult Crafter::BuildExternal(const ExternalDependency& dep, std::s } } - std::string keyMaterial = std::format("{}|{}|{}|{}|{}", dep.source.url, dep.source.branch, dep.source.commit, JoinOptions(dep.options), target); - std::size_t key = std::hash{}(keyMaterial); - fs::path cloneDir = externalRoot / std::format("{}-{:016x}", name, key); + fs::path cloneDir = ExternalCloneDir(dep, target); std::string fetchErr = FetchGit(dep.source, cloneDir); if (!fetchErr.empty()) { @@ -253,10 +270,7 @@ ExternalBuildResult Crafter::BuildExternal(const ExternalDependency& dep, std::s } } - for (const fs::path& include : dep.includeDirs) { - fs::path full = include.empty() ? cloneDir : cloneDir / include; - result.compileFlags.push_back(std::format("-I{}", fs::absolute(full).string())); - } + result.compileFlags = ExternalIncludeFlags(dep, target); if (dep.builder == ExternalBuilder::CMake) { // Each search path gets both a -L (link-time) and a -Wl,-rpath @@ -265,7 +279,7 @@ ExternalBuildResult Crafter::BuildExternal(const ExternalDependency& dep, std::s // LD_LIBRARY_PATH gymnastics. Static deps (.a) ignore the rpath // harmlessly. PE/COFF targets (mingw, msvc) resolve DLLs via PATH // or adjacency rather than rpath, so lld warns if we pass it. - bool isPe = target == "x86_64-w64-mingw32" || target == "x86_64-pc-windows-msvc"; + const bool isPe = target == "x86_64-w64-mingw32" || target == "x86_64-pc-windows-msvc"; std::string buildDirAbs = fs::absolute(cmakeBuildDir).string(); result.linkFlags.push_back(std::format("-L{}", buildDirAbs)); if (!isPe) result.linkFlags.push_back(std::format("-Wl,-rpath,{}", buildDirAbs)); @@ -361,7 +375,7 @@ Configuration* Crafter::GitProject(const GitProjectSpec& spec) { fs::path cloneDir = externalRoot / std::format("{}-{}", name, srcHash); { - bool exists = fs::exists(cloneDir); + const bool exists = fs::exists(cloneDir); Progress::Task task(std::format("{} {}", exists ? "Updating" : "Cloning", name)); if (std::string err = FetchGit(spec.source, cloneDir); !err.empty()) { throw std::runtime_error(std::format("GitProject({}): {}", spec.source.url, err)); @@ -402,7 +416,7 @@ fs::path Crafter::GitFetch(const GitSource& source) { fs::path cloneDir = externalRoot / std::format("{}-{:016x}", name, key); { - bool exists = fs::exists(cloneDir); + const bool exists = fs::exists(cloneDir); Progress::Task task(std::format("{} {}", exists ? "Updating" : "Cloning", name)); if (std::string err = FetchGit(source, cloneDir); !err.empty()) { throw std::runtime_error(std::format("GitFetch({}): {}", source.url, err)); diff --git a/implementations/Crafter.Build-Lint.cpp b/implementations/Crafter.Build-Lint.cpp index 8571e29..72cf5f3 100644 --- a/implementations/Crafter.Build-Lint.cpp +++ b/implementations/Crafter.Build-Lint.cpp @@ -50,20 +50,55 @@ namespace { LibHandle handle = nullptr; std::string error; // non-empty exactly when handle is null - decltype(&clang_createIndex) CreateIndex = nullptr; - decltype(&clang_disposeIndex) DisposeIndex = nullptr; - decltype(&clang_parseTranslationUnit) ParseTranslationUnit = nullptr; - decltype(&clang_disposeTranslationUnit) DisposeTranslationUnit = nullptr; - decltype(&clang_getFile) GetFile = nullptr; - decltype(&clang_getLocationForOffset) GetLocationForOffset = nullptr; - decltype(&clang_getRange) GetRange = nullptr; - decltype(&clang_getRangeStart) GetRangeStart = nullptr; - decltype(&clang_getRangeEnd) GetRangeEnd = nullptr; - decltype(&clang_getFileLocation) GetFileLocation = nullptr; - decltype(&clang_tokenize) Tokenize = nullptr; - decltype(&clang_disposeTokens) DisposeTokens = nullptr; - decltype(&clang_getTokenKind) GetTokenKind = nullptr; - decltype(&clang_getTokenExtent) GetTokenExtent = nullptr; + decltype(&clang_createIndex) createIndex = nullptr; + decltype(&clang_disposeIndex) disposeIndex = nullptr; + decltype(&clang_parseTranslationUnit) parseTranslationUnit = nullptr; + decltype(&clang_disposeTranslationUnit) disposeTranslationUnit = nullptr; + decltype(&clang_getFile) getFile = nullptr; + decltype(&clang_getLocationForOffset) getLocationForOffset = nullptr; + decltype(&clang_getRange) getRange = nullptr; + decltype(&clang_getRangeStart) getRangeStart = nullptr; + decltype(&clang_getRangeEnd) getRangeEnd = nullptr; + decltype(&clang_getFileLocation) getFileLocation = nullptr; + decltype(&clang_tokenize) tokenize = nullptr; + 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_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; + decltype(&clang_getResultType) getResultType = nullptr; + decltype(&clang_Cursor_getBinaryOpcode) getBinaryOpcode = nullptr; + decltype(&clang_getCursorUnaryOperatorKind) getUnaryOperatorKind = nullptr; + decltype(&clang_isConstQualifiedType) isConstQualifiedType = nullptr; + decltype(&clang_CXXMethod_isConst) methodIsConst = nullptr; + decltype(&clang_CXXMethod_isStatic) methodIsStatic = nullptr; + decltype(&clang_getArgType) getArgType = nullptr; + decltype(&clang_getNumArgTypes) getNumArgTypes = nullptr; + decltype(&clang_getPointeeType) getPointeeType = nullptr; + decltype(&clang_Cursor_Evaluate) evaluate = nullptr; + decltype(&clang_EvalResult_getKind) evalResultKind = nullptr; + decltype(&clang_EvalResult_dispose) disposeEvalResult = nullptr; }; LibClang LoadLibClang() { @@ -105,20 +140,65 @@ namespace { slot = reinterpret_cast>(LibrarySymbol(lib.handle, name)); if (!slot) missing.push_back(name); }; - bind(lib.CreateIndex, "clang_createIndex"); - bind(lib.DisposeIndex, "clang_disposeIndex"); - bind(lib.ParseTranslationUnit, "clang_parseTranslationUnit"); - bind(lib.DisposeTranslationUnit, "clang_disposeTranslationUnit"); - bind(lib.GetFile, "clang_getFile"); - bind(lib.GetLocationForOffset, "clang_getLocationForOffset"); - bind(lib.GetRange, "clang_getRange"); - bind(lib.GetRangeStart, "clang_getRangeStart"); - bind(lib.GetRangeEnd, "clang_getRangeEnd"); - bind(lib.GetFileLocation, "clang_getFileLocation"); - bind(lib.Tokenize, "clang_tokenize"); - bind(lib.DisposeTokens, "clang_disposeTokens"); - bind(lib.GetTokenKind, "clang_getTokenKind"); - bind(lib.GetTokenExtent, "clang_getTokenExtent"); + bind(lib.createIndex, "clang_createIndex"); + bind(lib.disposeIndex, "clang_disposeIndex"); + bind(lib.parseTranslationUnit, "clang_parseTranslationUnit"); + bind(lib.disposeTranslationUnit, "clang_disposeTranslationUnit"); + bind(lib.getFile, "clang_getFile"); + bind(lib.getLocationForOffset, "clang_getLocationForOffset"); + bind(lib.getRange, "clang_getRange"); + bind(lib.getRangeStart, "clang_getRangeStart"); + bind(lib.getRangeEnd, "clang_getRangeEnd"); + bind(lib.getFileLocation, "clang_getFileLocation"); + bind(lib.tokenize, "clang_tokenize"); + 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.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"); + bind(lib.getResultType, "clang_getResultType"); + bind(lib.getBinaryOpcode, "clang_Cursor_getBinaryOpcode"); + bind(lib.getUnaryOperatorKind, "clang_getCursorUnaryOperatorKind"); + bind(lib.isConstQualifiedType, "clang_isConstQualifiedType"); + bind(lib.methodIsConst, "clang_CXXMethod_isConst"); + bind(lib.methodIsStatic, "clang_CXXMethod_isStatic"); + bind(lib.getArgType, "clang_getArgType"); + bind(lib.getNumArgTypes, "clang_getNumArgTypes"); + bind(lib.getPointeeType, "clang_getPointeeType"); + bind(lib.evaluate, "clang_Cursor_Evaluate"); + bind(lib.evalResultKind, "clang_EvalResult_getKind"); + bind(lib.disposeEvalResult, "clang_EvalResult_dispose"); + bind(lib.getBinaryOpcode, "clang_Cursor_getBinaryOpcode"); + bind(lib.getUnaryOperatorKind, "clang_getCursorUnaryOperatorKind"); + bind(lib.isConstQualifiedType, "clang_isConstQualifiedType"); + bind(lib.methodIsConst, "clang_CXXMethod_isConst"); + bind(lib.methodIsStatic, "clang_CXXMethod_isStatic"); + bind(lib.getArgType, "clang_getArgType"); + bind(lib.getNumArgTypes, "clang_getNumArgTypes"); + bind(lib.getPointeeType, "clang_getPointeeType"); + bind(lib.evaluate, "clang_Cursor_Evaluate"); + bind(lib.evalResultKind, "clang_EvalResult_getKind"); + bind(lib.disposeEvalResult, "clang_EvalResult_dispose"); if (!missing.empty()) { lib.handle = nullptr; lib.error = std::format("loaded {} but it is missing {}", candidates.front(), join(missing)); @@ -182,39 +262,552 @@ namespace { unsaved.Contents = content.data(); unsaved.Length = static_cast(content.size()); - CXIndex index = lc.CreateIndex(0, 0); + CXIndex index = lc.createIndex(0, 0); if (!index) return {}; - CXTranslationUnit tu = lc.ParseTranslationUnit(index, path.c_str(), args.data(), static_cast(args.size()), &unsaved, 1, CXTranslationUnit_SingleFileParse | CXTranslationUnit_SkipFunctionBodies | CXTranslationUnit_KeepGoing); + CXTranslationUnit tu = lc.parseTranslationUnit(index, path.c_str(), args.data(), static_cast(args.size()), &unsaved, 1, CXTranslationUnit_SingleFileParse | CXTranslationUnit_SkipFunctionBodies | CXTranslationUnit_KeepGoing); if (!tu) { - lc.DisposeIndex(index); + lc.disposeIndex(index); return {}; } std::vector tokens; - if (CXFile cxFile = lc.GetFile(tu, path.c_str())) { - CXSourceRange whole = lc.GetRange(lc.GetLocationForOffset(tu, cxFile, 0), lc.GetLocationForOffset(tu, cxFile, static_cast(content.size()))); + if (CXFile cxFile = lc.getFile(tu, path.c_str())) { + CXSourceRange whole = lc.getRange(lc.getLocationForOffset(tu, cxFile, 0), lc.getLocationForOffset(tu, cxFile, static_cast(content.size()))); CXToken* raw = nullptr; std::uint32_t count = 0; - lc.Tokenize(tu, whole, &raw, &count); + lc.tokenize(tu, whole, &raw, &count); tokens.reserve(count); for (std::uint32_t i = 0; i < count; ++i) { - CXSourceRange extent = lc.GetTokenExtent(tu, raw[i]); + CXSourceRange extent = lc.getTokenExtent(tu, raw[i]); std::uint32_t line = 0; std::uint32_t column = 0; std::uint32_t begin = 0; std::uint32_t end = 0; - lc.GetFileLocation(lc.GetRangeStart(extent), nullptr, &line, &column, &begin); - lc.GetFileLocation(lc.GetRangeEnd(extent), nullptr, nullptr, nullptr, &end); + lc.getFileLocation(lc.getRangeStart(extent), nullptr, &line, &column, &begin); + lc.getFileLocation(lc.getRangeEnd(extent), nullptr, nullptr, nullptr, &end); if (end < begin || begin > content.size()) continue; - tokens.push_back({MapTokenKind(lc.GetTokenKind(raw[i])), begin, std::min(end - begin, content.size() - begin), line, column}); + tokens.push_back({MapTokenKind(lc.getTokenKind(raw[i])), begin, std::min(end - begin, content.size() - begin), line, column}); } - if (raw) lc.DisposeTokens(tu, raw, count); + if (raw) lc.disposeTokens(tu, raw, count); } - lc.DisposeTranslationUnit(tu); - lc.DisposeIndex(index); + lc.disposeTranslationUnit(tu); + lc.disposeIndex(index); return tokens; } + // ---------------- AST ---------------- + + std::string TakeString(const LibClang& lc, CXString s) { + // const char* because clang_getCString returns one. no-char-pointer + // works this out for itself now: the initialiser resolves to a + // declaration in clang-c/, outside the project, so the declaration is + // flagged as foreign API and exempt. No suppression comment needed. + 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 + std::int32_t externCDepth = 0; // inside how many extern "C" blocks + // >0 while visiting a subtree whose value is being WRITTEN: the left + // side of an assignment, the operand of ++/--, or anything whose + // address is taken or which binds to a non-const reference. + std::int32_t writeDepth = 0; + // Set while visiting the binding of a range-for. + bool inLoopBinding = false; + // Name offset -> index, so a DeclRefExpr can be resolved back to the + // declaration it names without comparing USR strings. + std::unordered_map byNameOffset; + }; + + bool IsScalarTypeKind(CXTypeKind kind) { + switch (kind) { + case CXType_Bool: + case CXType_Char_U: + case CXType_UChar: + case CXType_UShort: + case CXType_UInt: + case CXType_ULong: + case CXType_ULongLong: + case CXType_Char_S: + case CXType_SChar: + case CXType_Short: + case CXType_Int: + case CXType_Long: + case CXType_LongLong: + case CXType_Float: + case CXType_Double: + case CXType_LongDouble: + case CXType_Enum: + case CXType_Pointer: + return true; + default: + return false; + } + } + + bool IsAssignmentOpcode(CX_BinaryOperatorKind opcode) { + return opcode >= CX_BO_Assign && opcode <= CX_BO_OrAssign; + } + + bool IsWritableReference(const LibClang& lc, CXType type) { + if (type.kind != CXType_LValueReference) return false; + return lc.isConstQualifiedType(lc.getPointeeType(type)) == 0; + } + + CXChildVisitResult CollectChild(CXCursor cursor, CXCursor, CXClientData data) { + static_cast*>(data)->push_back(cursor); + return CXChildVisit_Continue; + } + + std::vector ChildrenOf(const LibClang& lc, CXCursor cursor) { + std::vector children; + lc.visitChildren(cursor, &CollectChild, &children); + return children; + } + + // Whether a CXCursor_LinkageSpec is `extern "C"` as opposed to + // `extern "C++"`. libclang exposes no query, and the spelling is empty, but + // the extent starts at the `extern` keyword so the source answers it. + bool IsExternCLinkage(std::string_view text) { + std::size_t quote = text.find('"'); + if (quote == std::string_view::npos) return false; + std::size_t close = text.find('"', quote + 1); + if (close == std::string_view::npos) return false; + return text.substr(quote + 1, close - quote - 1) == "C"; + } + + 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); + } + + void ProcessCursor(CXCursor cursor, DeclWalk& walk); + + CXChildVisitResult VisitDecl(CXCursor cursor, CXCursor, CXClientData data) { + ProcessCursor(cursor, *static_cast(data)); + return CXChildVisit_Continue; + } + + // Visit a cursor's children with the write-context flag raised, so every + // DeclRefExpr inside counts as a write to what it names. + void ProcessAsWrite(CXCursor cursor, DeclWalk& walk) { + ++walk.writeDepth; + ProcessCursor(cursor, walk); + --walk.writeDepth; + } + + void ProcessCursor(CXCursor cursor, DeclWalk& walk) { + 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; + + const CXCursorKind kind = lc.getCursorKind(cursor); + const LintDeclKind mapped = MapCursorKind(kind); + if (mapped == LintDeclKind::Other) { + // A foreign reference inside a VARIABLE, FIELD or PARAMETER is an + // initialiser binding that declaration to somebody else's API — + // `char* p = getenv(...)`. Deliberately not applied when the + // enclosing declaration is a function: a function that merely + // touches libc++ somewhere in its body would otherwise exempt its + // own signature. + if (!walk.stack.empty()) { + LintDecl& enclosing = (*walk.out)[walk.stack.back()]; + const bool initialiserContext = enclosing.kind == LintDeclKind::Variable || enclosing.kind == LintDeclKind::Field || enclosing.kind == LintDeclKind::Parameter; + if (initialiserContext && ResolvesOutsideProject(lc, cursor, *walk.projectRoot)) { + enclosing.isForeignApi = true; + } + } + + // ---- mutation analysis ---- + // A name used where a value is being written marks that + // declaration mutated. Resolving through getCursorReferenced means + // shadowing and qualified names come out right. + if (kind == CXCursor_DeclRefExpr && walk.writeDepth > 0) { + CXCursor target = lc.getCursorReferenced(cursor); + if (!lc.cursorIsNull(target)) { + std::uint32_t targetOffset = 0; + lc.getFileLocation(lc.getCursorLocation(target), nullptr, nullptr, nullptr, &targetOffset); + if (auto it = walk.byNameOffset.find(targetOffset); it != walk.byNameOffset.end()) { + (*walk.out)[it->second].isMutated = true; + } + } + } + // The left side of an assignment is written; the right side is read. + if (kind == CXCursor_BinaryOperator || kind == CXCursor_CompoundAssignOperator) { + if (IsAssignmentOpcode(lc.getBinaryOpcode(cursor))) { + std::vector children = ChildrenOf(lc, cursor); + if (!children.empty()) { + ProcessAsWrite(children.front(), walk); + for (std::size_t c = 1; c < children.size(); ++c) ProcessCursor(children[c], walk); + return; + } + } + } + // ++/-- write their operand; & lets it be written elsewhere, which + // we cannot follow, so it counts as mutated. + if (kind == CXCursor_UnaryOperator) { + const CXUnaryOperatorKind unary = lc.getUnaryOperatorKind(cursor); + const bool writes = unary == CXUnaryOperator_PreInc || unary == CXUnaryOperator_PreDec || unary == CXUnaryOperator_PostInc || unary == CXUnaryOperator_PostDec || unary == CXUnaryOperator_AddrOf; + if (writes) { + for (CXCursor child : ChildrenOf(lc, cursor)) ProcessAsWrite(child, walk); + return; + } + } + // An argument bound to a non-const lvalue reference can be written + // by the callee. + if (kind == CXCursor_CallExpr) { + std::vector children = ChildrenOf(lc, cursor); + CXType callee = lc.getCursorType(lc.getCursorReferenced(cursor)); + std::int32_t params = lc.getNumArgTypes(callee); + if (params > 0) { + // Children are [callee?, args...]; line them up from the end + // so an implicit callee child does not shift the mapping. + std::size_t firstArg = children.size() > static_cast(params) + ? children.size() - static_cast(params) : 0; + for (std::size_t c = 0; c < children.size(); ++c) { + const bool byWritableRef = c >= firstArg && IsWritableReference(lc, lc.getArgType(callee, static_cast(c - firstArg))); + if (byWritableRef) ProcessAsWrite(children[c], walk); + else ProcessCursor(children[c], walk); + } + return; + } + } + // The first child of a range-for is its binding; the rest are the + // range expression and the body. + if (kind == CXCursor_CXXForRangeStmt) { + std::vector children = ChildrenOf(lc, cursor); + for (std::size_t c = 0; c < children.size(); ++c) { + const bool binding = c == 0 && lc.getCursorKind(children[c]) == CXCursor_VarDecl; + walk.inLoopBinding = binding; + ProcessCursor(children[c], walk); + walk.inLoopBinding = false; + } + return; + } + // Recursed by hand rather than with CXChildVisit_Recurse so the + // enclosing-declaration stack stays accurate: the callback is never + // told when a subtree ends. + if (kind == CXCursor_LinkageSpec) { + CXSourceRange extent = lc.getCursorExtent(cursor); + std::uint32_t specBegin = 0; + std::uint32_t specEnd = 0; + lc.getFileLocation(lc.getRangeStart(extent), nullptr, nullptr, nullptr, &specBegin); + lc.getFileLocation(lc.getRangeEnd(extent), nullptr, nullptr, nullptr, &specEnd); + std::string_view text; + if (specBegin < walk.content->size()) { + text = std::string_view(walk.content->data() + specBegin, std::min(specEnd - specBegin, walk.content->size() - specBegin)); + } + const bool isC = IsExternCLinkage(text); + if (isC) ++walk.externCDepth; + lc.visitChildren(cursor, &VisitDecl, &walk); + if (isC) --walk.externCDepth; + return; + } + lc.visitChildren(cursor, &VisitDecl, &walk); + return; + } + + LintDecl decl; + decl.kind = mapped; + decl.name = TakeString(lc, lc.getCursorSpelling(cursor)); + // For anything callable, `type` is the RESULT type rather than the + // whole function type: the parameters arrive as their own Parameter + // declarations, so spelling them here too would make every rule + // reading `type` report each one twice. + const bool callable = mapped == LintDeclKind::Function || mapped == LintDeclKind::Method; + decl.type = TakeString(lc, lc.getTypeSpelling(callable ? lc.getResultType(lc.getCursorType(cursor)) : 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; + decl.nameOffset = nameOffset; + 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; + // Inside an extern "C" block, where a C API's spelling is not ours to + // modernise. NOT clang_getCursorLanguage: its default answer for a + // plain function, variable or parameter is CXLanguage_C even in a C++ + // translation unit, so trusting it exempted essentially everything. + decl.isExternC = walk.externCDepth > 0; + // 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(); + + CXType declaredType = lc.getCursorType(cursor); + decl.isConst = lc.isConstQualifiedType(declaredType) != 0; + decl.isScalar = IsScalarTypeKind(declaredType.kind); + decl.isLoopVariable = walk.inLoopBinding; + // Ask clang whether the initialiser is a constant expression instead of + // inspecting its tokens. Evaluating a VarDecl evaluates its initialiser, + // so this covers sizeof, a fold over other constants, and anything else + // that folds — none of which a token scan can recognise — and it does + // not mistake a literal with a non-constexpr user-defined suffix for a + // constant. + if (mapped == LintDeclKind::Variable || mapped == LintDeclKind::Field) { + if (CXEvalResult evaluated = lc.evaluate(cursor)) { + decl.isConstantInitialised = lc.evalResultKind(evaluated) != CXEval_UnExposed; + lc.disposeEvalResult(evaluated); + } + } + if (mapped == LintDeclKind::Method) { + decl.isConstMethod = lc.methodIsConst(cursor) != 0; + decl.isStaticMethod = lc.methodIsStatic(cursor) != 0; + } + + walk.out->push_back(std::move(decl)); + std::size_t index = walk.out->size() - 1; + walk.byNameOffset.emplace(nameOffset, index); + walk.stack.push_back(index); + // Binding a name to a non-const reference — `auto& r = x;` — lets x be + // written through r, which we cannot follow, so x counts as mutated. + if (IsWritableReference(lc, declaredType)) { + for (CXCursor child : ChildrenOf(lc, cursor)) ProcessAsWrite(child, walk); + } else { + lc.visitChildren(cursor, &VisitDecl, &walk); + } + walk.stack.pop_back(); + } + + 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 +891,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 +983,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 +1066,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 +1136,60 @@ 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) { + std::map astFailures; + std::set skippedAstRules; + + // 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); + // Sources that #include an external dependency's headers — + // Crafter.Build-Shader.cpp and glslang here — need those -I + // flags to parse at all. Build appends its own authoritative + // set after the external build; these come straight from the + // declaration, which is all a parse needs. + command = assembled.command + assembled.externalIncludeFlags; + 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 +1199,37 @@ 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; + // No compile command means this file is not a translation unit + // of the build graph — project.cpp, which LoadProject compiles + // with its own flags, or a header. A semantic rule does not + // apply there, the same way a rule self-filters by extension, + // so skip it quietly. That is a different thing from a file we + // SHOULD have been able to parse and could not, which is an + // error: reporting nothing for it would be indistinguishable + // from reporting it clean. + if (ctx.compileCommand.empty()) continue; + if (!ctx.AstAvailable()) { + // Recorded once per file and reported as a single grouped + // 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; + } + } // 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. @@ -621,6 +1318,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(), [](const LintFinding& a, const LintFinding& b) { return std::tie(a.file, a.line) < std::tie(b.file, b.line); diff --git a/implementations/Crafter.Build-Platform.cpp b/implementations/Crafter.Build-Platform.cpp index 55e035b..62ee0ab 100644 --- a/implementations/Crafter.Build-Platform.cpp +++ b/implementations/Crafter.Build-Platform.cpp @@ -814,7 +814,7 @@ std::string Crafter::BuildStdPcm(const Configuration& config, fs::path stdPcm) { return ""; } } else { - bool isWasm = config.target.starts_with("wasm32"); + const bool isWasm = config.target.starts_with("wasm32"); // wasi-sdk drops std.cppm at /share/libc++/v1/, the rest of // the libc++ ecosystem (e.g. /opt/aarch64-rootfs) follows FHS at // /usr/share/libc++/v1/. @@ -906,7 +906,7 @@ namespace { // Re-derived under the lock: another builder may have refreshed the // cache from its own sources while we waited. std::string stamp = CrafterBuildSourceStamp(sourceDir, CrafterBuildModules); - bool upToDate = ReadCacheStamp(cacheDir) == stamp; + const bool upToDate = ReadCacheStamp(cacheDir) == stamp; for (std::string_view name : CrafterBuildModules) { fs::path cppmPath = sourceDir / std::format("{}.cppm", name); fs::path pcmPath = cacheDir / std::format("{}.pcm", name); @@ -961,7 +961,7 @@ Configuration Crafter::LoadProject(const fs::path& projectFile, std::span(EShMsgDefault | EShMsgVulkanRules | EShMsgSpvRules); + constexpr EShMessages Messages = static_cast(EShMsgDefault | EShMsgVulkanRules | EShMsgSpvRules); std::ifstream fileStream(path, std::ios::in | std::ios::binary); if (!fileStream) { return fail("failed to open shader source", {}); @@ -71,12 +71,12 @@ namespace Crafter { std::string src = contents.str(); std::string pathStr = path.string(); - const char* file_name_list[1] = { pathStr.c_str() }; + const char* fileNameList[1] = { pathStr.c_str() }; const char* shaderSource = src.data(); const std::int32_t shaderSourceLen = static_cast(src.size()); glslang::TShader shader(glslangType); - shader.setStringsWithLengthsAndNames(&shaderSource, &shaderSourceLen, file_name_list, 1); + shader.setStringsWithLengthsAndNames(&shaderSource, &shaderSourceLen, fileNameList, 1); shader.setEntryPoint(entrypoint.c_str()); shader.setSourceEntryPoint(entrypoint.c_str()); shader.setEnvTarget(glslang::EShTargetSpv, glslang::EShTargetSpv_1_4); @@ -86,13 +86,13 @@ namespace Crafter { includeDir.pushExternalLocalDirectory(dir.generic_string()); } - if (!shader.parse(GetDefaultResources(), 100, false, messages, includeDir)) { + if (!shader.parse(GetDefaultResources(), 100, false, Messages, includeDir)) { return fail("GLSL parse failed", std::string(shader.getInfoLog()) + shader.getInfoDebugLog()); } glslang::TProgram program; program.addShader(&shader); - if (!program.link(messages)) { + if (!program.link(Messages)) { return fail("GLSL link failed", std::string(program.getInfoLog()) + program.getInfoDebugLog()); } diff --git a/interfaces/Crafter.Build-Clang.cppm b/interfaces/Crafter.Build-Clang.cppm index ed4f3fa..ed18187 100644 --- a/interfaces/Crafter.Build-Clang.cppm +++ b/interfaces/Crafter.Build-Clang.cppm @@ -149,6 +149,89 @@ 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 nameOffset = 0; // byte offset of the declared name + 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 + // For the FIRST declarator of a statement, `begin` is the start of the + // shared type and so precedes `nameOffset`. For a continuation + // declarator — the `b` of `int* a, b;` — the extent starts at the name, + // so begin == nameOffset. That is how a multi-declarator statement is + // recognised without re-parsing the text, and each declarator's `type` + // is its OWN resolved type: `int *` for a, plain `int` for b. + 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. + // ---- constness ---- + // The declared type is const-qualified. + bool isConst = false; + // A scalar: integer, floating, bool, enum or pointer. For these, + // "is it ever written" is decidable from assignments, ++/--, address-of + // and reference bindings alone — there are no member calls that could + // mutate it — so isMutated is exact rather than a guess. + bool isScalar = false; + // Written to somewhere in this file: assigned, incremented, had its + // address taken, or bound to a non-const reference. Only meaningful + // for a declaration whose uses are all in this file, so a local rather + // than something with external linkage. + bool isMutated = false; + // The initialiser is a constant expression, as decided by clang's own + // constant evaluator rather than by inspecting its tokens. Only set for + // Variable and Field. + bool isConstantInitialised = false; + // Method declared const. Only set for Method. + bool isConstMethod = false; + // Method declared static. Only set for Method. + bool isStaticMethod = false; + // The binding of a range-for: `for (T x : range)`. A loop binding is + // not a variable a reader thinks of as assignable, so constness advice + // about it is noise. + bool isLoopVariable = false; + 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 +306,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 +336,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 +352,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 +472,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 @@ -418,9 +541,52 @@ export namespace Crafter { std::vector requires_; }; + // Directory holding the std module PCM for this Configuration. Keyed on + // target+march because a BMI is only loadable by a TU with matching target + // features, and suffixed with the wasm variant flags for the same reason. + CRAFTER_API fs::path StdPcmDir(const Configuration& config); + + // The clang invocation every C++ translation unit in this Configuration is + // compiled with, before anything that depends on work having happened + // (dependency public flags, external dependency flags — Build appends + // those itself). A pure function of the Configuration. + // + // Anything that needs to PARSE this project's sources rather than build + // them — the linter's AST layer, a future compile_commands.json — must go + // through this instead of assembling its own flags. A precompiled module + // is rejected outright by a TU whose target features differ from the one + // that wrote it, so approximately-right flags fail hard rather than + // degrade: omitting -march=native alone produces hundreds of "compiled + // with the target feature '+avx512bw' but the current translation unit is + // not" errors and no usable parse. + struct CompileCommand { + std::string command; // full C++ compile prefix, shell-ready + // Sub-sets Build also needs on its own: the .c compile path takes the + // includes, defines and user flags but not the module-only bits. + std::string includeFlags; + std::string defineFlags; + std::string userFlags; + std::string ltoCompileFlags; + std::string ltoLinkFlags; + // -I flags from external dependencies' declared includeDirs, for this + // configuration and its dependencies. Deliberately NOT folded into + // `command`: Build appends the authoritative set from the external build + // results instead. Exposed for callers that only PARSE sources and so + // cannot wait for a build to tell them where the headers are. + std::string externalIncludeFlags; + // ThinLTO is on: objects hold bitcode, so archiving needs llvm-ar. + bool useLto = false; + fs::path stdPcmDir; + fs::path pcmDir; + }; + CRAFTER_API CompileCommand GetCompileCommand(const Configuration& config); + CRAFTER_API BuildResult Build(Configuration& config, std::unordered_map>& depResults, std::mutex& depMutex); - CRAFTER_API int Run(int argc, char** argv); + // Takes main's argv verbatim so the CLI entry point is a one-liner; + // the shape is inherited from the language, not chosen here. + // lint-disable-next-line no-char-pointer + CRAFTER_API std::int32_t Run(std::int32_t argc, char** argv); // Delete the bin/ and build/ trees beside `projectFile`, returning the paths // that existed and were removed. Backs `crafter-build clean`. diff --git a/interfaces/Crafter.Build-External.cppm b/interfaces/Crafter.Build-External.cppm index 868b943..df5b190 100644 --- a/interfaces/Crafter.Build-External.cppm +++ b/interfaces/Crafter.Build-External.cppm @@ -44,6 +44,16 @@ export namespace Crafter { fs::file_time_type latestArtifact = fs::file_time_type::min(); }; + // Where this dependency's clone lives in the cache. A pure function of the + // declaration and target — BuildExternal derives its own working directory + // through this, so a caller that only needs to know where the headers ended + // up cannot drift from where they actually are. + CRAFTER_API fs::path ExternalCloneDir(const ExternalDependency& dep, std::string_view target); + // The -I flags this dependency contributes, from its declared includeDirs. + // Also pure, which is what lets the linter's AST layer parse a source that + // includes an external's headers without running a build to find out where + // they are. The paths only resolve once something has fetched the clone. + CRAFTER_API std::vector ExternalIncludeFlags(const ExternalDependency& dep, std::string_view target); CRAFTER_API ExternalBuildResult BuildExternal(const ExternalDependency& dep, std::string_view target, std::atomic& cancelled); // Specification for a sibling crafter-build project to fetch and depend on. 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/lint-rules.h b/lint-rules.h index 3cf8c92..b701980 100644 --- a/lint-rules.h +++ b/lint-rules.h @@ -70,13 +70,28 @@ inline bool IsCamelCase(std::string_view name) { return !name.empty() && ((name[0] >= 'a' && name[0] <= 'z') || name[0] == '_') && name.find('_', 1) == std::string_view::npos; } -// The identifier ending right before position `pos` (exclusive), or empty. -inline std::string_view WordBefore(std::string_view s, std::size_t pos) { - std::size_t end = pos; - while (end > 0 && (s[end - 1] == ' ' || s[end - 1] == '\t')) --end; - std::size_t begin = end; - while (begin > 0 && (IsWordChar(s[begin - 1]) || s[begin - 1] == '~')) --begin; - return s.substr(begin, end - begin); +// clang spells a pointer type "int *"; house style attaches the star to the +// type. Only affects spelling, never which type is meant. +inline std::string NormalisePointerSpelling(std::string_view type) { + std::string out; + out.reserve(type.size()); + for (char c : type) { + if ((c == '*' || c == '&') && out.ends_with(' ')) out.pop_back(); + out += c; + } + return out; +} + +// True for a pointer to char, as clang spells a type: "char *", +// "const char *", "char **". Deliberately not signed/unsigned char, which are +// byte buffers rather than text and were never the target. +inline bool IsCharPointer(std::string_view type) { + std::size_t star = type.find('*'); + if (star == std::string_view::npos) return false; + std::string_view base = Trim(type.substr(0, star)); + if (base.starts_with("const ")) base.remove_prefix(6); + if (base.starts_with("volatile ")) base.remove_prefix(9); + return base == "char"; } inline void AddProjectLintRules(Crafter::Configuration& cfg) { @@ -106,145 +121,99 @@ inline void AddProjectLintRules(Crafter::Configuration& cfg) { } }); - // Naming: functions/types PascalCase, variables camelCase, statics and - // namespace-scope globals PascalCase. A line-based scope tracker decides - // whether a declaration sits at namespace scope (global) or inside a - // function/type (local/member). Heuristic by nature — report-only. - cfg.AddLintRule("naming", [](LintContext& ctx) { + // Naming: types and functions PascalCase, variables camelCase, with + // statics, namespace-scope globals and constexpr constants PascalCase. + // + // Reads the AST. The previous version needed four std::regex, a hand-rolled + // {/} scope stack, a cumulative paren-depth counter to avoid mistaking a + // wrapped parameter list for a declaration, a keyword denylist, and a + // "function-shaped line" guess whose own comment conceded it was heuristic. + // All of it existed to answer two questions clang answers directly: what + // kind of declaration is this, and what encloses it. + cfg.AddAstLintRule("naming", [](LintContext& ctx) { if (!IsCppFile(ctx)) return; - std::vector lines = Lines(ctx.CommentStripped()); - - enum class Scope { Namespace, Type, Function, Other }; - std::vector stack; - auto currentScope = [&]() { return stack.empty() ? Scope::Namespace : stack.back(); }; - - static const std::regex typeDecl(R"(\b(?:class|struct|union)\s+(?:CRAFTER_API\s+)?([A-Za-z_]\w*))"); - static const std::regex enumDecl(R"(\benum\s+(?:class\s+|struct\s+)?([A-Za-z_]\w*))"); - static const std::regex usingDecl(R"(^\s*using\s+([A-Za-z_]\w*)\s*=)"); - static const std::regex varDecl( - R"(^\s*((?:static|constexpr|const|inline|mutable|thread_local|export|CRAFTER_API)\s+)*)" - R"((?:std::)?[A-Za-z_][\w:]*(?:<[^;={]*>)?(?:\s*[&*])*\s+([A-Za-z_]\w*)\s*(=|;|\{))"); - static const std::unordered_set keywords = { - "if", "for", "while", "switch", "catch", "return", "else", "do", "case", "goto", - "new", "delete", "throw", "using", "namespace", "template", "typedef", "friend", - "public", "private", "protected", "class", "struct", "enum", "union", "import", - "module", "export", "break", "continue", "co_return", "co_await", "co_yield", - "sizeof", "alignof", "decltype", "static_assert", "operator", "try", "requires", - }; - - // Cumulative paren depth at the start of each line: declaration and - // function checks only run at depth 0, so wrapped parameter lists and - // continuation lines (`) {` closers) never look like declarations. - std::int64_t parenDepth = 0; - for (std::size_t i = 0; i < lines.size(); ++i) { - std::string_view trimmed = Trim(lines[i]); - std::string lineStr(lines[i]); - std::smatch m; - bool atDepth0 = parenDepth == 0; - parenDepth = std::max(0, parenDepth + ParenDelta(lines[i])); - - if (!trimmed.starts_with('#') && atDepth0) { - // Type / alias names must be PascalCase. - if (std::regex_search(lineStr, m, typeDecl) || std::regex_search(lineStr, m, enumDecl)) { - std::string name = m[1].str(); - if (!keywords.contains(name) && !IsPascalCase(name)) { - ctx.Report(i + 1, std::format("type '{}' should be PascalCase", name)); + using Kind = Crafter::LintDeclKind; + std::span decls = ctx.Decls(); + for (const Crafter::LintDecl& decl : decls) { + if (decl.name.empty()) continue; // anonymous namespace, unnamed struct + // A declaration with C language linkage is named by the C library + // it mirrors — `extern "C" int setenv(...)` is not ours to rename. + if (decl.isExternC) continue; + // Namespace scope is now a lookup rather than a brace count, and it + // is exact for a local declared inside a function body. + bool atNamespaceScope = decl.parent == Crafter::LintNoParent + || decls[decl.parent].kind == Kind::Namespace; + switch (decl.kind) { + case Kind::Class: + case Kind::Struct: + case Kind::Union: + case Kind::Enum: + if (!IsPascalCase(decl.name)) { + ctx.Report(decl.line, std::format("type '{}' should be PascalCase", decl.name)); } - } - if (std::regex_search(lineStr, m, usingDecl) && !IsPascalCase(m[1].str())) { - ctx.Report(i + 1, std::format("type alias '{}' should be PascalCase", m[1].str())); - } - - // Function definitions: identifier before the first '(' on a - // line that ends where a body opens or closes — a multi-line - // def's trailing '{' or a one-liner's trailing '}'. Statement - // calls with inline lambda arguments look similar - // (`bool x = std::any_of(..., [](T v) { ... });`) but end in - // ';' and/or carry '=' before the name — both excluded. - // Lambdas ("](") and control keywords are skipped; ctors/ - // dtors pass the Pascal check by construction; `main` and - // operators exempt. - std::size_t bodyBrace = trimmed.find('{'); - bool functionShaped = trimmed.ends_with('{') || trimmed.ends_with('}'); - if (functionShaped && currentScope() != Scope::Function && !trimmed.starts_with("return")) { - std::size_t paren = trimmed.find('('); - if (paren != std::string_view::npos && paren > 0 && paren < bodyBrace) { - std::string_view name = WordBefore(trimmed, paren); - char before = trimmed[paren - 1]; - bool looksLikeDef = !name.empty() && IsWordChar(before) - && !keywords.contains(name) && name != "main" - && !name.starts_with('~'); - // Require a type token before the name (or a - // qualified Class::Name), with no '=' in front — - // plain calls and initializations don't match. - if (looksLikeDef) { - std::size_t nameStart = trimmed.rfind(name, paren); - std::string_view prefix = Trim(trimmed.substr(0, nameStart)); - looksLikeDef = !prefix.empty() && !prefix.contains('=') - && (IsWordChar(prefix.back()) || prefix.back() == '>' - || prefix.back() == '*' || prefix.back() == '&' - || prefix.ends_with("::")); + break; + case Kind::TypeAlias: + if (!IsPascalCase(decl.name)) { + ctx.Report(decl.line, std::format("type alias '{}' should be PascalCase", decl.name)); + } + break; + case Kind::Function: + case Kind::Method: + // main is spelled by the language; operators by their + // symbol. Constructors and destructors take their type's + // name and are separate kinds, so they never arrive here. + if (decl.name == "main" || decl.name.starts_with("operator")) break; + if (!IsPascalCase(decl.name)) { + ctx.Report(decl.line, std::format("function '{}' should be PascalCase", decl.name)); + } + break; + case Kind::Variable: + // constexpr variables are compile-time constants and take + // constant naming, like statics and globals. + if (decl.isStatic || decl.isConstexpr || atNamespaceScope) { + if (!IsPascalCase(decl.name)) { + ctx.Report(decl.line, std::format("{} '{}' should be PascalCase", + decl.isStatic ? "static variable" + : decl.isConstexpr ? "constexpr constant" + : "global variable", decl.name)); } - if (looksLikeDef && !IsPascalCase(name)) { - ctx.Report(i + 1, std::format("function '{}' should be PascalCase", name)); + } else if (!IsCamelCase(decl.name)) { + ctx.Report(decl.line, std::format("variable '{}' should be camelCase", decl.name)); + } + break; + case Kind::Field: + if (decl.isStatic || decl.isConstexpr) { + if (!IsPascalCase(decl.name)) { + ctx.Report(decl.line, std::format("static member '{}' should be PascalCase", decl.name)); } + } else if (!IsCamelCase(decl.name)) { + ctx.Report(decl.line, std::format("member '{}' should be camelCase", decl.name)); } - } - - // Variable declarations: camelCase locally, PascalCase for - // statics and namespace-scope globals. - if (std::regex_search(lineStr, m, varDecl)) { - std::string name = m[2].str(); - std::string_view typeToken = Trim(std::string_view(lineStr).substr(0, static_cast(m.position(2)))); - std::string_view firstWord = typeToken.substr(0, typeToken.find_first_of(" \t<")); - if (!keywords.contains(firstWord) && !keywords.contains(name)) { - bool isStatic = lineStr.contains("static "); - // constexpr variables are compile-time constants — - // constant naming (PascalCase) like statics/globals. - bool isConstexpr = lineStr.contains("constexpr "); - bool global = currentScope() == Scope::Namespace; - if ((isStatic || global || isConstexpr) && !IsPascalCase(name)) { - ctx.Report(i + 1, std::format("{} '{}' should be PascalCase", - isStatic ? "static variable" - : isConstexpr ? "constexpr constant" - : "global variable", name)); - } else if (!isStatic && !global && !isConstexpr && !IsCamelCase(name)) { - ctx.Report(i + 1, std::format("variable '{}' should be camelCase", name)); - } - } - } - } - - // Scope tracking: classify each '{' opened on this line; pop on '}'. - for (std::size_t c = 0; c < lines[i].size(); ++c) { - if (lines[i][c] == '{') { - Scope kind = Scope::Other; - std::string_view upTo = Trim(lines[i].substr(0, c)); - if (trimmed.starts_with("namespace") || upTo.contains("namespace ")) { - kind = Scope::Namespace; - } else if (std::regex_search(lineStr, typeDecl) || std::regex_search(lineStr, enumDecl)) { - kind = Scope::Type; - } else if (upTo.ends_with(')') || upTo.ends_with("const") || upTo.ends_with("noexcept") - || upTo.ends_with("->") || trimmed.starts_with("extern")) { - kind = Scope::Function; // function/lambda/control body — all non-global - } - stack.push_back(kind); - } else if (lines[i][c] == '}') { - if (!stack.empty()) stack.pop_back(); - } + break; + default: + break; } } }); + // Scoped enums only. Kept on tokens rather than moved to the AST, even + // though LintDecl carries an exact isScopedEnum: an AST is one + // configuration's slice, so a plain enum inside a preprocessor branch that + // is inactive for the host would stop being reported. Tokens are lexed + // without evaluating #if, so every platform's code stays covered. cfg.AddLintRule("enum-class", [](LintContext& ctx) { if (!IsCppFile(ctx)) return; - std::vector lines = Lines(ctx.CommentStripped()); - static const std::regex plainEnum(R"(\benum\s+(?!class\b|struct\b)[A-Za-z_])"); - for (std::size_t i = 0; i < lines.size(); ++i) { - std::string lineStr(lines[i]); - if (std::regex_search(lineStr, plainEnum)) { - ctx.Report(i + 1, "use enum class instead of plain enum"); - } + std::span tokens = ctx.Tokens(); + for (std::size_t i = 0; i < tokens.size(); ++i) { + if (tokens[i].kind != Crafter::LintTokenKind::Keyword) continue; + if (ctx.TokenText(tokens[i]) != "enum") continue; + // Asking for the next TOKEN rather than the rest of the line means a + // declaration split across lines reads identically to one that is + // not — the regex this replaced required the name to follow `enum` + // on the same line, so `enum\n Color {` went unreported. + std::string_view next = i + 1 < tokens.size() ? ctx.TokenText(tokens[i + 1]) : std::string_view{}; + if (next == "class" || next == "struct") continue; + ctx.Report(tokens[i].line, "use enum class instead of plain enum"); } }); @@ -260,23 +229,42 @@ inline void AddProjectLintRules(Crafter::Configuration& cfg) { } }); - // Prefer std::string / std::string_view over char*. OS interop stays: - // getenv returns char*, argv is char**, extern "C" prototypes mirror C. - cfg.AddLintRule("no-char-pointer", [](LintContext& ctx) { + // Prefer std::string / std::string_view over char* in our own declarations. + // + // Reads the AST rather than the text, which retires the substring denylist + // this rule used to carry (argv, getenv, setenv, dlerror, c_str, .data(, + // reinterpret_cast, extern "). Every entry was a patch for one interop + // site, the list could only grow, and it disabled the rule for a whole LINE + // whenever one appeared. The question is now asked directly: whose header + // dictates this spelling? A declaration with C language linkage, or one + // that binds to an entity declared outside the project, is somebody else's + // API and keeps its spelling. + // + // Only declarations are considered. A char* inside a cast or an expression + // is not an interface, and reinterpret_cast for binary IO — the case + // the denylist existed to permit — is no longer a finding to suppress. + cfg.AddAstLintRule("no-char-pointer", [](LintContext& ctx) { if (!IsCppFile(ctx)) return; - std::vector lines = Lines(ctx.CommentStripped()); - static const std::regex charPtr(R"(\bchar\s*\*)"); - for (std::size_t i = 0; i < lines.size(); ++i) { - std::string lineStr(lines[i]); - // C-interop stays char*: argv, getenv/setenv, dlerror, C APIs fed - // by c_str()/data(), binary IO reinterpret_casts, extern "C" - // prototypes. (extern " matches with the literal body blanked.) - if (lineStr.contains("argv") || lineStr.contains("getenv") || lineStr.contains("setenv") - || lineStr.contains("dlerror") || lineStr.contains("c_str") || lineStr.contains(".data(") - || lineStr.contains("reinterpret_cast") || lineStr.contains("extern \"")) continue; - if (std::regex_search(lineStr, charPtr)) { - ctx.Report(i + 1, "prefer std::string / std::string_view over char*"); + std::span decls = ctx.Decls(); + for (const Crafter::LintDecl& decl : decls) { + // main's signature is fixed by the language, so its argv is no more + // ours to modernise than a C API's is. + if (decl.parent != Crafter::LintNoParent && decls[decl.parent].name == "main") continue; + if (decl.kind == Crafter::LintDeclKind::Function && decl.name == "main") continue; + switch (decl.kind) { + case Crafter::LintDeclKind::Variable: + case Crafter::LintDeclKind::Parameter: + case Crafter::LintDeclKind::Field: + case Crafter::LintDeclKind::TypeAlias: + case Crafter::LintDeclKind::Function: + case Crafter::LintDeclKind::Method: + break; + default: + continue; } + if (decl.isExternC || decl.isForeignApi) continue; + if (!IsCharPointer(decl.type)) continue; + ctx.Report(decl.line, std::format("prefer std::string / std::string_view over char* ('{}' is '{}')", decl.name, decl.type)); } }); @@ -286,6 +274,67 @@ inline void AddProjectLintRules(Crafter::Configuration& cfg) { // groups, literals) are REWRITTEN automatically; anything the operand // scanner can't prove safe — raw-string lines, ternaries, mixed // operators, multi-line expressions — is reported for a human instead. + // A local that is never written should say so. Restricted to SCALARS — + // integers, bools, enums, pointers, floating types — which is what makes + // the answer exact rather than a guess: a scalar has no member functions, + // so the only ways to write one are assignment, ++/--, having its address + // taken, or binding to a non-const reference, and the AST layer tracks all + // four. For a class type, a non-const method call could mutate it and + // deciding that needs the whole-program analysis clang-tidy does. + // + // Report-only. Adding const is a judgement about intent as much as + // mechanics, and a wrong suggestion should cost a glance, not a build. + cfg.AddAstLintRule("const-local", [](LintContext& ctx) { + if (!IsCppFile(ctx)) return; + std::span decls = ctx.Decls(); + for (const Crafter::LintDecl& decl : decls) { + if (decl.kind != Crafter::LintDeclKind::Variable) continue; + if (decl.parent == Crafter::LintNoParent) continue; + // Locals only: a namespace-scope or static variable may be written + // from a translation unit this parse cannot see. + Crafter::LintDeclKind enclosing = decls[decl.parent].kind; + bool isLocal = enclosing == Crafter::LintDeclKind::Function || enclosing == Crafter::LintDeclKind::Method + || enclosing == Crafter::LintDeclKind::Constructor || enclosing == Crafter::LintDeclKind::Destructor; + if (!isLocal || decl.isStatic) continue; + if (decl.isConst || decl.isConstexpr) continue; + if (!decl.isScalar || decl.isMutated) continue; + if (decl.name.empty()) continue; + // A range-for binding is not what a reader pictures as an + // assignable variable, and `for (T* const x : …)` is not a spelling + // anybody writes. + if (decl.isLoopVariable) continue; + // Likewise `T* const p` — the useful constness for a pointer local + // is almost always on the pointee, which this rule cannot advise + // on. Restricting to value types keeps the advice actionable. + if (decl.type.contains('*')) continue; + ctx.Report(decl.line, std::format("'{}' is never modified — declare it const", decl.name)); + } + }); + + // A constant whose value is already a constant expression can be constexpr, + // which moves it into the type system instead of leaving it to the + // optimiser. Constant-ness of the initialiser is decided by clang's own + // evaluator, so `sizeof(T)`, a fold over other constants and anything else + // that folds all qualify, while `Compute()` does not. + // + // Restricted to scalars on purpose: for a class type, constexpr may be + // unavailable even when the initialiser folds (a std::string constant + // cannot be constexpr at namespace scope), and the evaluator answering + // "this folds" is not the same question as "constexpr is legal here". + cfg.AddAstLintRule("constexpr-constant", [](LintContext& ctx) { + if (!IsCppFile(ctx)) return; + for (const Crafter::LintDecl& decl : ctx.Decls()) { + if (decl.kind != Crafter::LintDeclKind::Variable && decl.kind != Crafter::LintDeclKind::Field) continue; + if (!decl.isConst || decl.isConstexpr || !decl.isScalar) continue; + // A pointer's value is an address, which is rarely a constant + // expression and never an interesting one to promote. + if (decl.type.contains('*')) continue; + if (!decl.isConstantInitialised) continue; + if (decl.name.empty()) continue; + ctx.Report(decl.line, std::format("'{}' has a constant initialiser — declare it constexpr", decl.name)); + } + }); + cfg.AddLintRule("format-concat", [](LintContext& ctx) { if (!IsCppFile(ctx)) return; const std::string& code = ctx.CommentStripped(); @@ -520,11 +569,40 @@ inline void AddProjectLintRules(Crafter::Configuration& cfg) { // short/int/long → fixed-width types. Lines mentioning main/argc/argv or // extern "C" keep their C-conventional ints. - cfg.AddLintRule("fixed-width-types", [](LintContext& ctx) { + cfg.AddAstLintRule("fixed-width-types", [](LintContext& ctx) { if (!IsCppFile(ctx)) return; const std::string& code = ctx.CommentStripped(); struct Rep { std::size_t pos; std::size_t len; std::string to; }; std::vector reps; + + // Byte ranges whose integer spelling is not ours to change. A wrong + // rewrite here does not merely over-report — it produces code that no + // longer matches the API it is calling — so this is derived from the + // AST rather than from substrings on the line. + // + // Replaces the old per-LINE textual exemption (`int main`, `argc`, + // `argv`, `extern "`), which both missed cases and disabled the rule + // for everything else sharing a line with one of those words. + std::vector> protectedRanges; + for (const Crafter::LintDecl& decl : ctx.Decls()) { + if (decl.isExternC || decl.isForeignApi) { + protectedRanges.emplace_back(decl.begin, decl.end); + continue; + } + // main's signature is fixed by the language. Only the signature: + // its body is ordinary code, and the declaration's extent covers + // the whole function. + if (decl.kind == Crafter::LintDeclKind::Function && decl.name == "main") { + std::size_t bodyBrace = ctx.content.find('{', decl.begin); + protectedRanges.emplace_back(decl.begin, bodyBrace == std::string::npos ? decl.end : bodyBrace); + } + } + auto isProtected = [&protectedRanges](std::size_t offset) { + return std::any_of(protectedRanges.begin(), protectedRanges.end(), + [offset](const std::pair& range) { + return offset >= range.first && offset < range.second; + }); + }; // Builtin integer type specifiers combine in any order (`unsigned // long`, `long unsigned int`, ...), so match whole RUNS of these // keywords and classify the run, rather than the words one by one. @@ -536,13 +614,8 @@ inline void AddProjectLintRules(Crafter::Configuration& cfg) { std::size_t lineEnd = code.find('\n', lineStart); if (lineEnd == std::string::npos) lineEnd = code.size(); std::string_view line(code.data() + lineStart, lineEnd - lineStart); - // `int main` / argc / argv keep their C-conventional type; so do - // extern "C" prototypes (the literal body is blanked in the - // stripped text, so match `extern "`). - bool exempt = line.contains("int main") || line.contains("argc") || line.contains("argv") - || line.contains("extern \""); std::size_t pos = 0; - while (!exempt && pos < line.size()) { + while (pos < line.size()) { if (!IsWordChar(line[pos])) { ++pos; continue; } std::size_t wordEnd = pos; while (wordEnd < line.size() && IsWordChar(line[wordEnd])) ++wordEnd; @@ -579,6 +652,8 @@ inline void AddProjectLintRules(Crafter::Configuration& cfg) { if (hasChar && !hasUnsigned && !hasSigned) continue; if (nextWord == "double") continue; + if (isProtected(lineStart + runBegin)) continue; + std::string_view width = hasChar ? "8" : hasShort ? "16" : hasLong ? "64" : "32"; reps.push_back({lineStart + runBegin, runEnd - runBegin, std::format("std::{}int{}_t", hasUnsigned ? "u" : "", width)}); @@ -596,41 +671,76 @@ inline void AddProjectLintRules(Crafter::Configuration& cfg) { // `Type a = x, b = y;`; anything with pointers/references, parens, or // templates in the declarators is only reported (splitting `int* a, b;` // would change b's type). - cfg.AddLintRule("single-declaration", [](LintContext& ctx) { + cfg.AddAstLintRule("single-declaration", [](LintContext& ctx) { if (!IsCppFile(ctx)) return; - std::vector stripped = Lines(ctx.CommentStripped()); - // The declarator char class excludes quotes: string-literal bodies are - // blanked in the stripped text, so reconstructing them would corrupt - // the file — those lines are left alone. - static const std::regex simpleMulti( - R"(^(\s*)((?:std::)?[A-Za-z_][\w:]*)\s+([A-Za-z_]\w*\s*=\s*[^,;()<>*&"]+(?:,\s*[A-Za-z_]\w*\s*=\s*[^,;()<>*&"]+)+);\s*$)"); - std::string out; - bool changed = false; - for (std::size_t i = 0; i < stripped.size(); ++i) { - std::string lineStr(stripped[i]); - std::smatch m; - std::string_view raw = i + 1 <= ctx.lines.size() ? ctx.Line(i + 1) : std::string_view{}; - if (!ctx.LineHasComment(i + 1) && !ctx.Suppressed("single-declaration", i + 1) - && std::regex_match(lineStr, m, simpleMulti)) { - std::string indent = m[1].str(), type = m[2].str(), decls = m[3].str(); - std::size_t start = 0; - bool first = true; - while (start < decls.size()) { - std::size_t comma = decls.find(',', start); - std::string_view d = Trim(std::string_view(decls).substr(start, comma == std::string::npos ? std::string::npos : comma - start)); - if (!first) out += '\n'; - out += std::format("{}{} {};", indent, type, d); - first = false; - if (comma == std::string::npos) break; - start = comma + 1; - } - changed = true; - } else { - out += raw; + std::span decls = ctx.Decls(); + std::span tokens = ctx.Tokens(); + + struct Edit { std::size_t begin; std::size_t end; std::string text; }; + std::vector edits; + + for (std::size_t i = 0; i < decls.size(); ++i) { + const Crafter::LintDecl& head = decls[i]; + if (head.kind != Crafter::LintDeclKind::Variable && head.kind != Crafter::LintDeclKind::Field) continue; + // A group head owns the shared type, so its extent starts before + // its name. Continuation declarators start AT their name. + if (head.begin == head.nameOffset) continue; + + std::size_t last = i; + while (last + 1 < decls.size()) { + const Crafter::LintDecl& next = decls[last + 1]; + if (next.kind != head.kind || next.parent != head.parent) break; + if (next.begin != next.nameOffset) break; // starts a new statement + ++last; } - if (i + 1 < stripped.size() || ctx.content.ends_with('\n')) out += '\n'; + if (last == i) continue; // a single declarator: nothing to split + + // The statement runs to the ';' after the final declarator. + auto semicolon = std::ranges::find_if(tokens, [&](const Crafter::LintToken& t) { + return t.offset >= decls[last].end && ctx.TokenText(t) == ";"; + }); + if (semicolon == tokens.end()) continue; + std::size_t stmtEnd = semicolon->offset + 1; + + // A comment anywhere inside the statement would be swallowed by the + // rewrite. One AFTER the ';' is outside the replaced range and + // survives, which the line-based version could not manage — it + // refused the whole line. + bool hasInnerComment = std::ranges::any_of(tokens, [&](const Crafter::LintToken& t) { + return t.kind == Crafter::LintTokenKind::Comment && t.offset >= head.begin && t.offset < stmtEnd; + }); + if (hasInnerComment) continue; + if (ctx.Suppressed("single-declaration", head.line)) continue; + + // Indent from the head's own line, so a statement that starts + // mid-line is left alone rather than reflowed. + std::size_t lineBegin = ctx.content.rfind('\n', head.begin); + lineBegin = lineBegin == std::string::npos ? 0 : lineBegin + 1; + if (!Trim(std::string_view(ctx.content).substr(lineBegin, head.begin - lineBegin)).empty()) continue; + std::string indent(std::string_view(ctx.content).substr(lineBegin, head.begin - lineBegin)); + + std::string replacement; + for (std::size_t d = i; d <= last; ++d) { + // Each declarator gets its OWN type. This is the whole reason + // this rule needed the AST: copying the head's type prefix + // turns `int* a, b;` into `int* a; int* b;` and silently + // changes b's type. clang has already resolved b as plain int. + std::string type = NormalisePointerSpelling(decls[d].type); + std::string_view declarator = std::string_view(ctx.content).substr(decls[d].nameOffset, decls[d].end - decls[d].nameOffset); + if (d > i) replacement += std::format("\n{}", indent); + // NormalisePointerSpelling already attached the star to the + // type, so the separator is always a single space: `int* a`. + replacement += std::format("{} {};", type, Trim(declarator)); + } + edits.push_back({head.begin, stmtEnd, std::move(replacement)}); + i = last; } - if (changed) ctx.SetContent(std::move(out)); + + if (edits.empty()) return; + std::string out = ctx.content; + std::ranges::sort(edits, [](const Edit& a, const Edit& b) { return a.begin > b.begin; }); + for (const Edit& e : edits) out.replace(e.begin, e.end - e.begin, e.text); + ctx.SetContent(std::move(out)); }); // K&R braces: `{` on its own line after a `)`/else/do/try header joins diff --git a/tests/CleanProject/main.cpp b/tests/CleanProject/main.cpp index 4a9c8f0..ecad097 100644 --- a/tests/CleanProject/main.cpp +++ b/tests/CleanProject/main.cpp @@ -64,7 +64,7 @@ int main() { fs::path projectFile = root / "project.cpp"; std::ofstream(projectFile) << "\n"; fs::path cwdBin = fs::current_path() / "bin"; - bool cwdBinExisted = fs::exists(cwdBin); + const bool cwdBinExisted = fs::exists(cwdBin); CleanProject(projectFile); Check(!fs::exists(root / "bin"), "the named project's bin/ is gone"); diff --git a/tests/HouseRules/main.cpp b/tests/HouseRules/main.cpp index 7afa0a5..e4c8058 100644 --- a/tests/HouseRules/main.cpp +++ b/tests/HouseRules/main.cpp @@ -112,10 +112,32 @@ int main() { Check(r.text.contains("if (b) {"), "braced if body untouched"); } - // single-declaration: simple multi-decl splits; pointer decl only reports. + // single-declaration splits on the AST, so each declarator carries its own + // resolved type. That is the whole reason it is not a token rule: copying + // the head's type prefix turns `int* a, b;` into `int* a; int* b;` and + // silently changes b from int to int*. { - RuleRun r = RunRule("void F() {\n bool a = false, b = true;\n}\n", "single-declaration", LintMode::Apply); - Check(r.text.contains("bool a = false;\n bool b = true;"), "multi-declaration split"); + RuleRun r = RunRule("#include \n" + "void F() {\n" + " bool a = false, b = true;\n" + " int* ptrA = nullptr, *ptrB = nullptr;\n" + " int* mixed = nullptr, plain = 7;\n" + " std::vector tmplA{}, tmplB{};\n" + " int callA = g(), callB = h();\n" + " int keep = 1, kept = 2; // trailing comment\n" + "}\n", + "single-declaration", LintMode::Apply); + Check(r.text.contains("bool a = false;\n bool b = true;"), "single-declaration: simple split"); + Check(r.text.contains("int* ptrA = nullptr;\n int* ptrB = nullptr;"), "single-declaration: pointers split, star kept on the type"); + // The case a token-based splitter gets wrong. + Check(r.text.contains("int* mixed = nullptr;\n int plain = 7;"), "single-declaration: only the starred declarator is a pointer"); + Check(r.text.contains("std::vector tmplA{};\n std::vector tmplB{};"), "single-declaration: template arguments are not a bail-out"); + Check(r.text.contains("int callA = g();\n int callB = h();"), "single-declaration: call initialisers are not a bail-out"); + // A comment after the ';' is outside the replaced range, so it survives; + // the line-based version refused the whole line instead. + Check(r.text.contains("int keep = 1;\n int kept = 2; // trailing comment"), "single-declaration: trailing comment survives the split"); + RuleRun again = RunRule(r.text, "single-declaration", LintMode::Apply); + Check(again.summary.changedFiles.empty(), "single-declaration: idempotent"); } // wrap-join: short wrapped call joins; operator chain joins; long stays. @@ -295,6 +317,194 @@ int main() { Check(r.text == source, "fixed-width-types leaves type names inside a raw string alone"); } + // no-char-pointer reads the AST, so it distinguishes OUR char* from one + // whose spelling belongs to somebody else's header. This is what replaced + // the substring denylist (argv, getenv, c_str, reinterpret_cast, …): each + // entry there disabled the rule for a whole line, and the list could only + // grow as new libraries arrived. + { + RuleRun r = RunRule("#include \n" + "#include \n" + "extern \"C\" const char* CApiEntry(const char* path);\n" + "char* OurBadApi(char* input) { return input; }\n" + "void F() {\n" + " char* fromLibc = std::getenv(\"HOME\");\n" + " std::string mine = \"ok\";\n" + " const char* toLibc = mine.c_str();\n" + " auto raw = reinterpret_cast(&mine);\n" + "}\n", + "no-char-pointer", LintMode::Report); + // Ours, so reported: the declaration and its parameter. + Check(HasFinding(r.summary, "'OurBadApi'"), "no-char-pointer: our own char* return is reported"); + Check(HasFinding(r.summary, "'input'"), "no-char-pointer: our own char* parameter is reported"); + // Foreign, so exempt — each for a reason, not by name. + Check(!HasFinding(r.summary, "'CApiEntry'"), "no-char-pointer: extern \"C\" declaration is exempt"); + Check(!HasFinding(r.summary, "'path'"), "no-char-pointer: extern \"C\" parameter is exempt"); + Check(!HasFinding(r.summary, "'fromLibc'"), "no-char-pointer: a value from libc is exempt"); + Check(!HasFinding(r.summary, "'toLibc'"), "no-char-pointer: a value from c_str() is exempt"); + // A local whose deduced type is char* is still our declaration, so it + // is reported. The old denylist exempted every line mentioning + // reinterpret_cast; a deliberate low-level cast now takes an explicit + // lint-disable comment, which is at least visible at the site. + Check(HasFinding(r.summary, "'raw'"), "no-char-pointer: a deduced char* local is still ours"); + } + + // Cases the line-based heuristic structurally could not see. It only looked + // at declarations starting at cumulative paren depth 0 and inferred scope + // from a running {/} count, so a wrapped signature, a specifier split over + // two lines, or a local shadowing a member all escaped it. + { + RuleRun r = RunRule("#include \n" + "namespace Outer {\n" + " int wrapped_function(\n" + " int first,\n" + " int second) { return first + second; }\n" + " static\n" + " int splitStatic = 1;\n" + " struct Holder {\n" + " int Member = 0;\n" + " void Method() {\n" + " int Local = 1;\n" + " (void)Local;\n" + " }\n" + " };\n" + "}\n" + "extern \"C\" int c_api_entry(const char* name);\n", + "naming", LintMode::Report); + // A signature wrapped over lines is still a function declaration. + Check(HasFinding(r.summary, "'wrapped_function' should be PascalCase"), "naming: wrapped signature is seen"); + // `static` on its own line still applies to the declaration below it. + Check(HasFinding(r.summary, "'splitStatic' should be PascalCase"), "naming: split specifier is seen"); + // Members are camelCase; the enclosing kind decides, not a brace count. + Check(HasFinding(r.summary, "'Member' should be camelCase"), "naming: member is checked as a member"); + // A local inside a method is a local, not a member and not a global. + Check(HasFinding(r.summary, "'Local' should be camelCase"), "naming: local inside a method is a local"); + // Named by libc, not by us. + Check(!HasFinding(r.summary, "c_api_entry"), "naming: extern \"C\" declaration is exempt"); + Check(!HasFinding(r.summary, "'Method'"), "naming: PascalCase method passes"); + Check(!HasFinding(r.summary, "'Holder'"), "naming: PascalCase type passes"); + } + + // fixed-width-types must not rewrite an integer whose width somebody else's + // header chose — the rewrite would leave code that no longer matches the API + // it calls. Derived from the AST, so the exemption is per-declaration rather + // than the old per-line textual one, which both missed cases and disabled + // the rule for anything else sharing a line with `argc`/`extern "`. + { + RuleRun r = RunRule("#include \n" + "extern \"C\" unsigned long CApiCall(unsigned int flags);\n" + "long OurOwnApi(int value) { return value; }\n" + "void F() {\n" + " unsigned long fromLibc = std::strtoul(\"1\", nullptr, 10);\n" + " unsigned ours = 1;\n" + " (void)fromLibc; (void)ours;\n" + "}\n" + "int main(int argc, char** argv) {\n" + " for (int i = 0; i < argc; ++i) { (void)argv[i]; }\n" + " return 0;\n" + "}\n", + "fixed-width-types", LintMode::Apply); + // Somebody else's widths, kept. + Check(r.text.contains("extern \"C\" unsigned long CApiCall(unsigned int flags);"), "fixed-width: extern \"C\" signature keeps its widths"); + Check(r.text.contains("unsigned long fromLibc = std::strtoul"), "fixed-width: a value from a C library keeps its width"); + Check(r.text.contains("int main(int argc, char** argv)"), "fixed-width: main's signature is untouched"); + // Ours, converted. + Check(r.text.contains("std::int64_t OurOwnApi(std::int32_t value)"), "fixed-width: our own signature converts"); + Check(r.text.contains("std::uint32_t ours = 1;"), "fixed-width: our own local converts"); + // The loop counter inside main's BODY is ordinary code. The old rule + // skipped it because `argc` appeared on the same line; only the + // signature is exempt now, not everything near it. + Check(r.text.contains("for (std::int32_t i = 0;"), "fixed-width: main's body is not exempt, only its signature"); + } + + // enum-class asks for the next TOKEN after `enum`, so a declaration split + // over lines reads the same as one that is not. The regex it replaced + // required the name to follow `enum` on the same line. + { + RuleRun r = RunRule("enum Plain { A };\n" + "enum\n" + " Split { B };\n" + "enum class Scoped { C };\n" + "enum\n" + " class SplitScoped { D };\n" + "enum struct AlsoFine { E };\n", + "enum-class", LintMode::Report); + Check(r.summary.findings.size() == 2, std::format("enum-class: exactly the two plain enums ({} found)", r.summary.findings.size())); + const bool onFirst = std::any_of(r.summary.findings.begin(), r.summary.findings.end(), [](const LintFinding& f) { return f.line == 1; }); + const bool onSplit = std::any_of(r.summary.findings.begin(), r.summary.findings.end(), [](const LintFinding& f) { return f.line == 2; }); + Check(onFirst, "enum-class: single-line plain enum reported"); + Check(onSplit, "enum-class: line-split plain enum reported at the keyword"); + } + + // const-local's mutation analysis is exact for scalars: assignment, ++/--, + // address-of and binding to a non-const reference are the only ways to + // write one, and all four are tracked. Both directions matter — a missed + // write means advising const on something that cannot be const. + { + RuleRun r = RunRule("void Mutate(int& out);\n" + "void ReadOnly(const int& in);\n" + "void ByValue(int v);\n" + "int Compute();\n" + "void F() {\n" + " int neverWritten = 1;\n" + " int assigned = 1; assigned = 2;\n" + " int incremented = 1; ++incremented;\n" + " int compound = 1; compound += 2;\n" + " int addressed = 1; int* taken = &addressed;\n" + " int toMutatingRef = 1; Mutate(toMutatingRef);\n" + " int toConstRef = 1; ReadOnly(toConstRef);\n" + " int toByValue = 1; ByValue(toByValue);\n" + " int boundToRef = 1; int& alias = boundToRef;\n" + " for (int loop = 0; loop < 1; ++loop) { (void)loop; }\n" + " (void)taken; (void)alias;\n" + "}\n", + "const-local", LintMode::Report); + Check(HasFinding(r.summary, "'neverWritten'"), "const-local: an unwritten local is reported"); + Check(HasFinding(r.summary, "'toConstRef'"), "const-local: passing to a const& is not a write"); + Check(HasFinding(r.summary, "'toByValue'"), "const-local: passing by value is not a write"); + Check(!HasFinding(r.summary, "'assigned'"), "const-local: assignment is a write"); + Check(!HasFinding(r.summary, "'incremented'"), "const-local: ++ is a write"); + Check(!HasFinding(r.summary, "'compound'"), "const-local: += is a write"); + Check(!HasFinding(r.summary, "'addressed'"), "const-local: taking an address counts as a write"); + Check(!HasFinding(r.summary, "'toMutatingRef'"), "const-local: binding to a non-const& parameter is a write"); + Check(!HasFinding(r.summary, "'boundToRef'"), "const-local: binding to a non-const& local is a write"); + Check(!HasFinding(r.summary, "'loop'"), "const-local: a mutated loop counter is not reported"); + // Pointers and range-for bindings are excluded: `T* const p` and + // `for (T* const x : …)` are not spellings anybody writes. + Check(!HasFinding(r.summary, "'taken'"), "const-local: pointer locals are out of scope"); + } + { + RuleRun r = RunRule("void F() {\n" " for (int each : Range()) { (void)each; }\n" "}\n", "const-local", LintMode::Report); + Check(!HasFinding(r.summary, "'each'"), "const-local: a range-for binding is not reported"); + } + + // constexpr-constant asks clang's constant evaluator, not the tokens of the + // initialiser. sizeof and a fold over other constants are constant + // expressions that no token scan can recognise; a call result is not. + { + RuleRun r = RunRule("int Compute();\n" + "constexpr int Base = 4;\n" + "struct Big { int a, b; };\n" + "void F() {\n" + " const int literal = 4;\n" + " const int folded = 1 << 4;\n" + " const int fromSizeof = sizeof(Big);\n" + " const int fromOtherConstant = Base + 1;\n" + " const int fromCall = Compute();\n" + " constexpr int already = 8;\n" + " (void)literal; (void)folded; (void)fromSizeof;\n" + " (void)fromOtherConstant; (void)fromCall; (void)already;\n" + "}\n", + "constexpr-constant", LintMode::Report); + Check(HasFinding(r.summary, "'literal'"), "constexpr: a literal constant is reported"); + Check(HasFinding(r.summary, "'folded'"), "constexpr: an operator fold over literals is reported"); + // Neither of these is reachable from a token scan of the initialiser. + Check(HasFinding(r.summary, "'fromSizeof'"), "constexpr: sizeof folds"); + Check(HasFinding(r.summary, "'fromOtherConstant'"), "constexpr: a fold over another constant folds"); + Check(!HasFinding(r.summary, "'fromCall'"), "constexpr: a call result is not a constant expression"); + Check(!HasFinding(r.summary, "'already'"), "constexpr: an existing constexpr is not re-reported"); + } + if (Failures > 0) { std::println(std::cerr, "{} assertions failed", Failures); return 1; 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..69bd436 100644 --- a/tests/Lint/main.cpp +++ b/tests/Lint/main.cpp @@ -54,7 +54,7 @@ namespace { out.reserve(ctx.content.size()); for (std::size_t n = 1; n <= ctx.lines.size(); ++n) { std::string_view line = ctx.Line(n); - bool crlf = line.ends_with('\r'); + const bool crlf = line.ends_with('\r'); if (crlf) line.remove_suffix(1); while (line.ends_with(' ') || line.ends_with('\t')) line.remove_suffix(1); out += line; @@ -226,7 +226,7 @@ int main() { LintSummary sum = RunLint(cfg, Mode(LintMode::Apply)); Check(s.Read("a") == "hello\nworld\n", "Apply rewrites the file"); Check(sum.changedFiles.size() == 1, "one file formatted"); - bool hasNote = std::any_of(sum.findings.begin(), sum.findings.end(), [](const LintFinding& f) { return f.rule == "note"; }); + const bool hasNote = std::any_of(sum.findings.begin(), sum.findings.end(), [](const LintFinding& f) { return f.rule == "note"; }); Check(hasNote, "report-only findings still recorded in Apply mode"); Check(sum.errors == 0, "clean apply has no errors"); } @@ -259,8 +259,8 @@ int main() { ctx.SetContent(std::move(c)); }); LintSummary rep = RunLint(cfg, Mode(LintMode::Report)); - bool one = std::any_of(rep.findings.begin(), rep.findings.end(), [](const LintFinding& f) { return f.rule == "one"; }); - bool two = std::any_of(rep.findings.begin(), rep.findings.end(), [](const LintFinding& f) { return f.rule == "two"; }); + const bool one = std::any_of(rep.findings.begin(), rep.findings.end(), [](const LintFinding& f) { return f.rule == "one"; }); + const bool two = std::any_of(rep.findings.begin(), rep.findings.end(), [](const LintFinding& f) { return f.rule == "two"; }); Check(one && two, "chained transforms both attributed"); Check(s.Read("a") == "AAA\n", "Report leaves chain input untouched"); LintSummary app = RunLint(cfg, Mode(LintMode::Apply)); @@ -281,7 +281,7 @@ int main() { Check(s.Read("a") == "keep\n", "throwing transform reverted, disk untouched"); Check(sum.errors == 1, "exception counted as error"); Check(sum.changedFiles.empty(), "reverted transform is not a change"); - bool threw = std::any_of(sum.findings.begin(), sum.findings.end(), + const bool threw = std::any_of(sum.findings.begin(), sum.findings.end(), [](const LintFinding& f) { return f.line == 0 && f.message.contains("mid-transform"); }); @@ -301,7 +301,7 @@ int main() { } }); LintSummary rep = RunLint(cfg, Mode(LintMode::Report)); - bool wholeFile = std::any_of(rep.findings.begin(), rep.findings.end(), + const bool wholeFile = std::any_of(rep.findings.begin(), rep.findings.end(), [](const LintFinding& f) { return f.rule == "final-newline" && f.line == 0; }); @@ -323,8 +323,8 @@ int main() { ctx.SetContent(std::move(c)); }); LintSummary rep = RunLint(cfg, Mode(LintMode::Report)); - bool flagged = std::any_of(rep.findings.begin(), rep.findings.end(), [](const LintFinding& f) { return f.message == "flagged"; }); - bool reformat = std::any_of(rep.findings.begin(), rep.findings.end(), [](const LintFinding& f) { return f.message == "would reformat"; }); + const bool flagged = std::any_of(rep.findings.begin(), rep.findings.end(), [](const LintFinding& f) { return f.message == "flagged"; }); + const bool reformat = std::any_of(rep.findings.begin(), rep.findings.end(), [](const LintFinding& f) { return f.message == "would reformat"; }); Check(flagged && reformat, "mixed rule records both finding kinds"); RunLint(cfg, Mode(LintMode::Apply)); Check(s.Read("a") == "bad\n", "mixed rule's transform applied"); @@ -389,10 +389,10 @@ int main() { }); AddTrimRule(cfg); LintSummary sum = RunLint(cfg, Mode(LintMode::Report)); - bool line2Silent = std::none_of(sum.findings.begin(), sum.findings.end(), [](const LintFinding& f) { return f.line == 2 && f.rule != "trim"; }); - bool line3Loud = std::count_if(sum.findings.begin(), sum.findings.end(), [](const LintFinding& f) { return f.line == 3; }) >= 2; + const bool line2Silent = std::none_of(sum.findings.begin(), sum.findings.end(), [](const LintFinding& f) { return f.line == 2 && f.rule != "trim"; }); + const bool line3Loud = std::count_if(sum.findings.begin(), sum.findings.end(), [](const LintFinding& f) { return f.line == 3; }) >= 2; Check(line2Silent, "both named rules suppressed on the target line"); - bool trimStillFires = std::any_of(sum.findings.begin(), sum.findings.end(), [](const LintFinding& f) { return f.line == 2 && f.rule == "trim"; }); + const bool trimStillFires = std::any_of(sum.findings.begin(), sum.findings.end(), [](const LintFinding& f) { return f.line == 2 && f.rule == "trim"; }); Check(trimStillFires, "unnamed rule still fires on the target line"); Check(line3Loud, "unsuppressed line reports from both rules"); } @@ -419,7 +419,7 @@ int main() { AddTrimRule(cfg); cfg.AddLintRule("flag", [](LintContext& ctx) { ctx.Report(2, "bad"); }); LintSummary rep = RunLint(cfg, Mode(LintMode::Report)); - bool onlyTrim = !rep.findings.empty() && std::all_of(rep.findings.begin(), rep.findings.end(), [](const LintFinding& f) { return f.rule == "trim"; }); + const bool onlyTrim = !rep.findings.empty() && std::all_of(rep.findings.begin(), rep.findings.end(), [](const LintFinding& f) { return f.rule == "trim"; }); Check(onlyTrim, "file-level rule directive kills that rule, trim still fires"); RunLint(cfg, Mode(LintMode::Apply)); Check(s.Read("a") == "// lint-disable-file flag\nbad\n", "other rules still format"); @@ -458,12 +458,12 @@ int main() { Check(offsetsSound, "tokens: every offset/length addresses real bytes"); // Ordered by offset, so binary search in TokensOnLine is valid. - bool ordered = std::ranges::is_sorted(toks, {}, &LintToken::offset); + const bool ordered = std::ranges::is_sorted(toks, {}, &LintToken::offset); Check(ordered, "tokens: stream is in source order"); // Inactive #ifdef branch is still lexed — this is what keeps token // rules covering every platform, unlike an AST. - bool sawHidden = std::ranges::any_of(toks, [&](const LintToken& t) { + const bool sawHidden = std::ranges::any_of(toks, [&](const LintToken& t) { return t.kind == LintTokenKind::Identifier && ctx.TokenText(t) == "HiddenBranch"; }); Check(sawHidden, "tokens: inactive #ifdef branch is lexed"); @@ -555,6 +555,169 @@ 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"); + // 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. + { + 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;