From b51a5bffb0ff7138bce0574a4659946a32e20fd9 Mon Sep 17 00:00:00 2001 From: Jorijn van der Graaf Date: Fri, 2 Oct 2026 20:42:21 +0200 Subject: [PATCH] changed rule --- README.md | 14 ++------- interfaces/Crafter.LintRules.cppm | 37 ++++++++---------------- tests/HouseRules/main.cpp | 47 +++++++++++++++++++++++++------ 3 files changed, 53 insertions(+), 45 deletions(-) diff --git a/README.md b/README.md index ad0f4e8..9b3f2e7 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # Crafter.LintRules -The Catcrafts house-style lint rules for [crafter-build](https://forgejo.catcrafts.net/Catcrafts/Crafter.Build), packaged as one C++ module. Naming conventions, K&R braces, `enum class`, `std::println` over iostream, fixed-width integer types, `std::format` over concatenation, line wrapping, whitespace hygiene and a few AST-backed checks (`const` locals, `constexpr` constants, no `char*`). Most rules are transforms, so `crafter-build format` applies them. +The Catcrafts house-style lint rules for [crafter-build](https://forgejo.catcrafts.net/Catcrafts/Crafter.Build) projects, packaged as one C++ module. ## Use @@ -35,7 +35,7 @@ Suppress a finding with `// lint-disable-next-line ` or a whole file with | --- | --- | --- | | `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, functions, statics and constants; camelCase variables | +| `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 | @@ -49,12 +49,4 @@ Suppress a finding with `// lint-disable-next-line ` or a whole file with | `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 | - -## Tests - -`tests/HouseRules` exercises every rule against small inputs. It links crafter-build to run the linter in-process, so it fetches and builds it from git; with a sibling checkout at `../Crafter.Build` use `crafter-build test --local`. - -## Building for other targets - -The module imports `Crafter.Build`, which only exists for the host. That is all a rule ever needs: rules run inside the crafter-build process that loads your `project.cpp`. +| `final-newline` | transform | files end with a newline | \ No newline at end of file diff --git a/interfaces/Crafter.LintRules.cppm b/interfaces/Crafter.LintRules.cppm index 0a6c2d6..e9a60da 100644 --- a/interfaces/Crafter.LintRules.cppm +++ b/interfaces/Crafter.LintRules.cppm @@ -132,8 +132,12 @@ void AddRules(Crafter::Configuration& cfg) { } }); - // Naming: types and functions PascalCase, variables camelCase, with - // statics, namespace-scope globals and constexpr constants PascalCase. + // 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 @@ -150,10 +154,6 @@ void AddRules(Crafter::Configuration& cfg) { // 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: @@ -179,26 +179,13 @@ void AddRules(Crafter::Configuration& cfg) { } 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)); - } - } 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)); + // 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: diff --git a/tests/HouseRules/main.cpp b/tests/HouseRules/main.cpp index a2fd210..256d0c2 100644 --- a/tests/HouseRules/main.cpp +++ b/tests/HouseRules/main.cpp @@ -180,7 +180,8 @@ int main() { Check(r.text.contains("G(x);"), "paren padding removed"); } - // naming: wrong-case function/type/static/global flagged; camel local passes. + // naming: wrong-case function/type/global flagged; camelCase local and + // static pass. { RuleRun r = RunRule("namespace {\n" " std::int32_t bad_global = 0;\n" @@ -191,12 +192,44 @@ int main() { " std::int32_t fineLocal = 1;\n" "}\n", "naming", LintMode::Report); - Check(HasFinding(r.summary, "'bad_global' should be PascalCase"), "lowercase global flagged"); + Check(HasFinding(r.summary, "'bad_global' should be camelCase"), "snake_case global flagged"); Check(HasFinding(r.summary, "'lint_thing' should be PascalCase"), "lowercase type flagged"); Check(HasFinding(r.summary, "'lower_func' should be PascalCase"), "lowercase function flagged"); - Check(HasFinding(r.summary, "'lowerStatic' should be PascalCase"), "camel static flagged"); + Check(!HasFinding(r.summary, "lowerStatic"), "camelCase static passes"); Check(!HasFinding(r.summary, "fineLocal"), "camelCase local passes"); } + + // naming: statics and constants are variables, so camelCase like any + // other wherever they live — a static local, a static or constexpr + // member, a namespace-scope constant or a plain global. + { + RuleRun r = RunRule("namespace {\n" + " constexpr int MaxSize = 4;\n" + " const int DEFAULT_PORT = 80;\n" + " static int kCount = 0;\n" + " int plainGlobal = 0;\n" + "}\n" + "struct Holder {\n" + " static int Instances;\n" + " static constexpr int kLimit = 8;\n" + " int Field = 0;\n" + "};\n" + "void Fn() {\n" + " static int Counter = 0;\n" + " static constexpr int localMax = 2;\n" + " (void)Counter;\n" + "}\n", + "naming", LintMode::Report); + Check(HasFinding(r.summary, "'MaxSize' should be camelCase"), "naming: PascalCase constexpr constant flagged"); + Check(HasFinding(r.summary, "'DEFAULT_PORT' should be camelCase"), "naming: SCREAMING_CASE constant flagged"); + Check(!HasFinding(r.summary, "kCount"), "naming: camelCase static global passes"); + Check(!HasFinding(r.summary, "plainGlobal"), "naming: camelCase global passes"); + Check(HasFinding(r.summary, "'Instances' should be camelCase"), "naming: PascalCase static member flagged"); + Check(!HasFinding(r.summary, "kLimit"), "naming: camelCase static constexpr member passes"); + Check(HasFinding(r.summary, "'Field' should be camelCase"), "naming: PascalCase member flagged"); + Check(HasFinding(r.summary, "'Counter' should be camelCase"), "naming: PascalCase static local flagged"); + Check(!HasFinding(r.summary, "localMax"), "naming: camelCase static constexpr local passes"); + } { // Statement calls with inline lambda arguments are NOT function // definitions (the any_of regression); a genuine lowercase one-liner @@ -351,16 +384,14 @@ int main() { // 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. + // from a running {/} count, so a wrapped signature or a local shadowing a + // member 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" @@ -373,8 +404,6 @@ int main() { "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.