Catcrafts/Crafter.LintRules
Watch
2
0
Fork
You've already forked Crafter.LintRules
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

lint rules lite

Browse source
This commit is contained in:
Jorijn van der Graaf 2026-10-31 02:21:19 +01:00
commit 42a27016e3
Signed by: jorijnvdgraaf
GPG key ID: 2937E59CDCC1BCFB
4 changed files with 213 additions and 111 deletions
Download patch file Download diff file Expand all files Collapse all files

40
README.md
View file

@ -21,6 +21,8 @@ extern "C" Configuration CrafterBuildProject(std::span<const std::string_view> 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: Then `crafter-build lint` reports, `crafter-build format` rewrites, and `crafter-build format --check` gates CI. Pin a revision for reproducible builds:
```cpp ```cpp
@ -31,22 +33,22 @@ Suppress a finding with `// lint-disable-next-line <rule>` or a whole file with
## Rules ## Rules
| Rule | Kind | What it wants | | Rule | Kind | Lite | What it wants |
| --- | --- | --- | | --- | --- | --- | --- |
| `spdx-header` | report | `// SPDX-License-Identifier:` on line 1, `// SPDX-FileCopyrightText:` on line 2, blank line 3 | | `spdx-header` | report | ✓ | `// SPDX-License-Identifier:` on line 1, `// SPDX-FileCopyrightText:` on line 2, blank line 3 |
| `no-tabs` | report | spaces | | `no-tabs` | report | | spaces |
| `naming` | report, AST | PascalCase types and functions; camelCase variables, including statics and constants | | `naming` | report, AST | | PascalCase types and functions; camelCase variables, including statics and constants |
| `enum-class` | report | `enum class`, never a plain `enum` | | `enum-class` | report | ✓ | `enum class`, never a plain `enum` |
| `no-iostream-print` | report | `std::println` instead of `std::cout`/`std::cerr` streaming | | `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 | | `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 | | `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 | | `constexpr-constant` | report, AST | ✓ | `constexpr` on a constant with a constant initialiser |
| `format-concat` | transform | `std::format` instead of `+` with string literals | | `format-concat` | transform | ✓ | `std::format` instead of `+` with string literals |
| `fixed-width-types` | transform, AST | `std::int32_t` and friends instead of `int`, `long`, … | | `fixed-width-types` | transform, AST | ✓ | `std::int32_t` and friends instead of `int`, `long`, … |
| `single-declaration` | transform, AST | one declaration per statement | | `single-declaration` | transform, AST | | one declaration per statement |
| `brace-style` | transform | K&R braces | | `brace-style` | transform | | K&R braces |
| `if-single-line` | transform | `if (x) return;` on one line when it fits | | `if-single-line` | transform | | `if (x) return;` on one line when it fits |
| `wrap-join` | transform | join wrapped statements that fit on one line | | `wrap-join` | transform | | join wrapped statements that fit on one line |
| `paren-spacing` | transform | `if (` and `for (`, no space before a call's `(` | | `paren-spacing` | transform | | `if (` and `for (`, no space before a call's `(` |
| `trim-trailing-ws` | transform | no trailing whitespace | | `trim-trailing-ws` | transform | | no trailing whitespace |
| `final-newline` | transform | files end with a newline | | `final-newline` | transform | | files end with a newline |

192
interfaces/Crafter.LintRules.cppm
View file

@ -13,12 +13,19 @@ import Crafter.Build;
// ... // ...
// LintRules::AddRules(cfg); // 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 // Deliberately not part of the crafter-build library, so the tool itself
// never nudges anyone toward one team's style. // never nudges anyone toward one team's style.
// //
// Report-only rules are registered first so their file:line numbers always // 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 // 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 // All detection runs on ctx.CommentStripped() — comments and string/char
// literal bodies are blanked to spaces with newlines preserved, so byte // 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); if (base.starts_with("volatile ")) base.remove_prefix(9);
return base == "char"; return base == "char";
} }
}
export namespace Crafter::LintRules { // ---------------- report-only rules ----------------
void AddRules(Crafter::Configuration& cfg) { void AddCoreReports(Crafter::Configuration& cfg) {
using Crafter::LintContext; using Crafter::LintContext;
// ---------------- report-only rules ----------------
// License header: SPDX identifier on line 1, copyright on line 2, blank // License header: SPDX identifier on line 1, copyright on line 2, blank
// line 3. // line 3.
cfg.AddLintRule("spdx-header", [](LintContext& ctx) { 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<const Crafter::LintDecl> 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 // Scoped enums only. Kept on tokens rather than moved to the AST, even
// though LintDecl carries an exact isScopedEnum: an AST is one // though LintDecl carries an exact isScopedEnum: an AST is one
// configuration's slice, so a plain enum inside a preprocessor branch that // 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)); 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<const Crafter::LintDecl> 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 // String building via `+` with a literal operand → std::format. Plain
// `a += b` accumulation (builder pattern) stays legal. Single-line chains // `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); for (const Rep& r : reps) out.replace(r.pos, r.len, r.to);
ctx.SetContent(std::move(out)); ctx.SetContent(std::move(out));
}); });
}
void AddStyleTransforms(Crafter::Configuration& cfg) {
using Crafter::LintContext;
// One declaration per statement. Auto-splits the simple initialized form // One declaration per statement. Auto-splits the simple initialized form
// `Type a = x, b = y;`; anything with pointers/references, parens, or // `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);
}
} }

21
project.cpp
View file

@ -7,9 +7,6 @@ namespace fs = std::filesystem;
using namespace Crafter; using namespace Crafter;
extern "C" Configuration CrafterBuildProject(std::span<const std::string_view> args) { extern "C" Configuration CrafterBuildProject(std::span<const std::string_view> args) {
std::vector<std::string> depArgs(args.begin(), args.end());
const bool useLocal = std::erase(depArgs, "--local") > 0;
Configuration cfg; Configuration cfg;
cfg.path = "./"; cfg.path = "./";
cfg.name = "Crafter.LintRules"; cfg.name = "Crafter.LintRules";
@ -28,15 +25,15 @@ extern "C" Configuration CrafterBuildProject(std::span<const std::string_view> a
// The rule test compiles the module as its own interface and links // 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 // crafter-build to run the linter in-process: from git by default, or
// the sibling checkout with `crafter-build test --local`. Host only, // the sibling checkout with `crafter-build test --local`. Only when this
// like the rules themselves, and only when this is the project being // is the project being built: crafter-build itself loads this file as a
// built: crafter-build itself loads this file as a dependency, and the // dependency, and the test's GitProject would then load crafter-build's
// test's GitProject would then load crafter-build's project file again // project file again and recurse into the cycle guard. No target check:
// and recurse into the cycle guard. // crafter-build only runs on Linux, and the hard-coded triple this used
if (IsTopLevelProject() && cfg.target == "x86_64-pc-linux-gnu") { // to compare against silently dropped the test on any host that spells
Configuration* build = useLocal // its triple differently (the Flatpak's x86_64-unknown-linux-gnu).
? LocalProject({ .projectFile = "../Crafter.Build/project.cpp", .args = depArgs }) if (IsTopLevelProject()) {
: GitProject({ .source = { .url = "https://forgejo.catcrafts.net/Catcrafts/Crafter.Build.git" }, .args = depArgs }); Configuration* build = GitProject({ .source = { .url = "https://forgejo.catcrafts.net/Catcrafts/Crafter.Build.git" }, .args = depArgs });
cfg.AddTest("HouseRules", ifaces).Dependencies({ build }); cfg.AddTest("HouseRules", ifaces).Dependencies({ build });
} }
return cfg; return cfg;

71
tests/HouseRules/main.cpp
View file

@ -9,8 +9,10 @@ using namespace Crafter;
// Validates the house-style ruleset — the transforms and reports every repo // Validates the house-style ruleset — the transforms and reports every repo
// that imports Crafter.LintRules relies on. Each case writes a scratch file, // 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 // runs ONE rule via the glob filter (or all of them, for the cases about the
// rewritten bytes. // 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 { namespace {
std::int32_t Failures = 0; std::int32_t Failures = 0;
@ -26,7 +28,7 @@ namespace {
std::string text; // file content after the run 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; static std::int32_t Counter = 0;
fs::path dir = fs::temp_directory_path() / "crafter-build-house-rules" / std::format("case-{}", Counter++); fs::path dir = fs::temp_directory_path() / "crafter-build-house-rules" / std::format("case-{}", Counter++);
fs::remove_all(dir); fs::remove_all(dir);
@ -43,7 +45,7 @@ namespace {
std::array<fs::path, 0> ifaces = {}; std::array<fs::path, 0> ifaces = {};
std::array<fs::path, 1> impls = { "f" }; std::array<fs::path, 1> impls = { "f" };
cfg.GetInterfacesAndImplementations(ifaces, impls); cfg.GetInterfacesAndImplementations(ifaces, impls);
LintRules::AddRules(cfg); addRules(cfg);
RunLintOptions opts; RunLintOptions opts;
opts.mode = mode; opts.mode = mode;
opts.globs = { std::string(rule) }; opts.globs = { std::string(rule) };
@ -534,6 +536,67 @@ int main() {
Check(!HasFinding(r.summary, "'already'"), "constexpr: an existing constexpr is not re-reported"); 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<std::string> names;
for (const LintRule& rule : cfg.lintRules) names.push_back(rule.name);
return names;
};
std::vector<std::string> lite = namesOf(LintRules::AddLiteRules);
std::vector<std::string> full = namesOf(LintRules::AddRules);
std::vector<std::string> expectedLite = {
"spdx-header", "enum-class", "no-iostream-print", "no-char-pointer", "const-local", "constexpr-constant",
"format-concat", "fixed-width-types",
};
std::vector<std::string> 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 <cstdint>\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) { if (Failures > 0) {
std::println(std::cerr, "{} assertions failed", Failures); std::println(std::cerr, "{} assertions failed", Failures);
return 1; return 1;