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

.agents/skills/pgsql-lint/SKILL.md

@@ -80,7 +80,14 @@ await lintFiles(['./migrations']);          // files/dirs on disk
 `lintSqlText`/`lintFiles` slice out each top-level `CREATE FUNCTION` using the
 parser's `stmt_location`/`stmt_len`, lint each in isolation, and **re-anchor**
 findings to absolute file lines — so a mixed migration is never treated as one
-malformed definition.
+malformed definition. Those offsets are **bytes** into the UTF-8 source, not
+string indices: slice and count lines over `utf8(text)` (`src/util.ts`), never
+over the JS string, or one `→` in a comment shifts every later definition.
+
+A definition the parser rejects is a `parse-error` (`P0`) finding, never a
+silent skip — no rule ran, so it is not clean. `lintDefinition` returns it as
+`parseError` on the result; the file runner turns it into a finding whose
+severity is configured like any rule (`--warn parse-error`).
 
 ## Authoring a new rule
 

packages/lint/README.md

@@ -36,6 +36,10 @@ Agent skill (rules, CLI, authoring new rules):
 | `C3` | `require-qualified-refs` | an unqualified relation reference (`FROM users` → `FROM app_public.users`) |
 | `C4` | `no-dynamic-sql` | `EXECUTE`, `EXECUTE … USING`, `FOR … IN EXECUTE` |
 
+A definition the parser rejects is reported as `parse-error` (`P0`) rather than
+skipped: no rule ran over it, so it is not clean. Downgrade it like any rule
+(`--warn parse-error`) when the parser, not the SQL, is at fault.
+
 The rules encode a single discipline: never depend on `search_path` — fully
 qualify everything (`C1` + `C3`) — don't paper over ambiguity (`C2`), and treat
 dynamic SQL as opaque and exceptional (`C4`).

packages/lint/__tests__/__fixtures__/multibyte.sql

@@ -0,0 +1,25 @@
+-- Arrows (→) and dashes (—) are multi-byte in UTF-8: every parser offset
+-- after this comment is a byte offset, not a UTF-16 index.
+CREATE SCHEMA app_public;
+
+CREATE FUNCTION app_public.pinned() RETURNS void
+LANGUAGE sql
+SET search_path = app_public
+AS $$
+  SELECT 1
+$$;
+
+CREATE FUNCTION app_public.dynamic(x text) RETURNS void
+LANGUAGE plpgsql
+AS $$
+BEGIN
+  EXECUTE format('SELECT %s', x);
+END;
+$$;
+
+CREATE FUNCTION app_public.dirty() RETURNS setof app_public.users
+LANGUAGE sql
+AS $$
+  -- 日本語
+  SELECT * FROM users
+$$;

packages/lint/__tests__/file-runner.test.ts

@@ -73,3 +73,58 @@ describe('lintSource (adapters)', () => {
     expect(reports.some((r) => r.file.endsWith('migration.sql'))).toBe(true);
   });
 });
+
+describe('multi-byte characters before a definition', () => {
+  it('finds every rule after a non-ASCII comment', async () => {
+    const [report] = await lintFiles([path.join(FIXTURES, 'multibyte.sql')]);
+    expect(report.parseError).toBeUndefined();
+    const byCode = Object.fromEntries(
+      report.findings.map((f) => [f.code, { line: f.line, subject: f.subject }])
+    );
+    expect(byCode).toEqual({
+      C1: { line: 7, subject: 'app_public.pinned' },
+      C4: { line: 16, subject: 'app_public.dynamic' },
+      C3: { line: 24, subject: 'app_public.dirty' }
+    });
+  });
+
+  it('is byte-for-byte equivalent to the ASCII-folded source', async () => {
+    const source = `-- → —\nCREATE FUNCTION f(x text) RETURNS void LANGUAGE plpgsql AS $$\nBEGIN\n  EXECUTE format('SELECT %s', x);\nEND;\n$$;`;
+    const folded = await lintSqlText(source.replace('→', '->').replace('—', '-'));
+    const report = await lintSqlText(source);
+    expect(report.findings).toEqual(folded.findings);
+    expect(report.findings.map((f) => [f.code, f.line])).toEqual([['C4', 4]]);
+  });
+});
+
+describe('definitions that fail to parse', () => {
+  it('surface as a finding instead of being dropped', async () => {
+    const report = await lintSource(
+      sqlTextAdapter(
+        `CREATE FUNCTION f() RETURNS void LANGUAGE plpgsql AS $$ BEGIN this is not plpgsql; END; $$;`,
+        'bad.sql'
+      )
+    );
+    expect(report).toHaveLength(1);
+    expect(report[0].findings).toHaveLength(1);
+    expect(report[0].findings[0]).toMatchObject({
+      ruleId: 'parse-error',
+      code: 'P0',
+      severity: 'error',
+      subject: 'f',
+      line: 1
+    });
+  });
+
+  it('are not reported when parse-error is configured off', async () => {
+    const report = await lintSource(
+      sqlTextAdapter(
+        `CREATE FUNCTION f() RETURNS void LANGUAGE plpgsql AS $$ BEGIN this is not plpgsql; END; $$;`,
+        'bad.sql'
+      ),
+      { severity: { 'parse-error': 'off' } }
+    );
+    expect(report).toHaveLength(1);
+    expect(report[0].findings).toEqual([]);
+  });
+});

packages/lint/src/cli.ts

@@ -194,10 +194,6 @@ async function main(): Promise<void> {
   let warnings = 0;
   let acknowledged = 0;
   for (const report of reports) {
-    if (report.parseError) {
-      if (!argv.json) console.error(chalk.yellow(`⚠ ${report.file}: ${report.parseError}`));
-      continue;
-    }
     const shown = report.findings.filter((f) => !f.acknowledged || !argv.quiet);
     if (!argv.json && shown.length > 0) {
       console.log(chalk.underline(report.file));

packages/lint/src/engine.ts

@@ -15,6 +15,7 @@ import type {
   LintProblem,
   LintResult,
   LintRule,
+  LintRuleMeta,
   Severity,
   SeverityMap,
   SuppressedProblem
@@ -38,10 +39,22 @@ export interface LintOptions {
 }
 
 /** Resolve a rule's configured severity (id wins over code; default `error`). */
-export function severityOf(rule: LintRule, severity: SeverityMap = {}): Severity {
+export function severityOf(rule: LintRuleMeta, severity: SeverityMap = {}): Severity {
   return severity[rule.id] ?? severity[rule.code] ?? 'error';
 }
 
+/**
+ * The pseudo-rule a parse failure is reported under. Not a {@link LintRule}
+ * (it has no `run`): it exists so the failure has an id/code a consumer can
+ * configure (`off` / `warn`) exactly like a rule.
+ */
+export const PARSE_ERROR: LintRuleMeta = {
+  id: 'parse-error',
+  code: 'P0',
+  title: 'Definition could not be parsed',
+  reasonRequired: false
+};
+
 /** Lint a single function definition. */
 export async function lintDefinition(
   text: string,
@@ -63,9 +76,10 @@ export async function lintDefinition(
   if (selected.length === 0) return { problems: active, suppressed };
 
   const unit = await parseUnit(text, language, name);
-  // An unparseable definition produces no lint findings — dynamic/opaque bodies
-  // are a downstream concern (e.g. a call-graph), not the linter's.
-  if (unit.parseError) return { problems: active, suppressed };
+  // An unparseable definition was checked by no rule. That is never a pass:
+  // the failure is returned so the caller reports it (the file runner turns
+  // it into a `parse-error` finding).
+  if (unit.parseError) return { problems: active, suppressed, parseError: unit.parseError };
 
   const keywords = options.keyword
     ? (Array.isArray(options.keyword) ? options.keyword : [options.keyword])

packages/lint/src/file-runner.ts

@@ -14,7 +14,7 @@ import { promises as fs } from 'fs';
 import * as path from 'path';
 import { parse } from 'pgsql-parser';
 
-import { lintDefinition, LintOptions } from './engine';
+import { lintDefinition, LintOptions, PARSE_ERROR, severityOf } from './engine';
 import { applyIgnore } from './ignore';
 import type {
   LintDefinitionInput,
@@ -23,6 +23,7 @@ import type {
   SourceAdapter,
   SuppressionScope
 } from './types';
+import { newlinesBeforeByte, sliceBytes, utf8 } from './util';
 import { findAll } from './walk';
 
 /** A finding re-anchored to a source file. */
@@ -55,14 +56,6 @@ export interface FileReport {
   parseError?: string;
 }
 
-/** Count newlines before `offset` — the 0-based line the offset sits on. */
-function lineAtOffset(text: string, offset: number): number {
-  let n = 0;
-  const end = Math.min(Math.max(offset, 0), text.length);
-  for (let i = 0; i < end; i++) if (text.charCodeAt(i) === 10) n++;
-  return n;
-}
-
 /** Printable `schema.name` of a `CreateFunctionStmt.funcname`. */
 function funcName(createFnStmt: Record<string, unknown>): string {
   const funcname = createFnStmt.funcname;
@@ -98,6 +91,9 @@ function funcLanguage(createFnStmt: Record<string, unknown>): string {
  * `CREATE FUNCTION` is sliced back out with its absolute start line, so a
  * mixed migration (schema + tables + functions) yields one input per function
  * rather than being treated as a single malformed definition.
+ *
+ * `stmt_location` / `stmt_len` are byte offsets into the UTF-8 source, so the
+ * slicing happens over the encoded bytes (see `util.ts`).
  */
 export async function sqlTextDefinitions(
   source: string,
@@ -106,18 +102,19 @@ export async function sqlTextDefinitions(
   const parsed = (await parse(source)) as {
     stmts?: Array<{ stmt?: unknown; stmt_location?: number; stmt_len?: number }>;
   };
+  const bytes = utf8(source);
   const out: LintDefinitionInput[] = [];
   for (const entry of parsed.stmts ?? []) {
     const createFnStmt = entry.stmt ? findAll(entry.stmt, 'CreateFunctionStmt')[0] : undefined;
     if (!createFnStmt) continue;
     const start = entry.stmt_location ?? 0;
-    const len = entry.stmt_len ?? source.length - start;
+    const len = entry.stmt_len ?? bytes.length - start;
     out.push({
-      text: source.slice(start, start + len),
+      text: sliceBytes(bytes, start, len),
       language: funcLanguage(createFnStmt),
       name: funcName(createFnStmt),
       file,
-      startLine: lineAtOffset(source, start) // 0-based line the definition begins on
+      startLine: newlinesBeforeByte(bytes, start) // 0-based line the definition begins on
     });
   }
   return out;
@@ -132,7 +129,9 @@ export async function lintSqlText(source: string, options: LintOptions = {}): Pr
   try {
     inputs = await sqlTextDefinitions(source);
   } catch (err) {
-    return { file: '<source>', findings: [], parseError: (err as Error).message };
+    const message = (err as Error).message;
+    const finding = parseErrorFinding(message, '<file>', 1, options);
+    return { file: '<source>', findings: finding ? [finding] : [], parseError: message };
   }
   const findings = await lintInputs(inputs, options);
   return { file: '<source>', findings };
@@ -147,12 +146,16 @@ async function lintInputs(
   for (const input of inputs) {
     const subject = input.name ?? '<anonymous>';
     const startLine = input.startLine ?? 0;
-    const { problems, suppressed } = await lintDefinition(
+    const { problems, suppressed, parseError } = await lintDefinition(
       input.text,
       input.language,
       subject,
       options
     );
+    if (parseError) {
+      const finding = parseErrorFinding(parseError, subject, startLine + 1, options);
+      if (finding) findings.push(finding);
+    }
     const absLine = (p: LintProblem): number => startLine + p.line; // p.line is 1-based within input.text
     for (const p of problems) findings.push(toFinding(p, subject, absLine(p), false));
     for (const s of suppressed) {
@@ -190,6 +193,7 @@ function toFinding(
 }
 
 const RULE_CODE: Record<string, string> = {
+  [PARSE_ERROR.id]: PARSE_ERROR.code,
   'no-set-search-path': 'C1',
   'no-variable-conflict': 'C2',
   'require-qualified-refs': 'C3',
@@ -199,6 +203,33 @@ function codeFor(ruleId: string): string {
   return RULE_CODE[ruleId] ?? ruleId;
 }
 
+/**
+ * A definition (or file) the parser rejected. Nothing in it was checked, so
+ * the failure is reported as a finding rather than dropped: an unparseable
+ * function would otherwise pass every rule by default. Severity is configured
+ * like any rule (`parse-error` / `P0`): `error` unless downgraded, and `off`
+ * drops it like any other rule turned off.
+ */
+function parseErrorFinding(
+  message: string,
+  subject: string,
+  line: number,
+  options: LintOptions
+): FileFinding | undefined {
+  const severity = severityOf(PARSE_ERROR, options.severity);
+  if (severity === 'off') return undefined;
+  return {
+    ruleId: PARSE_ERROR.id,
+    code: PARSE_ERROR.code,
+    line,
+    message: `Definition could not be parsed, so no rule was checked: ${message}`,
+    hint: 'Fix the syntax (or, if the parser is wrong, downgrade `parse-error` in the lint config).',
+    severity,
+    subject,
+    acknowledged: false
+  };
+}
+
 /** Recursively collect `.sql` files under a directory. */
 async function sqlFilesUnder(dir: string): Promise<string[]> {
   const out: string[] = [];

packages/lint/src/index.ts

@@ -16,7 +16,7 @@ export { changedFiles, changedSqlFiles, resolveChangedBase } from './changed';
 export type { LintConfigFile, LoadedLintConfig } from './config';
 export { CONFIG_FILENAMES, configDir, findConfigFile, loadLintConfig } from './config';
 export type { LintOptions } from './engine';
-export { lintDefinition, severityOf } from './engine';
+export { lintDefinition, PARSE_ERROR, severityOf } from './engine';
 export {
   FileFinding,
   FileLintOptions,

packages/lint/src/parse-unit.ts

@@ -13,19 +13,15 @@ import { parsePlPgSQL } from 'libpg-query';
 import { parse } from 'pgsql-parser';
 
 import type { DynamicSqlSite, LintUnit, SqlFragment } from './types';
+import { newlinesBeforeByte, utf8 } from './util';
 import { findAll } from './walk';
 
-/** Count newlines in `s[0..offset)` — i.e. how many lines precede `offset`. */
-function newlinesBefore(s: string, offset: number): number {
-  let n = 0;
-  const end = Math.min(offset, s.length);
-  for (let i = 0; i < end; i++) if (s.charCodeAt(i) === 10) n++;
-  return n;
-}
-
 /** Absolute 1-based line of a char offset within `text`. */
 function lineOf(text: string, offset: number): number {
-  return newlinesBefore(text, offset) + 1;
+  let n = 0;
+  const end = Math.min(offset, text.length);
+  for (let i = 0; i < end; i++) if (text.charCodeAt(i) === 10) n++;
+  return n + 1;
 }
 
 /** The body string of a `CreateFunctionStmt` (`AS $$ … $$`), or null. */
@@ -138,13 +134,14 @@ export async function parseUnit(
   const lang = language.toLowerCase();
 
   if (lang === 'sql' && body !== null && bodyStartLine !== undefined) {
-    // A SQL-language body is itself SQL: parse it whole. Locations are
-    // relative to the body, so re-anchor them onto the definition.
+    // A SQL-language body is itself SQL: parse it whole. Locations are byte
+    // offsets relative to the body, so re-anchor them onto the definition.
     try {
       const ast = await parse(body);
+      const bodyBytes = utf8(body);
       fragments.push({
         ast,
-        lineForOffset: (offset) => bodyStartLine + newlinesBefore(body, offset)
+        lineForOffset: (offset) => bodyStartLine + newlinesBeforeByte(bodyBytes, offset)
       });
     } catch {
       // Body may contain positional parameters etc. that don't parse alone —

packages/lint/src/types.ts

@@ -84,6 +84,12 @@ export interface LintResult {
   problems: LintProblem[];
   /** Suppressed findings, kept for the "accepted risk" report bucket. */
   suppressed: SuppressedProblem[];
+  /**
+   * Set when the definition could not be parsed. `problems` is then empty
+   * because no rule ran — not because the definition is clean — so a consumer
+   * must report this rather than treat the result as a pass.
+   */
+  parseError?: string;
 }
 
 /** Static metadata for a lint rule. */