From 413514629acd81cfbae45552c67b5930c4ca795d Mon Sep 17 00:00:00 2001 From: Daniel Castro Date: Wed, 30 Nov 2022 19:07:24 +0000 Subject: [PATCH 01/10] draft --- package.json | 613 +++++++++--------- server/src/commands/validateWorkspace.ts | 1 + server/src/postgres/parsers/parseFunctions.ts | 72 +- .../queries/queryFileStaticAnalysis.ts | 101 ++- server/src/services/validation.ts | 10 +- server/src/settings.ts | 2 + 6 files changed, 469 insertions(+), 330 deletions(-) diff --git a/package.json b/package.json index ea474d3..7409038 100644 --- a/package.json +++ b/package.json @@ -1,306 +1,311 @@ { - "name": "vscode-plpgsql-lsp", - "displayName": "PL/pgSQL Language Server", - "description": "VSCode PL/pgSQL Language Server.", - "license": "MIT", - "version": "2.10.1", - "icon": "images/postgres.png", - "repository": { - "type": "git", - "url": "https://github.com/UniqueVision/plpgsql-lsp" - }, - "publisher": "uniquevision", - "categories": [], - "keywords": [ - "Postgres", - "PostgreSQL", - "plpgsql", - "Language Server" - ], - "engines": { - "vscode": "^1.68.0" - }, - "activationEvents": [ - "onLanguage:postgres", - "onCommand:plpgsql-lsp.executeFileQuery", - "onCommand:plpgsql-lsp.validateWorkspace" - ], - "main": "./client/out/extension", - "scripts": { - "vscode:prepublish": "npm run build", - "build": "npm run clean && webpack --mode production --config ./client/webpack.config.js && webpack --mode production --config ./server/webpack.config.js", - "build:dev": "npm run clean && webpack --mode none --config ./client/webpack.config.js && webpack --mode none --config ./server/webpack.config.js", - "tsc": "tsc --noEmit --project ./tsconfig.json && npm run tsc:client && npm run tsc:server", - "tsc:client": "tsc --noEmit --project ./client/tsconfig.json", - "tsc:server": "tsc --noEmit --project ./server/tsconfig.json", - "compile": "tsc -b", - "compile:client": "tsc -b ./client/tsconfig.json", - "compile:server": "tsc -b ./server/tsconfig.json", - "watch": "webpack --watch", - "package:linux": "vsce package --target linux-x64", - "package:mac": "vsce package --target darwin-x64", - "package:windows": "vsce package --target win32-x64", - "lint": "eslint ./client/src ./server/src --ext .ts,.tsx --fix", - "postinstall": "cd client && npm install && cd ../server && npm install && cd ..", - "test": "cd server && npm run test && cd ..", - "test:ci": "cd server && npm run test:ci && cd ..", - "clean": "rimraf client/out && rimraf server/out" - }, - "contributes": { - "commands": [ - { - "command": "plpgsql-lsp.executeFileQuery", - "title": "PL/pgSQL: Execute the Current File Query" - }, - { - "command": "plpgsql-lsp.validateWorkspace", - "title": "PL/pgSQL: Validate the Workspace Files" - } - ], - "menus": { - "commandPalette": [ - { - "command": "plpgsql-lsp.executeFileQuery", - "when": "editorLangId == postgres" - }, - { - "command": "plpgsql-lsp.validateWorkspace", - "when": "editorLangId == postgres" - } - ] - }, - "languages": [ - { - "id": "postgres", - "extensions": [ - ".pgsql", - ".psql" - ], - "aliases": [ - "Postgres" - ], - "configuration": "./language-configuration.json" - } - ], - "grammars": [ - { - "language": "postgres", - "scopeName": "source.pgsql", - "path": "./syntaxes/pgsql.tmLanguage" - } - ], - "configuration": { - "type": "object", - "title": "PostgreSQL connection configuration", - "properties": { - "plpgsqlLanguageServer.host": { - "scope": "resource", - "type": "string", - "default": "localhost", - "description": "Your database host." - }, - "plpgsqlLanguageServer.port": { - "scope": "resource", - "type": "number", - "default": 5432, - "description": "Your database port." - }, - "plpgsqlLanguageServer.database": { - "scope": "resource", - "type": "string", - "nullable": true, - "description": "Your database name." - }, - "plpgsqlLanguageServer.user": { - "scope": "resource", - "type": "string", - "nullable": true, - "description": "Your database user." - }, - "plpgsqlLanguageServer.password": { - "scope": "resource", - "type": "string", - "nullable": true, - "description": "Your database password." - }, - "plpgsqlLanguageServer.definitionFiles": { - "scope": "resource", - "type": "array", - "default": [ - "**/*.psql", - "**/*.pgsql" - ], - "description": "The pattern list of the definition files.", - "items": { - "type": "string" - } - }, - "plpgsqlLanguageServer.defaultSchema": { - "scope": "resource", - "type": "string", - "default": "public", - "description": "Default schema name." - }, - "plpgsqlLanguageServer.validateOn": { - "scope": "resource", - "type": "string", - "enum": [ - "save", - "change" - ], - "default": "change", - "description": "Specify when to validate files. Default: 'change'" - }, - "plpgsqlLanguageServer.queryParameterPattern": { - "scope": "resource", - "type": "string", - "default": "\\$[1-9][0-9]*", - "description": "Query parameter pattern. the pattern is described by regular expression." - }, - "plpgsqlLanguageServer.keywordQueryParameterPattern": { - "scope": "resource", - "type": "array", - "default": [], - "description": "Keyword query parameter patterns. Patterns must include \"{keyword}\", as in \"@{keyword}\".", - "items": { - "type": "string" - } - }, - "plpgsqlLanguageServer.statements": { - "properties": { - "separatorPattern": { - "scope": "resource", - "type": "string", - "required": true, - "example": "-- name:[\\s]+.*", - "description": "Prepared statement separator regex pattern." - }, - "diagnosticsLevels": { - "scope": "resource", - "type": "object", - "default": null, - "description": "Override diagnostic levels for statements.", - "properties": { - "disableFlag": { - "scope": "resource", - "type": "string", - "default": "disable", - "enum": [ - "disable", - "warning" - ], - "description": "Set diagnostic level for the disabled validation flag." - } - } - } - }, - "default": null, - "scope": "resource", - "type": "object", - "description": "If set, multiple statements in a file can be analyzed based on a separator." - }, - "plpgsqlLanguageServer.migrations": { - "properties": { - "upFiles": { - "scope": "resource", - "type": "array", - "items": { - "type": "string" - }, - "description": "Up migrations file glob pattern. Executed by sorted filename order.", - "required": true, - "example": [ - "*.up.pgsql" - ] - }, - "downFiles": { - "scope": "resource", - "type": "array", - "items": { - "type": "string" - }, - "description": "Down migrations file glob pattern. Executed by sorted filename order.", - "required": true, - "example": [ - "*.down.pgsql" - ] - }, - "postMigrationFiles": { - "scope": "resource", - "type": "array", - "items": { - "type": "string" - }, - "description": "Post-migration file glob pattern. Executed by sorted filename order.", - "required": true, - "default": [], - "example": [ - "*.post.pgsql" - ] - }, - "target": { - "scope": "resource", - "type": "string", - "enum": [ - "all", - "up/down" - ], - "default": "up/down", - "description": "Specify target files to be migrated in the background. Default: 'up/down'" - } - }, - "scope": "resource", - "default": null, - "type": "object", - "description": "If set, migrations will be applied for all analyses. If the current file is a migration file, execution will run until the previous migration." - }, - "plpgsqlLanguageServer.enableExecuteFileQueryCommand": { - "scope": "resource", - "type": "boolean", - "default": true, - "description": "Enable/Disable \"Execute the Current File Query\" command." - }, - "plpgsqlLanguageServer.workspaceValidationTargetFiles": { - "scope": "resource", - "type": "array", - "default": [], - "description": "The pattern list of the workspace validation target files.", - "items": { - "type": "string" - } - } - }, - "required": [ - "plpgsqlLanguageServer.host", - "plpgsqlLanguageServer.port", - "plpgsqlLanguageServer.defaultSchema" - ] - } - }, - "devDependencies": { - "@types/node": "^18.0.0", - "@typescript-eslint/eslint-plugin": "^5.30.0", - "@typescript-eslint/parser": "^5.30.0", - "eslint": "^8.18.0", - "eslint-config-prettier": "^8.5.0", - "eslint-config-standard": "^17.0.0", - "eslint-plugin-import": "^2.26.0", - "eslint-plugin-node": "^11.1.0", - "eslint-plugin-prettier": "^4.2.1", - "eslint-plugin-promise": "^6.0.0", - "eslint-plugin-simple-import-sort": "^7.0.0", - "jest": "^28.1.2", - "merge-options": "^3.0.4", - "node-loader": "^2.0.0", - "rimraf": "^3.0.2", - "ts-jest": "^28.0.5", - "ts-loader": "^9.3.1", - "ts-node": "^10.8.1", - "tsconfig-paths-webpack-plugin": "^3.5.2", - "typescript": "^4.7.4", - "vsce": "^2.9.2", - "webpack": "^5.73.0", - "webpack-cli": "^4.10.0", - "webpack-node-externals": "^3.0.0" - } + "name": "vscode-plpgsql-lsp", + "displayName": "PL/pgSQL Language Server", + "description": "VSCode PL/pgSQL Language Server.", + "license": "MIT", + "version": "2.10.1", + "icon": "images/postgres.png", + "repository": { + "type": "git", + "url": "https://github.com/UniqueVision/plpgsql-lsp" + }, + "publisher": "uniquevision", + "categories": [], + "keywords": [ + "Postgres", + "PostgreSQL", + "plpgsql", + "Language Server" + ], + "engines": { + "vscode": "^1.68.0" + }, + "activationEvents": [ + "onLanguage:postgres", + "onCommand:plpgsql-lsp.executeFileQuery", + "onCommand:plpgsql-lsp.validateWorkspace" + ], + "main": "./client/out/extension", + "scripts": { + "vscode:prepublish": "npm run build", + "build": "npm run clean && webpack --mode production --config ./client/webpack.config.js && webpack --mode production --config ./server/webpack.config.js", + "build:dev": "npm run clean && webpack --mode none --config ./client/webpack.config.js && webpack --mode none --config ./server/webpack.config.js", + "tsc": "tsc --noEmit --project ./tsconfig.json && npm run tsc:client && npm run tsc:server", + "tsc:client": "tsc --noEmit --project ./client/tsconfig.json", + "tsc:server": "tsc --noEmit --project ./server/tsconfig.json", + "compile": "tsc -b", + "compile:client": "tsc -b ./client/tsconfig.json", + "compile:server": "tsc -b ./server/tsconfig.json", + "watch": "webpack --watch", + "package:linux": "vsce package --target linux-x64", + "package:mac": "vsce package --target darwin-x64", + "package:windows": "vsce package --target win32-x64", + "lint": "eslint ./client/src ./server/src --ext .ts,.tsx --fix", + "postinstall": "cd client && npm install && cd ../server && npm install && cd ..", + "test": "cd server && npm run test && cd ..", + "test:ci": "cd server && npm run test:ci && cd ..", + "clean": "rimraf client/out && rimraf server/out" + }, + "contributes": { + "commands": [ + { + "command": "plpgsql-lsp.executeFileQuery", + "title": "PL/pgSQL: Execute the Current File Query" + }, + { + "command": "plpgsql-lsp.validateWorkspace", + "title": "PL/pgSQL: Validate the Workspace Files" + } + ], + "menus": { + "commandPalette": [ + { + "command": "plpgsql-lsp.executeFileQuery", + "when": "editorLangId == postgres" + }, + { + "command": "plpgsql-lsp.validateWorkspace", + "when": "editorLangId == postgres" + } + ] + }, + "languages": [ + { + "id": "postgres", + "extensions": [ + ".pgsql", + ".psql" + ], + "aliases": [ + "Postgres" + ], + "configuration": "./language-configuration.json" + } + ], + "grammars": [ + { + "language": "postgres", + "scopeName": "source.pgsql", + "path": "./syntaxes/pgsql.tmLanguage" + } + ], + "configuration": { + "type": "object", + "title": "PostgreSQL connection configuration", + "properties": { + "plpgsqlLanguageServer.host": { + "scope": "resource", + "type": "string", + "default": "localhost", + "description": "Your database host." + }, + "plpgsqlLanguageServer.port": { + "scope": "resource", + "type": "number", + "default": 5432, + "description": "Your database port." + }, + "plpgsqlLanguageServer.database": { + "scope": "resource", + "type": "string", + "nullable": true, + "description": "Your database name." + }, + "plpgsqlLanguageServer.user": { + "scope": "resource", + "type": "string", + "nullable": true, + "description": "Your database user." + }, + "plpgsqlLanguageServer.password": { + "scope": "resource", + "type": "string", + "nullable": true, + "description": "Your database password." + }, + "plpgsqlLanguageServer.definitionFiles": { + "scope": "resource", + "type": "array", + "default": [ + "**/*.psql", + "**/*.pgsql" + ], + "description": "The pattern list of the definition files.", + "items": { + "type": "string" + } + }, + "plpgsqlLanguageServer.defaultSchema": { + "scope": "resource", + "type": "string", + "default": "public", + "description": "Default schema name." + }, + "plpgsqlLanguageServer.plpgsqlCheckSchema": { + "scope": "resource", + "type": "string", + "description": "Schema where plpgsql_check is installed." + }, + "plpgsqlLanguageServer.validateOn": { + "scope": "resource", + "type": "string", + "enum": [ + "save", + "change" + ], + "default": "change", + "description": "Specify when to validate files. Default: 'change'" + }, + "plpgsqlLanguageServer.queryParameterPattern": { + "scope": "resource", + "type": "string", + "default": "\\$[1-9][0-9]*", + "description": "Query parameter pattern. the pattern is described by regular expression." + }, + "plpgsqlLanguageServer.keywordQueryParameterPattern": { + "scope": "resource", + "type": "array", + "default": [], + "description": "Keyword query parameter patterns. Patterns must include \"{keyword}\", as in \"@{keyword}\".", + "items": { + "type": "string" + } + }, + "plpgsqlLanguageServer.statements": { + "properties": { + "separatorPattern": { + "scope": "resource", + "type": "string", + "required": true, + "example": "-- name:[\\s]+.*", + "description": "Prepared statement separator regex pattern." + }, + "diagnosticsLevels": { + "scope": "resource", + "type": "object", + "default": null, + "description": "Override diagnostic levels for statements.", + "properties": { + "disableFlag": { + "scope": "resource", + "type": "string", + "default": "disable", + "enum": [ + "disable", + "warning" + ], + "description": "Set diagnostic level for the disabled validation flag." + } + } + } + }, + "default": null, + "scope": "resource", + "type": "object", + "description": "If set, multiple statements in a file can be analyzed based on a separator." + }, + "plpgsqlLanguageServer.migrations": { + "properties": { + "upFiles": { + "scope": "resource", + "type": "array", + "items": { + "type": "string" + }, + "description": "Up migrations file glob pattern. Executed by sorted filename order.", + "required": true, + "example": [ + "*.up.pgsql" + ] + }, + "downFiles": { + "scope": "resource", + "type": "array", + "items": { + "type": "string" + }, + "description": "Down migrations file glob pattern. Executed by sorted filename order.", + "required": true, + "example": [ + "*.down.pgsql" + ] + }, + "postMigrationFiles": { + "scope": "resource", + "type": "array", + "items": { + "type": "string" + }, + "description": "Post-migration file glob pattern. Executed by sorted filename order.", + "required": true, + "default": [], + "example": [ + "*.post.pgsql" + ] + }, + "target": { + "scope": "resource", + "type": "string", + "enum": [ + "all", + "up/down" + ], + "default": "up/down", + "description": "Specify target files to be migrated in the background. Default: 'up/down'" + } + }, + "scope": "resource", + "default": null, + "type": "object", + "description": "If set, migrations will be applied for all analyses. If the current file is a migration file, execution will run until the previous migration." + }, + "plpgsqlLanguageServer.enableExecuteFileQueryCommand": { + "scope": "resource", + "type": "boolean", + "default": true, + "description": "Enable/Disable \"Execute the Current File Query\" command." + }, + "plpgsqlLanguageServer.workspaceValidationTargetFiles": { + "scope": "resource", + "type": "array", + "default": [], + "description": "The pattern list of the workspace validation target files.", + "items": { + "type": "string" + } + } + }, + "required": [ + "plpgsqlLanguageServer.host", + "plpgsqlLanguageServer.port", + "plpgsqlLanguageServer.defaultSchema" + ] + } + }, + "devDependencies": { + "@types/node": "^18.0.0", + "@typescript-eslint/eslint-plugin": "^5.30.0", + "@typescript-eslint/parser": "^5.30.0", + "eslint": "^8.18.0", + "eslint-config-prettier": "^8.5.0", + "eslint-config-standard": "^17.0.0", + "eslint-plugin-import": "^2.26.0", + "eslint-plugin-node": "^11.1.0", + "eslint-plugin-prettier": "^4.2.1", + "eslint-plugin-promise": "^6.0.0", + "eslint-plugin-simple-import-sort": "^7.0.0", + "jest": "^28.1.2", + "merge-options": "^3.0.4", + "node-loader": "^2.0.0", + "rimraf": "^3.0.2", + "ts-jest": "^28.0.5", + "ts-loader": "^9.3.1", + "ts-node": "^10.8.1", + "tsconfig-paths-webpack-plugin": "^3.5.2", + "typescript": "^4.7.4", + "vsce": "^2.9.2", + "webpack": "^5.73.0", + "webpack-cli": "^4.10.0", + "webpack-node-externals": "^3.0.0" + } } diff --git a/server/src/commands/validateWorkspace.ts b/server/src/commands/validateWorkspace.ts index d44b6ca..6d78375 100644 --- a/server/src/commands/validateWorkspace.ts +++ b/server/src/commands/validateWorkspace.ts @@ -64,6 +64,7 @@ export async function validateFile( options.hasDiagnosticRelatedInformationCapability, queryParameterInfo, statements: settings.statements, + plpgsqlCheckSchema: settings.plpgsqlCheckSchema, }, settings, logger, diff --git a/server/src/postgres/parsers/parseFunctions.ts b/server/src/postgres/parsers/parseFunctions.ts index 77d9231..09cf056 100644 --- a/server/src/postgres/parsers/parseFunctions.ts +++ b/server/src/postgres/parsers/parseFunctions.ts @@ -12,14 +12,20 @@ export interface FunctionInfo { location: number | undefined, } +export interface TriggerInfo { + functionName: string, + location: number | undefined, + relname: string, +} + export async function parseFunctions( uri: URI, queryParameterInfo: QueryParameterInfo | null, logger: Logger, -): Promise { +): Promise<[FunctionInfo[], TriggerInfo[]]> { const fileText = await readFileFromUri(uri) if (fileText === null) { - return [] + return [[], []] } const [sanitizedFileText] = sanitizeFileWithQueryParameters( @@ -28,23 +34,37 @@ export async function parseFunctions( const stmtements = await parseStmtements(uri, sanitizedFileText, logger) if (stmtements === undefined) { - return [] + return [[], []] } - - return stmtements.flatMap( + const functions: FunctionInfo[] = [] + const triggers: TriggerInfo[] = [] + stmtements.forEach( (statement) => { - if (statement?.stmt?.CreateFunctionStmt !== undefined) { + logger.info(`Statically analyzing: ${JSON.stringify(statement)}`) + + if (statement?.stmt?.CreateFunctionStmt !== undefined ) { try { - return getCreateFunctions(statement) + functions.push(...getCreateFunctions(statement)) } catch (error: unknown) { logger.error(`ParseFunctionError: ${(error as Error).message} (${uri})`) } } - return [] + if (statement?.stmt?.CreateTrigStmt !== undefined ) { + try { + triggers.push(...getCreateTriggers(statement)) + } + catch (error: unknown) { + logger.error(`ParseFunctionError: ${(error as Error).message} (${uri})`) + } + } }, ) + + logger.error(JSON.stringify(triggers)) + + return [functions, triggers] } function getCreateFunctions( @@ -83,3 +103,39 @@ function getCreateFunctions( }, ) } + + +function getCreateTriggers( + statement: Statement, +): TriggerInfo[] { + const createTriggerStmt = statement?.stmt?.CreateTrigStmt + if (createTriggerStmt === undefined) { + return [] + } + + const funcname = createTriggerStmt.funcname + if (funcname === undefined) { + throw new ParsedTypeError("createTriggerStmt.funcname is undefined!") + } + const relname = createTriggerStmt.relation?.relname + if (relname === undefined) { + throw new ParsedTypeError("createTriggerStmt.relation?.relname is undefined!") + } + + return funcname.flatMap( + (funcname) => { + const functionName = funcname.String.str + if (functionName === undefined) { + return [] + } + + return [ + { + functionName, + location: undefined, + relname, + }, + ] + }, + ) +} diff --git a/server/src/postgres/queries/queryFileStaticAnalysis.ts b/server/src/postgres/queries/queryFileStaticAnalysis.ts index 39e8fa1..da0a584 100644 --- a/server/src/postgres/queries/queryFileStaticAnalysis.ts +++ b/server/src/postgres/queries/queryFileStaticAnalysis.ts @@ -6,7 +6,7 @@ import { QueryParameterInfo, sanitizeFileWithQueryParameters, } from "@/postgres/parameters" -import { FunctionInfo } from "@/postgres/parsers/parseFunctions" +import { FunctionInfo, TriggerInfo } from "@/postgres/parsers/parseFunctions" import { getLineRangeFromBuffer, getTextAllRange } from "@/utilities/text" export interface StaticAnalysisErrorRow { @@ -32,12 +32,14 @@ export interface StaticAnalysisError { export type StaticAnalysisOptions = { isComplete: boolean, queryParameterInfo: QueryParameterInfo | null + plpgsqlCheckSchema?: string } export async function queryFileStaticAnalysis( pgPool: PostgresPool, document: TextDocument, functionInfos: FunctionInfo[], + triggerInfos: TriggerInfo[], options: StaticAnalysisOptions, logger: Logger, ): Promise { @@ -46,12 +48,27 @@ export async function queryFileStaticAnalysis( document.getText(), options.queryParameterInfo, logger, ) + const plpgsqlCheckSchema = options.plpgsqlCheckSchema + const pgClient = await pgPool.connect() + if (plpgsqlCheckSchema) { + await pgClient.query(` + SELECT + set_config( + 'search_path', + current_setting('search_path') || ',${plpgsqlCheckSchema}', + false + ) + WHERE current_setting('search_path') !~ '(^|,)${plpgsqlCheckSchema}(,|$)' + `) + } + try { await pgClient.query("BEGIN") await pgClient.query( fileText, Array(parameterNumber).fill(null), ) + const extensionCheck = await pgClient.query(` SELECT extname @@ -61,7 +78,10 @@ export async function queryFileStaticAnalysis( extname = 'plpgsql_check' `) + if (extensionCheck.rowCount === 0) { + logger.warn("plpgsql_check is not installed in the database.") + return [] } @@ -91,26 +111,53 @@ export async function queryFileStaticAnalysis( continue } - rows.forEach( - (row) => { - const range = (() => { - return (location === undefined) - ? getTextAllRange(document) - : getLineRangeFromBuffer( - fileText, - location, - row.lineno ? row.lineno - 1 : 0, - ) ?? getTextAllRange(document) - })() - - errors.push({ - level: row.level, range, message: row.message, - }) - }, + extractError(rows, location) + } + } + catch (error: unknown) { + await pgClient.query("ROLLBACK") + await pgClient.query("BEGIN") + if (options.isComplete) { + const message = (error as Error).message + logger.error(`StaticAnalysisError: ${message} (${document.uri})`) + } + } + + try { + for (const { functionName, location, relname } of triggerInfos) { + logger.warn(`trigger::: relname: ${relname} -- functionName: ${functionName}`) + + const result = await pgClient.query( + ` + SELECT + (pcf).functionid::regprocedure AS procedure, + (pcf).lineno AS lineno, + (pcf).statement AS statement, + (pcf).sqlstate AS sqlstate, + (pcf).message AS message, + (pcf).detail AS detail, + (pcf).hint AS hint, + (pcf).level AS level, + (pcf)."position" AS position, + (pcf).query AS query, + (pcf).context AS context + FROM + plpgsql_check_function_tb($1, $2) AS pcf + `, + [functionName, relname], ) + + const rows: StaticAnalysisErrorRow[] = result.rows + if (rows.length === 0) { + continue + } + + extractError(rows, location) } } catch (error: unknown) { + await pgClient.query("ROLLBACK") + await pgClient.query("BEGIN") if (options.isComplete) { const message = (error as Error).message logger.error(`StaticAnalysisError: ${message} (${document.uri})`) @@ -122,4 +169,24 @@ export async function queryFileStaticAnalysis( } return errors + + function extractError(rows: StaticAnalysisErrorRow[], location: number | undefined) { + rows.forEach( + (row) => { + const range = (() => { + return (location === undefined) + ? getTextAllRange(document) + : getLineRangeFromBuffer( + fileText, + location, + row.lineno ? row.lineno - 1 : 0, + ) ?? getTextAllRange(document) + })() + + errors.push({ + level: row.level, range, message: row.message, + }) + }, + ) + } } diff --git a/server/src/services/validation.ts b/server/src/services/validation.ts index 2fd40c0..dea2cff 100644 --- a/server/src/services/validation.ts +++ b/server/src/services/validation.ts @@ -13,6 +13,7 @@ type ValidateTextDocumentOptions = { hasDiagnosticRelatedInformationCapability: boolean, queryParameterInfo: QueryParameterInfo | null, statements?: StatementsSettings, + plpgsqlCheckSchema?: string, } export async function validateTextDocument( @@ -91,13 +92,20 @@ async function validateStaticAnalysis( options: ValidateTextDocumentOptions, logger: Logger, ): Promise { + const [functions, triggers] = await parseFunctions( + document.uri, + options.queryParameterInfo, + logger, + ) const errors = await queryFileStaticAnalysis( pgPool, document, - await parseFunctions(document.uri, options.queryParameterInfo, logger), + functions, + triggers, { isComplete: options.isComplete, queryParameterInfo: options.queryParameterInfo, + plpgsqlCheckSchema: options.plpgsqlCheckSchema, }, logger, ) diff --git a/server/src/settings.ts b/server/src/settings.ts index b6f798e..6e0917d 100644 --- a/server/src/settings.ts +++ b/server/src/settings.ts @@ -6,6 +6,7 @@ export interface Settings { password?: string; definitionFiles: string[]; defaultSchema: string; + plpgsqlCheckSchema?: string; queryParameterPattern: string | string[]; keywordQueryParameterPattern?: string | string[]; enableExecuteFileQueryCommand: boolean; @@ -41,6 +42,7 @@ export const DEFAULT_SETTINGS: Settings = { password: undefined, definitionFiles: ["**/*.psql", "**/*.pgsql"], defaultSchema: "public", + plpgsqlCheckSchema: undefined, queryParameterPattern: /\$[1-9][0-9]*/.source, keywordQueryParameterPattern: undefined, enableExecuteFileQueryCommand: true, From 325227b2f68abc1afd5cd1bc928ddfec3fca6dcc Mon Sep 17 00:00:00 2001 From: Daniel Castro Date: Wed, 30 Nov 2022 19:58:06 +0000 Subject: [PATCH 02/10] trying to run migrations for static analysis - should share with syntax --- .../postgres/parsers/parseCreateStatements.ts | 4 +- server/src/postgres/parsers/parseFunctions.ts | 2 +- server/src/postgres/queries/migrations.ts | 112 +++++++++++++++++ .../queries/queryFileStaticAnalysis.ts | 34 ++++- .../queries/queryFileSyntaxAnalysis.ts | 116 +----------------- server/src/services/validation.ts | 2 + server/src/utilities/regex.ts | 9 ++ 7 files changed, 162 insertions(+), 117 deletions(-) create mode 100644 server/src/postgres/queries/migrations.ts diff --git a/server/src/postgres/parsers/parseCreateStatements.ts b/server/src/postgres/parsers/parseCreateStatements.ts index 6b85ccd..a198844 100644 --- a/server/src/postgres/parsers/parseCreateStatements.ts +++ b/server/src/postgres/parsers/parseCreateStatements.ts @@ -298,10 +298,10 @@ export function parseIndexCreateStatements( return [] } - const idxname = IndexStmt?.idxname + let idxname = IndexStmt?.idxname if (idxname === undefined) { - throw new ParsedTypeError("IndexStmt.idxname is undefined!") + idxname = "" } const indexNameLocation = findIndexFromBuffer( diff --git a/server/src/postgres/parsers/parseFunctions.ts b/server/src/postgres/parsers/parseFunctions.ts index 09cf056..c4c20c3 100644 --- a/server/src/postgres/parsers/parseFunctions.ts +++ b/server/src/postgres/parsers/parseFunctions.ts @@ -40,7 +40,7 @@ export async function parseFunctions( const triggers: TriggerInfo[] = [] stmtements.forEach( (statement) => { - logger.info(`Statically analyzing: ${JSON.stringify(statement)}`) + // logger.info(`Statically analyzing: ${JSON.stringify(statement)}`) if (statement?.stmt?.CreateFunctionStmt !== undefined ) { try { diff --git a/server/src/postgres/queries/migrations.ts b/server/src/postgres/queries/migrations.ts new file mode 100644 index 0000000..a463d76 --- /dev/null +++ b/server/src/postgres/queries/migrations.ts @@ -0,0 +1,112 @@ +import fs from "fs/promises" +import glob from "glob-promise" +import path from "path" +import { DatabaseError } from "pg" +import { + Logger, +} from "vscode-languageserver" +import { TextDocument } from "vscode-languageserver-textdocument" + +import { MigrationError } from "@/errors" +import { PostgresClient } from "@/postgres" +import { MigrationsSettings } from "@/settings" +import { asyncFlatMap } from "@/utilities/functool" +import { BEGIN_RE, COMMIT_RE, ROLLBACK_RE } from "@/utilities/regex" + + +export async function runMigration( + pgClient: PostgresClient, + document: TextDocument, + migrations: MigrationsSettings, + logger: Logger, +): Promise { + const upMigrationFiles = ( + await asyncFlatMap( + migrations.upFiles, + (filePattern) => glob.promise(filePattern), + )) + .sort((a, b) => a.localeCompare(b, undefined, { numeric: true })) + + const downMigrationFiles = ( + await asyncFlatMap( + migrations.downFiles, + (filePattern) => glob.promise(filePattern), + )) + .sort((a, b) => b.localeCompare(a, undefined, { numeric: true })) + + const postMigrationFiles = ( + await asyncFlatMap( + migrations.postMigrationFiles ?? [], + (filePattern) => glob.promise(filePattern), + )) + .sort((a, b) => a.localeCompare(b, undefined, { numeric: true })) + + const migrationTarget = migrations?.target ?? "up/down" + + if (migrationTarget === "up/down" + && ( + // Check if it is not a migration file. + upMigrationFiles.filter(file => document.uri.endsWith(file)).length + + downMigrationFiles.filter(file => document.uri.endsWith(file)).length === 0 + ) + ) { + return + } + + let shouldContinue = true + + if (shouldContinue) { + shouldContinue = await queryMigrations( + pgClient, document, downMigrationFiles, logger, + ) + } + + if (shouldContinue) { + shouldContinue = await queryMigrations( + pgClient, document, upMigrationFiles, logger, + ) + } + + if (shouldContinue) { + shouldContinue = await queryMigrations( + pgClient, document, postMigrationFiles, logger, + ) + } +} + +async function queryMigrations( + pgClient: PostgresClient, + document: TextDocument, + files: string[], + logger: Logger, +): Promise { + for await (const file of files) { + try { + if (document.uri.endsWith(file)) { + // allow us to revisit and work on any migration/post-migration file + logger.info("Stopping migration execution at the current file") + + return false + } + + logger.info(`Migration ${file}`) + + const migration = (await fs.readFile(file, { encoding: "utf8" })) + .replace(BEGIN_RE, (m) => "-".repeat(m.length)) + .replace(COMMIT_RE, (m) => "-".repeat(m.length)) + .replace(ROLLBACK_RE, (m) => "-".repeat(m.length)) + + await pgClient.query(migration) + } catch (error: unknown) { + const errorMessage = (error as DatabaseError).message + + logger.error( + `Stopping migration execution at ${path.basename(file)}: ${errorMessage}`, + ) + + throw new MigrationError(document, errorMessage) + } + } + + return true +} diff --git a/server/src/postgres/queries/queryFileStaticAnalysis.ts b/server/src/postgres/queries/queryFileStaticAnalysis.ts index da0a584..09e7f96 100644 --- a/server/src/postgres/queries/queryFileStaticAnalysis.ts +++ b/server/src/postgres/queries/queryFileStaticAnalysis.ts @@ -1,12 +1,15 @@ import { Logger, Range, uinteger } from "vscode-languageserver" import { TextDocument } from "vscode-languageserver-textdocument" +import { MigrationError } from "@/errors" import { PostgresPool } from "@/postgres" import { QueryParameterInfo, sanitizeFileWithQueryParameters, } from "@/postgres/parameters" import { FunctionInfo, TriggerInfo } from "@/postgres/parsers/parseFunctions" +import { runMigration } from "@/postgres/queries/migrations" +import { Settings } from "@/settings" import { getLineRangeFromBuffer, getTextAllRange } from "@/utilities/text" export interface StaticAnalysisErrorRow { @@ -33,6 +36,7 @@ export type StaticAnalysisOptions = { isComplete: boolean, queryParameterInfo: QueryParameterInfo | null plpgsqlCheckSchema?: string + migrations?: Settings["migrations"] } export async function queryFileStaticAnalysis( @@ -48,9 +52,10 @@ export async function queryFileStaticAnalysis( document.getText(), options.queryParameterInfo, logger, ) - const plpgsqlCheckSchema = options.plpgsqlCheckSchema - const pgClient = await pgPool.connect() + + const plpgsqlCheckSchema = options.plpgsqlCheckSchema + // outside transaction if (plpgsqlCheckSchema) { await pgClient.query(` SELECT @@ -65,6 +70,27 @@ export async function queryFileStaticAnalysis( try { await pgClient.query("BEGIN") + + if (options.migrations) { + await runMigration(pgClient, document, options.migrations, logger) + } + } catch (error: unknown) { + if (error instanceof MigrationError) { + errors.push({ + level: "", + range: getTextAllRange(document), + message: error.message, + }) + } + + // Restart transaction. + await pgClient.query("ROLLBACK") + await pgClient.query("BEGIN") + } finally { + await pgClient.query("SAVEPOINT migrations") + } + + try { await pgClient.query( fileText, Array(parameterNumber).fill(null), ) @@ -115,7 +141,7 @@ export async function queryFileStaticAnalysis( } } catch (error: unknown) { - await pgClient.query("ROLLBACK") + await pgClient.query("ROLLBACK to migrations") await pgClient.query("BEGIN") if (options.isComplete) { const message = (error as Error).message @@ -156,7 +182,7 @@ export async function queryFileStaticAnalysis( } } catch (error: unknown) { - await pgClient.query("ROLLBACK") + await pgClient.query("ROLLBACK to migrations") await pgClient.query("BEGIN") if (options.isComplete) { const message = (error as Error).message diff --git a/server/src/postgres/queries/queryFileSyntaxAnalysis.ts b/server/src/postgres/queries/queryFileSyntaxAnalysis.ts index e904c95..1ddeda0 100644 --- a/server/src/postgres/queries/queryFileSyntaxAnalysis.ts +++ b/server/src/postgres/queries/queryFileSyntaxAnalysis.ts @@ -1,6 +1,3 @@ -import fs from "fs/promises" -import glob from "glob-promise" -import path from "path" import { DatabaseError } from "pg" import { Diagnostic, DiagnosticSeverity, Logger, uinteger, @@ -8,23 +5,19 @@ import { import { TextDocument } from "vscode-languageserver-textdocument" import { MigrationError } from "@/errors" -import { PostgresClient, PostgresPool } from "@/postgres" +import { PostgresPool } from "@/postgres" import { getQueryParameterInfo, QueryParameterInfo, sanitizeFileWithQueryParameters, } from "@/postgres/parameters" -import { MigrationsSettings, Settings, StatementsSettings } from "@/settings" -import { asyncFlatMap } from "@/utilities/functool" +import { runMigration } from "@/postgres/queries/migrations" +import { Settings, StatementsSettings } from "@/settings" import { neverReach } from "@/utilities/neverReach" +import { + BEGIN_RE, COMMIT_RE, DISABLE_STATEMENT_VALIDATION_RE, ROLLBACK_RE, SQL_COMMENT_RE, +} from "@/utilities/regex" import { getCurrentLineFromIndex, getTextAllRange } from "@/utilities/text" -const SQL_COMMENT_RE = /\/\*[\s\S]*?\*\/|([^:]|^)--.*$/gm -const BEGIN_RE = /^([\s]*begin[\s]*;)/igm -const COMMIT_RE = /^([\s]*commit[\s]*;)/igm -const ROLLBACK_RE = /^([\s]*rollback[\s]*;)/igm - -const DISABLE_STATEMENT_VALIDATION_RE = /^ *-- +plpgsql-language-server:disable *$/m - export type SyntaxAnalysisOptions = { isComplete: boolean queryParameterInfo: QueryParameterInfo | null @@ -141,103 +134,6 @@ function statementError( } } -async function runMigration( - pgClient: PostgresClient, - document: TextDocument, - migrations: MigrationsSettings, - logger: Logger, -): Promise { - const upMigrationFiles = ( - await asyncFlatMap( - migrations.upFiles, - (filePattern) => glob.promise(filePattern), - )) - .sort((a, b) => a.localeCompare(b, undefined, { numeric: true })) - - const downMigrationFiles = ( - await asyncFlatMap( - migrations.downFiles, - (filePattern) => glob.promise(filePattern), - )) - .sort((a, b) => b.localeCompare(a, undefined, { numeric: true })) - - const postMigrationFiles = ( - await asyncFlatMap( - migrations.postMigrationFiles ?? [], - (filePattern) => glob.promise(filePattern), - )) - .sort((a, b) => a.localeCompare(b, undefined, { numeric: true })) - - const migrationTarget = migrations?.target ?? "up/down" - - if (migrationTarget === "up/down" - && ( - // Check if it is not a migration file. - upMigrationFiles.filter(file => document.uri.endsWith(file)).length - + downMigrationFiles.filter(file => document.uri.endsWith(file)).length === 0 - ) - ) { - return - } - - let shouldContinue = true - - if (shouldContinue) { - shouldContinue = await queryMigrations( - pgClient, document, downMigrationFiles, logger, - ) - } - - if (shouldContinue) { - shouldContinue = await queryMigrations( - pgClient, document, upMigrationFiles, logger, - ) - } - - if (shouldContinue) { - shouldContinue = await queryMigrations( - pgClient, document, postMigrationFiles, logger, - ) - } -} - -async function queryMigrations( - pgClient: PostgresClient, - document: TextDocument, - files: string[], - logger: Logger, -): Promise { - for await (const file of files) { - try { - if (document.uri.endsWith(file)) { - // allow us to revisit and work on any migration/post-migration file - logger.info("Stopping migration execution at the current file") - - return false - } - - logger.info(`Migration ${file}`) - - const migration = (await fs.readFile(file, { encoding: "utf8" })) - .replace(BEGIN_RE, (m) => "-".repeat(m.length)) - .replace(COMMIT_RE, (m) => "-".repeat(m.length)) - .replace(ROLLBACK_RE, (m) => "-".repeat(m.length)) - - await pgClient.query(migration) - } catch (error: unknown) { - const errorMessage = (error as DatabaseError).message - - logger.error( - `Stopping migration execution at ${path.basename(file)}: ${errorMessage}`, - ) - - throw new MigrationError(document, errorMessage) - } - } - - return true -} - function queryStatement( document: TextDocument, statement: string, diff --git a/server/src/services/validation.ts b/server/src/services/validation.ts index dea2cff..7f6dfac 100644 --- a/server/src/services/validation.ts +++ b/server/src/services/validation.ts @@ -14,6 +14,7 @@ type ValidateTextDocumentOptions = { queryParameterInfo: QueryParameterInfo | null, statements?: StatementsSettings, plpgsqlCheckSchema?: string, + migrations?: Settings["migrations"] } export async function validateTextDocument( @@ -106,6 +107,7 @@ async function validateStaticAnalysis( isComplete: options.isComplete, queryParameterInfo: options.queryParameterInfo, plpgsqlCheckSchema: options.plpgsqlCheckSchema, + migrations: options.migrations, }, logger, ) diff --git a/server/src/utilities/regex.ts b/server/src/utilities/regex.ts index 8630ae9..96a90e2 100644 --- a/server/src/utilities/regex.ts +++ b/server/src/utilities/regex.ts @@ -1,3 +1,12 @@ export function escapeRegex(string: string): string { return string.replace(/[-/\\^$*+?.()|[\]{}]/g, "\\$&") } + + +export const SQL_COMMENT_RE = /\/\*[\s\S]*?\*\/|([^:]|^)--.*$/gm +export const BEGIN_RE = /^([\s]*begin[\s]*;)/igm +export const COMMIT_RE = /^([\s]*commit[\s]*;)/igm +export const ROLLBACK_RE = /^([\s]*rollback[\s]*;)/igm + +// eslint-disable-next-line max-len +export const DISABLE_STATEMENT_VALIDATION_RE = /^ *-- +plpgsql-language-server:disable *$/m From 566ea3d015fc47861bb4f50c18b2bf1ebfddfe17 Mon Sep 17 00:00:00 2001 From: Daniel Castro Date: Wed, 30 Nov 2022 20:56:16 +0000 Subject: [PATCH 03/10] TODO - static analysis should run all parsed statements individually --- .../src/postgres/queries/queryFileStaticAnalysis.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/server/src/postgres/queries/queryFileStaticAnalysis.ts b/server/src/postgres/queries/queryFileStaticAnalysis.ts index 09e7f96..09f5d19 100644 --- a/server/src/postgres/queries/queryFileStaticAnalysis.ts +++ b/server/src/postgres/queries/queryFileStaticAnalysis.ts @@ -48,7 +48,7 @@ export async function queryFileStaticAnalysis( logger: Logger, ): Promise { const errors: StaticAnalysisError[] = [] - const [fileText, parameterNumber] = sanitizeFileWithQueryParameters( + const [fileText] = sanitizeFileWithQueryParameters( document.getText(), options.queryParameterInfo, logger, ) @@ -91,9 +91,9 @@ export async function queryFileStaticAnalysis( } try { - await pgClient.query( - fileText, Array(parameterNumber).fill(null), - ) + // await pgClient.query( + // fileText, Array(parameterNumber).fill(null), + // ) const extensionCheck = await pgClient.query(` SELECT @@ -134,6 +134,8 @@ export async function queryFileStaticAnalysis( const rows: StaticAnalysisErrorRow[] = result.rows if (rows.length === 0) { + await pgClient.query("SAVEPOINT migrations") + continue } @@ -175,6 +177,8 @@ export async function queryFileStaticAnalysis( const rows: StaticAnalysisErrorRow[] = result.rows if (rows.length === 0) { + await pgClient.query("SAVEPOINT migrations") + continue } From 06f5221df62e0eb193181b22041f3d5e7780bd9a Mon Sep 17 00:00:00 2001 From: Daniel Castro Date: Wed, 30 Nov 2022 22:57:37 +0000 Subject: [PATCH 04/10] migrations run once and correct trigger analysis --- server/src/errors.ts | 5 +- .../queries/queryFileStaticAnalysis.ts | 66 ++----------- .../queries/queryFileSyntaxAnalysis.ts | 36 +------ .../queries => services}/migrations.ts | 22 ++--- server/src/services/validation.ts | 96 ++++++++++++++++--- 5 files changed, 111 insertions(+), 114 deletions(-) rename server/src/{postgres/queries => services}/migrations.ts (84%) diff --git a/server/src/errors.ts b/server/src/errors.ts index dfce272..662bf26 100644 --- a/server/src/errors.ts +++ b/server/src/errors.ts @@ -71,7 +71,10 @@ export class WorkspaceValidationTargetFilesEmptyError export class MigrationError extends PlpgsqlLanguageServerError { - constructor(public document: TextDocument, message: string) { + public migrationPath: string + + constructor(public document: TextDocument, message: string, migrationPath: string) { super(message) + this.migrationPath = migrationPath } } diff --git a/server/src/postgres/queries/queryFileStaticAnalysis.ts b/server/src/postgres/queries/queryFileStaticAnalysis.ts index 09f5d19..7369e03 100644 --- a/server/src/postgres/queries/queryFileStaticAnalysis.ts +++ b/server/src/postgres/queries/queryFileStaticAnalysis.ts @@ -1,14 +1,12 @@ import { Logger, Range, uinteger } from "vscode-languageserver" import { TextDocument } from "vscode-languageserver-textdocument" -import { MigrationError } from "@/errors" -import { PostgresPool } from "@/postgres" +import { PostgresClient } from "@/postgres" import { QueryParameterInfo, sanitizeFileWithQueryParameters, } from "@/postgres/parameters" import { FunctionInfo, TriggerInfo } from "@/postgres/parsers/parseFunctions" -import { runMigration } from "@/postgres/queries/migrations" import { Settings } from "@/settings" import { getLineRangeFromBuffer, getTextAllRange } from "@/utilities/text" @@ -40,7 +38,7 @@ export type StaticAnalysisOptions = { } export async function queryFileStaticAnalysis( - pgPool: PostgresPool, + pgClient: PostgresClient, document: TextDocument, functionInfos: FunctionInfo[], triggerInfos: TriggerInfo[], @@ -52,49 +50,7 @@ export async function queryFileStaticAnalysis( document.getText(), options.queryParameterInfo, logger, ) - const pgClient = await pgPool.connect() - - const plpgsqlCheckSchema = options.plpgsqlCheckSchema - // outside transaction - if (plpgsqlCheckSchema) { - await pgClient.query(` - SELECT - set_config( - 'search_path', - current_setting('search_path') || ',${plpgsqlCheckSchema}', - false - ) - WHERE current_setting('search_path') !~ '(^|,)${plpgsqlCheckSchema}(,|$)' - `) - } - try { - await pgClient.query("BEGIN") - - if (options.migrations) { - await runMigration(pgClient, document, options.migrations, logger) - } - } catch (error: unknown) { - if (error instanceof MigrationError) { - errors.push({ - level: "", - range: getTextAllRange(document), - message: error.message, - }) - } - - // Restart transaction. - await pgClient.query("ROLLBACK") - await pgClient.query("BEGIN") - } finally { - await pgClient.query("SAVEPOINT migrations") - } - - try { - // await pgClient.query( - // fileText, Array(parameterNumber).fill(null), - // ) - const extensionCheck = await pgClient.query(` SELECT extname @@ -134,8 +90,6 @@ export async function queryFileStaticAnalysis( const rows: StaticAnalysisErrorRow[] = result.rows if (rows.length === 0) { - await pgClient.query("SAVEPOINT migrations") - continue } @@ -143,7 +97,7 @@ export async function queryFileStaticAnalysis( } } catch (error: unknown) { - await pgClient.query("ROLLBACK to migrations") + await pgClient.query("ROLLBACK to validated_syntax") await pgClient.query("BEGIN") if (options.isComplete) { const message = (error as Error).message @@ -153,7 +107,11 @@ export async function queryFileStaticAnalysis( try { for (const { functionName, location, relname } of triggerInfos) { - logger.warn(`trigger::: relname: ${relname} -- functionName: ${functionName}`) + logger.warn(` + trigger::: + relname: ${relname} + functionName: ${functionName} + location: ${location}`) const result = await pgClient.query( ` @@ -177,8 +135,6 @@ export async function queryFileStaticAnalysis( const rows: StaticAnalysisErrorRow[] = result.rows if (rows.length === 0) { - await pgClient.query("SAVEPOINT migrations") - continue } @@ -186,17 +142,13 @@ export async function queryFileStaticAnalysis( } } catch (error: unknown) { - await pgClient.query("ROLLBACK to migrations") + await pgClient.query("ROLLBACK to validated_syntax") await pgClient.query("BEGIN") if (options.isComplete) { const message = (error as Error).message logger.error(`StaticAnalysisError: ${message} (${document.uri})`) } } - finally { - await pgClient.query("ROLLBACK") - pgClient.release() - } return errors diff --git a/server/src/postgres/queries/queryFileSyntaxAnalysis.ts b/server/src/postgres/queries/queryFileSyntaxAnalysis.ts index 1ddeda0..8efa802 100644 --- a/server/src/postgres/queries/queryFileSyntaxAnalysis.ts +++ b/server/src/postgres/queries/queryFileSyntaxAnalysis.ts @@ -4,13 +4,11 @@ import { } from "vscode-languageserver" import { TextDocument } from "vscode-languageserver-textdocument" -import { MigrationError } from "@/errors" -import { PostgresPool } from "@/postgres" +import { PostgresClient } from "@/postgres" import { getQueryParameterInfo, QueryParameterInfo, sanitizeFileWithQueryParameters, } from "@/postgres/parameters" -import { runMigration } from "@/postgres/queries/migrations" import { Settings, StatementsSettings } from "@/settings" import { neverReach } from "@/utilities/neverReach" import { @@ -25,7 +23,7 @@ export type SyntaxAnalysisOptions = { }; export async function queryFileSyntaxAnalysis( - pgPool: PostgresPool, + pgClient: PostgresClient, document: TextDocument, options: SyntaxAnalysisOptions, settings: Settings, @@ -40,29 +38,6 @@ export async function queryFileSyntaxAnalysis( statementSepRE = new RegExp(`(${options.statements.separatorPattern})`, "g") preparedStatements = documentText.split(statementSepRE) } - const pgClient = await pgPool.connect() - - try { - await pgClient.query("BEGIN") - - if (settings.migrations) { - await runMigration(pgClient, document, settings.migrations, logger) - } - } catch (error: unknown) { - if (error instanceof MigrationError) { - diagnostics.push({ - severity: DiagnosticSeverity.Error, - range: getTextAllRange(document), - message: error.message, - }) - } - - // Restart transaction. - await pgClient.query("ROLLBACK") - await pgClient.query("BEGIN") - } finally { - await pgClient.query("SAVEPOINT migrations") - } const statementNames: string[] = [] for (let i = 0; i < preparedStatements.length; i++) { @@ -93,12 +68,11 @@ export async function queryFileSyntaxAnalysis( currentTextIndex, logger, )) - } finally { - await pgClient.query("ROLLBACK TO migrations") + if (preparedStatements.length > 0) { + await pgClient.query("ROLLBACK TO migrations") + } } } - await pgClient.query("ROLLBACK") - pgClient.release() return diagnostics } diff --git a/server/src/postgres/queries/migrations.ts b/server/src/services/migrations.ts similarity index 84% rename from server/src/postgres/queries/migrations.ts rename to server/src/services/migrations.ts index a463d76..8c90ee4 100644 --- a/server/src/postgres/queries/migrations.ts +++ b/server/src/services/migrations.ts @@ -1,6 +1,5 @@ import fs from "fs/promises" import glob from "glob-promise" -import path from "path" import { DatabaseError } from "pg" import { Logger, @@ -19,7 +18,7 @@ export async function runMigration( document: TextDocument, migrations: MigrationsSettings, logger: Logger, -): Promise { +): Promise { const upMigrationFiles = ( await asyncFlatMap( migrations.upFiles, @@ -42,15 +41,12 @@ export async function runMigration( .sort((a, b) => a.localeCompare(b, undefined, { numeric: true })) const migrationTarget = migrations?.target ?? "up/down" + const currentFileIsMigration = + upMigrationFiles.filter(file => document.uri.endsWith(file)).length + + downMigrationFiles.filter(file => document.uri.endsWith(file)).length !== 0 - if (migrationTarget === "up/down" - && ( - // Check if it is not a migration file. - upMigrationFiles.filter(file => document.uri.endsWith(file)).length - + downMigrationFiles.filter(file => document.uri.endsWith(file)).length === 0 - ) - ) { - return + if (migrationTarget === "up/down" && !currentFileIsMigration) { + return currentFileIsMigration } let shouldContinue = true @@ -72,6 +68,8 @@ export async function runMigration( pgClient, document, postMigrationFiles, logger, ) } + + return currentFileIsMigration } async function queryMigrations( @@ -101,10 +99,10 @@ async function queryMigrations( const errorMessage = (error as DatabaseError).message logger.error( - `Stopping migration execution at ${path.basename(file)}: ${errorMessage}`, + `Stopping migration execution at ${file}: ${errorMessage}`, ) - throw new MigrationError(document, errorMessage) + throw new MigrationError(document, errorMessage, file) } } diff --git a/server/src/services/validation.ts b/server/src/services/validation.ts index 7f6dfac..cf4865f 100644 --- a/server/src/services/validation.ts +++ b/server/src/services/validation.ts @@ -1,12 +1,18 @@ +import path from "path" import { Diagnostic, DiagnosticSeverity, Logger } from "vscode-languageserver" import { TextDocument } from "vscode-languageserver-textdocument" +import { URI } from "vscode-uri" +import { uriToFsPath } from "vscode-uri/lib/umd/uri" -import { PostgresPool } from "@/postgres" +import { MigrationError } from "@/errors" +import { PostgresClient, PostgresPool } from "@/postgres" import { QueryParameterInfo } from "@/postgres/parameters" import { parseFunctions } from "@/postgres/parsers/parseFunctions" import { queryFileStaticAnalysis } from "@/postgres/queries/queryFileStaticAnalysis" import { queryFileSyntaxAnalysis } from "@/postgres/queries/queryFileSyntaxAnalysis" +import { runMigration } from "@/services/migrations" import { Settings, StatementsSettings } from "@/settings" +import { getTextAllRange } from "@/utilities/text" type ValidateTextDocumentOptions = { isComplete: boolean, @@ -24,29 +30,93 @@ export async function validateTextDocument( settings: Settings, logger: Logger, ): Promise { - let diagnostics: Diagnostic[] = [] - diagnostics = await validateSyntaxAnalysis( - pgPool, + const diagnostics: Diagnostic[] = [] + + const pgClient = await pgPool.connect() + + await setupEnvironment(pgClient, options) + + try { + await pgClient.query("BEGIN") + + if (settings.migrations) { + await runMigration( + pgClient, + document, + settings.migrations, + logger, + ) + } + } catch (error: unknown) { + if (error instanceof MigrationError) { + diagnostics.push({ + severity: DiagnosticSeverity.Error, + range: getTextAllRange(document), + message: `${error.migrationPath}: ${error.message}`, + relatedInformation: [ + { + location: { + uri: path.resolve(error.migrationPath), + range: getTextAllRange(document), + }, + message: error.message, + }, + ], + }) + } + + // Restart transaction. + await pgClient.query("ROLLBACK") + await pgClient.query("BEGIN") + } finally { + await pgClient.query("SAVEPOINT migrations") + } + + const syntaxDiagnostics = await validateSyntaxAnalysis( + pgClient, document, options, settings, logger, ) + diagnostics.push(...syntaxDiagnostics) - // TODO static analysis for statements - // if (diagnostics.filter(d => d.severity === DiagnosticSeverity.Error).length === 0) { - if (diagnostics.length === 0) { - diagnostics = await validateStaticAnalysis( - pgPool, + if (diagnostics.filter(d => d.severity === DiagnosticSeverity.Error).length === 0) { + await pgClient.query("SAVEPOINT validated_syntax") + const staticDiagnostics = await validateStaticAnalysis( + pgClient, document, options, logger, ) + diagnostics.push(...staticDiagnostics) } + await pgClient.query("ROLLBACK") + pgClient.release() + return diagnostics } +async function setupEnvironment( + pgClient: PostgresClient, + options: ValidateTextDocumentOptions, +) { + const plpgsqlCheckSchema = options.plpgsqlCheckSchema + // outside transaction + if (plpgsqlCheckSchema) { + await pgClient.query(` + SELECT + set_config( + 'search_path', + current_setting('search_path') || ',${plpgsqlCheckSchema}', + false + ) + WHERE current_setting('search_path') !~ '(^|,)${plpgsqlCheckSchema}(,|$)' + `) + } +} + export async function isCorrectFileValidation( pgPool: PostgresPool, document: TextDocument, @@ -72,14 +142,14 @@ export async function isCorrectFileValidation( } async function validateSyntaxAnalysis( - pgPool: PostgresPool, + pgClient: PostgresClient, document: TextDocument, options: ValidateTextDocumentOptions, settings: Settings, logger: Logger, ): Promise { return await queryFileSyntaxAnalysis( - pgPool, + pgClient, document, options, settings, @@ -88,7 +158,7 @@ async function validateSyntaxAnalysis( } async function validateStaticAnalysis( - pgPool: PostgresPool, + pgClient: PostgresClient, document: TextDocument, options: ValidateTextDocumentOptions, logger: Logger, @@ -99,7 +169,7 @@ async function validateStaticAnalysis( logger, ) const errors = await queryFileStaticAnalysis( - pgPool, + pgClient, document, functions, triggers, From f709ce5d8ded74e0c7d0e36c0a21ac93db2a2607 Mon Sep 17 00:00:00 2001 From: Daniel Castro Date: Thu, 1 Dec 2022 08:27:20 +0000 Subject: [PATCH 05/10] take trigger relation schema into account --- server/src/postgres/parsers/parseFunctions.ts | 11 ++++++++--- server/src/postgres/parsers/statement.ts | 1 + server/src/services/migrations.ts | 4 ++++ server/src/services/validation.ts | 2 -- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/server/src/postgres/parsers/parseFunctions.ts b/server/src/postgres/parsers/parseFunctions.ts index c4c20c3..35542d6 100644 --- a/server/src/postgres/parsers/parseFunctions.ts +++ b/server/src/postgres/parsers/parseFunctions.ts @@ -40,7 +40,6 @@ export async function parseFunctions( const triggers: TriggerInfo[] = [] stmtements.forEach( (statement) => { - // logger.info(`Statically analyzing: ${JSON.stringify(statement)}`) if (statement?.stmt?.CreateFunctionStmt !== undefined ) { try { @@ -52,6 +51,7 @@ export async function parseFunctions( } if (statement?.stmt?.CreateTrigStmt !== undefined ) { + logger.info(`Statically analyzing trigger: ${JSON.stringify(statement)}`) try { triggers.push(...getCreateTriggers(statement)) } @@ -117,11 +117,16 @@ function getCreateTriggers( if (funcname === undefined) { throw new ParsedTypeError("createTriggerStmt.funcname is undefined!") } - const relname = createTriggerStmt.relation?.relname - if (relname === undefined) { + let relname = createTriggerStmt.relation?.relname || "" + if (relname === "") { throw new ParsedTypeError("createTriggerStmt.relation?.relname is undefined!") } + const schema = createTriggerStmt.relation?.schemaname + if (schema) { + relname = `${schema}.${relname}` + } + return funcname.flatMap( (funcname) => { const functionName = funcname.String.str diff --git a/server/src/postgres/parsers/statement.ts b/server/src/postgres/parsers/statement.ts index 340b3ef..b0117bd 100644 --- a/server/src/postgres/parsers/statement.ts +++ b/server/src/postgres/parsers/statement.ts @@ -112,6 +112,7 @@ export interface CreateTrigStmt { } export interface CreateTrigStmtRelation { + schemaname?: string relname: string inh: boolean relpersistence: string diff --git a/server/src/services/migrations.ts b/server/src/services/migrations.ts index 8c90ee4..db5dc7a 100644 --- a/server/src/services/migrations.ts +++ b/server/src/services/migrations.ts @@ -1,5 +1,6 @@ import fs from "fs/promises" import glob from "glob-promise" +import path from "path" import { DatabaseError } from "pg" import { Logger, @@ -25,6 +26,7 @@ export async function runMigration( (filePattern) => glob.promise(filePattern), )) .sort((a, b) => a.localeCompare(b, undefined, { numeric: true })) + .map(file => path.normalize(file)) const downMigrationFiles = ( await asyncFlatMap( @@ -32,6 +34,7 @@ export async function runMigration( (filePattern) => glob.promise(filePattern), )) .sort((a, b) => b.localeCompare(a, undefined, { numeric: true })) + .map(file => path.normalize(file)) const postMigrationFiles = ( await asyncFlatMap( @@ -39,6 +42,7 @@ export async function runMigration( (filePattern) => glob.promise(filePattern), )) .sort((a, b) => a.localeCompare(b, undefined, { numeric: true })) + .map(file => path.normalize(file)) const migrationTarget = migrations?.target ?? "up/down" const currentFileIsMigration = diff --git a/server/src/services/validation.ts b/server/src/services/validation.ts index cf4865f..ed25adf 100644 --- a/server/src/services/validation.ts +++ b/server/src/services/validation.ts @@ -1,8 +1,6 @@ import path from "path" import { Diagnostic, DiagnosticSeverity, Logger } from "vscode-languageserver" import { TextDocument } from "vscode-languageserver-textdocument" -import { URI } from "vscode-uri" -import { uriToFsPath } from "vscode-uri/lib/umd/uri" import { MigrationError } from "@/errors" import { PostgresClient, PostgresPool } from "@/postgres" From 42807719897ca27fe26fe15642b84664fbfff0b9 Mon Sep 17 00:00:00 2001 From: Daniel Castro Date: Thu, 1 Dec 2022 10:10:10 +0000 Subject: [PATCH 06/10] show exact static analysis errors location - pending possible getLineRangeFromBuffer fix --- server/src/postgres/parsers/parseFunctions.ts | 4 +- .../queries/queryFileStaticAnalysis.ts | 63 ++++++++++++------- server/src/services/validation.ts | 6 +- 3 files changed, 46 insertions(+), 27 deletions(-) diff --git a/server/src/postgres/parsers/parseFunctions.ts b/server/src/postgres/parsers/parseFunctions.ts index 35542d6..8e57832 100644 --- a/server/src/postgres/parsers/parseFunctions.ts +++ b/server/src/postgres/parsers/parseFunctions.ts @@ -14,8 +14,8 @@ export interface FunctionInfo { export interface TriggerInfo { functionName: string, - location: number | undefined, relname: string, + stmtLocation?: number, } export async function parseFunctions( @@ -137,8 +137,8 @@ function getCreateTriggers( return [ { functionName, - location: undefined, relname, + stmtLocation: statement?.stmt_location, }, ] }, diff --git a/server/src/postgres/queries/queryFileStaticAnalysis.ts b/server/src/postgres/queries/queryFileStaticAnalysis.ts index 7369e03..6a46657 100644 --- a/server/src/postgres/queries/queryFileStaticAnalysis.ts +++ b/server/src/postgres/queries/queryFileStaticAnalysis.ts @@ -49,6 +49,7 @@ export async function queryFileStaticAnalysis( const [fileText] = sanitizeFileWithQueryParameters( document.getText(), options.queryParameterInfo, logger, ) + logger.info(`fileText.length: ${fileText.length}`) try { const extensionCheck = await pgClient.query(` @@ -101,17 +102,18 @@ export async function queryFileStaticAnalysis( await pgClient.query("BEGIN") if (options.isComplete) { const message = (error as Error).message - logger.error(`StaticAnalysisError: ${message} (${document.uri})`) + logger.error(`StaticAnalysisError (1): ${message} (${document.uri})`) } } try { - for (const { functionName, location, relname } of triggerInfos) { + for (const triggerInfo of triggerInfos) { + const { functionName, stmtLocation, relname } = triggerInfo logger.warn(` trigger::: relname: ${relname} functionName: ${functionName} - location: ${location}`) + stmtLocation: ${stmtLocation}`) const result = await pgClient.query( ` @@ -138,7 +140,7 @@ export async function queryFileStaticAnalysis( continue } - extractError(rows, location) + extractError(rows, stmtLocation) } } catch (error: unknown) { @@ -146,29 +148,46 @@ export async function queryFileStaticAnalysis( await pgClient.query("BEGIN") if (options.isComplete) { const message = (error as Error).message - logger.error(`StaticAnalysisError: ${message} (${document.uri})`) + logger.error(`StaticAnalysisError (2): ${message} (${document.uri})`) } } return errors - function extractError(rows: StaticAnalysisErrorRow[], location: number | undefined) { + function extractError( + rows: StaticAnalysisErrorRow[], + location: number | undefined, + ) { rows.forEach( - (row) => { - const range = (() => { - return (location === undefined) - ? getTextAllRange(document) - : getLineRangeFromBuffer( - fileText, - location, - row.lineno ? row.lineno - 1 : 0, - ) ?? getTextAllRange(document) - })() - - errors.push({ - level: row.level, range, message: row.message, - }) - }, - ) + (row) => { + let range: Range + // FIXME getLineRangeFromBuffer + // range may be larger than byte count for some cases at the end of the doc and throw err reading length of undefined. + // both fileText.length and location from parsed stmt are correct + try { + range = (() => { + return (location === undefined) + ? getTextAllRange(document) + : getLineRangeFromBuffer( + fileText, + location, + row.lineno ? row.lineno - 1 : 0, + ) ?? getTextAllRange(document) + })() + } catch (error: unknown) { + logger.error(`Could not extract error from row. + message: ${JSON.stringify(row.message)} + lineno: ${row.lineno} + location: ${location}`) + range = getTextAllRange(document) + } + errors.push({ + level: row.level, range, message: row.message, + }) + + } + , + ) + } } diff --git a/server/src/services/validation.ts b/server/src/services/validation.ts index ed25adf..eb34174 100644 --- a/server/src/services/validation.ts +++ b/server/src/services/validation.ts @@ -161,7 +161,7 @@ async function validateStaticAnalysis( options: ValidateTextDocumentOptions, logger: Logger, ): Promise { - const [functions, triggers] = await parseFunctions( + const [functionInfos, triggerInfos] = await parseFunctions( document.uri, options.queryParameterInfo, logger, @@ -169,8 +169,8 @@ async function validateStaticAnalysis( const errors = await queryFileStaticAnalysis( pgClient, document, - functions, - triggers, + functionInfos, + triggerInfos, { isComplete: options.isComplete, queryParameterInfo: options.queryParameterInfo, From 9f10c0158cff5f771c8cb200c7c731a3d3223bc8 Mon Sep 17 00:00:00 2001 From: Daniel Castro Date: Thu, 1 Dec 2022 15:16:39 +0000 Subject: [PATCH 07/10] fixed error ranges and add trigger tests --- ..._error_trigger_column_does_not_exist.pgsql | 32 ++ ...r_trigger_column_does_not_exist.pgsql.json | 328 ++++++++++++++++++ server/src/postgres/parsers/parseFunctions.ts | 4 +- .../queries/queryFileStaticAnalysis.ts | 54 +-- server/src/services/validation.test.ts | 18 + server/src/utilities/text.ts | 2 +- 6 files changed, 413 insertions(+), 25 deletions(-) create mode 100644 sample/definitions/function/syntax_error_trigger_column_does_not_exist.pgsql create mode 100644 sample/definitions/function/syntax_error_trigger_column_does_not_exist.pgsql.json diff --git a/sample/definitions/function/syntax_error_trigger_column_does_not_exist.pgsql b/sample/definitions/function/syntax_error_trigger_column_does_not_exist.pgsql new file mode 100644 index 0000000..9700a48 --- /dev/null +++ b/sample/definitions/function/syntax_error_trigger_column_does_not_exist.pgsql @@ -0,0 +1,32 @@ +CREATE TABLE users_1 ( + id integer not null PRIMARY KEY, + updated_at timestamp with time zone not null DEFAULT now() +); +CREATE TABLE users_2 ( + id integer not null PRIMARY KEY +); +CREATE TABLE users_3 ( + id integer not null PRIMARY KEY +); + +create or replace function update_updated_at_column () + returns trigger + language plpgsql + as $function$ +begin + new.updated_at = NOW(); + return new; +end; +$function$; + +create trigger update_users_3_modtime -- should raise error + before update on users_3 for each row + execute function update_updated_at_column (); + +create trigger update_users_1_modtime + before update on users_1 for each row + execute function update_updated_at_column (); + +create trigger update_users_2_modtime -- should raise error + before update on users_2 for each row + execute function update_updated_at_column (); diff --git a/sample/definitions/function/syntax_error_trigger_column_does_not_exist.pgsql.json b/sample/definitions/function/syntax_error_trigger_column_does_not_exist.pgsql.json new file mode 100644 index 0000000..3c80cb1 --- /dev/null +++ b/sample/definitions/function/syntax_error_trigger_column_does_not_exist.pgsql.json @@ -0,0 +1,328 @@ +[ + { + "RawStmt": { + "stmt": { + "CreateStmt": { + "relation": { + "relname": "users_1", + "inh": true, + "relpersistence": "p" + }, + "tableElts": [ + { + "ColumnDef": { + "colname": "id", + "typeName": { + "names": [ + { + "String": { + "str": "pg_catalog" + } + }, + { + "String": { + "str": "int4" + } + } + ], + "typemod": -1 + }, + "is_local": true, + "constraints": [ + { + "Constraint": { + "contype": "CONSTR_NOTNULL" + } + }, + { + "Constraint": { + "contype": "CONSTR_PRIMARY" + } + } + ] + } + }, + { + "ColumnDef": { + "colname": "updated_at", + "typeName": { + "names": [ + { + "String": { + "str": "pg_catalog" + } + }, + { + "String": { + "str": "timestamptz" + } + } + ], + "typemod": -1 + }, + "is_local": true, + "constraints": [ + { + "Constraint": { + "contype": "CONSTR_NOTNULL" + } + }, + { + "Constraint": { + "contype": "CONSTR_DEFAULT", + "raw_expr": { + "FuncCall": { + "funcname": [ + { + "String": { + "str": "now" + } + } + ] + } + } + } + } + ] + } + } + ], + "oncommit": "ONCOMMIT_NOOP" + } + }, + "stmt_len": 120 + } + }, + { + "RawStmt": { + "stmt": { + "CreateStmt": { + "relation": { + "relname": "users_2", + "inh": true, + "relpersistence": "p" + }, + "tableElts": [ + { + "ColumnDef": { + "colname": "id", + "typeName": { + "names": [ + { + "String": { + "str": "pg_catalog" + } + }, + { + "String": { + "str": "int4" + } + } + ], + "typemod": -1 + }, + "is_local": true, + "constraints": [ + { + "Constraint": { + "contype": "CONSTR_NOTNULL" + } + }, + { + "Constraint": { + "contype": "CONSTR_PRIMARY" + } + } + ] + } + } + ], + "oncommit": "ONCOMMIT_NOOP" + } + }, + "stmt_len": 59 + } + }, + { + "RawStmt": { + "stmt": { + "CreateStmt": { + "relation": { + "relname": "users_3", + "inh": true, + "relpersistence": "p" + }, + "tableElts": [ + { + "ColumnDef": { + "colname": "id", + "typeName": { + "names": [ + { + "String": { + "str": "pg_catalog" + } + }, + { + "String": { + "str": "int4" + } + } + ], + "typemod": -1 + }, + "is_local": true, + "constraints": [ + { + "Constraint": { + "contype": "CONSTR_NOTNULL" + } + }, + { + "Constraint": { + "contype": "CONSTR_PRIMARY" + } + } + ] + } + } + ], + "oncommit": "ONCOMMIT_NOOP" + } + }, + "stmt_len": 59 + } + }, + { + "RawStmt": { + "stmt": { + "CreateFunctionStmt": { + "replace": true, + "funcname": [ + { + "String": { + "str": "update_updated_at_column" + } + } + ], + "returnType": { + "names": [ + { + "String": { + "str": "trigger" + } + } + ], + "typemod": -1 + }, + "options": [ + { + "DefElem": { + "defname": "language", + "arg": { + "String": { + "str": "plpgsql" + } + }, + "defaction": "DEFELEM_UNSPEC" + } + }, + { + "DefElem": { + "defname": "as", + "arg": { + "List": { + "items": [ + { + "String": { + "str": "\nbegin\n new.updated_at = NOW();\n return new;\nend;\n" + } + } + ] + } + }, + "defaction": "DEFELEM_UNSPEC" + } + } + ] + } + }, + "stmt_len": 171 + } + }, + { + "RawStmt": { + "stmt": { + "CreateTrigStmt": { + "trigname": "update_users_3_modtime", + "relation": { + "relname": "users_3", + "inh": true, + "relpersistence": "p" + }, + "funcname": [ + { + "String": { + "str": "update_updated_at_column" + } + } + ], + "row": true, + "timing": 2, + "events": 16 + } + }, + "stmt_len": 148 + } + }, + { + "RawStmt": { + "stmt": { + "CreateTrigStmt": { + "trigname": "update_users_1_modtime", + "relation": { + "relname": "users_1", + "inh": true, + "relpersistence": "p" + }, + "funcname": [ + { + "String": { + "str": "update_updated_at_column" + } + } + ], + "row": true, + "timing": 2, + "events": 16 + } + }, + "stmt_len": 126 + } + }, + { + "RawStmt": { + "stmt": { + "CreateTrigStmt": { + "trigname": "update_users_2_modtime", + "relation": { + "relname": "users_2", + "inh": true, + "relpersistence": "p" + }, + "funcname": [ + { + "String": { + "str": "update_updated_at_column" + } + } + ], + "row": true, + "timing": 2, + "events": 16 + } + }, + "stmt_len": 148 + } + } +] \ No newline at end of file diff --git a/server/src/postgres/parsers/parseFunctions.ts b/server/src/postgres/parsers/parseFunctions.ts index 8e57832..27cddc0 100644 --- a/server/src/postgres/parsers/parseFunctions.ts +++ b/server/src/postgres/parsers/parseFunctions.ts @@ -16,6 +16,7 @@ export interface TriggerInfo { functionName: string, relname: string, stmtLocation?: number, + stmtLen: number, } export async function parseFunctions( @@ -138,7 +139,8 @@ function getCreateTriggers( { functionName, relname, - stmtLocation: statement?.stmt_location, + stmtLocation: statement.stmt_location, + stmtLen: statement.stmt_len, }, ] }, diff --git a/server/src/postgres/queries/queryFileStaticAnalysis.ts b/server/src/postgres/queries/queryFileStaticAnalysis.ts index 6a46657..c7ae4e6 100644 --- a/server/src/postgres/queries/queryFileStaticAnalysis.ts +++ b/server/src/postgres/queries/queryFileStaticAnalysis.ts @@ -8,7 +8,10 @@ import { } from "@/postgres/parameters" import { FunctionInfo, TriggerInfo } from "@/postgres/parsers/parseFunctions" import { Settings } from "@/settings" -import { getLineRangeFromBuffer, getTextAllRange } from "@/utilities/text" +import { + getLineRangeFromBuffer, + getRangeFromBuffer, getTextAllRange, +} from "@/utilities/text" export interface StaticAnalysisErrorRow { procedure: string @@ -108,7 +111,7 @@ export async function queryFileStaticAnalysis( try { for (const triggerInfo of triggerInfos) { - const { functionName, stmtLocation, relname } = triggerInfo + const { functionName, stmtLocation, relname, stmtLen } = triggerInfo logger.warn(` trigger::: relname: ${relname} @@ -140,7 +143,7 @@ export async function queryFileStaticAnalysis( continue } - extractError(rows, stmtLocation) + extractError(rows, stmtLocation, stmtLen) } } catch (error: unknown) { @@ -157,30 +160,35 @@ export async function queryFileStaticAnalysis( function extractError( rows: StaticAnalysisErrorRow[], location: number | undefined, + stmtLen?: number, ) { rows.forEach( (row) => { - let range: Range - // FIXME getLineRangeFromBuffer - // range may be larger than byte count for some cases at the end of the doc and throw err reading length of undefined. - // both fileText.length and location from parsed stmt are correct - try { - range = (() => { - return (location === undefined) - ? getTextAllRange(document) - : getLineRangeFromBuffer( - fileText, - location, - row.lineno ? row.lineno - 1 : 0, - ) ?? getTextAllRange(document) + const range = (() => { + if (location === undefined) { + return getTextAllRange(document) + } + if (stmtLen) { + return getRangeFromBuffer( + fileText, + location + 1, + location + 1 + stmtLen, + ) + } + + const lineRange = getLineRangeFromBuffer( + fileText, + location, + row.lineno ? row.lineno - 1 : 0, + ) + + if (!lineRange) { + return getTextAllRange(document) + } + + return lineRange })() - } catch (error: unknown) { - logger.error(`Could not extract error from row. - message: ${JSON.stringify(row.message)} - lineno: ${row.lineno} - location: ${location}`) - range = getTextAllRange(document) - } + errors.push({ level: row.level, range, message: row.message, }) diff --git a/server/src/services/validation.test.ts b/server/src/services/validation.test.ts index db3cc09..6f6a536 100644 --- a/server/src/services/validation.test.ts +++ b/server/src/services/validation.test.ts @@ -68,6 +68,24 @@ describe("Validate Tests", () => { ]) }) + it("TRIGGER on inexistent field", async () => { + const diagnostics = await validateSampleFile( + "definitions/function/syntax_error_trigger_column_does_not_exist.pgsql", + ) + + expect(diagnostics).toStrictEqual([ + { + severity: DiagnosticSeverity.Error, + message: 'record "new" has no field "updated_at"', + range: Range.create(20, 0, 23, 47), + }, { + severity: DiagnosticSeverity.Error, + message: 'record "new" has no field "updated_at"', + range: Range.create(28, 0, 31, 47), + }, + ]) + }) + it("FUNCTION column does not exists", async () => { const diagnostics = await validateSampleFile( "definitions/function/syntax_error_function_column_does_not_exist.pgsql", diff --git a/server/src/utilities/text.ts b/server/src/utilities/text.ts index ce2880a..28802ae 100644 --- a/server/src/utilities/text.ts +++ b/server/src/utilities/text.ts @@ -88,7 +88,7 @@ export function getLineRangeFromBuffer( fileText: string, index: uinteger, offsetLine: uinteger = 0, ): Range | undefined { const textLines = Buffer.from(fileText) - .slice(0, index) + .subarray(0, index) .toString() .split("\n") From 5a0a2bf29a8272b1bec1234f9491d482b2b06eb3 Mon Sep 17 00:00:00 2001 From: Daniel Castro Date: Thu, 1 Dec 2022 15:38:01 +0000 Subject: [PATCH 08/10] fix ci --- ..._error_trigger_column_does_not_exist.pgsql | 4 + ...r_trigger_column_does_not_exist.pgsql.json | 79 ++++++++++++++++++- server/src/services/validation.test.ts | 4 +- 3 files changed, 83 insertions(+), 4 deletions(-) diff --git a/sample/definitions/function/syntax_error_trigger_column_does_not_exist.pgsql b/sample/definitions/function/syntax_error_trigger_column_does_not_exist.pgsql index 9700a48..4552e2f 100644 --- a/sample/definitions/function/syntax_error_trigger_column_does_not_exist.pgsql +++ b/sample/definitions/function/syntax_error_trigger_column_does_not_exist.pgsql @@ -1,3 +1,7 @@ +DROP TABLE IF EXISTS users_1 CASCADE; +DROP TABLE IF EXISTS users_2 CASCADE; +DROP TABLE IF EXISTS users_3 CASCADE; + CREATE TABLE users_1 ( id integer not null PRIMARY KEY, updated_at timestamp with time zone not null DEFAULT now() diff --git a/sample/definitions/function/syntax_error_trigger_column_does_not_exist.pgsql.json b/sample/definitions/function/syntax_error_trigger_column_does_not_exist.pgsql.json index 3c80cb1..2adb046 100644 --- a/sample/definitions/function/syntax_error_trigger_column_does_not_exist.pgsql.json +++ b/sample/definitions/function/syntax_error_trigger_column_does_not_exist.pgsql.json @@ -1,4 +1,79 @@ [ + { + "RawStmt": { + "stmt": { + "DropStmt": { + "objects": [ + { + "List": { + "items": [ + { + "String": { + "str": "users_1" + } + } + ] + } + } + ], + "removeType": "OBJECT_TABLE", + "behavior": "DROP_CASCADE", + "missing_ok": true + } + }, + "stmt_len": 36 + } + }, + { + "RawStmt": { + "stmt": { + "DropStmt": { + "objects": [ + { + "List": { + "items": [ + { + "String": { + "str": "users_2" + } + } + ] + } + } + ], + "removeType": "OBJECT_TABLE", + "behavior": "DROP_CASCADE", + "missing_ok": true + } + }, + "stmt_len": 37 + } + }, + { + "RawStmt": { + "stmt": { + "DropStmt": { + "objects": [ + { + "List": { + "items": [ + { + "String": { + "str": "users_3" + } + } + ] + } + } + ], + "removeType": "OBJECT_TABLE", + "behavior": "DROP_CASCADE", + "missing_ok": true + } + }, + "stmt_len": 37 + } + }, { "RawStmt": { "stmt": { @@ -90,7 +165,7 @@ "oncommit": "ONCOMMIT_NOOP" } }, - "stmt_len": 120 + "stmt_len": 122 } }, { @@ -272,7 +347,7 @@ "events": 16 } }, - "stmt_len": 148 + "stmt_len": 190 } }, { diff --git a/server/src/services/validation.test.ts b/server/src/services/validation.test.ts index 6f6a536..5045e40 100644 --- a/server/src/services/validation.test.ts +++ b/server/src/services/validation.test.ts @@ -77,11 +77,11 @@ describe("Validate Tests", () => { { severity: DiagnosticSeverity.Error, message: 'record "new" has no field "updated_at"', - range: Range.create(20, 0, 23, 47), + range: Range.create(24, 0, 27, 47), }, { severity: DiagnosticSeverity.Error, message: 'record "new" has no field "updated_at"', - range: Range.create(28, 0, 31, 47), + range: Range.create(32, 0, 35, 47), }, ]) }) From 3895ee74ce490250fe84607b539e943504a380a7 Mon Sep 17 00:00:00 2001 From: Daniel Castro Date: Thu, 1 Dec 2022 16:12:06 +0000 Subject: [PATCH 09/10] add tests for correct schemas and fix naming --- .../function/static_error_disabled.pgsql | 21 + ....json => static_error_disabled.pgsql.json} | 0 ...error_trigger_column_does_not_exist.pgsql} | 0 ...r_trigger_column_does_not_exist.pgsql.json | 403 ++++++++++++++++++ .../schemas/correct_schema_1.sql | 0 .../schemas/correct_schema_2.sql | 0 .../schemas/correct_schema_3.sql | 0 server/src/__tests__/__fixtures__/schemas | 1 + server/src/__tests__/helpers/file.ts | 2 +- server/src/services/validation.test.ts | 29 +- 10 files changed, 453 insertions(+), 3 deletions(-) create mode 100644 sample/definitions/function/static_error_disabled.pgsql rename sample/definitions/function/{syntax_error_trigger_column_does_not_exist.pgsql.json => static_error_disabled.pgsql.json} (100%) rename sample/definitions/function/{syntax_error_trigger_column_does_not_exist.pgsql => static_error_trigger_column_does_not_exist.pgsql} (100%) create mode 100644 sample/definitions/function/static_error_trigger_column_does_not_exist.pgsql.json rename server/src/__tests__/__fixtures__/schemas/correct_schema_1.pgsql => sample/schemas/correct_schema_1.sql (100%) rename server/src/__tests__/__fixtures__/schemas/correct_schema_2.pgsql => sample/schemas/correct_schema_2.sql (100%) rename server/src/__tests__/__fixtures__/schemas/correct_schema_3.pgsql => sample/schemas/correct_schema_3.sql (100%) create mode 120000 server/src/__tests__/__fixtures__/schemas diff --git a/sample/definitions/function/static_error_disabled.pgsql b/sample/definitions/function/static_error_disabled.pgsql new file mode 100644 index 0000000..10f858b --- /dev/null +++ b/sample/definitions/function/static_error_disabled.pgsql @@ -0,0 +1,21 @@ +DROP TABLE IF EXISTS users_2 CASCADE; + +CREATE TABLE users_2 ( + id integer not null PRIMARY KEY +); + +create or replace function update_updated_at_column () + returns trigger + language plpgsql + as $function$ +begin + new.updated_at = NOW(); + return new; +end; +$function$; + +-- TODO some kind of flag for disabling static analysis only per statement +-- plpgsql-language-server:disable-static +create trigger update_users_2_modtime -- should raise error + before update on users_2 for each row + execute function update_updated_at_column (); diff --git a/sample/definitions/function/syntax_error_trigger_column_does_not_exist.pgsql.json b/sample/definitions/function/static_error_disabled.pgsql.json similarity index 100% rename from sample/definitions/function/syntax_error_trigger_column_does_not_exist.pgsql.json rename to sample/definitions/function/static_error_disabled.pgsql.json diff --git a/sample/definitions/function/syntax_error_trigger_column_does_not_exist.pgsql b/sample/definitions/function/static_error_trigger_column_does_not_exist.pgsql similarity index 100% rename from sample/definitions/function/syntax_error_trigger_column_does_not_exist.pgsql rename to sample/definitions/function/static_error_trigger_column_does_not_exist.pgsql diff --git a/sample/definitions/function/static_error_trigger_column_does_not_exist.pgsql.json b/sample/definitions/function/static_error_trigger_column_does_not_exist.pgsql.json new file mode 100644 index 0000000..2adb046 --- /dev/null +++ b/sample/definitions/function/static_error_trigger_column_does_not_exist.pgsql.json @@ -0,0 +1,403 @@ +[ + { + "RawStmt": { + "stmt": { + "DropStmt": { + "objects": [ + { + "List": { + "items": [ + { + "String": { + "str": "users_1" + } + } + ] + } + } + ], + "removeType": "OBJECT_TABLE", + "behavior": "DROP_CASCADE", + "missing_ok": true + } + }, + "stmt_len": 36 + } + }, + { + "RawStmt": { + "stmt": { + "DropStmt": { + "objects": [ + { + "List": { + "items": [ + { + "String": { + "str": "users_2" + } + } + ] + } + } + ], + "removeType": "OBJECT_TABLE", + "behavior": "DROP_CASCADE", + "missing_ok": true + } + }, + "stmt_len": 37 + } + }, + { + "RawStmt": { + "stmt": { + "DropStmt": { + "objects": [ + { + "List": { + "items": [ + { + "String": { + "str": "users_3" + } + } + ] + } + } + ], + "removeType": "OBJECT_TABLE", + "behavior": "DROP_CASCADE", + "missing_ok": true + } + }, + "stmt_len": 37 + } + }, + { + "RawStmt": { + "stmt": { + "CreateStmt": { + "relation": { + "relname": "users_1", + "inh": true, + "relpersistence": "p" + }, + "tableElts": [ + { + "ColumnDef": { + "colname": "id", + "typeName": { + "names": [ + { + "String": { + "str": "pg_catalog" + } + }, + { + "String": { + "str": "int4" + } + } + ], + "typemod": -1 + }, + "is_local": true, + "constraints": [ + { + "Constraint": { + "contype": "CONSTR_NOTNULL" + } + }, + { + "Constraint": { + "contype": "CONSTR_PRIMARY" + } + } + ] + } + }, + { + "ColumnDef": { + "colname": "updated_at", + "typeName": { + "names": [ + { + "String": { + "str": "pg_catalog" + } + }, + { + "String": { + "str": "timestamptz" + } + } + ], + "typemod": -1 + }, + "is_local": true, + "constraints": [ + { + "Constraint": { + "contype": "CONSTR_NOTNULL" + } + }, + { + "Constraint": { + "contype": "CONSTR_DEFAULT", + "raw_expr": { + "FuncCall": { + "funcname": [ + { + "String": { + "str": "now" + } + } + ] + } + } + } + } + ] + } + } + ], + "oncommit": "ONCOMMIT_NOOP" + } + }, + "stmt_len": 122 + } + }, + { + "RawStmt": { + "stmt": { + "CreateStmt": { + "relation": { + "relname": "users_2", + "inh": true, + "relpersistence": "p" + }, + "tableElts": [ + { + "ColumnDef": { + "colname": "id", + "typeName": { + "names": [ + { + "String": { + "str": "pg_catalog" + } + }, + { + "String": { + "str": "int4" + } + } + ], + "typemod": -1 + }, + "is_local": true, + "constraints": [ + { + "Constraint": { + "contype": "CONSTR_NOTNULL" + } + }, + { + "Constraint": { + "contype": "CONSTR_PRIMARY" + } + } + ] + } + } + ], + "oncommit": "ONCOMMIT_NOOP" + } + }, + "stmt_len": 59 + } + }, + { + "RawStmt": { + "stmt": { + "CreateStmt": { + "relation": { + "relname": "users_3", + "inh": true, + "relpersistence": "p" + }, + "tableElts": [ + { + "ColumnDef": { + "colname": "id", + "typeName": { + "names": [ + { + "String": { + "str": "pg_catalog" + } + }, + { + "String": { + "str": "int4" + } + } + ], + "typemod": -1 + }, + "is_local": true, + "constraints": [ + { + "Constraint": { + "contype": "CONSTR_NOTNULL" + } + }, + { + "Constraint": { + "contype": "CONSTR_PRIMARY" + } + } + ] + } + } + ], + "oncommit": "ONCOMMIT_NOOP" + } + }, + "stmt_len": 59 + } + }, + { + "RawStmt": { + "stmt": { + "CreateFunctionStmt": { + "replace": true, + "funcname": [ + { + "String": { + "str": "update_updated_at_column" + } + } + ], + "returnType": { + "names": [ + { + "String": { + "str": "trigger" + } + } + ], + "typemod": -1 + }, + "options": [ + { + "DefElem": { + "defname": "language", + "arg": { + "String": { + "str": "plpgsql" + } + }, + "defaction": "DEFELEM_UNSPEC" + } + }, + { + "DefElem": { + "defname": "as", + "arg": { + "List": { + "items": [ + { + "String": { + "str": "\nbegin\n new.updated_at = NOW();\n return new;\nend;\n" + } + } + ] + } + }, + "defaction": "DEFELEM_UNSPEC" + } + } + ] + } + }, + "stmt_len": 171 + } + }, + { + "RawStmt": { + "stmt": { + "CreateTrigStmt": { + "trigname": "update_users_3_modtime", + "relation": { + "relname": "users_3", + "inh": true, + "relpersistence": "p" + }, + "funcname": [ + { + "String": { + "str": "update_updated_at_column" + } + } + ], + "row": true, + "timing": 2, + "events": 16 + } + }, + "stmt_len": 190 + } + }, + { + "RawStmt": { + "stmt": { + "CreateTrigStmt": { + "trigname": "update_users_1_modtime", + "relation": { + "relname": "users_1", + "inh": true, + "relpersistence": "p" + }, + "funcname": [ + { + "String": { + "str": "update_updated_at_column" + } + } + ], + "row": true, + "timing": 2, + "events": 16 + } + }, + "stmt_len": 126 + } + }, + { + "RawStmt": { + "stmt": { + "CreateTrigStmt": { + "trigname": "update_users_2_modtime", + "relation": { + "relname": "users_2", + "inh": true, + "relpersistence": "p" + }, + "funcname": [ + { + "String": { + "str": "update_updated_at_column" + } + } + ], + "row": true, + "timing": 2, + "events": 16 + } + }, + "stmt_len": 148 + } + } +] \ No newline at end of file diff --git a/server/src/__tests__/__fixtures__/schemas/correct_schema_1.pgsql b/sample/schemas/correct_schema_1.sql similarity index 100% rename from server/src/__tests__/__fixtures__/schemas/correct_schema_1.pgsql rename to sample/schemas/correct_schema_1.sql diff --git a/server/src/__tests__/__fixtures__/schemas/correct_schema_2.pgsql b/sample/schemas/correct_schema_2.sql similarity index 100% rename from server/src/__tests__/__fixtures__/schemas/correct_schema_2.pgsql rename to sample/schemas/correct_schema_2.sql diff --git a/server/src/__tests__/__fixtures__/schemas/correct_schema_3.pgsql b/sample/schemas/correct_schema_3.sql similarity index 100% rename from server/src/__tests__/__fixtures__/schemas/correct_schema_3.pgsql rename to sample/schemas/correct_schema_3.sql diff --git a/server/src/__tests__/__fixtures__/schemas b/server/src/__tests__/__fixtures__/schemas new file mode 120000 index 0000000..e28eb7d --- /dev/null +++ b/server/src/__tests__/__fixtures__/schemas @@ -0,0 +1 @@ +../../../../sample/schemas/ \ No newline at end of file diff --git a/server/src/__tests__/helpers/file.ts b/server/src/__tests__/helpers/file.ts index 99d4e01..c760985 100644 --- a/server/src/__tests__/helpers/file.ts +++ b/server/src/__tests__/helpers/file.ts @@ -37,7 +37,7 @@ export async function loadSampleFile( } } -function sampleDirPath(): string { +export function sampleDirPath(): string { return path.join(__dirname, "..", "__fixtures__") } diff --git a/server/src/services/validation.test.ts b/server/src/services/validation.test.ts index 5045e40..bd10570 100644 --- a/server/src/services/validation.test.ts +++ b/server/src/services/validation.test.ts @@ -1,6 +1,10 @@ +import glob from "glob-promise" +import path from "path" import { Diagnostic, DiagnosticSeverity, Range } from "vscode-languageserver" -import { DEFAULT_LOAD_FILE_OPTIONS, LoadFileOptions } from "@/__tests__/helpers/file" +import { + DEFAULT_LOAD_FILE_OPTIONS, LoadFileOptions, sampleDirPath, +} from "@/__tests__/helpers/file" import { RecordLogger } from "@/__tests__/helpers/logger" import { setupTestServer } from "@/__tests__/helpers/server" import { SettingsBuilder } from "@/__tests__/helpers/settings" @@ -70,7 +74,7 @@ describe("Validate Tests", () => { it("TRIGGER on inexistent field", async () => { const diagnostics = await validateSampleFile( - "definitions/function/syntax_error_trigger_column_does_not_exist.pgsql", + "definitions/function/static_error_trigger_column_does_not_exist.pgsql", ) expect(diagnostics).toStrictEqual([ @@ -86,6 +90,15 @@ describe("Validate Tests", () => { ]) }) + // TODO + it.skip("static analysis disabled on invalid statement", async () => { + const diagnostics = await validateSampleFile( + "definitions/function/static_error_disabled.pgsql", + ) + + expect(diagnostics).toStrictEqual([]) + }) + it("FUNCTION column does not exists", async () => { const diagnostics = await validateSampleFile( "definitions/function/syntax_error_function_column_does_not_exist.pgsql", @@ -109,6 +122,18 @@ describe("Validate Tests", () => { expect(diagnostics).toStrictEqual([]) }) + + it("correct schemas", async () => { + const schemas = (await glob.promise(path.join(sampleDirPath(), "schemas/*.sql"))) + .map(file => path.relative(sampleDirPath(), file)) + + schemas.forEach(async (schema) => { + const diagnostics = await validateSampleFile(schema) + + expect(diagnostics).toStrictEqual([]) + }) + }) + it("Syntax error query", async () => { const diagnostics = await validateSampleFile( "queries/syntax_error_query_with_language_server_disable_comment.pgsql", From 70527781d0f2ed9511afef98846e642cdb6fd9ae Mon Sep 17 00:00:00 2001 From: Daniel Castro Date: Thu, 1 Dec 2022 18:52:45 +0000 Subject: [PATCH 10/10] comment per statement to disable static validation --- .../function/static_error_disabled.pgsql.json | 403 ------------------ .../static_error_disabled.pgsql | 5 +- .../trigger/static_error_disabled.pgsql.json | 184 ++++++++ ..._error_trigger_column_does_not_exist.pgsql | 0 ...r_trigger_column_does_not_exist.pgsql.json | 2 +- .../queries/queryFileStaticAnalysis.ts | 65 +-- server/src/services/validation.test.ts | 18 +- server/src/utilities/regex.ts | 4 +- 8 files changed, 240 insertions(+), 441 deletions(-) delete mode 100644 sample/definitions/function/static_error_disabled.pgsql.json rename sample/definitions/{function => trigger}/static_error_disabled.pgsql (75%) create mode 100644 sample/definitions/trigger/static_error_disabled.pgsql.json rename sample/definitions/{function => trigger}/static_error_trigger_column_does_not_exist.pgsql (100%) rename sample/definitions/{function => trigger}/static_error_trigger_column_does_not_exist.pgsql.json (99%) diff --git a/sample/definitions/function/static_error_disabled.pgsql.json b/sample/definitions/function/static_error_disabled.pgsql.json deleted file mode 100644 index 2adb046..0000000 --- a/sample/definitions/function/static_error_disabled.pgsql.json +++ /dev/null @@ -1,403 +0,0 @@ -[ - { - "RawStmt": { - "stmt": { - "DropStmt": { - "objects": [ - { - "List": { - "items": [ - { - "String": { - "str": "users_1" - } - } - ] - } - } - ], - "removeType": "OBJECT_TABLE", - "behavior": "DROP_CASCADE", - "missing_ok": true - } - }, - "stmt_len": 36 - } - }, - { - "RawStmt": { - "stmt": { - "DropStmt": { - "objects": [ - { - "List": { - "items": [ - { - "String": { - "str": "users_2" - } - } - ] - } - } - ], - "removeType": "OBJECT_TABLE", - "behavior": "DROP_CASCADE", - "missing_ok": true - } - }, - "stmt_len": 37 - } - }, - { - "RawStmt": { - "stmt": { - "DropStmt": { - "objects": [ - { - "List": { - "items": [ - { - "String": { - "str": "users_3" - } - } - ] - } - } - ], - "removeType": "OBJECT_TABLE", - "behavior": "DROP_CASCADE", - "missing_ok": true - } - }, - "stmt_len": 37 - } - }, - { - "RawStmt": { - "stmt": { - "CreateStmt": { - "relation": { - "relname": "users_1", - "inh": true, - "relpersistence": "p" - }, - "tableElts": [ - { - "ColumnDef": { - "colname": "id", - "typeName": { - "names": [ - { - "String": { - "str": "pg_catalog" - } - }, - { - "String": { - "str": "int4" - } - } - ], - "typemod": -1 - }, - "is_local": true, - "constraints": [ - { - "Constraint": { - "contype": "CONSTR_NOTNULL" - } - }, - { - "Constraint": { - "contype": "CONSTR_PRIMARY" - } - } - ] - } - }, - { - "ColumnDef": { - "colname": "updated_at", - "typeName": { - "names": [ - { - "String": { - "str": "pg_catalog" - } - }, - { - "String": { - "str": "timestamptz" - } - } - ], - "typemod": -1 - }, - "is_local": true, - "constraints": [ - { - "Constraint": { - "contype": "CONSTR_NOTNULL" - } - }, - { - "Constraint": { - "contype": "CONSTR_DEFAULT", - "raw_expr": { - "FuncCall": { - "funcname": [ - { - "String": { - "str": "now" - } - } - ] - } - } - } - } - ] - } - } - ], - "oncommit": "ONCOMMIT_NOOP" - } - }, - "stmt_len": 122 - } - }, - { - "RawStmt": { - "stmt": { - "CreateStmt": { - "relation": { - "relname": "users_2", - "inh": true, - "relpersistence": "p" - }, - "tableElts": [ - { - "ColumnDef": { - "colname": "id", - "typeName": { - "names": [ - { - "String": { - "str": "pg_catalog" - } - }, - { - "String": { - "str": "int4" - } - } - ], - "typemod": -1 - }, - "is_local": true, - "constraints": [ - { - "Constraint": { - "contype": "CONSTR_NOTNULL" - } - }, - { - "Constraint": { - "contype": "CONSTR_PRIMARY" - } - } - ] - } - } - ], - "oncommit": "ONCOMMIT_NOOP" - } - }, - "stmt_len": 59 - } - }, - { - "RawStmt": { - "stmt": { - "CreateStmt": { - "relation": { - "relname": "users_3", - "inh": true, - "relpersistence": "p" - }, - "tableElts": [ - { - "ColumnDef": { - "colname": "id", - "typeName": { - "names": [ - { - "String": { - "str": "pg_catalog" - } - }, - { - "String": { - "str": "int4" - } - } - ], - "typemod": -1 - }, - "is_local": true, - "constraints": [ - { - "Constraint": { - "contype": "CONSTR_NOTNULL" - } - }, - { - "Constraint": { - "contype": "CONSTR_PRIMARY" - } - } - ] - } - } - ], - "oncommit": "ONCOMMIT_NOOP" - } - }, - "stmt_len": 59 - } - }, - { - "RawStmt": { - "stmt": { - "CreateFunctionStmt": { - "replace": true, - "funcname": [ - { - "String": { - "str": "update_updated_at_column" - } - } - ], - "returnType": { - "names": [ - { - "String": { - "str": "trigger" - } - } - ], - "typemod": -1 - }, - "options": [ - { - "DefElem": { - "defname": "language", - "arg": { - "String": { - "str": "plpgsql" - } - }, - "defaction": "DEFELEM_UNSPEC" - } - }, - { - "DefElem": { - "defname": "as", - "arg": { - "List": { - "items": [ - { - "String": { - "str": "\nbegin\n new.updated_at = NOW();\n return new;\nend;\n" - } - } - ] - } - }, - "defaction": "DEFELEM_UNSPEC" - } - } - ] - } - }, - "stmt_len": 171 - } - }, - { - "RawStmt": { - "stmt": { - "CreateTrigStmt": { - "trigname": "update_users_3_modtime", - "relation": { - "relname": "users_3", - "inh": true, - "relpersistence": "p" - }, - "funcname": [ - { - "String": { - "str": "update_updated_at_column" - } - } - ], - "row": true, - "timing": 2, - "events": 16 - } - }, - "stmt_len": 190 - } - }, - { - "RawStmt": { - "stmt": { - "CreateTrigStmt": { - "trigname": "update_users_1_modtime", - "relation": { - "relname": "users_1", - "inh": true, - "relpersistence": "p" - }, - "funcname": [ - { - "String": { - "str": "update_updated_at_column" - } - } - ], - "row": true, - "timing": 2, - "events": 16 - } - }, - "stmt_len": 126 - } - }, - { - "RawStmt": { - "stmt": { - "CreateTrigStmt": { - "trigname": "update_users_2_modtime", - "relation": { - "relname": "users_2", - "inh": true, - "relpersistence": "p" - }, - "funcname": [ - { - "String": { - "str": "update_updated_at_column" - } - } - ], - "row": true, - "timing": 2, - "events": 16 - } - }, - "stmt_len": 148 - } - } -] \ No newline at end of file diff --git a/sample/definitions/function/static_error_disabled.pgsql b/sample/definitions/trigger/static_error_disabled.pgsql similarity index 75% rename from sample/definitions/function/static_error_disabled.pgsql rename to sample/definitions/trigger/static_error_disabled.pgsql index 10f858b..6f9c1a0 100644 --- a/sample/definitions/function/static_error_disabled.pgsql +++ b/sample/definitions/trigger/static_error_disabled.pgsql @@ -14,8 +14,11 @@ begin end; $function$; --- TODO some kind of flag for disabling static analysis only per statement -- plpgsql-language-server:disable-static +create trigger update_users_2_modtime_disabled -- error silenced + before update on users_2 for each row + execute function update_updated_at_column (); + create trigger update_users_2_modtime -- should raise error before update on users_2 for each row execute function update_updated_at_column (); diff --git a/sample/definitions/trigger/static_error_disabled.pgsql.json b/sample/definitions/trigger/static_error_disabled.pgsql.json new file mode 100644 index 0000000..8bc91f8 --- /dev/null +++ b/sample/definitions/trigger/static_error_disabled.pgsql.json @@ -0,0 +1,184 @@ +[ + { + "RawStmt": { + "stmt": { + "DropStmt": { + "objects": [ + { + "List": { + "items": [ + { + "String": { + "str": "users_2" + } + } + ] + } + } + ], + "removeType": "OBJECT_TABLE", + "behavior": "DROP_CASCADE", + "missing_ok": true + } + }, + "stmt_len": 36 + } + }, + { + "RawStmt": { + "stmt": { + "CreateStmt": { + "relation": { + "relname": "users_2", + "inh": true, + "relpersistence": "p" + }, + "tableElts": [ + { + "ColumnDef": { + "colname": "id", + "typeName": { + "names": [ + { + "String": { + "str": "pg_catalog" + } + }, + { + "String": { + "str": "int4" + } + } + ], + "typemod": -1 + }, + "is_local": true, + "constraints": [ + { + "Constraint": { + "contype": "CONSTR_NOTNULL" + } + }, + { + "Constraint": { + "contype": "CONSTR_PRIMARY" + } + } + ] + } + } + ], + "oncommit": "ONCOMMIT_NOOP" + } + }, + "stmt_len": 60 + } + }, + { + "RawStmt": { + "stmt": { + "CreateFunctionStmt": { + "replace": true, + "funcname": [ + { + "String": { + "str": "update_updated_at_column" + } + } + ], + "returnType": { + "names": [ + { + "String": { + "str": "trigger" + } + } + ], + "typemod": -1 + }, + "options": [ + { + "DefElem": { + "defname": "language", + "arg": { + "String": { + "str": "plpgsql" + } + }, + "defaction": "DEFELEM_UNSPEC" + } + }, + { + "DefElem": { + "defname": "as", + "arg": { + "List": { + "items": [ + { + "String": { + "str": "\nbegin\n new.updated_at = NOW();\n return new;\nend;\n" + } + } + ] + } + }, + "defaction": "DEFELEM_UNSPEC" + } + } + ] + } + }, + "stmt_len": 171 + } + }, + { + "RawStmt": { + "stmt": { + "CreateTrigStmt": { + "trigname": "update_users_2_modtime_disabled", + "relation": { + "relname": "users_2", + "inh": true, + "relpersistence": "p" + }, + "funcname": [ + { + "String": { + "str": "update_updated_at_column" + } + } + ], + "row": true, + "timing": 2, + "events": 16 + } + }, + "stmt_len": 195 + } + }, + { + "RawStmt": { + "stmt": { + "CreateTrigStmt": { + "trigname": "update_users_2_modtime", + "relation": { + "relname": "users_2", + "inh": true, + "relpersistence": "p" + }, + "funcname": [ + { + "String": { + "str": "update_updated_at_column" + } + } + ], + "row": true, + "timing": 2, + "events": 16 + } + }, + "stmt_len": 148 + } + } +] \ No newline at end of file diff --git a/sample/definitions/function/static_error_trigger_column_does_not_exist.pgsql b/sample/definitions/trigger/static_error_trigger_column_does_not_exist.pgsql similarity index 100% rename from sample/definitions/function/static_error_trigger_column_does_not_exist.pgsql rename to sample/definitions/trigger/static_error_trigger_column_does_not_exist.pgsql diff --git a/sample/definitions/function/static_error_trigger_column_does_not_exist.pgsql.json b/sample/definitions/trigger/static_error_trigger_column_does_not_exist.pgsql.json similarity index 99% rename from sample/definitions/function/static_error_trigger_column_does_not_exist.pgsql.json rename to sample/definitions/trigger/static_error_trigger_column_does_not_exist.pgsql.json index 2adb046..bda322f 100644 --- a/sample/definitions/function/static_error_trigger_column_does_not_exist.pgsql.json +++ b/sample/definitions/trigger/static_error_trigger_column_does_not_exist.pgsql.json @@ -347,7 +347,7 @@ "events": 16 } }, - "stmt_len": 190 + "stmt_len": 148 } }, { diff --git a/server/src/postgres/queries/queryFileStaticAnalysis.ts b/server/src/postgres/queries/queryFileStaticAnalysis.ts index c7ae4e6..48cd37c 100644 --- a/server/src/postgres/queries/queryFileStaticAnalysis.ts +++ b/server/src/postgres/queries/queryFileStaticAnalysis.ts @@ -8,6 +8,7 @@ import { } from "@/postgres/parameters" import { FunctionInfo, TriggerInfo } from "@/postgres/parsers/parseFunctions" import { Settings } from "@/settings" +import { DISABLE_STATIC_VALIDATION_RE } from "@/utilities/regex" import { getLineRangeFromBuffer, getRangeFromBuffer, getTextAllRange, @@ -162,40 +163,44 @@ export async function queryFileStaticAnalysis( location: number | undefined, stmtLen?: number, ) { - rows.forEach( - (row) => { - const range = (() => { - if (location === undefined) { - return getTextAllRange(document) - } - if (stmtLen) { - return getRangeFromBuffer( - fileText, - location + 1, - location + 1 + stmtLen, - ) + rows.forEach((row) => { + const range = (() => { + if (location === undefined) { + return getTextAllRange(document) + } + if (stmtLen) { + const stmt = fileText.slice(location + 1, location + 1 + stmtLen) + if (DISABLE_STATIC_VALIDATION_RE + .test(stmt)) { + return } - const lineRange = getLineRangeFromBuffer( + return getRangeFromBuffer( fileText, - location, - row.lineno ? row.lineno - 1 : 0, + location + 1, + location + 1 + stmtLen, ) + } + const lineRange = getLineRangeFromBuffer( + fileText, + location, + row.lineno ? row.lineno - 1 : 0, + ) + + if (!lineRange) { + return getTextAllRange(document) + } + + return lineRange + })() + + if (!range) { + return + } - if (!lineRange) { - return getTextAllRange(document) - } - - return lineRange - })() - - errors.push({ - level: row.level, range, message: row.message, - }) - - } - , - ) - + errors.push({ + level: row.level, range, message: row.message, + }) + }) } } diff --git a/server/src/services/validation.test.ts b/server/src/services/validation.test.ts index bd10570..07a10fa 100644 --- a/server/src/services/validation.test.ts +++ b/server/src/services/validation.test.ts @@ -74,7 +74,7 @@ describe("Validate Tests", () => { it("TRIGGER on inexistent field", async () => { const diagnostics = await validateSampleFile( - "definitions/function/static_error_trigger_column_does_not_exist.pgsql", + "definitions/trigger/static_error_trigger_column_does_not_exist.pgsql", ) expect(diagnostics).toStrictEqual([ @@ -90,13 +90,21 @@ describe("Validate Tests", () => { ]) }) - // TODO - it.skip("static analysis disabled on invalid statement", async () => { + it("static analysis disabled on invalid statement", async () => { const diagnostics = await validateSampleFile( - "definitions/function/static_error_disabled.pgsql", + "definitions/trigger/static_error_disabled.pgsql", ) - expect(diagnostics).toStrictEqual([]) + if (!diagnostics) { + throw new Error("") + } + if (diagnostics?.length === 0) { + throw new Error("") + } + + expect(diagnostics).toHaveLength(1) + expect(diagnostics[0].message) + .toContain("record \"new\" has no field \"updated_at\"") }) it("FUNCTION column does not exists", async () => { diff --git a/server/src/utilities/regex.ts b/server/src/utilities/regex.ts index 96a90e2..1894365 100644 --- a/server/src/utilities/regex.ts +++ b/server/src/utilities/regex.ts @@ -1,3 +1,5 @@ +/* eslint-disable max-len */ + export function escapeRegex(string: string): string { return string.replace(/[-/\\^$*+?.()|[\]{}]/g, "\\$&") } @@ -8,5 +10,5 @@ export const BEGIN_RE = /^([\s]*begin[\s]*;)/igm export const COMMIT_RE = /^([\s]*commit[\s]*;)/igm export const ROLLBACK_RE = /^([\s]*rollback[\s]*;)/igm -// eslint-disable-next-line max-len export const DISABLE_STATEMENT_VALIDATION_RE = /^ *-- +plpgsql-language-server:disable *$/m +export const DISABLE_STATIC_VALIDATION_RE = /^ *-- +plpgsql-language-server:disable-static *$/m