Skip to content

fix(lint): slice definitions by UTF-8 byte offset; report unparseable definitions - #374

Merged
pyramation merged 2 commits into
mainfrom
devin/1793678430-lint-byte-offsets
Nov 3, 2026
Merged

pyramation merged 2 commits into
mainfrom
devin/1793678430-lint-byte-offsets

Conversation

@pyramation

Copy link
Copy Markdown
Collaborator

Summary

@pgsql/lint applied libpg_query offsets (stmt_location/stmt_len, node locations) to the JS string, but those are byte offsets into the UTF-8 source. Any multi-byte character earlier in a file (→, — in a comment) shifted every later CREATE FUNCTION slice; the shifted text failed to parse, engine.ts dropped the parse error, and the file reported zero findings for C1–C4.

Changes in packages/lint:

  • util.ts: utf8(text), sliceBytes(bytes, start, len), newlinesBeforeByte(bytes, off); lineOfOffset now counts over bytes.
  • file-runner.ts: statement slicing and startLine re-anchoring go through the byte helpers.
  • parse-unit.ts: SQL-language body fragment offsets and the C1 SET clause locations are mapped via bytes as well.
  • Parse failures are no longer silent: lintDefinition returns parseError, and the file/sql-text runners emit a parse-error (P0) finding (PARSE_ERROR meta exported; severity configurable like any rule, e.g. --warn parse-error). The CLI's special-case warning path for parse errors is removed so they count like other findings.

Tests: __fixtures__/multibyte.sql (Unicode comment before functions tripping C1/C3/C4) with a Unicode-vs-ASCII-folded equivalence test and a P0 test; all three fail on main and pass here. README and .agents/skills/pgsql-lint updated.

Link to Devin session: https://app.devin.ai/sessions/55f7e7b0c9bb4304b9989b993362571e
Open in Devin Desktop: https://app.devin.ai/desktop/session/55f7e7b0c9bb4304b9989b993362571e?variant=devin
Requested by: @pyramation

… definitions

libpg_query reports stmt_location/stmt_len and node locations as byte
offsets into the UTF-8 source, but the file runner sliced the JS string by
code unit. Any multi-byte character earlier in a file shifted every later
CREATE FUNCTION slice, the definition failed to parse, and the engine
dropped the parse error — so the file reported no findings at all.

- slice statements and count lines over the encoded bytes (util.ts)
- SQL-language body fragments and the C1 SET clause map byte offsets too
- lintDefinition returns parseError; lintSqlText/lintFiles/lintSource
  report it as a parse-error (P0) finding, severity configurable
@devin-ai-integration

Copy link
Copy Markdown
Contributor

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@tenki-reviewer

tenki-reviewer Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Review complete. 🟡 1 medium

📍 Findings outside the diff (1) — 🟡 1 medium — defects on lines GitHub can't attach comments to

🟡 Medium — parse-error: off still emits a failing error finding · file-runner.ts:158–233 · unchanged line

// packages/lint/src/file-runner.ts
158	    if (parseError) findings.push(parseErrorFinding(parseError, subject, startLine + 1, options));
159	    const absLine = (p: LintProblem): number => startLine + p.line; // p.line is 1-based within input.text
160	    for (const p of problems) findings.push(toFinding(p, subject, absLine(p), false));
161	    for (const s of suppressed) {
162	      findings.push(toFinding(s, subject, absLine(s), true, s.reason, s.scope));
163	    }
164	  }
165	  findings.sort((a, b) => a.line - b.line || a.ruleId.localeCompare(b.ruleId));
166	  return findings;
167	}
168	
169	function toFinding(

parseErrorFinding resolves the configured severity but the caller pushes the finding unconditionally — the off case is never checked (packages/lint/src/file-runner.ts:158, built at packages/lint/src/file-runner.ts:224). The CLI then counts every non-warn, unacknowledged finding as an error and exits 1 (packages/lint/src/cli.ts:203-207). This contradicts the documented SeverityMap contract that off means the rule is not reported (packages/lint/src/types.ts:51), and diverges from regular rules, which lintDefinition filters at packages/lint/src/engine.ts:75 — so a user following the finding's own hint to set parse-error: off still gets a hard failure.


The change adds a parse-error detection pipeline to the new @pgsql/lint package: definitions that fail to parse now produce a PARSE_ERROR pseudo-rule finding instead of being silently skipped, and the CLI counts them toward exit-code semantics. UTF-8 byte offsets (stmt_location/stmt_len) are now handled explicitly via TextEncoder/TextDecoder so multibyte sources slice and line-map correctly, backed by a new multibyte.sql fixture and file-runner tests.

Files Change
packages/lint/src/parse-unit.ts, packages/lint/src/util.ts Add byte-accurate definition extraction (stmt_location/stmt_len over UTF-8 bytes) and byte-based line-offset helpers with multibyte fixture coverage.
packages/lint/src/engine.ts, packages/lint/src/types.ts, packages/lint/src/index.ts Introduce the PARSE_ERROR pseudo-rule, propagate parseError through LintResult, and document the severity/off configuration contract.
packages/lint/src/file-runner.ts, packages/lint/src/cli.ts Emit parse-error findings per definition, wire them into report totals and CLI exit codes.
packages/lint/__tests__/file-runner.test.ts, packages/lint/__tests__/__fixtures__/multibyte.sql Add tests and a multibyte fixture exercising parse-error reporting.
packages/lint/README.md, .agents/skills/pgsql-lint/SKILL.md Document parse-error behavior and configuration.

Reviewed commit: 0473fb0

parseErrorFinding resolved the configured severity but was pushed
unconditionally, so a config that turned the pseudo-rule off still failed
the run. Return nothing for off, as lintDefinition does for rules.
@pyramation
pyramation merged commit 382bab2 into main Nov 3, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant