fix: the fifteen bugs found by the 2026-07-28 audit

CHANGELOG.md

@@ -18,8 +18,27 @@ Notable changes to this project, following [Keep a Changelog](https://keepachang
 
   Nothing about what the plugin does changed. The split exists because the two halves reach different TypeScript versions: the library type-checks clean on TypeScript 7, while the plugin needs the classic compiler API that TypeScript 7 removed. A single package can only declare one peer range, and either choice would have misrepresented half the code.
 
+### Fixed
+
+An end-to-end audit of the library, the CLI, and the editor plugin; every entry below was reproduced before it was fixed.
+
+- Placeholders no longer collapse into a single unusable slot. A cast on a numbered placeholder (`$1::int`) now still binds by index, and two distinct `$name` placeholders get a slot each instead of intersecting into `never` ([#228](https://github.com/tiagolauer/OwlSQL/issues/228)).
+- `UPDATE`/`DELETE` without a `FROM`/`USING` clause no longer gains a fabricated second source that matched every table in the schema, which made every `RETURNING` column report `ambiguous column` in strict mode ([#229](https://github.com/tiagolauer/OwlSQL/issues/229)).
+- `INSERT ... SELECT` is callable again: its parameter tuple resolved to `never`, which rejects every call including the zero-argument one. It now falls back to `unknown[]` as documented ([#230](https://github.com/tiagolauer/OwlSQL/issues/230)).
+- `MERGE ... OUTPUT $action` no longer demands an extra argument or fails the placeholder-style check against the mssql executor — `$action` is a pseudo-column, not a placeholder ([#231](https://github.com/tiagolauer/OwlSQL/issues/231)).
+- A semicolon inside a quoted identifier (`"id;x"`, `` `id;x` ``, `[id;x]`) is no longer read as a stacked statement ([#232](https://github.com/tiagolauer/OwlSQL/issues/232)).
+- Strict mode now validates the `WHERE` clause of an `UPDATE`/`DELETE` written without `RETURNING`, which it previously skipped entirely ([#233](https://github.com/tiagolauer/OwlSQL/issues/233)).
+- The mssql and node:sqlite parameter scanners skip SQL comments and quoted identifiers. A `@name`/`$name`/`?` inside a comment was bound as a real parameter, shifting every value after it ([#234](https://github.com/tiagolauer/OwlSQL/issues/234)).
+- `createMssqlExecutor(request)` works for more than one query: a caller-supplied `Request` is reset before binding, instead of throwing on a repeated parameter name and leaking the previous query's values ([#235](https://github.com/tiagolauer/OwlSQL/issues/235)).
+- `owlsql generate --url file:./app.db` works; the bare `file:` prefix was detected but never stripped ([#236](https://github.com/tiagolauer/OwlSQL/issues/236)).
+- The node:sqlite adapter reports write metadata for a CTE-led write (`with ... insert ...`) and for a write with `RETURNING`, both of which previously reported none ([#237](https://github.com/tiagolauer/OwlSQL/issues/237)).
+- `:name` placeholders are typed. The adapter has always bound them; the type layer produced an empty parameter tuple and strict mode flagged them as unknown columns ([#238](https://github.com/tiagolauer/OwlSQL/issues/238)).
+- The editor plugin no longer loses the joined table in a plain `from a join b`, marking every reference to it as an unknown alias. `JOIN ... USING` and comma-joined `FROM` lists are handled too ([#242](https://github.com/tiagolauer/OwlSQL/issues/242)).
+
 ### Added
 
+- `owlsql generate` accepts a SQL Server named instance in the URL form (`mssql://user:pass@host\INSTANCE/db`), and explains what a valid connection string looks like when the URL cannot be parsed at all ([#239](https://github.com/tiagolauer/OwlSQL/issues/239)).
+- `owlsql generate` rejects a `--table` name that matches no table instead of silently generating a schema without it, and warns on an unmatched `--exclude` ([#240](https://github.com/tiagolauer/OwlSQL/issues/240)). Boolean flags now reject a value, so `--check=false` no longer turned the check on ([#241](https://github.com/tiagolauer/OwlSQL/issues/241)).
 - Integration tests running the adapters and `owlsql generate` against real PostgreSQL, MySQL, and SQL Server instances, alongside the existing faked-driver tests.
 - A type-instantiation budget (`npm run test:perf`) that fails CI when a change makes the type-level parser measurably more expensive.
 - [VERSIONING.md](VERSIONING.md), stating what counts as a breaking change for a library whose public API is the types it infers.

README.md

@@ -225,7 +225,7 @@ commit it, edit it by hand afterward, rename fields, anything. Running
 
 | Flag | Required | Description |
 | ---- | -------- | ----------- |
-| `--url` | yes | Connection string (or a file path for SQLite). SQL Server accepts both `mssql://user:pass@host:1433/db` (translated to a driver config; `?encrypt=false` and `?trustServerCertificate=true` supported) and an ADO string (`Server=host;Database=db;User Id=u;Password=p`). |
+| `--url` | yes | Connection string (or a file path for SQLite). SQL Server accepts both `mssql://user:pass@host:1433/db` (translated to a driver config; `?encrypt=false` and `?trustServerCertificate=true` supported, and a named instance may be written as `host\INSTANCE`) and an ADO string (`Server=host;Database=db;User Id=u;Password=p`). |
 | `--out` | no | Output file. Defaults to `./schema.ts`. |
 | `--dialect` | no | `postgres` \| `mysql` \| `sqlite` \| `mssql`. Auto-detected from the URL scheme (`postgres://`/`postgresql://`, `mysql://`, `mssql://`/`sqlserver://`); an ADO `Server=...` string also routes to `mssql` — falls back to `sqlite` for a bare file path, so it's only needed when that's ambiguous. |
 | `--schema` | no | Schema/database name to introspect. Defaults to `public` (Postgres), the connected database (MySQL), or `dbo` (SQL Server). Not used for SQLite. |
@@ -433,7 +433,8 @@ The error type propagates wherever you use the rows, surfacing the message in
 hovers and breaking any code that treats them as real data.
 
 Strict mode checks the `SELECT` list, the `WHERE` clause, and `JOIN ... ON`
-conditions:
+conditions — including the `WHERE` of an `UPDATE`/`DELETE` that returns no
+columns, where a typo is most expensive:
 
 ```ts
 const wrongSide = await db.query(
@@ -514,7 +515,9 @@ await db.query('select id from users where id = ?', 1);
 ```
 
 Styles: `'dollar'` (pg, postgres.js), `'question'` (mysql2), `'at'` (mssql).
-`node:sqlite` accepts all three, so leave the option off there.
+`node:sqlite` accepts all three plus `:name`, so leave the option off there.
+A `:name` placeholder is typed like any other but carries no style of its own,
+so it is never checked against a declared dialect.
 
 **Write metadata.** Adapters report driver metadata alongside the rows: on a
 successful `Result`, `result.meta?.rowCount` carries the affected-row count

src/adapters/mssql.ts

@@ -12,14 +12,29 @@ function isRequestSource(source: MssqlQueryable): source is ConnectionPool | Tra
   return typeof (source as { request?: unknown }).request === 'function';
 }
 
+// A ConnectionPool or an already-open Transaction each need `.request()` called
+// to get a Request bound to that connection/transaction; a Request passed
+// directly is already bound and used as-is - this is what lets a caller route a
+// query through an open transaction instead of always implicitly starting a
+// new, separately-committed request.
+//
+// That Request is reused across every query() on this executor, and node-mssql
+// throws on `input()` for a name it has already seen and never clears the bag
+// between calls - so without the reset a second query with the same @name
+// failed, and one with different names silently carried the first query's
+// values along (issue #235).
+function requestFor(source: MssqlQueryable): Request {
+  if (isRequestSource(source)) {
+    return source.request();
+  }
+
+  source.parameters = {};
+  return source;
+}
+
 export function createMssqlExecutor(source: MssqlQueryable): DialectExecutor<'at'> {
   return async (sql, params) => {
-    // A ConnectionPool or an already-open Transaction each need `.request()`
-    // called to get a Request bound to that connection/transaction; a
-    // Request passed directly is already bound and used as-is - this is what
-    // lets a caller route a query through an open transaction instead of
-    // always implicitly starting a new, separately-committed request.
-    const request = isRequestSource(source) ? source.request() : source;
+    const request = requestFor(source);
 
     collectNamedParameters(sql, MSSQL_PARAM_PREFIXES).forEach((name, index) => {
       request.input(name.slice(1), params[index] ?? null);

src/adapters/named-params.ts

@@ -1,6 +1,15 @@
 const NAMED_PARAM_BODY = /^[A-Za-z0-9_]+/;
 const DOLLAR_QUOTE_TAG = /^\$([A-Za-z0-9_]*)\$/;
 
+// The three quoting styles the type layer accepts (standard, MySQL, SQL
+// Server). Their bodies are names, not SQL, so a `@`/`$`/`?` inside one is not
+// a placeholder.
+const IDENTIFIER_QUOTE_CLOSERS: ReadonlyMap<string, string> = new Map([
+  ['"', '"'],
+  ['`', '`'],
+  ['[', ']'],
+]);
+
 // A quote preceded by an odd run of backslashes is backslash-escaped (MySQL's
 // and SQLite's default `\'`) and doesn't open or close a literal - the run has
 // to be odd, not merely non-empty, since `\\'` is an escaped backslash followed
@@ -15,47 +24,120 @@ function isLiteralDelimiter(sql: string, index: number): boolean {
   return backslashes % 2 === 0;
 }
 
-export function collectNamedParameters(sql: string, prefixes: ReadonlySet<string>): string[] {
-  const names: string[] = [];
-  let insideLiteral = false;
-
-  for (let index = 0; index < sql.length; index += 1) {
-    const char = sql[index] as string;
+function skipSingleQuotedLiteral(sql: string, openIndex: number): number {
+  let index = openIndex + 1;
 
-    if (char === "'" && isLiteralDelimiter(sql, index)) {
-      insideLiteral = !insideLiteral;
-      continue;
+  while (index < sql.length) {
+    if (sql[index] === "'" && isLiteralDelimiter(sql, index)) {
+      if (sql[index + 1] === "'") {
+        index += 2;
+        continue;
+      }
+      return index + 1;
     }
-    if (insideLiteral) {
+    index += 1;
+  }
+
+  return sql.length;
+}
+
+function skipDollarQuotedBody(sql: string, openIndex: number): number {
+  const open = DOLLAR_QUOTE_TAG.exec(sql.slice(openIndex));
+  if (!open) {
+    return -1;
+  }
+
+  const closer = `$${open[1]}$`;
+  const closeIndex = sql.indexOf(closer, openIndex + open[0].length);
+  return closeIndex === -1 ? sql.length : closeIndex + closer.length;
+}
+
+// Returns the index just past the non-SQL region opening at `index`, or -1 when
+// nothing opens there. Everything the type-level Normalize strips or masks
+// before computing Params<DB, Q> has to be skipped here too, or the two layers
+// disagree on how many placeholders a query has - comments were the gap, and
+// since binding is positional, one `@word` in a comment shifted every value
+// after it (issue #234).
+function skipNonParameterRegion(sql: string, index: number): number {
+  const char = sql[index];
+
+  if (char === "'") {
+    return skipSingleQuotedLiteral(sql, index);
+  }
+
+  if (char === '-' && sql[index + 1] === '-') {
+    const newline = sql.indexOf('\n', index + 2);
+    return newline === -1 ? sql.length : newline + 1;
+  }
+
+  if (char === '/' && sql[index + 1] === '*') {
+    const close = sql.indexOf('*/', index + 2);
+    return close === -1 ? sql.length : close + 2;
+  }
+
+  if (char === '$') {
+    return skipDollarQuotedBody(sql, index);
+  }
+
+  const closer = char === undefined ? undefined : IDENTIFIER_QUOTE_CLOSERS.get(char);
+  if (closer !== undefined) {
+    const close = sql.indexOf(closer, index + 1);
+    return close === -1 ? sql.length : close + 1;
+  }
+
+  return -1;
+}
+
+type ParameterToken = { kind: 'positional' } | { kind: 'named'; name: string };
+
+function scanParameters(
+  sql: string,
+  prefixes: ReadonlySet<string>,
+  visit: (token: ParameterToken) => void,
+): void {
+  let index = 0;
+
+  while (index < sql.length) {
+    const skipTo = skipNonParameterRegion(sql, index);
+    if (skipTo !== -1) {
+      index = skipTo;
       continue;
     }
 
-    if (char === '$') {
-      const open = DOLLAR_QUOTE_TAG.exec(sql.slice(index));
-      if (open) {
-        const closer = `$${open[1]}$`;
-        const closeIndex = sql.indexOf(closer, index + open[0].length);
-        index = closeIndex === -1 ? sql.length - 1 : closeIndex + closer.length - 1;
-        continue;
-      }
+    const char = sql[index] as string;
+
+    if (char === '?') {
+      visit({ kind: 'positional' });
+      index += 1;
+      continue;
     }
 
     if (prefixes.has(char)) {
       if (sql[index + 1] === char) {
-        index += 1;
+        index += 2;
         continue;
       }
 
       const body = NAMED_PARAM_BODY.exec(sql.slice(index + 1));
       if (body) {
-        const name = `${char}${body[0]}`;
-        if (!names.includes(name)) {
-          names.push(name);
-        }
-        index += body[0].length;
+        visit({ kind: 'named', name: `${char}${body[0]}` });
+        index += 1 + body[0].length;
+        continue;
       }
     }
+
+    index += 1;
   }
+}
+
+export function collectNamedParameters(sql: string, prefixes: ReadonlySet<string>): string[] {
+  const names: string[] = [];
+
+  scanParameters(sql, prefixes, (token) => {
+    if (token.kind === 'named' && !names.includes(token.name)) {
+      names.push(token.name);
+    }
+  });
 
   return names;
 }
@@ -77,54 +159,25 @@ export function resolveMixedParameters(
   values: readonly unknown[],
 ): MixedParameters {
   const named: Record<string, unknown> = {};
+  const bound = new Set<string>();
   const positional: unknown[] = [];
-  let insideLiteral = false;
   let valueIndex = 0;
 
-  for (let index = 0; index < sql.length; index += 1) {
-    const char = sql[index] as string;
-
-    if (char === "'" && isLiteralDelimiter(sql, index)) {
-      insideLiteral = !insideLiteral;
-      continue;
-    }
-    if (insideLiteral) {
-      continue;
-    }
-
-    if (char === '$') {
-      const open = DOLLAR_QUOTE_TAG.exec(sql.slice(index));
-      if (open) {
-        const closer = `$${open[1]}$`;
-        const closeIndex = sql.indexOf(closer, index + open[0].length);
-        index = closeIndex === -1 ? sql.length - 1 : closeIndex + closer.length - 1;
-        continue;
-      }
-    }
-
-    if (char === '?') {
+  scanParameters(sql, prefixes, (token) => {
+    if (token.kind === 'positional') {
       positional.push(values[valueIndex] ?? null);
       valueIndex += 1;
-      continue;
+      return;
     }
 
-    if (prefixes.has(char)) {
-      if (sql[index + 1] === char) {
-        index += 1;
-        continue;
-      }
-
-      const body = NAMED_PARAM_BODY.exec(sql.slice(index + 1));
-      if (body) {
-        const name = `${char}${body[0]}`;
-        if (!(name in named)) {
-          named[name] = values[valueIndex] ?? null;
-          valueIndex += 1;
-        }
-        index += body[0].length;
-      }
+    if (bound.has(token.name)) {
+      return;
     }
-  }
+
+    bound.add(token.name);
+    named[token.name] = values[valueIndex] ?? null;
+    valueIndex += 1;
+  });
 
   return { named, positional };
 }