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

refactor(lint): single-declaration splits on the AST, per-declarator type

Browse source 
Tokens are not enough for this one, which is worth stating because it is the
opposite of the enum-class case. Splitting a multi-declarator statement by
copying the shared type prefix is wrong in C++:

    int* a, b;   ->   int* a; int* b;    // b was int, not int*

Only per-declarator types get it right, and clang has already resolved them —
`int *` for a, plain `int` for b. A token-based splitter cannot know.

The regex it replaces bailed on `*`, `&`, `<>`, parens and quotes, so pointers,
templates and call initialisers were all left alone. All three split now, and
the fixture proves the mixed pointer case above comes out correctly.

Groups are found structurally rather than by matching a line shape: the first
declarator's extent starts at the shared type, so begin < nameOffset, while a
continuation declarator's starts at its own name, so begin == nameOffset. That
signal comes from the AST itself. nameOffset is now on LintDecl, which is also
what lets the replacement reuse each declarator's original text verbatim
instead of reconstructing it.

Replacing a byte range rather than rewriting whole lines means a comment after
the ';' is outside the edit and survives — the line-based version refused to
touch any line carrying a comment. A comment INSIDE the statement still bails,
since the rewrite would swallow it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jorijn van der Graaf 2026-07-31 00:35:52 +02:00
commit 5ca2b3e1df
4 changed files with 112 additions and 35 deletions
Download patch file Download diff file Expand all files Collapse all files

1
implementations/Crafter.Build-Lint.cpp
View file

@ -465,6 +465,7 @@ namespace {
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;

7
interfaces/Crafter.Build-Clang.cppm
View file

@ -183,8 +183,15 @@ export namespace Crafter {
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;

111
lint-rules.h
View file

@ -70,6 +70,18 @@ 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;
}
// 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.
@ -598,41 +610,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<std::string_view> 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<const Crafter::LintDecl> decls = ctx.Decls();
std::span<const Crafter::LintToken> tokens = ctx.Tokens();
struct Edit { std::size_t begin; std::size_t end; std::string text; };
std::vector<Edit> 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

28
tests/HouseRules/main.cpp
View file

@ -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 <vector>\n"
"void F() {\n"
" bool a = false, b = true;\n"
" int* ptrA = nullptr, *ptrB = nullptr;\n"
" int* mixed = nullptr, plain = 7;\n"
" std::vector<int> 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<int> tmplA{};\n std::vector<int> 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.