feat: better type handling and error reporting (#294)

.changeset/famous-fishes-arrive.md

@@ -0,0 +1,10 @@
+---
+"@ts-safeql/eslint-plugin": minor
+"@ts-safeql/generate": minor
+---
+
+**Improved Error Reporting** - Previously, errors could point to incorrect positions due to query transformations behind the scene. Now, errors align precisely with your source code, making debugging easier.
+
+**Better Type Handling** - String expressions previously required manual casting to work with stricter types like enums, often causing type errors. This update removes the need for manual casting by ensuring seamless compatibility.
+
+**Forceful Shadow Database Dropping** - When using the migrations strategy, shadow databases are now forcefully dropped (if supported). This is an internal improvement to prevent edge cases and ensure smoother migrations.

packages/eslint-plugin/src/rules/check-sql.rule.ts

@@ -1,5 +1,11 @@
 import { ResolvedTarget } from "@ts-safeql/generate";
-import { InvalidConfigError, PostgresError, doesMatchPattern, fmap } from "@ts-safeql/shared";
+import {
+  InvalidConfigError,
+  PostgresError,
+  QuerySourceMapEntry,
+  doesMatchPattern,
+  fmap,
+} from "@ts-safeql/shared";
 import {
   ESLintUtils,
   ParserServices,
@@ -119,11 +125,11 @@ function checkConnection(params: {
   return match(params.target).exhaustive();
 }
 
-const pgParseQueryE = (query: string) => {
+const pgParseQueryE = (query: string, sourcemaps: QuerySourceMapEntry[]) => {
   return pipe(
     E.tryCatch(
       () => parser.parseQuerySync(query),
-      (error) => PostgresError.to(query, error),
+      (error) => PostgresError.to(query, error, sourcemaps),
     ),
   );
 };
@@ -162,9 +168,15 @@ function reportCheck(params: {
         : E.right(parser.program.getTypeChecker());
     }),
     E.bindW("query", ({ parser, checker }) =>
-      mapTemplateLiteralToQueryText(tag.quasi, parser, checker, params.connection),
+      mapTemplateLiteralToQueryText(
+        tag.quasi,
+        parser,
+        checker,
+        params.connection,
+        params.context.sourceCode,
+      ),
     ),
-    E.bindW("pgParsed", ({ query }) => pgParseQueryE(query)),
+    E.bindW("pgParsed", ({ query }) => pgParseQueryE(query.text, query.sourcemaps)),
     E.bindW("result", ({ query, pgParsed }) => {
       return generateSyncE({ query, pgParsed, connection, target, projectDir });
     }),

packages/eslint-plugin/src/rules/check-sql.test.ts

@@ -3,13 +3,14 @@ import {
   setupTestDatabase,
   typeColumnTsTypeEntries,
 } from "@ts-safeql/test-utils";
-import { RuleTester } from "@typescript-eslint/rule-tester";
+import { InvalidTestCase, RuleTester } from "@typescript-eslint/rule-tester";
 
 import { afterAll, beforeAll, describe, it } from "vitest";
 import path from "path";
 import { Sql } from "postgres";
 import rules from ".";
 import { RuleOptionConnection, RuleOptions } from "./RuleOptions";
+import { normalizeIndent } from "@ts-safeql/shared";
 
 const tsconfigRootDir = path.resolve(__dirname, "../../");
 const project = "tsconfig.json";
@@ -567,6 +568,18 @@ RuleTester.describe("check-sql", () => {
           sql<Result>\`SELECT colname FROM test_nullable_boolean\`
         `,
       },
+      {
+        filename,
+        name: "select where enum column equals to one of the string literals",
+        options: withConnection(connections.withTag),
+        code: `
+          function run(cert: "HHA" | "RN" | "LPN" | "CNA" | "PCA" | "OTHER") {
+            sql\`select from caregiver WHERE certification = \${cert}\`
+            sql\`select from caregiver WHERE certification = \${"HHA"}\`
+            sql\`select from caregiver WHERE certification = 'HHA'\`
+          }
+        `,
+      },
     ],
     invalid: [
       {
@@ -621,9 +634,9 @@ RuleTester.describe("check-sql", () => {
               error: "Duplicate columns: caregiver.id, agency.id",
             },
             line: 1,
-            column: 30,
+            column: 31,
             endLine: 1,
-            endColumn: 36,
+            endColumn: 37,
           },
         ],
       },
@@ -824,6 +837,91 @@ RuleTester.describe("check-sql", () => {
     ],
   });
 
+  ruleTester.run("position", rules["check-sql"], {
+    valid: [
+      {
+        filename,
+        name: "control",
+        options: withConnection(connections.base),
+        code: `
+          function run(cert1: "HHA" | "RN", cert2: "LPN" | "CNA") {
+            return sql<{ id: number }[]>\`
+              select id
+              from caregiver
+              where true 
+                and certification = \${cert1}
+                and caregiver.id = 1
+                and certification = \${cert2}
+                and caregiver.id = 1
+            \`
+          }
+        `,
+      },
+    ],
+    invalid: [
+      invalidPositionTestCase({
+        code: "sql`select idd from caregiver`",
+        error: 'column "idd" does not exist',
+        line: 1,
+        columns: [12, 15],
+      }),
+      invalidPositionTestCase({
+        code: normalizeIndent`
+          sql\`
+            select
+              id
+            from
+              caregiverr
+          \`
+        `,
+        error: 'relation "caregiverr" does not exist',
+        line: 5,
+        columns: [5, 15],
+      }),
+      invalidPositionTestCase({
+        code: normalizeIndent`
+          function run(expr1: "HHA" | "RN", expr2: "LPN" | "CNN") {
+            sql\`
+              select id
+              from
+                caregiver
+              where true
+                and certification = \${expr1}
+                and caregiver.id = 1
+                and certification = \${expr2}
+                and caregiver.id = 1
+            \`
+          }
+        `,
+        error: 'invalid input value for enum certification: "CNN"',
+        line: 9,
+        columns: [27, 35],
+      }),
+      invalidPositionTestCase({
+        code: normalizeIndent`
+          function run(cert1: "HHA" | "RNA") {
+            return sql\`select id from caregiver where certification = \${cert1}\`
+          }
+        `,
+        error: 'invalid input value for enum certification: "RNA"',
+        line: 2,
+        columns: [61, 69],
+      }),
+      invalidPositionTestCase({
+        code: "sql`select id, id from caregiver`",
+        error: `Duplicate columns: caregiver.id, caregiver.id`,
+        line: 1,
+        columns: [12, 14],
+      }),
+      invalidPositionTestCase({
+        code: "sql`select id sele, certification sele from caregiver`",
+        error: `Duplicate columns: caregiver.id (alias: sele), caregiver.certification (alias: sele)`,
+        line: 1,
+        columns: [15, 19],
+      }),
+    ],
+  });
+
   ruleTester.run("pg type to ts type check (inline type)", rules["check-sql"], {
     valid: typeColumnTsTypeEntries.map(([colName, colType]) => ({
       name: `select ${colName} from table as ${colType} (using type reference)`,
@@ -1847,4 +1945,30 @@ RuleTester.describe("check-sql", () => {
       },
     ],
   });
+
+  function invalidPositionTestCase(params: {
+    only?: boolean;
+    line: number;
+    columns: [number, number];
+    error: string;
+    code: string;
+  }): InvalidTestCase<keyof (typeof rules)["check-sql"]["meta"]["messages"], RuleOptions> {
+    return {
+      filename,
+      name: `${params.line}:[${params.columns[0]}:${params.columns[1]}] - ${params.error}`,
+      only: params.only,
+      options: withConnection(connections.withTag),
+      code: params.code,
+      errors: [
+        {
+          messageId: "invalidQuery",
+          data: { error: params.error },
+          line: params.line,
+          endLine: params.line,
+          column: params.columns[0],
+          endColumn: params.columns[1],
+        },
+      ],
+    };
+  }
 });

packages/eslint-plugin/src/rules/check-sql.utils.ts

@@ -6,21 +6,22 @@ import {
   InvalidMigrationsPathError,
   InvalidQueryError,
   PostgresError,
+  QuerySourceMapEntry,
   fmap,
 } from "@ts-safeql/shared";
 import { TSESTree } from "@typescript-eslint/utils";
+import { SourceCode } from "@typescript-eslint/utils/ts-eslint";
 import crypto from "crypto";
 import fs from "fs";
 import path from "path";
 import { Sql } from "postgres";
 import { match } from "ts-pattern";
 import { z } from "zod";
-import { ESTreeUtils } from "../utils";
 import { E, TE, pipe } from "../utils/fp-ts";
 import { mapConnectionOptionsToString, parseConnection } from "../utils/pg.utils";
+import { WorkerError } from "../workers/check-sql.worker";
 import { RuleContext } from "./check-sql.rule";
 import { RuleOptionConnection, zConnectionMigration } from "./RuleOptions";
-import { WorkerError } from "../workers/check-sql.worker";
 
 type TypeReplacerString = string;
 type TypeReplacerFromTo = [string, string];
@@ -133,14 +134,16 @@ export function reportDuplicateColumns(params: {
 }) {
   const { tag, context, error } = params;
 
+  const location = getQueryErrorPosition({
+    tag: tag,
+    error: error,
+    sourceCode: context.sourceCode,
+  });
+
   return context.report({
     node: tag,
     messageId: "invalidQuery",
-    loc: ESTreeUtils.getSourceLocationFromStringPosition({
-      loc: tag.quasi.loc,
-      position: error.queryText.search(error.columns[0]) + 1,
-      value: error.queryText,
-    }),
+    loc: location.sourceLocation,
     data: {
       error: error.message,
     },
@@ -154,14 +157,16 @@ export function reportPostgresError(params: {
 }) {
   const { context, tag, error } = params;
 
+  const location = getQueryErrorPosition({
+    tag: tag,
+    error: error,
+    sourceCode: context.sourceCode,
+  });
+
   return context.report({
     node: tag,
     messageId: "invalidQuery",
-    loc: ESTreeUtils.getSourceLocationFromStringPosition({
-      loc: tag.quasi.loc,
-      position: parseInt(error.position, 10),
-      value: error.queryText,
-    }),
+    loc: location.sourceLocation,
     data: {
       error: error.message,
     },
@@ -480,3 +485,58 @@ function isNullableResolvedTarget(target: ResolvedTarget): boolean {
       return false;
   }
 }
+
+interface GetWordRangeInPositionParams {
+  error: {
+    position: number;
+    sourcemaps: QuerySourceMapEntry[];
+  };
+  tag: TSESTree.TaggedTemplateExpression;
+  sourceCode: Readonly<SourceCode>;
+}
+
+function getQueryErrorPosition(params: GetWordRangeInPositionParams) {
+  const range: [number, number] = [params.error.position, params.error.position + 1];
+
+  for (const entry of params.error.sourcemaps) {
+    const generatedLength = Math.max(0, entry.generated.end - entry.generated.start);
+    const originalLength = Math.max(0, entry.original.end - entry.original.start);
+    const adjustment = originalLength - generatedLength;
+
+    if (range[0] >= entry.generated.start && range[1] <= entry.generated.end) {
+      range[0] = entry.original.start + entry.offset;
+      range[1] = entry.original.start + entry.offset + 1;
+      continue;
+    }
+
+    if (params.error.position >= entry.generated.start) {
+      range[0] += adjustment;
+    }
+
+    if (params.error.position >= entry.generated.end) {
+      range[1] += adjustment;
+    }
+  }
+
+  const start = params.sourceCode.getLocFromIndex(params.tag.quasi.range[0] + range[0]);
+  const startLineText = params.sourceCode.getLines()[start.line - 1];
+  const remainingLineText = startLineText.substring(start.column);
+  const remainingWordLength = (remainingLineText.match(/^[\w.{}'$"]+/)?.at(0)?.length ?? 1) - 1;
+
+  const end = params.sourceCode.getLocFromIndex(params.tag.quasi.range[0] + range[1]);
+
+  const sourceLocation: TSESTree.SourceLocation = {
+    start: start,
+    end: {
+      line: end.line,
+      column: end.column + remainingWordLength,
+    },
+  };
+
+  return {
+    range,
+    sourceLocation: sourceLocation,
+    remainingLineText: remainingLineText,
+    remainingWordLength: remainingWordLength,
+  };
+}