Skip to content

Commit 4928637

Browse files
authored
fix(typegen): prevent duplication of generator-owned methods (readValidator/writeValidator) for Edm.Boolean fields during regeneration (#119)
<!-- CURSOR_SUMMARY --> > [!NOTE] > **Medium Risk** > Touches code generation logic that parses and rewrites method chains; mistakes could drop or misclassify user customizations in regenerated schemas, though new e2e coverage reduces risk. > > **Overview** > Prevents type regeneration from re-appending generator-owned chain methods (notably `readValidator`/`writeValidator` used for `Edm.Boolean`) when preserving existing field customizations. > > This extends customization extraction to accept additional “standard” method names derived by parsing the generated `fieldBuilder` chain, and adds e2e tests asserting validators are not duplicated and a user-level `.transform()` is still preserved. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 8203a33. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 6c6b569 commit 4928637

File tree

3 files changed

+209
-4
lines changed

3 files changed

+209
-4
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@proofkit/typegen": patch
3+
---
4+
5+
Fix typegen duplicating readValidator/writeValidator on Edm.Boolean fields during regeneration

packages/typegen/src/fmodata/generateODataTypes.ts

Lines changed: 108 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,7 @@ function generateTableOccurrence(
315315

316316
// Preserve user customizations from existing field
317317
if (matchedExistingField) {
318-
line = preserveUserCustomizations(matchedExistingField, line);
318+
line = preserveUserCustomizations(matchedExistingField, line, fieldBuilder);
319319
}
320320

321321
// Add comma if not the last field
@@ -435,7 +435,11 @@ interface ParsedTableOccurrence {
435435
/**
436436
* Extracts user customizations (like .inputValidator() and .outputValidator()) from a method chain
437437
*/
438-
function extractUserCustomizations(chainText: string, baseChainEnd: number): string {
438+
function extractUserCustomizations(
439+
chainText: string,
440+
baseChainEnd: number,
441+
additionalStandardMethods?: Set<string>,
442+
): string {
439443
// We want to preserve user-added chained calls even if they were placed:
440444
// - before a standard method (e.g. textField().inputValidator(...).entityId(...))
441445
// - on fields that have no standard methods at all (possible when reduceMetadata is true)
@@ -447,6 +451,11 @@ function extractUserCustomizations(chainText: string, baseChainEnd: number): str
447451
// that can be appended to the regenerated chain.
448452

449453
const standardMethodNames = new Set(["primaryKey", "readOnly", "notNull", "entityId", "comment"]);
454+
if (additionalStandardMethods) {
455+
for (const m of additionalStandardMethods) {
456+
standardMethodNames.add(m);
457+
}
458+
}
450459

451460
const start = Math.max(0, Math.min(baseChainEnd, chainText.length));
452461
const tail = chainText.slice(start);
@@ -824,10 +833,102 @@ function matchFieldByName(existingFields: Map<string, ParsedField>, fieldName: s
824833
return existingFields.get(fieldName) || null;
825834
}
826835

836+
/**
837+
* Extracts method names called in a chain expression.
838+
* e.g. "numberField().readValidator(...).writeValidator(...)" => {"readValidator", "writeValidator"}
839+
*/
840+
function extractMethodNamesFromChain(chain: string): Set<string> {
841+
const names = new Set<string>();
842+
let depth = 0;
843+
let i = 0;
844+
845+
function skipStringLiteral(quote: string): void {
846+
i++; // skip opening quote
847+
while (i < chain.length) {
848+
if (chain[i] === "\\") {
849+
i += 2;
850+
continue;
851+
}
852+
if (chain[i] === quote) {
853+
i++;
854+
break;
855+
}
856+
i++;
857+
}
858+
}
859+
860+
while (i < chain.length) {
861+
const ch = chain[i] ?? "";
862+
// Skip string literals (handles quotes inside parens correctly)
863+
if (ch === "'" || ch === '"' || ch === "`") {
864+
skipStringLiteral(ch);
865+
continue;
866+
}
867+
if (ch === "(") {
868+
depth++;
869+
i++;
870+
continue;
871+
}
872+
if (ch === ")") {
873+
depth--;
874+
i++;
875+
continue;
876+
}
877+
// Only match `.methodName(` at the top-level chain (depth 0)
878+
if (ch === "." && depth === 0) {
879+
i++;
880+
const nameStart = i;
881+
while (i < chain.length && REGEX_IDENT_CHAR.test(chain[i] ?? "")) {
882+
i++;
883+
}
884+
if (i > nameStart) {
885+
const name = chain.slice(nameStart, i);
886+
while (i < chain.length && REGEX_WHITESPACE.test(chain[i] ?? "")) {
887+
i++;
888+
}
889+
// Skip optional generic type args <...>
890+
if (i < chain.length && chain[i] === "<") {
891+
let angleDepth = 0;
892+
while (i < chain.length) {
893+
const c = chain[i] ?? "";
894+
if (c === "'" || c === '"' || c === "`") {
895+
skipStringLiteral(c);
896+
continue;
897+
}
898+
if (c === "<") {
899+
angleDepth++;
900+
} else if (c === ">") {
901+
angleDepth--;
902+
if (angleDepth === 0) {
903+
i++;
904+
break;
905+
}
906+
}
907+
i++;
908+
}
909+
while (i < chain.length && REGEX_WHITESPACE.test(chain[i] ?? "")) {
910+
i++;
911+
}
912+
}
913+
if (i < chain.length && chain[i] === "(") {
914+
names.add(name);
915+
}
916+
}
917+
continue;
918+
}
919+
i++;
920+
}
921+
return names;
922+
}
923+
827924
/**
828925
* Preserves user customizations from an existing field chain
829926
*/
830-
function preserveUserCustomizations(existingField: ParsedField | undefined, newChain: string): string {
927+
function preserveUserCustomizations(
928+
existingField: ParsedField | undefined,
929+
newChain: string,
930+
fieldBuilder: string,
931+
): string {
831932
if (!existingField) {
832933
return newChain;
833934
}
@@ -848,7 +949,10 @@ function preserveUserCustomizations(existingField: ParsedField | undefined, newC
848949
const existingChainText = existingField.fullChainText;
849950
const existingBaseEnd = existingChainText.startsWith(baseBuilderPrefix) ? baseBuilderPrefix.length : 0;
850951

851-
const userCustomizations = extractUserCustomizations(existingChainText, existingBaseEnd);
952+
// Methods in the generated field builder (e.g. readValidator, writeValidator for
953+
// Edm.Boolean) are generator-owned and must not be duplicated as user customizations.
954+
const generatorMethods = extractMethodNamesFromChain(fieldBuilder);
955+
const userCustomizations = extractUserCustomizations(existingChainText, existingBaseEnd, generatorMethods);
852956

853957
if (!userCustomizations) {
854958
return newChain;

packages/typegen/tests/e2e/fmodata-preserve-customizations.test.ts

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,102 @@ describe("fmodata generateODataTypes preserves user customizations", () => {
129129
}
130130
});
131131

132+
it("does not duplicate generator-owned methods for Edm.Boolean fields", async () => {
133+
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "proofkit-fmodata-preserve-"));
134+
135+
try {
136+
const entitySetName = "MyTable";
137+
const entityTypeName = "NS.MyTable";
138+
const metadata = makeMetadata({
139+
entitySetName,
140+
entityTypeName,
141+
fields: [{ name: "is_active", type: "Edm.Boolean", fieldId: "F1" }],
142+
});
143+
144+
const existingFilePath = path.join(tmpDir, "MyTable.ts");
145+
await fs.writeFile(
146+
existingFilePath,
147+
[
148+
`import { fmTableOccurrence, numberField } from "@proofkit/fmodata";`,
149+
`import { z } from "zod/v4";`,
150+
"",
151+
`export const MyTable = fmTableOccurrence("MyTable", {`,
152+
` is_active: numberField().readValidator(z.coerce.boolean()).writeValidator(z.boolean().transform((v) => (v ? 1 : 0))).entityId("F1"),`,
153+
"}, {",
154+
` entityId: "T1",`,
155+
"});",
156+
"",
157+
].join("\n"),
158+
"utf8",
159+
);
160+
161+
await generateODataTypes(metadata, {
162+
type: "fmodata",
163+
path: tmpDir,
164+
clearOldFiles: false,
165+
tables: [{ tableName: "MyTable" }],
166+
});
167+
168+
const regenerated = await fs.readFile(existingFilePath, "utf8");
169+
// readValidator and writeValidator should each appear exactly once
170+
const readValidatorCount = (regenerated.match(/readValidator/g) || []).length;
171+
const writeValidatorCount = (regenerated.match(/writeValidator/g) || []).length;
172+
expect(readValidatorCount).toBe(1);
173+
expect(writeValidatorCount).toBe(1);
174+
} finally {
175+
await fs.rm(tmpDir, { recursive: true, force: true });
176+
}
177+
});
178+
179+
it("preserves user .transform() on Edm.Boolean fields (not confused with nested .transform)", async () => {
180+
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "proofkit-fmodata-preserve-"));
181+
182+
try {
183+
const entitySetName = "MyTable";
184+
const entityTypeName = "NS.MyTable";
185+
const metadata = makeMetadata({
186+
entitySetName,
187+
entityTypeName,
188+
fields: [{ name: "is_active", type: "Edm.Boolean", fieldId: "F1" }],
189+
});
190+
191+
const existingFilePath = path.join(tmpDir, "MyTable.ts");
192+
await fs.writeFile(
193+
existingFilePath,
194+
[
195+
`import { fmTableOccurrence, numberField } from "@proofkit/fmodata";`,
196+
`import { z } from "zod/v4";`,
197+
"",
198+
`export const MyTable = fmTableOccurrence("MyTable", {`,
199+
` is_active: numberField().readValidator(z.coerce.boolean()).writeValidator(z.boolean().transform((v) => (v ? 1 : 0))).entityId("F1").transform((v) => !!v),`,
200+
"}, {",
201+
` entityId: "T1",`,
202+
"});",
203+
"",
204+
].join("\n"),
205+
"utf8",
206+
);
207+
208+
await generateODataTypes(metadata, {
209+
type: "fmodata",
210+
path: tmpDir,
211+
clearOldFiles: false,
212+
tables: [{ tableName: "MyTable" }],
213+
});
214+
215+
const regenerated = await fs.readFile(existingFilePath, "utf8");
216+
// The user-added .transform() should be preserved
217+
expect(regenerated).toContain(".transform((v) => !!v)");
218+
// readValidator and writeValidator should each appear exactly once
219+
const readValidatorCount = (regenerated.match(/readValidator/g) || []).length;
220+
const writeValidatorCount = (regenerated.match(/writeValidator/g) || []).length;
221+
expect(readValidatorCount).toBe(1);
222+
expect(writeValidatorCount).toBe(1);
223+
} finally {
224+
await fs.rm(tmpDir, { recursive: true, force: true });
225+
}
226+
});
227+
132228
it("preserves aliased imports when regenerating files", async () => {
133229
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "proofkit-fmodata-preserve-"));
134230

0 commit comments

Comments
 (0)