Skip to content

Commit 886727c

Browse files
Copilotmarkcowltimotheeguerin
authored
fix ICE in serializeValueAsJson (#9804)
- [x] Add test for known scalar constructors as ModelProperty default values in openapi3 - [x] Fix serializeValueAsJson to return undefined for unrecognized scalar constructors - [x] Remove inadvertent csharp emitter file changes (reverted to pre-PR state) <!-- START COPILOT ORIGINAL PROMPT --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>[core] ICE in serializeValueAsJson</issue_title> > <issue_description>### Describe the bug > > The core library throws an uncaught Error on a custom scalar initializer with no arguments in `serializeValueAsJson`. The Error is provoked by the OpenAPI3 emitter (as well as the JSON Schema emitter) and bubbles back up to the emitter entrypoint where it triggers an ICE. > > The logic attempts to use the first argument of the initializer, which isn't reliable. That logic works for "cast-like" scalar values (e.g. `uuid("...")`), but not for initializers (e.g. `uuid.random() // no first argument!`). > > ### Reproduction > > ```tsp > scalar S { > init i(); > } > > model M { p: S = S.i(); } > ``` > > https://typespec.io/playground/?e=%40typespec%2Fopenapi3&c=c2NhbGFyIFMgewogIGluaXQgaSgpOwp9Cgptb2RlbCBNIHsgcDogUyA9IFMuxBsgfQ%3D%3D&options=%7B%7D&vs=%7B%7D > > However, if you provide a first argument, this is enough to quell the ICE but the emit is still wrong: > > ```tsp > scalar S { > init i(s: string); > } > > // Default value renders as "foo", which is wrong. > model M { p: S = S.i("foo") } > ``` > > https://typespec.io/playground/?e=%40typespec%2Fopenapi3&c=c2NhbGFyIFMgewogIGluaXQgaShzOiBzdHJpbmcpOwp9Cgptb2RlbCBNIHsgcDogUyA9IFMuaSgiZm9vIik7IH0%3D&options=%7B%7D&vs=%7B%7D > > ### Checklist > > - [x] Follow our [Code of Conduct](https://github.com/microsoft/typespec/blob/main/CODE_OF_CONDUCT.md) > - [x] Check that there isn't already an issue that request the same bug to avoid creating a duplicate. > - [x] Check that this is a concrete bug. For Q&A open a [GitHub Discussion](https://github.com/Microsoft/typespec/discussions). > - [x] The provided reproduction is a [minimal reproducible example](https://stackoverflow.com/help/minimal-reproducible-example) of the bug.</issue_description> > > <agent_instructions>Instead we should drop the result if we don't recognize the constructor</agent_instructions> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > <comment_new><author>@timotheeguerin</author><body> > Drop the result like we do with `now()` if we can't resolve the scalar constructor</body></comment_new> > </comments> > </details> <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes #9781 <!-- START COPILOT CODING AGENT TIPS --> --- 🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. [Learn more about Advanced Security.](https://gh.io/cca-advanced-security) --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: markcowl <1054056+markcowl@users.noreply.github.com> Co-authored-by: Timothee Guerin <tiguerin@microsoft.com>
1 parent 6ca470a commit 886727c

File tree

4 files changed

+62
-3
lines changed

4 files changed

+62
-3
lines changed
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
changeKind: fix
3+
packages:
4+
- "@typespec/compiler"
5+
---
6+
7+
Fix crash when using custom scalar initializer in examples or default values
8+
[API] Fix crash in `serializeValueAsJson` when a custom scalar initializer has no recognized constructor (e.g. `S.i()` with no args). Now returns `undefined` instead of crashing.
9+

packages/compiler/src/lib/examples.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ function serializeScalarValueAsJson(
229229

230230
const result = resolveKnownScalar(program, value.scalar);
231231
if (result === undefined) {
232-
return serializeValueAsJson(program, value.value.args[0], value.value.args[0].type);
232+
return undefined;
233233
}
234234

235235
encodeAs = encodeAs ?? result.encodeAs;

packages/compiler/test/decorators/examples.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,28 @@ describe("@example", () => {
130130
code: "unassignable",
131131
});
132132
});
133+
134+
it("returns undefined for custom scalar with no-argument initializer", async () => {
135+
const { program, examples, target } = await getExamplesFor(`
136+
@example(test.i())
137+
@test scalar test {
138+
init i();
139+
}
140+
`);
141+
expect(examples).toHaveLength(1);
142+
expect(serializeValueAsJson(program, examples[0].value, target)).toBeUndefined();
143+
});
144+
145+
it("returns undefined for custom scalar with string-argument initializer", async () => {
146+
const { program, examples, target } = await getExamplesFor(`
147+
@example(test.name("Shorty"))
148+
@test scalar test {
149+
init name(value: string);
150+
}
151+
`);
152+
expect(examples).toHaveLength(1);
153+
expect(serializeValueAsJson(program, examples[0].value, target)).toBeUndefined();
154+
});
133155
});
134156

135157
describe("enum", () => {

packages/openapi3/test/models.test.ts

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ worksFor(supportedVersions, ({ diagnoseOpenApiFor, oapiForModel, openApiFor }) =
262262
});
263263
});
264264

265-
it("scalar used as a default value", async () => {
265+
it("scalar with unknown constructor used as a default value produces no default and no diagnostic", async () => {
266266
const res = await oapiForModel(
267267
"Pet",
268268
`
@@ -272,7 +272,35 @@ worksFor(supportedVersions, ({ diagnoseOpenApiFor, oapiForModel, openApiFor }) =
272272
`,
273273
);
274274

275-
expect(res.schemas.Pet.properties.name.default).toEqual("Shorty");
275+
expect(res.schemas.Pet.properties.name.default).toBeUndefined();
276+
});
277+
278+
it("scalar with no-argument initializer used as a default value does not crash", async () => {
279+
const res = await oapiForModel(
280+
"M",
281+
`
282+
scalar S { init i(); }
283+
284+
model M { p: S = S.i(); }
285+
`,
286+
);
287+
288+
expect(res.schemas.M.properties.p.default).toBeUndefined();
289+
});
290+
291+
it("known scalar constructors used as default values produce correct defaults", async () => {
292+
const res = await oapiForModel(
293+
"Foo",
294+
`
295+
model Foo {
296+
int32Prop: int32 = int32(12);
297+
stringProp: string = string("this is the string value");
298+
}
299+
`,
300+
);
301+
302+
expect(res.schemas.Foo.properties.int32Prop.default).toEqual(12);
303+
expect(res.schemas.Foo.properties.stringProp.default).toEqual("this is the string value");
276304
});
277305

278306
it("encode know scalar as a default value", async () => {

0 commit comments

Comments
 (0)