fix(lint): make the reflow guards token-accurate instead of textual

The transforms guard themselves against comments and raw strings before
joining or rewriting a line, because pulling text up past a `//` buries it
and reflowing a multi-line literal changes the string. Those guards were
substring probes over the raw line, so they answered the wrong question:

  Line(n).contains("//")   fires on // inside a string literal
  Line(n).contains("R\"")  fires on the characters R" inside a literal, and
                           MISSES a raw string opened on an earlier line

Both misfire on this repo's own sources. "MARKER" ends in R", and any string
mentioning a lint-disable directive contains //. Two wrapped call sites in
tests/Lint were being left unjoined for exactly these reasons; they join now,
and the results are in this commit.

Replaced by LineHasComment() (added with the token layer) and a new
LineHasMultiLineToken(), which reports whether any token actually covering
that line spans a line boundary — a raw string or a block comment. Backed by
a per-line bitmap derived from the token cache and invalidated with it.

The guards themselves stay: joining across a real comment or a real
multi-line literal is still unsafe, and there are tests for both. What
changes is that they now fire on comments and literals rather than on the
characters that spell them.

format-concat gets narrower as a result. It used to refuse any line
containing R" and ask for a manual fix; now only a literal that genuinely
spans lines does that, because a single-line raw string reduces to R"…" in
the stripped view, fails the plain-literal test, and travels through as an
argument with its spelling intact.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jorijn van der Graaf 2026-07-27 03:08:06 +02:00
commit 78fbf8f80c
5 changed files with 87 additions and 16 deletions

View file

@ -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; }); 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<bool> spanned(lines.size(), false);
for (const LintToken& token : Tokens()) {
std::size_t crossed = static_cast<std::size_t>(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) { void LintContext::Report(std::size_t line, std::string message) {
sink->push_back({file, line, activeRule, std::move(message)}); sink->push_back({file, line, activeRule, std::move(message)});
} }
@ -427,6 +442,7 @@ void LintContext::SetContent(std::string newContent) {
commentStrippedCache.reset(); commentStrippedCache.reset();
suppressionsCache.reset(); // line numbers may have shifted — re-parse suppressionsCache.reset(); // line numbers may have shifted — re-parse
tokenCache.reset(); // offsets refer to the old buffer — re-lex tokenCache.reset(); // offsets refer to the old buffer — re-lex
spannedLineCache.reset(); // derived from tokenCache
} }
void Configuration::AddLintRule(std::string name, std::function<void(LintContext&)> check) { void Configuration::AddLintRule(std::string name, std::function<void(LintContext&)> check) {

View file

@ -214,6 +214,14 @@ export namespace Crafter {
// True when a comment token starts on `line`. Replaces `Line(n).contains("//")`, // True when a comment token starts on `line`. Replaces `Line(n).contains("//")`,
// which false-positives on a `//` inside a string literal. // which false-positives on a `//` inside a string literal.
CRAFTER_API bool LineHasComment(std::size_t line); 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. // Driver wiring — set by RunLint before each check call. Not for rules.
std::string activeRule; std::string activeRule;
@ -221,6 +229,9 @@ export namespace Crafter {
std::optional<std::string> commentStrippedCache; std::optional<std::string> commentStrippedCache;
std::optional<LintSuppressions> suppressionsCache; std::optional<LintSuppressions> suppressionsCache;
std::optional<std::vector<LintToken>> tokenCache; std::optional<std::vector<LintToken>> tokenCache;
// Per-line flag, 0-based, for LineHasMultiLineToken. Derived from
// tokenCache and invalidated with it.
std::optional<std::vector<bool>> spannedLineCache;
}; };
// A named lint rule: `check` runs once per (rule, file) over the project's // A named lint rule: `check` runs once per (rule, file) over the project's

View file

@ -408,9 +408,12 @@ inline void AddProjectLintRules(Crafter::Configuration& cfg) {
lineStart += line.size() + 1; lineStart += line.size() + 1;
std::string_view trimmed = Trim(line); std::string_view trimmed = Trim(line);
if (trimmed.starts_with('#')) continue; if (trimmed.starts_with('#')) continue;
// Raw strings defeat the comment stripper's quote tracking; any // A chain is read within one line, so a literal that spans lines
// literal-+ pattern on such a line is a manual fix. // would be sliced in half by chainEnd. Single-line raw strings are
bool hasRaw = std::string_view(ctx.content).substr(lineOff, line.size()).contains("R\""); // 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; std::size_t searchFrom = 0;
while (searchFrom < line.size()) { while (searchFrom < line.size()) {
@ -427,8 +430,8 @@ inline void AddProjectLintRules(Crafter::Configuration& cfg) {
bool literalAdjacent = (leftEnd > 0 && line[leftEnd - 1] == '"') bool literalAdjacent = (leftEnd > 0 && line[leftEnd - 1] == '"')
|| (rightBegin < line.size() && line[rightBegin] == '"'); || (rightBegin < line.size() && line[rightBegin] == '"');
if (!literalAdjacent) continue; if (!literalAdjacent) continue;
if (hasRaw) { if (spansLines) {
ctx.Report(li + 1, "use std::format instead of string concatenation with + (raw-string line, fix manually)"); ctx.Report(li + 1, "use std::format instead of string concatenation with + (multi-line literal, fix manually)");
break; break;
} }
@ -607,7 +610,7 @@ inline void AddProjectLintRules(Crafter::Configuration& cfg) {
std::string lineStr(stripped[i]); std::string lineStr(stripped[i]);
std::smatch m; std::smatch m;
std::string_view raw = i + 1 <= ctx.lines.size() ? ctx.Line(i + 1) : std::string_view{}; 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::regex_match(lineStr, m, simpleMulti)) {
std::string indent = m[1].str(), type = m[2].str(), decls = m[3].str(); std::string indent = m[1].str(), type = m[2].str(), decls = m[3].str();
std::size_t start = 0; std::size_t start = 0;
@ -644,7 +647,7 @@ inline void AddProjectLintRules(Crafter::Configuration& cfg) {
if (trimmed == "{" && !outLines.empty()) { if (trimmed == "{" && !outLines.empty()) {
std::string_view prevTrim = i > 0 ? Trim(stripped[i - 1]) : std::string_view{}; 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 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); bool suppressed = ctx.Suppressed("brace-style", i) || ctx.Suppressed("brace-style", i + 1);
if (headerBefore && !hasComment && !suppressed) { if (headerBefore && !hasComment && !suppressed) {
std::string& prev = outLines.back(); 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) { if (i + 1 < ctx.lines.size() && std::regex_match(lineStr, ifHeader) && ParenDelta(stripped[i]) == 0) {
std::string_view body = Trim(stripped[i + 1]); std::string_view body = Trim(stripped[i + 1]);
bool joinable = !body.empty() && body != "{" && !body.starts_with("if") && body.ends_with(';') 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); && !ctx.Suppressed("if-single-line", i + 1) && !ctx.Suppressed("if-single-line", i + 2);
std::string joined = std::string(ctx.Line(i + 1)); std::string joined = std::string(ctx.Line(i + 1));
while (!joined.empty() && (joined.back() == ' ' || joined.back() == '\t')) joined.pop_back(); 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::string_view trimmed = Trim(stripped[i]);
std::int64_t delta = ParenDelta(stripped[i]); std::int64_t delta = ParenDelta(stripped[i]);
bool candidate = delta > 0 && !trimmed.empty() && !trimmed.starts_with('#') 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) { if (candidate) {
std::string joined(ctx.Line(i + 1)); std::string joined(ctx.Line(i + 1));
std::size_t j = i + 1; std::size_t j = i + 1;
@ -736,7 +739,7 @@ inline void AddProjectLintRules(Crafter::Configuration& cfg) {
// lambda body starts — leave those wrapped. // lambda body starts — leave those wrapped.
bool closes = delta + ParenDelta(stripped[j]) <= 0; bool closes = delta + ParenDelta(stripped[j]) <= 0;
if (next.empty() || (next.ends_with('{') && !closes) 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; ok = false;
break; break;
} }
@ -757,7 +760,7 @@ inline void AddProjectLintRules(Crafter::Configuration& cfg) {
continue; continue;
} }
} else if (delta == 0 && i + 1 < ctx.lines.size() && !trimmed.starts_with('#') } 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) { auto isOpStart = [](std::string_view s) {
return s.starts_with("&&") || s.starts_with("||") || s.starts_with("| "); 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; std::size_t j = i + 1;
bool ok = true; bool ok = true;
while (j < ctx.lines.size() && isOpStart(Trim(stripped[j]))) { 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; ok = false;
break; break;
} }

View file

@ -252,6 +252,49 @@ int main() {
Check(second.summary.changedFiles.empty(), "wrap-join is idempotent after the fixpoint"); 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) { if (Failures > 0) {
std::println(std::cerr, "{} assertions failed", Failures); std::println(std::cerr, "{} assertions failed", Failures);
return 1; return 1;

View file

@ -156,8 +156,7 @@ int main() {
Configuration cfg = FixtureConfig(); Configuration cfg = FixtureConfig();
cfg.AddLintRule("marker", [](LintContext& ctx) { cfg.AddLintRule("marker", [](LintContext& ctx) {
const std::string& code = ctx.CommentStripped(); const std::string& code = ctx.CommentStripped();
for (std::size_t pos = code.find("MARKER"); pos != std::string::npos; for (std::size_t pos = code.find("MARKER"); pos != std::string::npos; pos = code.find("MARKER", pos + 1)) {
pos = code.find("MARKER", pos + 1)) {
std::size_t line = 1 + std::count(code.begin(), code.begin() + pos, '\n'); std::size_t line = 1 + std::count(code.begin(), code.begin() + pos, '\n');
ctx.Report(line, "MARKER in code"); ctx.Report(line, "MARKER in code");
} }
@ -370,8 +369,7 @@ int main() {
Configuration cfg = s.Config({"a"}); Configuration cfg = s.Config({"a"});
AddTrimRule(cfg); AddTrimRule(cfg);
RunLint(cfg, Mode(LintMode::Apply)); RunLint(cfg, Mode(LintMode::Apply));
Check(s.Read("a") == "// lint-disable-next-line trim\nkeep \ntrim\n", Check(s.Read("a") == "// lint-disable-next-line trim\nkeep \ntrim\n", "suppressed line keeps its bytes; the unsuppressed one is fixed");
"suppressed line keeps its bytes; the unsuppressed one is fixed");
} }
// Multiple rule names on one directive (space- or comma-separated). // Multiple rule names on one directive (space- or comma-separated).