Skip to content

Commit 18f3231

Browse files
devin-ai-integration[bot]pyramation
devin-ai-integration[bot]
and
pyramation
committed
refactor: migrate quoteString usages to quoteIdentifier and remove deprecated methods
- Replace all quoteString usages in deparser.ts with quoteIdentifier - Replace all needsQuotesForString usages with quoteIdentifier checks - Remove deprecated quoteString and needsQuotesForString methods from quote-utils.ts - Update quoteIfNeeded() and String() methods to use quoteIdentifier - Migrate DefElem contexts and VariableSetStmt to use quoteIdentifier The quoteIdentifier function is more correct because it: - Properly escapes embedded double quotes as "" - Checks all keyword categories (not just RESERVED_KEYWORDS) - Follows PostgreSQL's exact quote_identifier() logic from ruleutils.c Co-Authored-By: Dan Lynch <[email protected]>
1 parent 30cbd40 commit 18f3231

File tree

2 files changed

+38
-73
lines changed

2 files changed

+38
-73
lines changed

‎packages/deparser/src/deparser.ts‎

Lines changed: 37 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -2458,7 +2458,7 @@ export class Deparser implements DeparserVisitor {
24582458
}
24592459

24602460
quoteIfNeeded(value: string): string {
2461-
return QuoteUtils.quoteString(value);
2461+
return QuoteUtils.quoteIdentifier(value);
24622462
}
24632463

24642464
preserveOperatorDefElemCase(defName: string): string {
@@ -2500,7 +2500,7 @@ export class Deparser implements DeparserVisitor {
25002500
}
25012501
}
25022502

2503-
return QuoteUtils.quoteString(value);
2503+
return QuoteUtils.quoteIdentifier(value);
25042504
}
25052505

25062506
Integer(node: t.Integer, context: DeparserContext): string {
@@ -4163,16 +4163,16 @@ export class Deparser implements DeparserVisitor {
41634163
}).join(', ') : '';
41644164

41654165
// Handle args - always include TO clause if args exist (even if empty string)
4166-
const paramName = QuoteUtils.quoteString(node.name);
4166+
const paramName = QuoteUtils.quoteIdentifier(node.name);
41674167
if (!node.args || node.args.length === 0) {
41684168
return `SET ${localPrefix}${paramName}`;
41694169
}
41704170
return `SET ${localPrefix}${paramName} TO ${args}`;
41714171
case 'VAR_SET_DEFAULT':
4172-
const defaultParamName = QuoteUtils.quoteString(node.name);
4172+
const defaultParamName = QuoteUtils.quoteIdentifier(node.name);
41734173
return `SET ${defaultParamName} TO DEFAULT`;
41744174
case 'VAR_SET_CURRENT':
4175-
const currentParamName = QuoteUtils.quoteString(node.name);
4175+
const currentParamName = QuoteUtils.quoteIdentifier(node.name);
41764176
return `SET ${currentParamName} FROM CURRENT`;
41774177
case 'VAR_SET_MULTI':
41784178
if (node.name === 'TRANSACTION' || node.name === 'SESSION CHARACTERISTICS') {
@@ -4240,7 +4240,7 @@ export class Deparser implements DeparserVisitor {
42404240
return `SET ${assignments}`;
42414241
}
42424242
case 'VAR_RESET':
4243-
const resetParamName = QuoteUtils.quoteString(node.name);
4243+
const resetParamName = QuoteUtils.quoteIdentifier(node.name);
42444244
return `RESET ${resetParamName}`;
42454245
case 'VAR_RESET_ALL':
42464246
return 'RESET ALL';
@@ -5687,7 +5687,7 @@ export class Deparser implements DeparserVisitor {
56875687
}
56885688

56895689
if (node.role) {
5690-
const roleName = QuoteUtils.quoteString(node.role);
5690+
const roleName = QuoteUtils.quoteIdentifier(node.role);
56915691
output.push(roleName);
56925692
}
56935693

@@ -5735,7 +5735,7 @@ export class Deparser implements DeparserVisitor {
57355735
if (context.parentNodeTypes.includes('DefineStmt') &&
57365736
['hashes', 'merges'].includes(node.defname.toLowerCase()) && !node.arg) {
57375737
if (node.defname !== node.defname.toLowerCase() && node.defname !== node.defname.toUpperCase()) {
5738-
return QuoteUtils.quoteString(node.defname);
5738+
return QuoteUtils.quoteIdentifier(node.defname);
57395739
}
57405740
return node.defname.charAt(0).toUpperCase() + node.defname.slice(1).toLowerCase();
57415741
}
@@ -5760,7 +5760,7 @@ export class Deparser implements DeparserVisitor {
57605760
? `'${argValue}'`
57615761
: argValue;
57625762

5763-
const quotedDefname = QuoteUtils.quoteString(node.defname);
5763+
const quotedDefname = QuoteUtils.quoteIdentifier(node.defname);
57645764

57655765
if (node.defaction === 'DEFELEM_ADD') {
57665766
return `ADD ${quotedDefname} ${finalValue}`;
@@ -5785,7 +5785,7 @@ export class Deparser implements DeparserVisitor {
57855785
return `SET ${node.defname} ${quotedValue}`;
57865786
}
57875787

5788-
const quotedDefname = QuoteUtils.quoteString(node.defname);
5788+
const quotedDefname = QuoteUtils.quoteIdentifier(node.defname);
57895789
return `${quotedDefname} ${quotedValue}`;
57905790
} else if (node.defaction === 'DEFELEM_DROP') {
57915791
// Handle DROP without argument
@@ -5851,7 +5851,7 @@ export class Deparser implements DeparserVisitor {
58515851
const quotedValue = typeof argValue === 'string'
58525852
? QuoteUtils.escape(argValue)
58535853
: argValue;
5854-
const quotedDefname = QuoteUtils.quoteString(node.defname);
5854+
const quotedDefname = QuoteUtils.quoteIdentifier(node.defname);
58555855
return `${quotedDefname} ${quotedValue}`;
58565856
}
58575857

@@ -5934,7 +5934,7 @@ export class Deparser implements DeparserVisitor {
59345934
if (this.getNodeType(item) === 'String') {
59355935
// Check if this identifier needs quotes to preserve case
59365936
const value = itemData.sval;
5937-
return QuoteUtils.quoteString(value);
5937+
return QuoteUtils.quoteIdentifier(value);
59385938
}
59395939
return this.visit(item, context);
59405940
});
@@ -6202,14 +6202,14 @@ export class Deparser implements DeparserVisitor {
62026202
// Handle boolean flags (no arguments) - preserve quoted case
62036203
if (['hashes', 'merges'].includes(node.defname.toLowerCase())) {
62046204
if (node.defname !== node.defname.toLowerCase() && node.defname !== node.defname.toUpperCase()) {
6205-
return QuoteUtils.quoteString(node.defname);
6205+
return QuoteUtils.quoteIdentifier(node.defname);
62066206
}
62076207
return preservedName.toUpperCase();
62086208
}
62096209

62106210
// Handle CREATE AGGREGATE quoted identifiers - preserve quotes when needed
6211-
if (QuoteUtils.needsQuotesForString(node.defname)) {
6212-
const quotedDefname = QuoteUtils.quoteString(node.defname);
6211+
const quotedDefname = QuoteUtils.quoteIdentifier(node.defname);
6212+
if (quotedDefname !== node.defname) {
62136213
if (node.arg) {
62146214
if (this.getNodeType(node.arg) === 'String') {
62156215
const stringData = this.getNodeData(node.arg);
@@ -6273,7 +6273,7 @@ export class Deparser implements DeparserVisitor {
62736273
if (context.parentNodeTypes.includes('DefineStmt') && !node.arg) {
62746274
// Check if the original defname appears to be quoted (mixed case that's not all upper/lower)
62756275
if (node.defname !== node.defname.toLowerCase() && node.defname !== node.defname.toUpperCase()) {
6276-
return QuoteUtils.quoteString(node.defname);
6276+
return QuoteUtils.quoteIdentifier(node.defname);
62776277
}
62786278
}
62796279

@@ -7137,7 +7137,7 @@ export class Deparser implements DeparserVisitor {
71377137
output.push('SERVER');
71387138

71397139
if (node.servername) {
7140-
output.push(QuoteUtils.quoteString(node.servername));
7140+
output.push(QuoteUtils.quoteIdentifier(node.servername));
71417141
}
71427142

71437143
if (node.options && node.options.length > 0) {
@@ -7198,7 +7198,7 @@ export class Deparser implements DeparserVisitor {
71987198
const output: string[] = ['CREATE', 'PUBLICATION'];
71997199

72007200
if (node.pubname) {
7201-
output.push(QuoteUtils.quoteString(node.pubname));
7201+
output.push(QuoteUtils.quoteIdentifier(node.pubname));
72027202
}
72037203

72047204
if (node.pubobjects && node.pubobjects.length > 0) {
@@ -7222,7 +7222,7 @@ export class Deparser implements DeparserVisitor {
72227222
const output: string[] = ['CREATE', 'SUBSCRIPTION'];
72237223

72247224
if (node.subname) {
7225-
output.push(QuoteUtils.quoteString(node.subname));
7225+
output.push(QuoteUtils.quoteIdentifier(node.subname));
72267226
}
72277227

72287228
output.push('CONNECTION');
@@ -7251,7 +7251,7 @@ export class Deparser implements DeparserVisitor {
72517251
const output: string[] = ['ALTER', 'PUBLICATION'];
72527252

72537253
if (node.pubname) {
7254-
output.push(QuoteUtils.quoteString(node.pubname));
7254+
output.push(QuoteUtils.quoteIdentifier(node.pubname));
72557255
}
72567256

72577257
if (node.action) {
@@ -7291,7 +7291,7 @@ export class Deparser implements DeparserVisitor {
72917291
const output: string[] = ['ALTER', 'SUBSCRIPTION'];
72927292

72937293
if (node.subname) {
7294-
output.push(QuoteUtils.quoteString(node.subname));
7294+
output.push(QuoteUtils.quoteIdentifier(node.subname));
72957295
}
72967296

72977297
if (node.kind) {
@@ -7343,7 +7343,7 @@ export class Deparser implements DeparserVisitor {
73437343
}
73447344

73457345
if (node.subname) {
7346-
output.push(QuoteUtils.quoteString(node.subname));
7346+
output.push(QuoteUtils.quoteIdentifier(node.subname));
73477347
}
73487348

73497349
if (node.behavior) {
@@ -7954,7 +7954,7 @@ export class Deparser implements DeparserVisitor {
79547954
output.push(this.RangeVar(node.relation, context));
79557955

79567956
if (node.indexname) {
7957-
output.push('USING', QuoteUtils.quoteString(node.indexname));
7957+
output.push('USING', QuoteUtils.quoteIdentifier(node.indexname));
79587958
}
79597959
}
79607960

@@ -8033,7 +8033,7 @@ export class Deparser implements DeparserVisitor {
80338033
}
80348034

80358035
if (node.name) {
8036-
output.push(QuoteUtils.quoteString(node.name));
8036+
output.push(QuoteUtils.quoteIdentifier(node.name));
80378037
}
80388038

80398039
return output.join(' ');
@@ -8080,7 +8080,7 @@ export class Deparser implements DeparserVisitor {
80808080
throw new Error('CreatedbStmt requires dbname');
80818081
}
80828082

8083-
output.push(QuoteUtils.quoteString(node.dbname));
8083+
output.push(QuoteUtils.quoteIdentifier(node.dbname));
80848084

80858085
if (node.options && node.options.length > 0) {
80868086
const options = ListUtils.unwrapList(node.options)
@@ -8103,7 +8103,7 @@ export class Deparser implements DeparserVisitor {
81038103
throw new Error('DropdbStmt requires dbname');
81048104
}
81058105

8106-
output.push(QuoteUtils.quoteString(node.dbname));
8106+
output.push(QuoteUtils.quoteIdentifier(node.dbname));
81078107

81088108
if (node.options && node.options.length > 0) {
81098109
const options = ListUtils.unwrapList(node.options)
@@ -8300,15 +8300,15 @@ export class Deparser implements DeparserVisitor {
83008300
}
83018301

83028302
if (node.renameType === 'OBJECT_COLUMN' && node.subname) {
8303-
output.push('RENAME COLUMN', QuoteUtils.quoteString(node.subname), 'TO');
8303+
output.push('RENAME COLUMN', QuoteUtils.quoteIdentifier(node.subname), 'TO');
83048304
} else if (node.renameType === 'OBJECT_DOMCONSTRAINT' && node.subname) {
8305-
output.push('RENAME CONSTRAINT', QuoteUtils.quoteString(node.subname), 'TO');
8305+
output.push('RENAME CONSTRAINT', QuoteUtils.quoteIdentifier(node.subname), 'TO');
83068306
} else if (node.renameType === 'OBJECT_TABCONSTRAINT' && node.subname) {
8307-
output.push('RENAME CONSTRAINT', QuoteUtils.quoteString(node.subname), 'TO');
8307+
output.push('RENAME CONSTRAINT', QuoteUtils.quoteIdentifier(node.subname), 'TO');
83088308
} else if (node.renameType === 'OBJECT_ATTRIBUTE' && node.subname) {
8309-
output.push('RENAME ATTRIBUTE', QuoteUtils.quoteString(node.subname), 'TO');
8309+
output.push('RENAME ATTRIBUTE', QuoteUtils.quoteIdentifier(node.subname), 'TO');
83108310
} else if (node.renameType === 'OBJECT_ROLE' && node.subname) {
8311-
output.push(QuoteUtils.quoteString(node.subname), 'RENAME TO');
8311+
output.push(QuoteUtils.quoteIdentifier(node.subname), 'RENAME TO');
83128312
} else if (node.renameType === 'OBJECT_SCHEMA' && node.subname) {
83138313
output.push(this.quoteIfNeeded(node.subname), 'RENAME TO');
83148314
} else if (node.renameType === 'OBJECT_RULE') {
@@ -8640,7 +8640,7 @@ export class Deparser implements DeparserVisitor {
86408640
const output: string[] = ['SECURITY LABEL'];
86418641

86428642
if (node.provider) {
8643-
output.push('FOR', QuoteUtils.quoteString(node.provider));
8643+
output.push('FOR', QuoteUtils.quoteIdentifier(node.provider));
86448644
}
86458645

86468646
output.push('ON');
@@ -9810,12 +9810,8 @@ export class Deparser implements DeparserVisitor {
98109810
const defValue = defElem.arg;
98119811

98129812
if (defName && defValue) {
9813-
let preservedDefName;
9814-
if (QuoteUtils.needsQuotesForString(defName)) {
9815-
preservedDefName = QuoteUtils.quoteString(defName);
9816-
} else {
9817-
preservedDefName = this.preserveOperatorDefElemCase(defName);
9818-
}
9813+
const quotedDefName = QuoteUtils.quoteIdentifier(defName);
9814+
const preservedDefName = quotedDefName !== defName ? quotedDefName : this.preserveOperatorDefElemCase(defName);
98199815

98209816
if ((defName.toLowerCase() === 'commutator' || defName.toLowerCase() === 'negator') && defValue.List) {
98219817
const listItems = ListUtils.unwrapList(defValue.List.items);
@@ -9831,7 +9827,7 @@ export class Deparser implements DeparserVisitor {
98319827
} else if (defName && !defValue) {
98329828
// Handle boolean flags like HASHES, MERGES - preserve original case
98339829
if (defName === 'Hashes' || defName === 'Merges') {
9834-
return QuoteUtils.quoteString(defName);
9830+
return QuoteUtils.quoteIdentifier(defName);
98359831
}
98369832
return this.preserveOperatorDefElemCase(defName).toUpperCase();
98379833
}
@@ -9974,12 +9970,8 @@ export class Deparser implements DeparserVisitor {
99749970
const defValue = defElem.arg;
99759971

99769972
if (defName && defValue) {
9977-
let preservedDefName;
9978-
if (QuoteUtils.needsQuotesForString(defName)) {
9979-
preservedDefName = QuoteUtils.quoteString(defName);
9980-
} else {
9981-
preservedDefName = defName;
9982-
}
9973+
const quotedDefName = QuoteUtils.quoteIdentifier(defName);
9974+
const preservedDefName = quotedDefName !== defName ? quotedDefName : defName;
99839975

99849976
// Handle String arguments with single quotes for string literals
99859977
if (defValue.String) {

‎packages/deparser/src/utils/quote-utils.ts‎

Lines changed: 1 addition & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,6 @@
1-
import { RESERVED_KEYWORDS, TYPE_FUNC_NAME_KEYWORDS, UNRESERVED_KEYWORDS, keywordKindOf } from '../kwlist';
1+
import { RESERVED_KEYWORDS, TYPE_FUNC_NAME_KEYWORDS, keywordKindOf } from '../kwlist';
22

33
export class QuoteUtils {
4-
/**
5-
* Checks if a value needs quoting for use in String nodes, DefElem, role names, etc.
6-
* Uses a different algorithm than needsQuotes - checks for mixed case and special characters.
7-
* This was previously Deparser.needsQuotes.
8-
*/
9-
static needsQuotesForString(value: string): boolean {
10-
if (!value) return false;
11-
12-
const needsQuotesRegex = /[a-z]+[\W\w]*[A-Z]+|[A-Z]+[\W\w]*[a-z]+|\W/;
13-
const isAllUppercase = /^[A-Z]+$/.test(value);
14-
15-
return needsQuotesRegex.test(value) ||
16-
RESERVED_KEYWORDS.has(value.toLowerCase()) ||
17-
isAllUppercase;
18-
}
19-
20-
/**
21-
* Quotes a string value if it needs quoting for String nodes.
22-
* Uses needsQuotesForString logic.
23-
*/
24-
static quoteString(value: string): string {
25-
if (QuoteUtils.needsQuotesForString(value)) {
26-
return `"${value}"`;
27-
}
28-
return value;
29-
}
30-
314
static needsQuotes(value: string): boolean {
325
if (!value || typeof value !== 'string') {
336
return false;

0 commit comments

Comments
 (0)