Skip to content

Commit

Permalink
feat(ignore): add ignore option
Browse files Browse the repository at this point in the history
  • Loading branch information
janpaepke authored and mmkal committed May 14, 2022
1 parent 91bfccf commit 0fa854e
Show file tree
Hide file tree
Showing 9 changed files with 326 additions and 89 deletions.
30 changes: 21 additions & 9 deletions packages/typegen/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,15 @@ Select statements, joins, and updates/inserts/deletes using `returning` are all
- [Installation](#installation)
- [Usage](#usage)
- [Configuration](#configuration)
- [Complex config types](#complex-config-types)
- [Testing `psqlCommand`](#testing-psqlcommand)
- [Example config](#example-config)
- [Advanced Configuration](#writetypes)
- [Advanced Configuration](#advanced-configuration)
- [Controlling write destination](#controlling-write-destination)
- [Modifying types](#modifying-types)
- [Modifying source files](#modifying-source-files)
- [Enhancing Return Types](#enhancing-return-types)
- [Ignoring Queries](#ignoring-queries)
- [Examples](#examples)
- [Migration from v0.8.0](#migration-from-v080)
- [SQL files](#sql-files)
Expand Down Expand Up @@ -134,7 +137,7 @@ CLI arguments will always have precedence over config options.
|`defaultType`|`--default-type`|`string`|`'unknown'`|TypeScript type when no mapping is found. This should usually be `unknown` (or `any` if you like to live dangerously).|
|`poolConfig`||`PoolConfig \| undefined`<br/>(see [below](#complex-config-types))|`undefined`|Slonik database pool configuration. Will be used to create a pool which issues queries to the database as the tool is running, and will have its type parsers inspected to ensure the generated types are correct. It's important to pass in a pool confguration which is the same as the one used in your application.|
|`logger`||`Logger`<br/>(see [below](#complex-config-types))|`console`|Logger object with `debug`, `info`, `warn` and `error` methods. Defaults to `console`.|
|`writeTypes`<br/>(experimental)||`WriteTypes`<br/>(see [below](#complex-config-types))|`typegen.`&thinsp;`defaultWriteTypes`|Control how files are written to disk. See the [writeTypes](#writetypes) section.|
|`writeTypes`<br/>(experimental)||`WriteTypes`<br/>(see [below](#complex-config-types))|`typegen.`&thinsp;`defaultWriteTypes`|Control how files are written to disk. See the [Advanced Configuration](#advanced-configuration) section.|
||`--config`|`string`|`'typegen.config.js'`|Path to configuration file.|
||`--migrate`|`'<=0.8.0'`|disabled|Before generating types, attempt to migrate a codebase which has used a prior version of this tool.|
||`--watch`|CLI argument|disabled|Run in watch mode.|
Expand All @@ -155,7 +158,7 @@ You can check if your `psql` is working, and that your postgres version supports
echo 'select 123 as abc \gdesc' \| psql "postgresql://postgres:postgres@localhost:5432/postgres" -f -
```

There are some more configuration options [documented in code](./src/types.ts), but these should be considered experimental, and might change without warning. You can try them out as documented [below](#writetypes), but please start a [discussion](https://github.com/mmkal/slonik-tools/discussions) on this library's project page with some info about your use case so the API can be stabilised in a sensible way.
There are some more configuration options [documented in code](./src/types.ts), but these should be considered experimental, and might change without warning. You can try them out as documented [below](#advanced-configuration), but please start a [discussion](https://github.com/mmkal/slonik-tools/discussions) on this library's project page with some info about your use case so the API can be stabilised in a sensible way.

### Example config

Expand All @@ -176,7 +179,7 @@ module.exports.default = {

Note that the `/** @type {import('@slonik/typegen').Options} */` comment is optional, but will ensure your IDE gives you type hints.

### writeTypes
### Advanced Configuration

The `writeTypes` option allows you to tweak what's written to disk. Note that the usage style isn't finalised and might change in future. If you use it, please create a discussion about it in https://github.com/mmkal/slonik-tools/discussions so that your use-case doesn't get taken away unexpectedly.

Expand Down Expand Up @@ -392,6 +395,14 @@ Note that you can't completely change a property type (say from `string` to `num
This is by design, because if you could, a change in the underlying table might cause typegen to detect a new type, which would be ignored, had you overwritten it. This would cause type changes to go unnoticed and we can't have that.
With intersections, the resulting property will be of type `never`, when an underlying column type changes. This will alert you to the change, so you can update your manual enhancements.
## Ignoring Queries
For file-based ignores, you can use the [exclude option](#configuration) to set a pattern or specific file(s) to be ignored via the config file or using the CLI option.
Typegen also automatically ignores all queries with zero chance of returning a result (i.e. sql fragments).
If you want to exclude a specific query from processing, you can add a `--typegen-ignore` or `/* typegen-ignore */` comment anywhere in the query.
## Examples
[The tests](./test) and [corresponding fixtures](./test/fixtures) are a good starting point to see what the code-generator will do.
Expand Down Expand Up @@ -432,18 +443,18 @@ In the above example, no type can be inferred because it's impossible to know wh

___

Queries with multiple statements will also not receive a type:
Queries with multiple statements will result in an error:

```ts
import {sql} from 'slonik'

sql`
insert into foo(a, b) values (1, 2);
insert into foo(a, b) values (3, 4);
update table set col=1 where id=1 returning 1;
update table set col=2 where id=2 returning 2;
`
```

This kind of query does not return a value when executed anyway.
The return type is not clearly assigned here. Every literal should only contain one query statement.

___

Expand All @@ -465,7 +476,8 @@ import {sql} from 'slonik'
sql`this is not even valid SQL!`
```

If you see errors being logged for SQL that you think is valid, feel free to [raise an issue](https://github.com/mmkal/slonik-tools/issues/new). In the meantime, you can create a variable `const _sql = sql` and use the `_sql` tag in the same way as `sql`. `_sql` will not be detected by the tool and can be used as normal.
If you see errors being logged for SQL that you think is valid, feel free to [raise an issue](https://github.com/mmkal/slonik-tools/issues/new).
In the meantime, you can use of of the [ignore options](#ignoring-queries) to skip processing the concerned queries.

___

Expand Down
35 changes: 21 additions & 14 deletions packages/typegen/src/extract/typescript.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import {ExtractedQuery, Options} from '../types'
import * as lodash from 'lodash'
import * as assert from 'assert'
import * as fs from 'fs'

import type * as ts from 'typescript'
import * as assert from 'assert'

import {ExtractedQuery, Options} from '../types'
import {isReturningQuery, tsCustom} from '../util'

const rawExtractWithTypeScript: Options['extractQueries'] = file => {
const ts: typeof import('typescript') = require('typescript')
Expand All @@ -12,22 +14,20 @@ const rawExtractWithTypeScript: Options['extractQueries'] = file => {
// adapted from https://github.com/Microsoft/TypeScript/wiki/Using-the-Compiler-API#traversing-the-ast-with-a-little-linter
const queries: ExtractedQuery[] = []

visitNodeGenerics(sourceFile, [])
visitNodeGenericsRecursive(sourceFile, [])

return queries

function visitNodeGenerics(node: ts.Node, context: string[]) {
function visitNodeGenericsRecursive(node: ts.Node, context: string[]) {
if (!ts.isTaggedTemplateExpression(node)) {
const newContext =
(ts.isVariableDeclaration(node) || ts.isPropertyAssignment(node) || ts.isFunctionDeclaration(node)) && node.name
? [...context, node.name.getText()]
: context
ts.forEachChild(node, n => visitNodeGenerics(n, newContext))
ts.forEachChild(node, n => visitNodeGenericsRecursive(n, newContext))
return
}
const isSqlIdentifier = (n: ts.Node) => ts.isIdentifier(n) && n.getText() === 'sql'
const sqlPropertyAccessor = ts.isPropertyAccessExpression(node.tag) && isSqlIdentifier(node.tag.name)
if (isSqlIdentifier(node.tag) || sqlPropertyAccessor) {
if (tsCustom.isSqlLiteral(node)) {
let template: string[] = []
if (ts.isNoSubstitutionTemplateLiteral(node.template)) {
template = [node.template.text]
Expand All @@ -38,17 +38,24 @@ const rawExtractWithTypeScript: Options['extractQueries'] = file => {

assert.ok(template.length > 0, `Couldn't get template for node at ${node.pos}`)

const sql = template
// join with $1. May not be correct if ${sql.identifier(['blah'])} is used. \gdesc will fail in that case.
.map((t, i) => `$${i}${t}`)
.join('')
.slice(2) // slice off $0 at the start

if (!isReturningQuery(sql)) {
// this is likely a fragment. let's skip it.
return
}

queries.push({
text: node.getFullText(),
source,
file,
context,
line: node.getSourceFile().getLineAndCharacterOfPosition(node.pos).line + 1,
sql: template
// join with $1. May not be correct if ${sql.identifier(['blah'])} is used. \gdesc will fail in that case.
.map((t, i) => `$${i}${t}`)
.join('')
.slice(2), // slice off $0 at the start
sql,
template,
})
}
Expand Down
28 changes: 22 additions & 6 deletions packages/typegen/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,16 @@ import {psqlClient} from './pg'
import {AnalyseQueryError, columnInfoGetter, isUntypeable, removeSimpleComments, simplifySql} from './query'
import {parameterTypesGetter} from './query/parameters'
import {AnalysedQuery, DescribedQuery, ExtractedQuery, Options, QueryField, QueryParameter} from './types'
import {changedFiles, checkClean, globAsync, globList, maybeDo, truncateQuery, tryOrDefault} from './util'
import {
changedFiles,
checkClean,
containsIgnoreComment,
globAsync,
globList,
maybeDo,
truncateQuery,
tryOrDefault,
} from './util'
import * as write from './write'

import memoizee = require('memoizee')
Expand Down Expand Up @@ -171,16 +180,19 @@ export const generate = async (params: Partial<Options>) => {
// gather stats for log
const queriesTotal = processedFiles.reduce((sum, {total}) => sum + total, 0)
const queriesSuccessful = processedFiles.reduce((sum, {successful}) => sum + successful, 0)
const queriesProcessed = queriesSuccessful < queriesTotal ? `${queriesSuccessful}/${queriesTotal}` : queriesTotal
const filesSuccessful = processedFiles.reduce((sum, {successful}) => sum + (successful > 0 ? 1 : 0), 0)
const filesProcessed = filesSuccessful < files.length ? `${filesSuccessful}/${files.length}` : files.length
logger.info(`Finished processing ${queriesProcessed} queries in ${filesProcessed} files.`)
const queriesMsg = queriesSuccessful < queriesTotal ? `${queriesSuccessful}/${queriesTotal}` : queriesTotal
const filesMsg = filesSuccessful < files.length ? `${filesSuccessful}/${files.length}` : files.length
logger.info(`Finished processing ${queriesMsg} queries in ${filesMsg} files.`)
}

async function generateForFile(file: string) {
const queries = extractQueries(file)

const queriesToAnalyse = queries.map(async (query): Promise<AnalysedQuery | null> => {
const queriesToDescribe = queries.filter(({sql}) => !containsIgnoreComment(sql))
const ignoreCount = queries.length - queriesToDescribe.length

const queriesToAnalyse = queriesToDescribe.map(async (query): Promise<AnalysedQuery | null> => {
const describedQuery = await describeQuery(query)
if (describedQuery === null) {
return null
Expand All @@ -191,7 +203,10 @@ export const generate = async (params: Partial<Options>) => {
const analysedQueries = lodash.compact(await Promise.all(queriesToAnalyse))

if (queries.length > 0) {
logger.info(`${getLogPath(file)} finished. Processed ${analysedQueries.length}/${queries.length} queries.`)
const ignoreMsg = ignoreCount > 0 ? ` (${ignoreCount} ignored)` : ''
logger.info(
`${getLogPath(file)} finished. Processed ${analysedQueries.length}/${queries.length} queries${ignoreMsg}.`,
)
}
if (analysedQueries.length > 0) {
await writeTypes(analysedQueries)
Expand All @@ -200,6 +215,7 @@ export const generate = async (params: Partial<Options>) => {
return {
total: queries.length,
successful: analysedQueries.length,
ignored: ignoreCount,
}
}

Expand Down
36 changes: 35 additions & 1 deletion packages/typegen/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import {promisify} from 'util'
import * as glob from 'glob'
import * as lodash from 'lodash'

import ts = require('typescript')

export const globAsync = promisify(glob)

/** Trim and compact whitespace. Don't use on content where whitespace matters! */
Expand Down Expand Up @@ -43,7 +45,9 @@ export const attempt = <T>(context: string, action: () => T): T => {
try {
return action()
} catch (e) {
e.message = `Failure: ${context}: ${e}`
if (e instanceof Error) {
e.message = `Failure: ${context}: ${e}`
}
throw e
}
}
Expand Down Expand Up @@ -78,3 +82,33 @@ export const changedFiles = (params: {since: string; cwd: string}) =>
.filter(Boolean)

export const globList = (list: readonly string[]) => (list.length === 1 ? list[0] : `{${list.join(',')}}`)

export const tsCustom = (() => {
const isSqlIdentifier = (node: ts.Node) => ts.isIdentifier(node) && node.getText() === 'sql'
const isSqlPropertyAccessor = (e: ts.Expression) => ts.isPropertyAccessExpression(e) && isSqlIdentifier(e.name)
/**
* Checks if the supplied node is an sql template literal
*/
const isSqlLiteral = (node: ts.Node): node is ts.TaggedTemplateExpression =>
ts.isTaggedTemplateExpression(node) && (isSqlIdentifier(node.tag) || isSqlPropertyAccessor(node.tag))
return {
isSqlLiteral,
} as const
})()

/**
* Tests a string against a regex checking for the existence of specific keywords.
* The rationale here is that typegen should only consider queries containing for example `SELECT`, and skip query fragments.
*/
export const isReturningQuery = (() => {
const returningKeywords = /(^|\s|\()(SELECT|VALUES|RETURNING)\s/i
return (query: string) => returningKeywords.test(query)
})()

/**
* Checks if a string contains a sql-comment, meant to signal typegen to ignore it.
*/
export const containsIgnoreComment = (() => {
const ignoreKeywords = /--[^\S\n\r\f]*typegen-ignore|(\/\*\s*typegen-ignore\s*\*\/)/i
return (query: string) => ignoreKeywords.test(query)
})()
6 changes: 2 additions & 4 deletions packages/typegen/src/write/inline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import * as lodash from 'lodash'
import type * as ts from 'typescript'

import {TaggedQuery} from '../types'
import {relativeUnixPath} from '../util'
import {relativeUnixPath, tsCustom} from '../util'
import {tsPrettify} from './prettify'
import {queryInterfaces} from './typescript'
import {WriteFile} from '.'
Expand Down Expand Up @@ -85,9 +85,7 @@ export function getFileWriter({getQueriesModulePath = defaultGetQueriesModule, w
}

if (ts.isTaggedTemplateExpression(node)) {
const isSqlIdentifier = (e: ts.Node) => ts.isIdentifier(e) && e.getText() === 'sql'
const isSqlPropertyAccessor = (e: ts.Expression) => ts.isPropertyAccessExpression(e) && isSqlIdentifier(e.name)
if (!isSqlIdentifier(node.tag) && !isSqlPropertyAccessor(node.tag)) {
if (!tsCustom.isSqlLiteral(node)) {
return
}
const matchingQuery = group.find(q => q.text === node.getFullText())
Expand Down
Loading

0 comments on commit 0fa854e

Please sign in to comment.