diff --git a/implementations/Crafter.Build-Lint.cpp b/implementations/Crafter.Build-Lint.cpp index 7317011..e233a88 100644 --- a/implementations/Crafter.Build-Lint.cpp +++ b/implementations/Crafter.Build-Lint.cpp @@ -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; diff --git a/interfaces/Crafter.Build-Clang.cppm b/interfaces/Crafter.Build-Clang.cppm index def137b..ba639fb 100644 --- a/interfaces/Crafter.Build-Clang.cppm +++ b/interfaces/Crafter.Build-Clang.cppm @@ -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; diff --git a/lint-rules.h b/lint-rules.h index 74d59ee..afcc522 100644 --- a/lint-rules.h +++ b/lint-rules.h @@ -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 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 decls = ctx.Decls(); + std::span tokens = ctx.Tokens(); + + struct Edit { std::size_t begin; std::size_t end; std::string text; }; + std::vector 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 diff --git a/tests/HouseRules/main.cpp b/tests/HouseRules/main.cpp index f71b46c..c4ee7c8 100644 --- a/tests/HouseRules/main.cpp +++ b/tests/HouseRules/main.cpp @@ -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 \n" + "void F() {\n" + " bool a = false, b = true;\n" + " int* ptrA = nullptr, *ptrB = nullptr;\n" + " int* mixed = nullptr, plain = 7;\n" + " std::vector 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 tmplA{};\n std::vector 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.