diff --git a/implementations/Crafter.Build-Clang.cpp b/implementations/Crafter.Build-Clang.cpp index 6a5d2ab..942769f 100644 --- a/implementations/Crafter.Build-Clang.cpp +++ b/implementations/Crafter.Build-Clang.cpp @@ -1638,7 +1638,7 @@ Exit status: } // lint-disable-next-line no-char-pointer -int Crafter::Run(int argc, char** argv) { +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"; @@ -1654,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); diff --git a/interfaces/Crafter.Build-Clang.cppm b/interfaces/Crafter.Build-Clang.cppm index d35ac23..def137b 100644 --- a/interfaces/Crafter.Build-Clang.cppm +++ b/interfaces/Crafter.Build-Clang.cppm @@ -554,7 +554,7 @@ export namespace Crafter { // 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 int Run(int argc, char** argv); + 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/lint-rules.h b/lint-rules.h index b18c9fc..f4054a7 100644 --- a/lint-rules.h +++ b/lint-rules.h @@ -487,11 +487,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. @@ -503,13 +532,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; @@ -546,6 +570,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)}); diff --git a/tests/HouseRules/main.cpp b/tests/HouseRules/main.cpp index e34adbb..a82871e 100644 --- a/tests/HouseRules/main.cpp +++ b/tests/HouseRules/main.cpp @@ -363,6 +363,38 @@ int main() { 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"); + } + if (Failures > 0) { std::println(std::cerr, "{} assertions failed", Failures); return 1;