From f1a976dffae2a91ceace810324a9508b2b28b4da Mon Sep 17 00:00:00 2001 From: Eliya Cohen Date: Fri, 6 Dec 2024 01:33:08 +0200 Subject: [PATCH] feat: better type handling and error reporting (#294) --- .changeset/famous-fishes-arrive.md | 10 ++ .../eslint-plugin/src/rules/check-sql.rule.ts | 22 ++- .../eslint-plugin/src/rules/check-sql.test.ts | 130 +++++++++++++++- .../src/rules/check-sql.utils.ts | 84 +++++++++-- .../eslint-plugin/src/utils/estree.utils.ts | 46 ------ packages/eslint-plugin/src/utils/pg.utils.ts | 13 +- .../eslint-plugin/src/utils/ts-pg.utils.ts | 141 +++++++++++++++--- .../eslint-plugin/src/utils/watch-manager.ts | 1 - .../src/workers/check-sql.worker.ts | 3 +- packages/generate/src/generate.test.ts | 2 +- packages/generate/src/generate.ts | 19 ++- packages/shared/src/errors.ts | 63 ++++++-- 12 files changed, 429 insertions(+), 105 deletions(-) create mode 100644 .changeset/famous-fishes-arrive.md diff --git a/.changeset/famous-fishes-arrive.md b/.changeset/famous-fishes-arrive.md new file mode 100644 index 00000000..1d5d09d7 --- /dev/null +++ b/.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. diff --git a/packages/eslint-plugin/src/rules/check-sql.rule.ts b/packages/eslint-plugin/src/rules/check-sql.rule.ts index 4c6ee643..18851721 100644 --- a/packages/eslint-plugin/src/rules/check-sql.rule.ts +++ b/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 }); }), diff --git a/packages/eslint-plugin/src/rules/check-sql.test.ts b/packages/eslint-plugin/src/rules/check-sql.test.ts index e2f6c157..105cb071 100644 --- a/packages/eslint-plugin/src/rules/check-sql.test.ts +++ b/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\`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 { + 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], + }, + ], + }; + } }); diff --git a/packages/eslint-plugin/src/rules/check-sql.utils.ts b/packages/eslint-plugin/src/rules/check-sql.utils.ts index c413220c..c157b010 100644 --- a/packages/eslint-plugin/src/rules/check-sql.utils.ts +++ b/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; +} + +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, + }; +} diff --git a/packages/eslint-plugin/src/utils/estree.utils.ts b/packages/eslint-plugin/src/utils/estree.utils.ts index 12b845b9..73f4711c 100644 --- a/packages/eslint-plugin/src/utils/estree.utils.ts +++ b/packages/eslint-plugin/src/utils/estree.utils.ts @@ -1,51 +1,5 @@ import { TSESTree } from "@typescript-eslint/utils"; -export function getSourceLocationFromStringPosition(params: { - loc: TSESTree.SourceLocation; - position: number; - value: string; -}): TSESTree.SourceLocation { - const valueUntilError = params.value.substring(0, params.position); - const errorInLine = valueUntilError.match(/\n/g)?.length ?? 0; - const errorFromColumn = valueUntilError.split(/\n/g)[errorInLine].length - 1; - const errorToColumn = - errorFromColumn + - (() => { - const rest = params.value.split(/\n/g)[errorInLine].substring(errorFromColumn); - const amountOfSpacesLeft = rest.match(/\s/g)?.length ?? 0; - - if (amountOfSpacesLeft > 0) { - return rest.indexOf(" "); - } - - return rest.length; - })(); - - if (errorInLine === 0) { - return { - start: { - line: params.loc.start.line + errorInLine, - column: params.loc.start.column + errorFromColumn + 1, - }, - end: { - line: params.loc.start.line + errorInLine, - column: params.loc.start.column + errorToColumn + 1, - }, - }; - } - - return { - start: { - line: params.loc.start.line + errorInLine, - column: errorFromColumn, - }, - end: { - line: params.loc.start.line + errorInLine, - column: errorToColumn, - }, - }; -} - export function isIdentifier(node?: TSESTree.Node): node is TSESTree.Identifier { return node?.type === TSESTree.AST_NODE_TYPES.Identifier; } diff --git a/packages/eslint-plugin/src/utils/pg.utils.ts b/packages/eslint-plugin/src/utils/pg.utils.ts index cf380e88..14ddd9d0 100644 --- a/packages/eslint-plugin/src/utils/pg.utils.ts +++ b/packages/eslint-plugin/src/utils/pg.utils.ts @@ -64,10 +64,15 @@ export function createDatabase(sql: Sql, database: string) { } export function dropDatabase(sql: Sql, database: string) { - return TE.tryCatch( - () => sql.unsafe(`DROP DATABASE IF EXISTS ${database}`), - DatabaseInitializationError.to, - ); + return TE.tryCatch(async () => { + const [{ withForce }] = await sql.unsafe(` + SELECT (string_to_array(version(), ' '))[2]::numeric >= 13 AS "withForce" + `); + + return withForce + ? sql.unsafe(`DROP DATABASE IF EXISTS ${database} WITH (FORCE)`) + : sql.unsafe(`DROP DATABASE IF EXISTS ${database}`); + }, DatabaseInitializationError.to); } function isDefined(value: T | null | undefined): value is T { diff --git a/packages/eslint-plugin/src/utils/ts-pg.utils.ts b/packages/eslint-plugin/src/utils/ts-pg.utils.ts index 8aadc61d..30e1c7c1 100644 --- a/packages/eslint-plugin/src/utils/ts-pg.utils.ts +++ b/packages/eslint-plugin/src/utils/ts-pg.utils.ts @@ -3,9 +3,10 @@ import { doesMatchPattern, InvalidQueryError, normalizeIndent, + QuerySourceMapEntry, } from "@ts-safeql/shared"; import { TSESTreeToTSNode } from "@typescript-eslint/typescript-estree"; -import { ParserServices, TSESTree } from "@typescript-eslint/utils"; +import { ParserServices, TSESLint, TSESTree } from "@typescript-eslint/utils"; import ts, { TypeChecker } from "typescript"; import { RuleOptionConnection } from "../rules/RuleOptions"; import { E, pipe } from "./fp-ts"; @@ -16,18 +17,21 @@ export function mapTemplateLiteralToQueryText( parser: ParserServices, checker: ts.TypeChecker, options: RuleOptionConnection, + sourceCode: Readonly, ) { let $idx = 0; let $queryText = ""; + const sourcemaps: QuerySourceMapEntry[] = []; - for (const $quasi of quasi.quasis) { + for (const [quasiIdx, $quasi] of quasi.quasis.entries()) { $queryText += $quasi.value.raw; if ($quasi.tail) { break; } - const expression = quasi.expressions[$idx]; + const position = $queryText.length; + const expression = quasi.expressions[quasiIdx]; const pgType = pipe(mapExpressionToTsTypeString({ expression, parser, checker }), (params) => getPgTypeFromTsType({ ...params, checker, options }), @@ -39,10 +43,95 @@ export function mapTemplateLiteralToQueryText( const pgTypeValue = pgType.right; - $queryText += pgTypeValue === null ? `$${++$idx}` : `$${++$idx}::${pgTypeValue}`; + if (pgTypeValue === null) { + const placeholder = `$${++$idx}`; + $queryText += placeholder; + + sourcemaps.push({ + original: { + text: sourceCode.text.slice(expression.range[0] - 2, expression.range[1] + 1), + start: expression.range[0] - quasi.range[0] - 2, + end: expression.range[1] - quasi.range[0] + 1, + }, + generated: { + text: placeholder, + start: position, + end: position + placeholder.length, + }, + offset: 0, + }); + + continue; + } + + if (pgTypeValue.kind === "literal") { + const placeholder = pgTypeValue.value; + $queryText += placeholder; + + sourcemaps.push({ + original: { + start: expression.range[0] - quasi.range[0] - 2, + end: expression.range[1] - quasi.range[0] + 1, + text: sourceCode.text.slice(expression.range[0] - 2, expression.range[1] + 1), + }, + generated: { + start: position, + end: position + placeholder.length, + text: placeholder, + }, + offset: 0, + }); + + continue; + } + + if (pgTypeValue.kind === "one-of" && $queryText.trimEnd().endsWith("=")) { + const textFromEquals = $queryText.slice($queryText.lastIndexOf("=")); + const placeholder = `IN (${pgTypeValue.types.map((t) => `'${t}'`).join(", ")})`; + const expressionText = sourceCode.text.slice( + expression.range[0] - 2, + expression.range[1] + 1, + ); + + $queryText = $queryText.replace(/(=)\s*$/, ""); + $queryText += placeholder; + + sourcemaps.push({ + original: { + start: expression.range[0] - quasi.range[0] - 2 - textFromEquals.length, + end: expression.range[1] - quasi.range[0] + 2 - textFromEquals.length, + text: `${textFromEquals}${expressionText}`, + }, + generated: { + start: position - textFromEquals.length + 1, + end: position + placeholder.length - textFromEquals.length, + text: placeholder, + }, + offset: textFromEquals.length, + }); + + continue; + } + + const placeholder = `$${++$idx}::${pgTypeValue.cast}`; + $queryText += placeholder; + + sourcemaps.push({ + original: { + start: expression.range[0] - quasi.range[0] - 2, + end: expression.range[1] - quasi.range[0], + text: sourceCode.text.slice(expression.range[0] - 2, expression.range[1] + 1), + }, + generated: { + start: position, + end: position + placeholder.length, + text: placeholder, + }, + offset: 0, + }); } - return E.right($queryText); + return E.right({ text: $queryText, sourcemaps }); } function mapExpressionToTsTypeString(params: { @@ -68,7 +157,6 @@ const tsTypeToPgTypeMap: Record = { }; const tsKindToPgTypeMap: Record = { - [ts.SyntaxKind.StringLiteral]: "text", [ts.SyntaxKind.NumericLiteral]: "int", [ts.SyntaxKind.TrueKeyword]: "boolean", [ts.SyntaxKind.FalseKeyword]: "boolean", @@ -97,8 +185,18 @@ const tsFlagToPgTypeMap: Record = { [ts.TypeFlags.BigIntLiteral]: "bigint", }; -function getPgTypeFromTsTypeUnion(params: { types: ts.Type[] }) { +function getPgTypeFromTsTypeUnion(params: { types: ts.Type[] }): E.Either { const types = params.types.filter((t) => t.flags !== ts.TypeFlags.Null); + const isStringLiterals = types.every((t) => t.flags === ts.TypeFlags.StringLiteral); + + if (isStringLiterals) { + return E.right({ + kind: "one-of", + types: types.map((t) => (t as ts.StringLiteralType).value), + cast: "text", + }); + } + const isUnionOfTheSameType = types.every((t) => t.flags === types[0].flags); const pgType = tsFlagToPgTypeMap[types[0].flags]; @@ -106,15 +204,20 @@ function getPgTypeFromTsTypeUnion(params: { types: ts.Type[] }) { return E.left(createMixedTypesInUnionErrorMessage(types.map((t) => t.flags))); } - return E.right(pgType); + return E.right({ kind: "cast", cast: pgType }); } +type PgTypeStrategy = + | { kind: "cast"; cast: string } + | { kind: "literal"; value: string; cast: string } + | { kind: "one-of"; types: string[]; cast: string }; + function getPgTypeFromTsType(params: { checker: TypeChecker; node: TSESTreeToTSNode; type: ts.Type; options: RuleOptionConnection; -}): E.Either { +}): E.Either { const { checker, node, type, options } = params; // Utility function to get PostgreSQL type from flags @@ -137,7 +240,7 @@ function getPgTypeFromTsType(params: { ); } - return E.right(whenTrueType); + return E.right({ kind: "cast", cast: whenTrueType }); } // Check for identifier @@ -154,20 +257,24 @@ function getPgTypeFromTsType(params: { return elementTypeUnion ? pipe( getPgTypeFromTsTypeUnion({ types: elementTypeUnion }), - E.map((pgType) => `${pgType}[]`), + E.map((pgType) => ({ kind: "cast", cast: `${pgType.cast}[]` })), ) : E.left("Invalid array union type"); } } + if (node.kind === ts.SyntaxKind.StringLiteral) { + return E.right({ kind: "literal", value: `'${node.text}'`, cast: "text" }); + } + // Check for known SyntaxKind mappings if (node.kind in tsKindToPgTypeMap) { - return E.right(tsKindToPgTypeMap[node.kind]); + return E.right({ kind: "cast", cast: tsKindToPgTypeMap[node.kind] }); } // Check for known type flags if (type.flags in tsFlagToPgTypeMap) { - return E.right(tsFlagToPgTypeMap[type.flags]); + return E.right({ kind: "cast", cast: tsFlagToPgTypeMap[type.flags] }); } // Handle null type @@ -179,7 +286,7 @@ function getPgTypeFromTsType(params: { if (TSUtils.isTsUnionType(type)) { const matchingType = type.types.find((t) => t.flags in tsFlagToPgTypeMap); return matchingType - ? E.right(tsFlagToPgTypeMap[matchingType.flags]) + ? E.right({ kind: "cast", cast: tsFlagToPgTypeMap[matchingType.flags] }) : E.left("Unsupported union type"); } @@ -190,7 +297,7 @@ function getPgTypeFromTsType(params: { const singularPgType = tsTypeToPgTypeMap[singularType]; if (singularPgType) { - return E.right(isArray ? `${singularPgType}[]` : singularPgType); + return E.right({ kind: "cast", cast: isArray ? `${singularPgType}[]` : singularPgType }); } if (checker.isArrayType(type)) { @@ -199,7 +306,7 @@ function getPgTypeFromTsType(params: { elementType?.isUnion() && elementType.types.every((t) => t.flags === elementType.types[0].flags) ) { - return E.right(`${getPgTypeFromFlags(elementType.types[0].flags)}[]`); + return E.right({ kind: "cast", cast: `${getPgTypeFromFlags(elementType.types[0].flags)}[]` }); } } @@ -214,7 +321,7 @@ function getPgTypeFromTsType(params: { if (override) { const [pgType] = override; - return E.right(isArray ? `${pgType}[]` : pgType); + return E.right({ kind: "cast", cast: isArray ? `${pgType}[]` : pgType }); } // Fallback for unsupported types diff --git a/packages/eslint-plugin/src/utils/watch-manager.ts b/packages/eslint-plugin/src/utils/watch-manager.ts index 94e2ab86..b1aeefe5 100644 --- a/packages/eslint-plugin/src/utils/watch-manager.ts +++ b/packages/eslint-plugin/src/utils/watch-manager.ts @@ -49,7 +49,6 @@ export function createWatchManager() { }, }); - console.log("migrationPath", migrationPath); watcher.add(migrationPath); }; diff --git a/packages/eslint-plugin/src/workers/check-sql.worker.ts b/packages/eslint-plugin/src/workers/check-sql.worker.ts index 12bbb7c0..f0ca642a 100644 --- a/packages/eslint-plugin/src/workers/check-sql.worker.ts +++ b/packages/eslint-plugin/src/workers/check-sql.worker.ts @@ -9,6 +9,7 @@ import { InternalError, InvalidMigrationError, InvalidMigrationsPathError, + QuerySourceMapEntry, } from "@ts-safeql/shared"; import * as LibPgQueryAST from "@ts-safeql/sql-ast"; import path from "path"; @@ -30,7 +31,7 @@ import { ConnectionTarget, RuleOptionConnection } from "../rules/RuleOptions"; export interface CheckSQLWorkerParams { connection: RuleOptionConnection; target: ConnectionTarget; - query: string; + query: { text: string; sourcemaps: QuerySourceMapEntry[] }; projectDir: string; pgParsed: LibPgQueryAST.ParseResult; } diff --git a/packages/generate/src/generate.test.ts b/packages/generate/src/generate.test.ts index c83110f5..55d33675 100644 --- a/packages/generate/src/generate.test.ts +++ b/packages/generate/src/generate.test.ts @@ -183,7 +183,7 @@ const testQuery = async (params: { generateTE({ sql, pgParsed, - query, + query: { text: query, sourcemaps: [] }, cacheKey, fieldTransform: undefined, overrides: { diff --git a/packages/generate/src/generate.ts b/packages/generate/src/generate.ts index abd6063d..d2297998 100644 --- a/packages/generate/src/generate.ts +++ b/packages/generate/src/generate.ts @@ -8,6 +8,7 @@ import { IdentiferCase, isPostgresError, PostgresError, + QuerySourceMapEntry, toCase, } from "@ts-safeql/shared"; import * as LibPgQueryAST from "@ts-safeql/sql-ast"; @@ -37,7 +38,7 @@ export type GenerateResult = { output: Extract | null; unknownColumns: string[]; stmt: postgres.Statement; - query: string; + query: { text: string; sourcemaps: QuerySourceMapEntry[] }; }; export type GenerateError = DuplicateColumnsError | PostgresError; @@ -50,7 +51,7 @@ type Overrides = { export interface GenerateParams { sql: Sql; - query: string; + query: { text: string; sourcemaps: QuerySourceMapEntry[] }; pgParsed: LibPgQueryAST.ParseResult; cacheMetadata?: boolean; cacheKey: CacheKey; @@ -207,7 +208,7 @@ async function generate( }); try { - const result = await sql.unsafe(query, [], { prepare: true }).describe(); + const result = await sql.unsafe(query.text, [], { prepare: true }).describe(); if (result.columns === undefined || result.columns === null || result.columns.length === 0) { return either.right({ output: null, unknownColumns: [], stmt: result, query: query }); @@ -232,9 +233,16 @@ async function generate( }; }); + const dupePosition = (() => { + const match = query.text.search(new RegExp(`\\b${duplicateCols[0].name}\\b`)); + return (match > -1 ? match : query.text.search(/SELECT/i)) + 1; + })(); + return either.left( DuplicateColumnsError.of({ - queryText: query, + queryText: query.text, + sourcemaps: query.sourcemaps, + position: dupePosition, columns: dupes.map((x) => { return x.column !== x.originalColumn ? `${x.table}.${x.originalColumn} (alias: ${x.column})` @@ -299,7 +307,8 @@ async function generate( if (isPostgresError(e)) { return either.left( PostgresError.of({ - queryText: query, + queryText: query.text, + sourcemaps: query.sourcemaps, message: e.message, line: e.line, position: e.position, diff --git a/packages/shared/src/errors.ts b/packages/shared/src/errors.ts index 4463c32d..be712171 100644 --- a/packages/shared/src/errors.ts +++ b/packages/shared/src/errors.ts @@ -169,14 +169,28 @@ export class DuplicateColumnsError extends Error { _tag = "DuplicateColumnsError" as const; columns: string[]; queryText: string; - - constructor(params: { columns: string[]; queryText: string }) { + position: number; + sourcemaps: QuerySourceMapEntry[]; + + constructor(params: { + columns: string[]; + queryText: string; + position: number; + sourcemaps: QuerySourceMapEntry[]; + }) { super(`Duplicate columns: ${params.columns.join(", ")}`); this.columns = params.columns; this.queryText = params.queryText; + this.position = params.position; + this.sourcemaps = params.sourcemaps; } - static of(params: { columns: string[]; queryText: string }) { + static of(params: { + columns: string[]; + queryText: string; + position: number; + sourcemaps: QuerySourceMapEntry[]; + }) { return new DuplicateColumnsError(params); } @@ -186,37 +200,60 @@ export class DuplicateColumnsError extends Error { message: this.message, columns: this.columns, queryText: this.queryText, + position: this.position, + sourcemaps: this.sourcemaps, }; } } +export interface QuerySourceMapEntry { + original: { start: number; end: number; text: string }; + generated: { start: number; end: number; text: string }; + offset: number; +} + export class PostgresError extends Error { _tag = "PostgresError" as const; queryText: string; message: string; line: string; - position: string; - - constructor(params: { queryText: string; message: string; line: string; position: string }) { + position: number; + sourcemaps: QuerySourceMapEntry[]; + + constructor(params: { + queryText: string; + message: string; + line: string; + position: number | string; + sourcemaps: QuerySourceMapEntry[]; + }) { super(params.message); this.queryText = params.queryText; this.message = params.message; this.line = params.line; - this.position = params.position; + this.position = Number(params.position); + this.sourcemaps = params.sourcemaps; } - static of(params: { queryText: string; message: string; line: string; position: string }) { + static of(params: { + queryText: string; + message: string; + line: string; + position: number | string; + sourcemaps: QuerySourceMapEntry[]; + }) { return new PostgresError(params); } - static to(query: string, error: unknown) { + static to(query: string, error: unknown, sourcemaps: QuerySourceMapEntry[]) { if (isPostgresError(error)) { return PostgresError.of({ queryText: query, message: error.message, line: error.line, position: error.position, + sourcemaps, }); } @@ -224,7 +261,8 @@ export class PostgresError extends Error { queryText: query, message: `${error}`, line: "1", - position: "1", + position: isPgParserError(error) ? error.cursorPosition : 0, + sourcemaps: sourcemaps, }); } @@ -235,6 +273,7 @@ export class PostgresError extends Error { message: this.message, line: this.line, position: this.position, + sourcemaps: this.sourcemaps, }; } } @@ -250,3 +289,7 @@ export function isPostgresError(e: unknown): e is postgres.PostgresError { return false; } + +function isPgParserError(error: unknown): error is Error & { cursorPosition: number } { + return error instanceof Error && "cursorPosition" in error; +}