Skip to content

Commit c6de409

Browse files
pnappapnappa
authored
feat!: Allow enforcing non-empty array parameters in config (#603)
When using array parameters (including array spread and pick), providing an empty array to a parameter will usually cause a crash at runtime due to a syntax error. For the following query, if an empty array is provided for userIds: ``` /* @name GetNotifications @param userIds -> (...) */ SELECT * FROM notifications WHERE id in :userIds! ``` As the query at runtime becomes: ``` SELECT * FROM notifications WHERE id in () ``` Which results in: ``` ERROR: syntax error at or near ")" ``` Therefore, this config forces the generated parameter type to only allow non-empty arrays, i.e. the type `[T, ...T[]]` rather than `T[]`.
1 parent 48e9da0 commit c6de409

File tree

4 files changed

+261
-2
lines changed

4 files changed

+261
-2
lines changed

‎docs-new/docs/cli.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ For a full list of options, see the [Configuration file format](#configuration-f
6060
"srcDir": "./src/", // Directory to scan or watch for query files
6161
"failOnError": false, // Whether to fail on a file processing error and abort generation (can be omitted - default is false)
6262
"camelCaseColumnNames": false, // convert to camelCase column names of result interface
63+
"nonEmptyArrayParams": false, // Whether the type for an array parameter should exclude empty arrays
6364
"dbUrl": "postgres://user:password@host/database", // DB URL (optional - will be merged with db if provided)
6465
"db": {
6566
"dbName": "testdb", // DB name
@@ -92,6 +93,7 @@ Configuration file can be also be written in CommonJS format and default exporte
9293
| `failOnError?` | `boolean` | Whether to fail on a file processing error and abort generation. **Default:** `false` |
9394
| `dbUrl?` | `string` | A connection string to the database. Example: `postgres://user:password@host/database`. Overrides (merged) with `db` config. |
9495
| `camelCaseColumnNames?` | `boolean` | Whether to convert column names to camelCase. _Note that this only coverts the types. You need to do this at runtime independently using a library like `pg-camelcase`_. |
96+
| `nonEmptyArrayParams?` | `boolean` | Whether the types for arrays parameters exclude empty arrays. This helps prevent runtime errors when accidentally providing empty input to a query. |
9597
| `typesOverrides?` | `Record<string, string>` | A map of type overrides. Similarly to `camelCaseColumnNames`, this only affects the types. _You need to do this at runtime independently using a library like `pg-types`._ |
9698
| `maxWorkerThreads` | `number` | The maximum number of worker threads to use for type generation. **The default is based on the number of available CPUs.** |
9799

‎packages/cli/src/config.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ const configParser = t.type({
5757
failOnError: t.union([t.boolean, t.undefined]),
5858
camelCaseColumnNames: t.union([t.boolean, t.undefined]),
5959
hungarianNotation: t.union([t.boolean, t.undefined]),
60+
nonEmptyArrayParams: t.union([t.boolean, t.undefined]),
6061
dbUrl: t.union([t.string, t.undefined]),
6162
db: t.union([
6263
t.type({
@@ -99,6 +100,7 @@ export interface ParsedConfig {
99100
failOnError: boolean;
100101
camelCaseColumnNames: boolean;
101102
hungarianNotation: boolean;
103+
nonEmptyArrayParams: boolean;
102104
transforms: IConfig['transforms'];
103105
srcDir: IConfig['srcDir'];
104106
typesOverrides: Record<string, Partial<TypeDefinition>>;
@@ -198,6 +200,7 @@ export function parseConfig(
198200
failOnError,
199201
camelCaseColumnNames,
200202
hungarianNotation,
203+
nonEmptyArrayParams,
201204
typesOverrides,
202205
} = configObject as IConfig;
203206

@@ -242,6 +245,7 @@ export function parseConfig(
242245
failOnError: failOnError ?? false,
243246
camelCaseColumnNames: camelCaseColumnNames ?? false,
244247
hungarianNotation: hungarianNotation ?? true,
248+
nonEmptyArrayParams: nonEmptyArrayParams ?? false,
245249
typesOverrides: parsedTypesOverrides,
246250
maxWorkerThreads,
247251
};

‎packages/cli/src/generator.test.ts‎

Lines changed: 235 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,241 @@ export interface IGetNotificationsQuery {
418418
expect(result).toEqual(expected);
419419
});
420420

421+
test(`Non-empty array parameters (${mode})`, async () => {
422+
const queryStringSQL = `
423+
/*
424+
@name GetNotifications
425+
@param userIds -> (...)
426+
*/
427+
SELECT payload, type FROM notifications WHERE id in :userIds;
428+
`;
429+
const queryStringTS = `
430+
const getNotifications = sql\`SELECT payload, type FROM notifications WHERE id in $userIds\`;
431+
`;
432+
const queryString =
433+
mode === ProcessingMode.SQL ? queryStringSQL : queryStringTS;
434+
const mockTypes: IQueryTypes = {
435+
returnTypes: [
436+
{
437+
returnName: 'payload',
438+
columnName: 'payload',
439+
type: 'json',
440+
nullable: false,
441+
},
442+
{
443+
returnName: 'type',
444+
columnName: 'type',
445+
type: { name: 'PayloadType', enumValues: ['message', 'dynamite'] },
446+
nullable: false,
447+
},
448+
],
449+
paramMetadata: {
450+
params: ['uuid'],
451+
mapping: [
452+
{
453+
name: 'userIds',
454+
type: ParameterTransform.Spread,
455+
assignedIndex: 1,
456+
required: false,
457+
},
458+
],
459+
},
460+
};
461+
const typeSource = async (_: any) => mockTypes;
462+
const types = new TypeAllocator(TypeMapping());
463+
// Test out imports
464+
types.use(
465+
{ name: 'PreparedQuery', from: '@pgtyped/runtime' },
466+
TypeScope.Return,
467+
);
468+
const result = await queryToTypeDeclarations(
469+
parsedQuery(mode, queryString),
470+
typeSource,
471+
types,
472+
{ nonEmptyArrayParams: true, hungarianNotation: true } as ParsedConfig,
473+
);
474+
const expectedTypes = `import { PreparedQuery } from '@pgtyped/runtime';
475+
476+
export type PayloadType = 'dynamite' | 'message';
477+
478+
export type Json = null | boolean | number | string | Json[] | { [key: string]: Json };\n`;
479+
480+
expect(types.declaration('file.ts')).toEqual(expectedTypes);
481+
const expected = `/** 'GetNotifications' parameters type */
482+
export interface IGetNotificationsParams {
483+
userIds: readonly [string | null | void, ...(string | null | void)[]];
484+
}
485+
486+
/** 'GetNotifications' return type */
487+
export interface IGetNotificationsResult {
488+
payload: Json;
489+
type: PayloadType;
490+
}
491+
492+
/** 'GetNotifications' query type */
493+
export interface IGetNotificationsQuery {
494+
params: IGetNotificationsParams;
495+
result: IGetNotificationsResult;
496+
}\n\n`;
497+
expect(result).toEqual(expected);
498+
});
499+
500+
test(`Required non-empty array parameters (${mode})`, async () => {
501+
const queryStringSQL = `
502+
/*
503+
@name GetNotifications
504+
@param userIds -> (...)
505+
*/
506+
SELECT payload, type FROM notifications WHERE id in :userIds!;
507+
`;
508+
const queryStringTS = `
509+
const getNotifications = sql\`SELECT payload, type FROM notifications WHERE id in $userIds!\`;
510+
`;
511+
const queryString =
512+
mode === ProcessingMode.SQL ? queryStringSQL : queryStringTS;
513+
const mockTypes: IQueryTypes = {
514+
returnTypes: [
515+
{
516+
returnName: 'payload',
517+
columnName: 'payload',
518+
type: 'json',
519+
nullable: false,
520+
},
521+
{
522+
returnName: 'type',
523+
columnName: 'type',
524+
type: { name: 'PayloadType', enumValues: ['message', 'dynamite'] },
525+
nullable: false,
526+
},
527+
],
528+
paramMetadata: {
529+
params: ['uuid'],
530+
mapping: [
531+
{
532+
name: 'userIds',
533+
type: ParameterTransform.Spread,
534+
assignedIndex: 1,
535+
required: true,
536+
},
537+
],
538+
},
539+
};
540+
const typeSource = async (_: any) => mockTypes;
541+
const types = new TypeAllocator(TypeMapping());
542+
// Test out imports
543+
types.use(
544+
{ name: 'PreparedQuery', from: '@pgtyped/runtime' },
545+
TypeScope.Return,
546+
);
547+
const result = await queryToTypeDeclarations(
548+
parsedQuery(mode, queryString),
549+
typeSource,
550+
types,
551+
{ nonEmptyArrayParams: true, hungarianNotation: true } as ParsedConfig,
552+
);
553+
const expectedTypes = `import { PreparedQuery } from '@pgtyped/runtime';
554+
555+
export type PayloadType = 'dynamite' | 'message';
556+
557+
export type Json = null | boolean | number | string | Json[] | { [key: string]: Json };\n`;
558+
559+
expect(types.declaration('file.ts')).toEqual(expectedTypes);
560+
const expected = `/** 'GetNotifications' parameters type */
561+
export interface IGetNotificationsParams {
562+
userIds: readonly [string, ...(string)[]];
563+
}
564+
565+
/** 'GetNotifications' return type */
566+
export interface IGetNotificationsResult {
567+
payload: Json;
568+
type: PayloadType;
569+
}
570+
571+
/** 'GetNotifications' query type */
572+
export interface IGetNotificationsQuery {
573+
params: IGetNotificationsParams;
574+
result: IGetNotificationsResult;
575+
}\n\n`;
576+
expect(result).toEqual(expected);
577+
});
578+
579+
test(`Non-empty object spread insert (${mode})`, async () => {
580+
const queryStringSQL = `
581+
/*
582+
@name InsertNotifications
583+
@param notification -> ((payload, user_id, type)...)
584+
*/
585+
INSERT INTO notifications (payload, user_id, type) VALUES :notification
586+
`;
587+
const queryStringTS = `const insertNotifications = sql\`INSERT INTO notifications (payload, user_id, type) VALUES $notification(payload, user_id, type)\`;`;
588+
const queryString =
589+
mode === ProcessingMode.SQL ? queryStringSQL : queryStringTS;
590+
const mockTypes: IQueryTypes = {
591+
returnTypes: [],
592+
paramMetadata: {
593+
params: ['json', 'uuid', 'text'],
594+
mapping: [
595+
{
596+
name: 'notification',
597+
type: ParameterTransform.PickSpread,
598+
dict: {
599+
payload: {
600+
name: 'payload',
601+
assignedIndex: 1,
602+
required: false,
603+
type: ParameterTransform.Scalar,
604+
},
605+
user_id: {
606+
name: 'user_id',
607+
assignedIndex: 2,
608+
required: false,
609+
type: ParameterTransform.Scalar,
610+
},
611+
type: {
612+
name: 'type',
613+
assignedIndex: 3,
614+
required: false,
615+
type: ParameterTransform.Scalar,
616+
},
617+
},
618+
},
619+
],
620+
},
621+
};
622+
const types = new TypeAllocator(TypeMapping());
623+
const typeSource = async (_: any) => mockTypes;
624+
const result = await queryToTypeDeclarations(
625+
parsedQuery(mode, queryString),
626+
typeSource,
627+
types,
628+
{ nonEmptyArrayParams: true, hungarianNotation: true } as ParsedConfig,
629+
);
630+
const expected = `/** 'InsertNotifications' parameters type */
631+
export interface IInsertNotificationsParams {
632+
notification: readonly [{
633+
payload: Json | null | void,
634+
user_id: string | null | void,
635+
type: string | null | void
636+
}, ...({
637+
payload: Json | null | void,
638+
user_id: string | null | void,
639+
type: string | null | void
640+
})[]];
641+
}
642+
643+
/** 'InsertNotifications' return type */
644+
export type IInsertNotificationsResult = void;
645+
646+
/** 'InsertNotifications' query type */
647+
export interface IInsertNotificationsQuery {
648+
params: IInsertNotificationsParams;
649+
result: IInsertNotificationsResult;
650+
}
651+
652+
`;
653+
expect(result).toEqual(expected);
654+
});
655+
421656
test(`Columns without nullable info should be nullable (${mode})`, async () => {
422657
const queryStringSQL = `
423658
/* @name GetNotifications */

‎packages/cli/src/generator.ts‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,24 @@ export async function queryToTypeDeclarations(
172172
});
173173
});
174174

175+
const formatArrayType = (type: string): string => {
176+
if (config.nonEmptyArrayParams) {
177+
// Only allow accepting an non-empty array. This prevents runtime errors.
178+
// For example, the following query will throw a runtime error if an empty
179+
// array is passed:
180+
// /*
181+
// @name GetNotifications
182+
// @param userIds -> (...)
183+
// */
184+
// SELECT * FROM notifications WHERE id in :userIds!
185+
// As the query at runtime becomes:
186+
// SELECT * FROM notifications WHERE id in ()
187+
// Which is a syntax error for postgres.
188+
return `readonly [${type}, ...(${type})[]]`;
189+
}
190+
return `readonly (${type})[]`;
191+
};
192+
175193
const { params } = paramMetadata;
176194
for (const param of paramMetadata.mapping) {
177195
if (
@@ -197,7 +215,7 @@ export async function queryToTypeDeclarations(
197215
paramFieldTypes.push({
198216
optional,
199217
fieldName: param.name,
200-
fieldType: isArray ? `readonly (${tsTypeName})[]` : tsTypeName,
218+
fieldType: isArray ? formatArrayType(tsTypeName) : tsTypeName,
201219
});
202220
} else {
203221
const isArray = param.type === ParameterTransform.PickSpread;
@@ -214,7 +232,7 @@ export async function queryToTypeDeclarations(
214232
.join(',\n');
215233
fieldType = `{\n${fieldType}\n }`;
216234
if (isArray) {
217-
fieldType = `readonly (${fieldType})[]`;
235+
fieldType = formatArrayType(fieldType);
218236
}
219237
paramFieldTypes.push({
220238
fieldName: param.name,

0 commit comments

Comments
 (0)