diff --git a/README.md b/README.md index e5c72ca..6cb9a0b 100644 --- a/README.md +++ b/README.md @@ -21,6 +21,8 @@ extern "C" Configuration CrafterBuildProject(std::span a } ``` +For a project that keeps its own formatting and naming, call `LintRules::AddLiteRules(cfg)` instead. It registers only the rules marked lite below, leaving layout, whitespace and naming alone; under `format` it still applies `format-concat` and `fixed-width-types`, and nothing else. + Then `crafter-build lint` reports, `crafter-build format` rewrites, and `crafter-build format --check` gates CI. Pin a revision for reproducible builds: ```cpp @@ -31,22 +33,22 @@ Suppress a finding with `// lint-disable-next-line ` or a whole file with ## Rules -| Rule | Kind | What it wants | -| --- | --- | --- | -| `spdx-header` | report | `// SPDX-License-Identifier:` on line 1, `// SPDX-FileCopyrightText:` on line 2, blank line 3 | -| `no-tabs` | report | spaces | -| `naming` | report, AST | PascalCase types and functions; camelCase variables, including statics and constants | -| `enum-class` | report | `enum class`, never a plain `enum` | -| `no-iostream-print` | report | `std::println` instead of `std::cout`/`std::cerr` streaming | -| `no-char-pointer` | report, AST | `std::string` / `std::string_view` over `char*` outside C interop | -| `const-local` | report, AST | `const` on a scalar local that is never written | -| `constexpr-constant` | report, AST | `constexpr` on a constant with a constant initialiser | -| `format-concat` | transform | `std::format` instead of `+` with string literals | -| `fixed-width-types` | transform, AST | `std::int32_t` and friends instead of `int`, `long`, … | -| `single-declaration` | transform, AST | one declaration per statement | -| `brace-style` | transform | K&R braces | -| `if-single-line` | transform | `if (x) return;` on one line when it fits | -| `wrap-join` | transform | join wrapped statements that fit on one line | -| `paren-spacing` | transform | `if (` and `for (`, no space before a call's `(` | -| `trim-trailing-ws` | transform | no trailing whitespace | -| `final-newline` | transform | files end with a newline | +| Rule | Kind | Lite | What it wants | +| --- | --- | --- | --- | +| `spdx-header` | report | ✓ | `// SPDX-License-Identifier:` on line 1, `// SPDX-FileCopyrightText:` on line 2, blank line 3 | +| `no-tabs` | report | | spaces | +| `naming` | report, AST | | PascalCase types and functions; camelCase variables, including statics and constants | +| `enum-class` | report | ✓ | `enum class`, never a plain `enum` | +| `no-iostream-print` | report | ✓ | `std::println` instead of `std::cout`/`std::cerr` streaming | +| `no-char-pointer` | report, AST | ✓ | `std::string` / `std::string_view` over `char*` outside C interop | +| `const-local` | report, AST | ✓ | `const` on a scalar local that is never written | +| `constexpr-constant` | report, AST | ✓ | `constexpr` on a constant with a constant initialiser | +| `format-concat` | transform | ✓ | `std::format` instead of `+` with string literals | +| `fixed-width-types` | transform, AST | ✓ | `std::int32_t` and friends instead of `int`, `long`, … | +| `single-declaration` | transform, AST | | one declaration per statement | +| `brace-style` | transform | | K&R braces | +| `if-single-line` | transform | | `if (x) return;` on one line when it fits | +| `wrap-join` | transform | | join wrapped statements that fit on one line | +| `paren-spacing` | transform | | `if (` and `for (`, no space before a call's `(` | +| `trim-trailing-ws` | transform | | no trailing whitespace | +| `final-newline` | transform | | files end with a newline | diff --git a/interfaces/Crafter.LintRules.cppm b/interfaces/Crafter.LintRules.cppm index e9a60da..1dd9516 100644 --- a/interfaces/Crafter.LintRules.cppm +++ b/interfaces/Crafter.LintRules.cppm @@ -13,12 +13,19 @@ import Crafter.Build; // ... // LintRules::AddRules(cfg); // +// AddLiteRules(cfg) in its place registers the same rules minus the style +// ones (layout, whitespace and naming), for a project that keeps its own. +// // Deliberately not part of the crafter-build library, so the tool itself // never nudges anyone toward one team's style. // // Report-only rules are registered first so their file:line numbers always // match the on-disk file; transforms run last because each one sees the -// previous transform's output, which can shift line numbers. +// previous transform's output, which can shift line numbers. Lite has to +// keep that order too, so the rules are grouped twice over: by kind, and +// into style and core (everything that is not style). AddRules registers +// all four groups and AddLiteRules the two core ones, so dropping the style +// rules never reorders what is left. // // All detection runs on ctx.CommentStripped() — comments and string/char // literal bodies are blanked to spaces with newlines preserved, so byte @@ -101,15 +108,12 @@ bool IsCharPointer(std::string_view type) { if (base.starts_with("volatile ")) base.remove_prefix(9); return base == "char"; } -} -export namespace Crafter::LintRules { +// ---------------- report-only rules ---------------- -void AddRules(Crafter::Configuration& cfg) { +void AddCoreReports(Crafter::Configuration& cfg) { using Crafter::LintContext; - // ---------------- report-only rules ---------------- - // License header: SPDX identifier on line 1, copyright on line 2, blank // line 3. cfg.AddLintRule("spdx-header", [](LintContext& ctx) { @@ -125,75 +129,6 @@ void AddRules(Crafter::Configuration& cfg) { } }); - cfg.AddLintRule("no-tabs", [](LintContext& ctx) { - if (!IsCppFile(ctx)) return; - for (std::size_t n = 1; n <= ctx.lines.size(); ++n) { - if (ctx.Line(n).contains('\t')) ctx.Report(n, "tab character (use spaces)"); - } - }); - - // Naming: types and functions PascalCase, everything that holds a value - // camelCase — locals, members, globals, statics and constants alike. - // Storage and constness used to pick the case, which meant `static int x` - // and `namespace { int x; }` were spelled differently for no reason a - // reader could see; now where a variable lives changes nothing about - // its name. - // - // 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; - 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; - 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)); - } - 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: - case Kind::Field: - // A static data member arrives as a Variable whose parent - // is the class; a Field is a per-instance member. Both - // are camelCase, so the label is the only difference. - if (!IsCamelCase(decl.name)) { - ctx.Report(decl.line, std::format("{} '{}' should be camelCase", - decl.kind == Kind::Field ? "member" : "variable", decl.name)); - } - 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 @@ -326,8 +261,85 @@ void AddRules(Crafter::Configuration& cfg) { ctx.Report(decl.line, std::format("'{}' has a constant initialiser — declare it constexpr", decl.name)); } }); +} - // ---------------- transforms (auto-fixed by `crafter-build format`) ---------------- +void AddStyleReports(Crafter::Configuration& cfg) { + using Crafter::LintContext; + + cfg.AddLintRule("no-tabs", [](LintContext& ctx) { + if (!IsCppFile(ctx)) return; + for (std::size_t n = 1; n <= ctx.lines.size(); ++n) { + if (ctx.Line(n).contains('\t')) ctx.Report(n, "tab character (use spaces)"); + } + }); + + // Naming: types and functions PascalCase, everything that holds a value + // camelCase — locals, members, globals, statics and constants alike. + // Storage and constness used to pick the case, which meant `static int x` + // and `namespace { int x; }` were spelled differently for no reason a + // reader could see; now where a variable lives changes nothing about + // its name. + // + // 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; + 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; + 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)); + } + 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: + case Kind::Field: + // A static data member arrives as a Variable whose parent + // is the class; a Field is a per-instance member. Both + // are camelCase, so the label is the only difference. + if (!IsCamelCase(decl.name)) { + ctx.Report(decl.line, std::format("{} '{}' should be camelCase", + decl.kind == Kind::Field ? "member" : "variable", decl.name)); + } + break; + default: + break; + } + } + }); +} + +// ---------------- transforms (auto-fixed by `crafter-build format`) ---------------- + +void AddCoreTransforms(Crafter::Configuration& cfg) { + using Crafter::LintContext; // String building via `+` with a literal operand → std::format. Plain // `a += b` accumulation (builder pattern) stays legal. Single-line chains @@ -664,6 +676,10 @@ void AddRules(Crafter::Configuration& cfg) { for (const Rep& r : reps) out.replace(r.pos, r.len, r.to); ctx.SetContent(std::move(out)); }); +} + +void AddStyleTransforms(Crafter::Configuration& cfg) { + using Crafter::LintContext; // One declaration per statement. Auto-splits the simple initialized form // `Type a = x, b = y;`; anything with pointers/references, parens, or @@ -965,5 +981,29 @@ void AddRules(Crafter::Configuration& cfg) { } }); } +} + +export namespace Crafter::LintRules { + +// The full house style. +void AddRules(Crafter::Configuration& cfg) { + AddCoreReports(cfg); + AddStyleReports(cfg); + AddCoreTransforms(cfg); + AddStyleTransforms(cfg); +} + +// The house rules without the house look, for a project that keeps its own +// formatter and naming conventions. Leaves out every rule that only decides +// how code is laid out or named — braces, wrapping, paren spacing, +// whitespace, tabs, one declaration per statement, naming — and keeps the +// ones that ask for different code: the SPDX header, enum class, +// std::string over char*, const, constexpr, std::println, std::format and +// fixed-width integers. `crafter-build format` therefore still rewrites +// string concatenation and builtin integer types, and nothing else. +void AddLiteRules(Crafter::Configuration& cfg) { + AddCoreReports(cfg); + AddCoreTransforms(cfg); +} } diff --git a/project.cpp b/project.cpp index 8403d20..11da1ec 100644 --- a/project.cpp +++ b/project.cpp @@ -7,9 +7,6 @@ namespace fs = std::filesystem; using namespace Crafter; extern "C" Configuration CrafterBuildProject(std::span args) { - std::vector depArgs(args.begin(), args.end()); - const bool useLocal = std::erase(depArgs, "--local") > 0; - Configuration cfg; cfg.path = "./"; cfg.name = "Crafter.LintRules"; @@ -28,15 +25,15 @@ extern "C" Configuration CrafterBuildProject(std::span a // The rule test compiles the module as its own interface and links // crafter-build to run the linter in-process: from git by default, or - // the sibling checkout with `crafter-build test --local`. Host only, - // like the rules themselves, and only when this is the project being - // built: crafter-build itself loads this file as a dependency, and the - // test's GitProject would then load crafter-build's project file again - // and recurse into the cycle guard. - if (IsTopLevelProject() && cfg.target == "x86_64-pc-linux-gnu") { - Configuration* build = useLocal - ? LocalProject({ .projectFile = "../Crafter.Build/project.cpp", .args = depArgs }) - : GitProject({ .source = { .url = "https://forgejo.catcrafts.net/Catcrafts/Crafter.Build.git" }, .args = depArgs }); + // the sibling checkout with `crafter-build test --local`. Only when this + // is the project being built: crafter-build itself loads this file as a + // dependency, and the test's GitProject would then load crafter-build's + // project file again and recurse into the cycle guard. No target check: + // crafter-build only runs on Linux, and the hard-coded triple this used + // to compare against silently dropped the test on any host that spells + // its triple differently (the Flatpak's x86_64-unknown-linux-gnu). + if (IsTopLevelProject()) { + Configuration* build = GitProject({ .source = { .url = "https://forgejo.catcrafts.net/Catcrafts/Crafter.Build.git" }, .args = depArgs }); cfg.AddTest("HouseRules", ifaces).Dependencies({ build }); } return cfg; diff --git a/tests/HouseRules/main.cpp b/tests/HouseRules/main.cpp index 256d0c2..017e422 100644 --- a/tests/HouseRules/main.cpp +++ b/tests/HouseRules/main.cpp @@ -9,8 +9,10 @@ using namespace Crafter; // Validates the house-style ruleset — the transforms and reports every repo // that imports Crafter.LintRules relies on. Each case writes a scratch file, -// runs ONE rule via the glob filter, and asserts the findings and/or -// rewritten bytes. +// runs ONE rule via the glob filter (or all of them, for the cases about the +// ruleset as a whole), and asserts the findings and/or rewritten bytes. The +// lite cases pin AddLiteRules to AddRules minus the style rules: a style rule +// leaking into lite, or a core rule falling out of either set, fails them. namespace { std::int32_t Failures = 0; @@ -26,7 +28,7 @@ namespace { std::string text; // file content after the run }; - RuleRun RunRule(std::string_view content, std::string_view rule, LintMode mode) { + RuleRun RunRule(std::string_view content, std::string_view rule, LintMode mode, void (*addRules)(Configuration&) = LintRules::AddRules) { static std::int32_t Counter = 0; fs::path dir = fs::temp_directory_path() / "crafter-build-house-rules" / std::format("case-{}", Counter++); fs::remove_all(dir); @@ -43,7 +45,7 @@ namespace { std::array ifaces = {}; std::array impls = { "f" }; cfg.GetInterfacesAndImplementations(ifaces, impls); - LintRules::AddRules(cfg); + addRules(cfg); RunLintOptions opts; opts.mode = mode; opts.globs = { std::string(rule) }; @@ -534,6 +536,67 @@ int main() { Check(!HasFinding(r.summary, "'already'"), "constexpr: an existing constexpr is not re-reported"); } + // AddLiteRules is AddRules minus the style rules — the rest unchanged and + // in the same order, so reports still run before every transform. Both + // lists are spelled out: the full one so a rule lost while regrouping + // fails here, the lite one so a style rule slipping into a core group + // does. + { + auto namesOf = [](void (*addRules)(Configuration&)) { + Configuration cfg; + addRules(cfg); + std::vector names; + for (const LintRule& rule : cfg.lintRules) names.push_back(rule.name); + return names; + }; + std::vector lite = namesOf(LintRules::AddLiteRules); + std::vector full = namesOf(LintRules::AddRules); + std::vector expectedLite = { + "spdx-header", "enum-class", "no-iostream-print", "no-char-pointer", "const-local", "constexpr-constant", + "format-concat", "fixed-width-types", + }; + std::vector expectedFull = { + "spdx-header", "enum-class", "no-iostream-print", "no-char-pointer", "const-local", "constexpr-constant", + "no-tabs", "naming", + "format-concat", "fixed-width-types", + "single-declaration", "brace-style", "if-single-line", "wrap-join", "paren-spacing", "trim-trailing-ws", "final-newline", + }; + Check(lite == expectedLite, std::format("lite: exactly the non-style rules, in order (got {})", lite)); + Check(full == expectedFull, std::format("full: all 17 rules, reports before transforms (got {})", full)); + } + + // The same file through both entry points. It breaks the style rules — an + // Allman brace, tabs, snake_case functions, a wrapped call padded inside + // its parens, trailing whitespace, no final newline — and two core ones: + // a plain enum, which lite reports, and an `int`, which lite rewrites. + // Lite must do exactly that and leave every other byte alone. The full run + // is the control: it shows this fixture really does trip the style rules, + // which is what makes lite leaving them alone mean something. + { + std::string_view source = "// SPDX-License-Identifier: MIT\n" + "// SPDX-FileCopyrightText: Copyright (C) 2026 Fixture\n" + "\n" + "#include \n" + "enum Color { Red };\n" + "void call_me(bool alpha, bool beta);\n" + "int snake_case_fn(bool alpha, bool beta)\n" + "{\n" + "\tcall_me( alpha,\n" + "\t beta ); \n" + "\treturn 0;\n" + "}"; + std::string expected(source); + expected.replace(expected.find("int snake_case_fn"), 3, "std::int32_t"); + RuleRun lite = RunRule(source, "*", LintMode::Apply, LintRules::AddLiteRules); + Check(lite.text == expected, "lite: rewrites the int and leaves every style violation byte for byte"); + Check(lite.summary.errors == 0, "lite: every rule ran, the AST ones included"); + Check(lite.summary.findings.size() == 1 && lite.summary.findings[0].rule == "enum-class", std::format("lite: the plain enum is the only finding ({} found)", lite.summary.findings.size())); + RuleRun full = RunRule(source, "*", LintMode::Apply); + Check(full.text != lite.text, "full: the style transforms rewrite the same file further"); + Check(HasFinding(full.summary, "should be PascalCase"), "full: naming reports the same file"); + Check(HasFinding(full.summary, "tab character"), "full: no-tabs reports the same file"); + } + if (Failures > 0) { std::println(std::cerr, "{} assertions failed", Failures); return 1;