diff --git a/implementations/Crafter.Build-Lint.cpp b/implementations/Crafter.Build-Lint.cpp index b010b8a..8571e29 100644 --- a/implementations/Crafter.Build-Lint.cpp +++ b/implementations/Crafter.Build-Lint.cpp @@ -364,6 +364,21 @@ bool LintContext::LineHasComment(std::size_t line) { return std::ranges::any_of(onLine, [](const LintToken& t) { return t.kind == LintTokenKind::Comment; }); } +bool LintContext::LineHasMultiLineToken(std::size_t line) { + if (!spannedLineCache) { + std::vector spanned(lines.size(), false); + for (const LintToken& token : Tokens()) { + std::size_t crossed = static_cast(std::ranges::count(TokenText(token), '\n')); + if (crossed == 0) continue; + for (std::size_t n = token.line; n <= token.line + crossed && n <= spanned.size(); ++n) { + spanned[n - 1] = true; + } + } + spannedLineCache = std::move(spanned); + } + return line >= 1 && line <= spannedLineCache->size() && (*spannedLineCache)[line - 1]; +} + void LintContext::Report(std::size_t line, std::string message) { sink->push_back({file, line, activeRule, std::move(message)}); } @@ -427,6 +442,7 @@ void LintContext::SetContent(std::string newContent) { commentStrippedCache.reset(); suppressionsCache.reset(); // line numbers may have shifted — re-parse tokenCache.reset(); // offsets refer to the old buffer — re-lex + spannedLineCache.reset(); // derived from tokenCache } void Configuration::AddLintRule(std::string name, std::function check) { diff --git a/interfaces/Crafter.Build-Clang.cppm b/interfaces/Crafter.Build-Clang.cppm index e5a1f1f..be52f4b 100644 --- a/interfaces/Crafter.Build-Clang.cppm +++ b/interfaces/Crafter.Build-Clang.cppm @@ -214,6 +214,14 @@ export namespace Crafter { // True when a comment token starts on `line`. Replaces `Line(n).contains("//")`, // which false-positives on a `//` inside a string literal. CRAFTER_API bool LineHasComment(std::size_t line); + // True when `line` (1-based) is touched by a token that spans more + // than one line — a raw string or a block comment. A transform that + // joins, splits or rewrites such a line changes what is inside that + // token, so this is the guard to consult before reflowing anything. + // Replaces `Line(n).contains("R\"")`, which both misses raw strings + // opened on an earlier line and fires on the characters R" appearing + // inside an ordinary literal. + CRAFTER_API bool LineHasMultiLineToken(std::size_t line); // Driver wiring — set by RunLint before each check call. Not for rules. std::string activeRule; @@ -221,6 +229,9 @@ export namespace Crafter { std::optional commentStrippedCache; std::optional suppressionsCache; std::optional> tokenCache; + // Per-line flag, 0-based, for LineHasMultiLineToken. Derived from + // tokenCache and invalidated with it. + std::optional> spannedLineCache; }; // A named lint rule: `check` runs once per (rule, file) over the project's diff --git a/lint-rules.h b/lint-rules.h index 12461fd..3cf8c92 100644 --- a/lint-rules.h +++ b/lint-rules.h @@ -408,9 +408,12 @@ inline void AddProjectLintRules(Crafter::Configuration& cfg) { lineStart += line.size() + 1; std::string_view trimmed = Trim(line); if (trimmed.starts_with('#')) continue; - // Raw strings defeat the comment stripper's quote tracking; any - // literal-+ pattern on such a line is a manual fix. - bool hasRaw = std::string_view(ctx.content).substr(lineOff, line.size()).contains("R\""); + // A chain is read within one line, so a literal that spans lines + // would be sliced in half by chainEnd. Single-line raw strings are + // fine: they reduce to R"…" in the stripped view, so they fail the + // "is a plain literal" test below and travel through as an + // argument, spelling and all. + bool spansLines = ctx.LineHasMultiLineToken(li + 1); std::size_t searchFrom = 0; while (searchFrom < line.size()) { @@ -427,8 +430,8 @@ inline void AddProjectLintRules(Crafter::Configuration& cfg) { bool literalAdjacent = (leftEnd > 0 && line[leftEnd - 1] == '"') || (rightBegin < line.size() && line[rightBegin] == '"'); if (!literalAdjacent) continue; - if (hasRaw) { - ctx.Report(li + 1, "use std::format instead of string concatenation with + (raw-string line, fix manually)"); + if (spansLines) { + ctx.Report(li + 1, "use std::format instead of string concatenation with + (multi-line literal, fix manually)"); break; } @@ -607,7 +610,7 @@ inline void AddProjectLintRules(Crafter::Configuration& cfg) { 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 (!raw.contains("//") && !ctx.Suppressed("single-declaration", i + 1) + 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; @@ -644,7 +647,7 @@ inline void AddProjectLintRules(Crafter::Configuration& cfg) { if (trimmed == "{" && !outLines.empty()) { std::string_view prevTrim = i > 0 ? Trim(stripped[i - 1]) : std::string_view{}; bool headerBefore = prevTrim.ends_with(')') || prevTrim == "else" || prevTrim == "do" || prevTrim == "try"; - bool hasComment = ctx.Line(i + 1).contains("//") || (i > 0 && ctx.Line(i).contains("//")); + bool hasComment = ctx.LineHasComment(i + 1) || (i > 0 && ctx.LineHasComment(i)); bool suppressed = ctx.Suppressed("brace-style", i) || ctx.Suppressed("brace-style", i + 1); if (headerBefore && !hasComment && !suppressed) { std::string& prev = outLines.back(); @@ -680,7 +683,7 @@ inline void AddProjectLintRules(Crafter::Configuration& cfg) { if (i + 1 < ctx.lines.size() && std::regex_match(lineStr, ifHeader) && ParenDelta(stripped[i]) == 0) { std::string_view body = Trim(stripped[i + 1]); bool joinable = !body.empty() && body != "{" && !body.starts_with("if") && body.ends_with(';') - && !ctx.Line(i + 1).contains("//") && !ctx.Line(i + 2).contains("//") + && !ctx.LineHasComment(i + 1) && !ctx.LineHasComment(i + 2) && !ctx.Suppressed("if-single-line", i + 1) && !ctx.Suppressed("if-single-line", i + 2); std::string joined = std::string(ctx.Line(i + 1)); while (!joined.empty() && (joined.back() == ' ' || joined.back() == '\t')) joined.pop_back(); @@ -724,7 +727,7 @@ inline void AddProjectLintRules(Crafter::Configuration& cfg) { std::string_view trimmed = Trim(stripped[i]); std::int64_t delta = ParenDelta(stripped[i]); bool candidate = delta > 0 && !trimmed.empty() && !trimmed.starts_with('#') - && !trimmed.ends_with('{') && !ctx.Line(i + 1).contains("//") && !ctx.Line(i + 1).contains("R\""); + && !trimmed.ends_with('{') && !ctx.LineHasComment(i + 1) && !ctx.LineHasMultiLineToken(i + 1); if (candidate) { std::string joined(ctx.Line(i + 1)); std::size_t j = i + 1; @@ -736,7 +739,7 @@ inline void AddProjectLintRules(Crafter::Configuration& cfg) { // lambda body starts — leave those wrapped. bool closes = delta + ParenDelta(stripped[j]) <= 0; if (next.empty() || (next.ends_with('{') && !closes) - || ctx.Line(j + 1).contains("//") || ctx.Line(j + 1).contains("R\"")) { + || ctx.LineHasComment(j + 1) || ctx.LineHasMultiLineToken(j + 1)) { ok = false; break; } @@ -757,7 +760,7 @@ inline void AddProjectLintRules(Crafter::Configuration& cfg) { continue; } } else if (delta == 0 && i + 1 < ctx.lines.size() && !trimmed.starts_with('#') - && !ctx.Line(i + 1).contains("//") && !ctx.Line(i + 1).contains("R\"")) { + && !ctx.LineHasComment(i + 1) && !ctx.LineHasMultiLineToken(i + 1)) { auto isOpStart = [](std::string_view s) { return s.starts_with("&&") || s.starts_with("||") || s.starts_with("| "); }; @@ -771,7 +774,7 @@ inline void AddProjectLintRules(Crafter::Configuration& cfg) { std::size_t j = i + 1; bool ok = true; while (j < ctx.lines.size() && isOpStart(Trim(stripped[j]))) { - if (ParenDelta(stripped[j]) != 0 || ctx.Line(j + 1).contains("//") || ctx.Line(j + 1).contains("R\"")) { + if (ParenDelta(stripped[j]) != 0 || ctx.LineHasComment(j + 1) || ctx.LineHasMultiLineToken(j + 1)) { ok = false; break; } diff --git a/tests/HouseRules/main.cpp b/tests/HouseRules/main.cpp index 0b3593e..7afa0a5 100644 --- a/tests/HouseRules/main.cpp +++ b/tests/HouseRules/main.cpp @@ -252,6 +252,49 @@ int main() { Check(second.summary.changedFiles.empty(), "wrap-join is idempotent after the fixpoint"); } + // The guards used to be substring probes over the raw line, so a literal + // whose TEXT contained `//` or `R"` looked like a comment or a raw string + // and silently disabled the rule. Both shapes occur in this repo's own + // sources — "MARKER" ends in the characters R", and any string mentioning + // a lint-disable directive contains //. + { + RuleRun r = RunRule("void F() {\n" " auto hit = text.find(\"MARKER\",\n" " start);\n" "}\n", "wrap-join", LintMode::Apply); + Check(r.text.contains("text.find(\"MARKER\", start);"), "R\" inside a literal no longer blocks wrap-join"); + } + { + RuleRun r = RunRule("void F() {\n" " Check(read() == \"// lint-disable-next-line trim\\n\",\n" " \"message\");\n" "}\n", "wrap-join", LintMode::Apply); + Check(r.text.contains("\\n\", \"message\");"), "// inside a literal no longer blocks wrap-join"); + } + + // A real trailing comment still blocks the join: text pulled up past a + // `//` would be swallowed by it. + { + RuleRun r = RunRule("void F() {\n" " auto v = g(alpha, // why\n" " beta);\n" "}\n", "wrap-join", LintMode::Apply); + Check(r.text.contains("g(alpha, // why\n"), "a real comment still blocks wrap-join"); + } + + // A genuinely multi-line literal is never reflowed — joining its lines + // would change the string's contents. + { + std::string_view source = "void F() {\n" + " auto text = R\"sql(SELECT a,\n" + " b FROM t)sql\";\n" + "}\n"; + RuleRun r = RunRule(source, "wrap-join", LintMode::Apply); + Check(r.text == source, "wrap-join leaves a multi-line raw string alone"); + RuleRun paren = RunRule(source, "paren-spacing", LintMode::Apply); + Check(paren.text == source, "paren-spacing leaves a multi-line raw string alone"); + } + + // Type keywords inside a literal are text, not declarations. + { + std::string_view source = "void F() {\n" + " auto sql = R\"q(int x; unsigned long y;)q\";\n" + "}\n"; + RuleRun r = RunRule(source, "fixed-width-types", LintMode::Apply); + Check(r.text == source, "fixed-width-types leaves type names inside a raw string alone"); + } + if (Failures > 0) { std::println(std::cerr, "{} assertions failed", Failures); return 1; diff --git a/tests/Lint/main.cpp b/tests/Lint/main.cpp index cad9b5e..fa81ec4 100644 --- a/tests/Lint/main.cpp +++ b/tests/Lint/main.cpp @@ -156,8 +156,7 @@ int main() { Configuration cfg = FixtureConfig(); cfg.AddLintRule("marker", [](LintContext& ctx) { const std::string& code = ctx.CommentStripped(); - for (std::size_t pos = code.find("MARKER"); pos != std::string::npos; - pos = code.find("MARKER", pos + 1)) { + for (std::size_t pos = code.find("MARKER"); pos != std::string::npos; pos = code.find("MARKER", pos + 1)) { std::size_t line = 1 + std::count(code.begin(), code.begin() + pos, '\n'); ctx.Report(line, "MARKER in code"); } @@ -370,8 +369,7 @@ int main() { Configuration cfg = s.Config({"a"}); AddTrimRule(cfg); RunLint(cfg, Mode(LintMode::Apply)); - Check(s.Read("a") == "// lint-disable-next-line trim\nkeep \ntrim\n", - "suppressed line keeps its bytes; the unsuppressed one is fixed"); + Check(s.Read("a") == "// lint-disable-next-line trim\nkeep \ntrim\n", "suppressed line keeps its bytes; the unsuppressed one is fixed"); } // Multiple rule names on one directive (space- or comma-separated).