From a1d5986875a826d464c14e9d37bb83bfdeb390ea Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Mon, 3 Oct 2022 11:14:35 -0700 Subject: [PATCH 01/12] Fix parent type references --- packages/compiler/core/checker.ts | 5 +- packages/compiler/core/projector.ts | 27 +++- packages/compiler/test/checker/model.test.ts | 116 ++++++++++-------- .../compiler/test/checker/projection.test.ts | 71 ++++++++++- 4 files changed, 159 insertions(+), 60 deletions(-) diff --git a/packages/compiler/core/checker.ts b/packages/compiler/core/checker.ts index 26db723ddf..6c271f200e 100644 --- a/packages/compiler/core/checker.ts +++ b/packages/compiler/core/checker.ts @@ -1921,8 +1921,9 @@ export function createChecker(program: Program): Checker { for (const prop of isBase.properties.values()) { type.properties.set( prop.name, - finishType({ - ...prop, + cloneType(prop, { + sourceProperty: prop, + model: type, }) ); } diff --git a/packages/compiler/core/projector.ts b/packages/compiler/core/projector.ts index 831dad9ac5..aa3b16f966 100644 --- a/packages/compiler/core/projector.ts +++ b/packages/compiler/core/projector.ts @@ -6,6 +6,7 @@ import { getParentTemplateNode, isNeverIndexer, isProjectedProgram, + isTemplateDeclaration, isTemplateInstance, ProjectedProgram, } from "./index.js"; @@ -163,7 +164,7 @@ export function createProjector( } } } - function projectNamespace(ns: Namespace, projectSubNamespace: boolean = true): Type { + function projectNamespace(ns: Namespace, projectSubNamespace: boolean = true): Namespace { const alreadyProjected = projectedTypes.get(ns) as Namespace; if (alreadyProjected) { if (projectSubNamespace) { @@ -201,7 +202,7 @@ export function createProjector( } projectedNamespaces.push(ns); - return applyProjection(ns, projectedNs); + return applyProjection(ns, projectedNs) as Namespace; } /** @@ -255,6 +256,10 @@ export function createProjector( } function projectModel(model: Model): Type { + if (model.name === "Foo") { + console.log("Project model", model.name, isTemplateDeclaration(model)); + console.log(""); + } const properties = new Map(); let templateArguments: Type[] | undefined; @@ -279,7 +284,7 @@ export function createProjector( projectedModel.indexer = { key: neverType, value: undefined }; } else { projectedModel.indexer = { - key: projectModel(model.indexer.key), + key: projectModel(model.indexer.key) as Model, value: projectType(model.indexer.value), }; } @@ -324,11 +329,21 @@ export function createProjector( const projectedType = projectType(prop.type); const projectedDecs = projectDecorators(prop.decorators); - const projectedProp = shallowClone(prop, { + const projectedProp: ModelProperty = shallowClone(prop, { type: projectedType, decorators: projectedDecs, }); + if (prop.model) { + const parentModel = projectType(prop.model) as Model; + projectedProp.model = parentModel; + } + + if (prop.sourceProperty) { + const sourceProperty = projectType(prop.sourceProperty) as ModelProperty; + projectedProp.sourceProperty = sourceProperty; + } + if (shouldFinishType(prop)) { finishTypeForProgram(projectedProgram, projectedProp); } @@ -410,6 +425,8 @@ export function createProjector( decorators: projectedDecs, }); + const parentUnion = projectType(variant.union) as Union; + projectedVariant.union = parentUnion; finishTypeForProgram(projectedProgram, projectedVariant); return projectedVariant; } @@ -557,7 +574,7 @@ export function createProjector( return projectedType; } - function shallowClone(type: T, additionalProps: Partial) { + function shallowClone(type: T, additionalProps: Partial): T { const scopeProps: any = {}; if ("namespace" in type && type.namespace !== undefined) { scopeProps.namespace = projectedNamespaceScope(); diff --git a/packages/compiler/test/checker/model.test.ts b/packages/compiler/test/checker/model.test.ts index 31cc921db6..90f0701ead 100644 --- a/packages/compiler/test/checker/model.test.ts +++ b/packages/compiler/test/checker/model.test.ts @@ -162,65 +162,81 @@ describe("compiler: models", () => { ]); }); - it("provides parent model of properties", async () => { - testHost.addCadlFile( - "main.cadl", - ` - @test + describe("link model with its properties", () => { + it("provides parent model of properties", async () => { + testHost.addCadlFile( + "main.cadl", + ` + @test + model A { + pA: int32; + } + + @test + model B { + pB: int32; + + } + ` + ); + + const { A, B } = (await testHost.compile("./")) as { A: Model; B: Model }; + + strictEqual(A.properties.get("pA")?.model, A); + strictEqual(B.properties.get("pB")?.model, B); + }); + + it("property merged via intersection", async () => { + testHost.addCadlFile( + "main.cadl", + ` model A { - pA: int32; + a: string; } - - @test model B { - pB: int32; - + b: string; } - @test - model C { - pC: int32; - } + @test model Test {prop: A & B} + ` + ); + const { Test } = (await testHost.compile("main.cadl")) as { Test: Model }; + const AB = Test.properties.get("prop")?.type; + + strictEqual(AB?.kind, "Model" as const); + strictEqual(AB.properties.get("a")?.model, AB); + strictEqual(AB.properties.get("b")?.model, AB); + }); - @test - model D { - ...A, - pD: B & C; + it("property copied via spread", async () => { + testHost.addCadlFile( + "main.cadl", + ` + model Foo { + prop: string; } + + @test model Test {...Foo} ` - ); + ); + const { Test } = (await testHost.compile("main.cadl")) as { Test: Model }; + strictEqual(Test.properties.get("prop")?.model, Test); + }); + + it("property copied via `is`", async () => { + testHost.addCadlFile( + "main.cadl", + ` + model Foo { + prop: string; + } - const { A, B, C, D } = await testHost.compile("./"); - - strictEqual(A.kind, "Model" as const); - strictEqual(A.properties.size, 1); - const pA = A.properties.get("pA"); - strictEqual(pA?.model, A); - - strictEqual(B.kind, "Model" as const); - strictEqual(B.properties.size, 1); - const pB = B.properties.get("pB"); - strictEqual(pB?.model, B); - - strictEqual(C.kind, "Model" as const); - strictEqual(C.properties.size, 1); - const pC = C.properties.get("pC"); - strictEqual(pC?.model, C); - - strictEqual(D.kind, "Model" as const); - strictEqual(D.properties.size, 2); - const pA_of_D = D.properties.get("pA"); - const pD = D.properties.get("pD"); - strictEqual(pA_of_D?.model, D); - strictEqual(pD?.model, D); - - const BC = pD.type; - strictEqual(BC.kind, "Model" as const); - strictEqual(BC.properties.size, 2); - const pB_of_BC = BC.properties.get("pB"); - const pC_of_BC = BC.properties.get("pC"); - strictEqual(pB_of_BC?.model, BC); - strictEqual(pC_of_BC?.model, BC); + @test model Test is Foo; + ` + ); + const { Test } = (await testHost.compile("main.cadl")) as { Test: Model }; + strictEqual(Test.properties.get("prop")?.model, Test); + }); }); describe("with extends", () => { diff --git a/packages/compiler/test/checker/projection.test.ts b/packages/compiler/test/checker/projection.test.ts index f1adf0a91a..cb2b175e6a 100644 --- a/packages/compiler/test/checker/projection.test.ts +++ b/packages/compiler/test/checker/projection.test.ts @@ -212,6 +212,37 @@ describe("compiler: projections", () => { }); describe("models", () => { + it("link projected model to projected properties", async () => { + const code = ` + @test model Foo { + name: string; + } + #suppress "projections-are-experimental" + projection model#test {to {}}`; + const result = (await testProjection(code)) as Model; + ok(result.projectionBase); + strictEqual(result.properties.get("name")?.model, result); + }); + + it("link projected property with sourceProperty", async () => { + const code = ` + model Bar { + name: string; + } + @test model Foo { + ...Bar + } + #suppress "projections-are-experimental" + projection model#test {to {}}`; + const Foo = (await testProjection(code)) as Model; + ok(Foo.projectionBase); + ok(Foo.properties.get("name")?.sourceProperty); + strictEqual( + Foo.properties.get("name")?.sourceProperty, + Foo.namespace!.models.get("Bar")?.properties.get("name") + ); + }); + it("works for versioning", async () => { const addedOnKey = Symbol("addedOn"); const removedOnKey = Symbol("removedOn"); @@ -512,6 +543,18 @@ describe("compiler: projections", () => { strictEqual((variant.type as Model).name, typeName); } + it("link projected model to projected properties", async () => { + const code = ` + @test union Foo { + one: {}; + } + #suppress "projections-are-experimental" + projection model#test {to {}}`; + const result = (await testProjection(code)) as Union; + ok(result.projectionBase); + strictEqual(result.variants.get("one")?.union, result); + }); + it("can rename itself", async () => { const code = ` ${unionCode} @@ -638,6 +681,18 @@ describe("compiler: projections", () => { ${projectionCode(body)} `; + it("link projected interfaces to its projected operations", async () => { + const code = ` + @test interface Foo { + op test(): string; + } + #suppress "projections-are-experimental" + projection interface#test {to {}}`; + const result = (await testProjection(code)) as Interface; + ok(result.projectionBase); + strictEqual(result.operations.get("test")?.interface, result); + }); + it("can rename itself", async () => { const code = ` ${interfaceCode} @@ -692,6 +747,14 @@ describe("compiler: projections", () => { ${projectionCode(body)} `; + it("link projected enum to projected members", async () => { + const code = defaultCode(""); + const result = (await testProjection(code)) as Enum; + ok(result.projectionBase); + strictEqual(result.members.get("one")?.enum, result); + strictEqual(result.members.get("two")?.enum, result); + }); + it("can rename itself", async () => { const code = ` ${enumCode} @@ -852,7 +915,9 @@ describe("compiler: projections", () => { let run = 0; testHost.addJsFile("mark.js", { - $mark: () => run++, + $mark: () => { + run++; + }, }); testHost.addCadlFile( @@ -889,7 +954,7 @@ describe("compiler: projections", () => { prop: string; } - model Instance is Foo; + model Instance {prop: Foo}; `, 1 ); @@ -902,7 +967,7 @@ describe("compiler: projections", () => { @mark(T) prop: string; } - model Instance is Foo; + model Instance {prop: Foo}; `, 1 ); From 42678c6ab29eb28e94283c44dc571da5251d92fa Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Mon, 3 Oct 2022 11:19:02 -0700 Subject: [PATCH 02/12] Changelog --- ...ctions-parent-source_2022-10-03-18-15.json | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 common/changes/@cadl-lang/compiler/fix-projections-parent-source_2022-10-03-18-15.json diff --git a/common/changes/@cadl-lang/compiler/fix-projections-parent-source_2022-10-03-18-15.json b/common/changes/@cadl-lang/compiler/fix-projections-parent-source_2022-10-03-18-15.json new file mode 100644 index 0000000000..93ff48922b --- /dev/null +++ b/common/changes/@cadl-lang/compiler/fix-projections-parent-source_2022-10-03-18-15.json @@ -0,0 +1,20 @@ +{ + "changes": [ + { + "packageName": "@cadl-lang/compiler", + "comment": "Fix: Property included via `model is` were not referencing the right model parent.", + "type": "patch" + }, + { + "packageName": "@cadl-lang/compiler", + "comment": "Fix: Projected types point to projected parent type for Model properties, Union variants.", + "type": "patch" + }, + { + "packageName": "@cadl-lang/compiler", + "comment": "Fix: Projected model property sourceProperty point to projected property", + "type": "patch" + } + ], + "packageName": "@cadl-lang/compiler" +} From 08ea0b97cf5835488a55c48f550fb8afb69fdc3c Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Mon, 3 Oct 2022 12:09:37 -0700 Subject: [PATCH 03/12] remove workaround for get effective property type --- packages/compiler/core/checker.ts | 18 ++---------------- packages/compiler/core/projector.ts | 5 ----- 2 files changed, 2 insertions(+), 21 deletions(-) diff --git a/packages/compiler/core/checker.ts b/packages/compiler/core/checker.ts index 6c271f200e..bb94155501 100644 --- a/packages/compiler/core/checker.ts +++ b/packages/compiler/core/checker.ts @@ -13,7 +13,6 @@ import { IntrinsicModelName, isIntrinsic, isNeverType, - isProjectedProgram, isUnknownType, isVoidType, JsSourceFileNode, @@ -4180,7 +4179,7 @@ export function getEffectiveModelType( if (model.name) { // named model - return getProjectedEffectiveModelType(program, model); + return model; } // We would need to change the algorithm if this doesn't hold. We @@ -4244,7 +4243,7 @@ export function getEffectiveModelType( } } - return match ? getProjectedEffectiveModelType(program, match) : model; + return match ? match : model; } /** @@ -4296,19 +4295,6 @@ export function filterModelProperties( return finishTypeForProgram(program, newModel); } -function getProjectedEffectiveModelType(program: Program | ProjectedProgram, type: Model): Model { - if (!isProjectedProgram(program)) { - return type; - } - - const projectedType = program.projector.projectType(type); - if (projectedType.kind !== "Model") { - compilerAssert(false, "Fail"); - } - - return projectedType; -} - export function* walkPropertiesInherited(model: Model) { let current: Model | undefined = model; diff --git a/packages/compiler/core/projector.ts b/packages/compiler/core/projector.ts index aa3b16f966..442fbdac79 100644 --- a/packages/compiler/core/projector.ts +++ b/packages/compiler/core/projector.ts @@ -6,7 +6,6 @@ import { getParentTemplateNode, isNeverIndexer, isProjectedProgram, - isTemplateDeclaration, isTemplateInstance, ProjectedProgram, } from "./index.js"; @@ -256,10 +255,6 @@ export function createProjector( } function projectModel(model: Model): Type { - if (model.name === "Foo") { - console.log("Project model", model.name, isTemplateDeclaration(model)); - console.log(""); - } const properties = new Map(); let templateArguments: Type[] | undefined; From c38e8dfe2aa3075621df1b5f67e157bfc66ce86c Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Mon, 3 Oct 2022 12:12:56 -0700 Subject: [PATCH 04/12] Update packages/compiler/core/checker.ts Co-authored-by: Nick Guerrera --- packages/compiler/core/checker.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/compiler/core/checker.ts b/packages/compiler/core/checker.ts index bb94155501..36441e9b79 100644 --- a/packages/compiler/core/checker.ts +++ b/packages/compiler/core/checker.ts @@ -4243,7 +4243,7 @@ export function getEffectiveModelType( } } - return match ? match : model; + return match ?? model; } /** From 2b32f9c500b1ca4d784c98f45ef5316d1f5951be Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Mon, 3 Oct 2022 12:18:04 -0700 Subject: [PATCH 05/12] remove toplevel workaround --- packages/rest/src/http/parameters.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/rest/src/http/parameters.ts b/packages/rest/src/http/parameters.ts index 3ef53a8a1a..aa9954568b 100644 --- a/packages/rest/src/http/parameters.ts +++ b/packages/rest/src/http/parameters.ts @@ -55,12 +55,7 @@ function getOperationParametersForVerb( ); function isImplicitPathParam(param: ModelProperty) { - // Only top-level parameters can be implicit path parameters. - // - // This check should be simpler: `param.model === operation.parameters`, - // but that is blocked by https://github.com/microsoft/cadl/issues/1069 - const isTopLevel = param === operation.parameters.properties.get(param.name); - + const isTopLevel = param.model === operation.parameters; return isTopLevel && knownPathParamNames.includes(param.name); } From 3123e7f8f75da4522dcdc65b5f9e67a7b8ebec8a Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Mon, 3 Oct 2022 14:04:51 -0700 Subject: [PATCH 06/12] Fix issue with order of execution --- packages/compiler/core/projector.ts | 38 +++++++++++--- .../compiler/test/checker/projection.test.ts | 52 +++++++++++++++---- 2 files changed, 73 insertions(+), 17 deletions(-) diff --git a/packages/compiler/core/projector.ts b/packages/compiler/core/projector.ts index 442fbdac79..a7fa526d88 100644 --- a/packages/compiler/core/projector.ts +++ b/packages/compiler/core/projector.ts @@ -288,7 +288,7 @@ export function createProjector( projectedTypes.set(model, projectedModel); for (const [key, prop] of model.properties) { - const projectedProp = projectType(prop); + const projectedProp = projectModelProperty(prop, projectedModel); if (projectedProp.kind === "ModelProperty") { properties.set(key, projectedProp); } @@ -320,18 +320,21 @@ export function createProjector( return !parentTemplate || isTemplateInstance(type); } - function projectModelProperty(prop: ModelProperty): Type { + function projectModelProperty(prop: ModelProperty, projectedModel?: Model): Type { + if (prop.model && projectedModel === undefined) { + return projectViaParent(prop, prop.model); + } + const projectedType = projectType(prop.type); const projectedDecs = projectDecorators(prop.decorators); - const projectedProp: ModelProperty = shallowClone(prop, { + const projectedProp = shallowClone(prop, { type: projectedType, decorators: projectedDecs, }); - if (prop.model) { - const parentModel = projectType(prop.model) as Model; - projectedProp.model = parentModel; + if (projectedModel) { + projectedProp.model = projectedModel; } if (prop.sourceProperty) { @@ -398,7 +401,7 @@ export function createProjector( }); for (const [key, variant] of union.variants) { - const projectedVariant = projectType(variant); + const projectedVariant = projectUnionVariant(variant, union); if (projectedVariant.kind === "UnionVariant" && projectedVariant.type !== neverType) { variants.set(key, projectedVariant); } @@ -411,7 +414,10 @@ export function createProjector( return applyProjection(union, projectedUnion); } - function projectUnionVariant(variant: UnionVariant) { + function projectUnionVariant(variant: UnionVariant, projectingUnion?: Union) { + if (projectingUnion === undefined) { + return projectViaParent(variant, variant.union); + } const projectedType = projectType(variant.type); const projectedDecs = projectDecorators(variant.decorators); @@ -599,4 +605,20 @@ export function createProjector( projectedTypes.set(type, clone); return clone; } + + /** + * Project the given type by projecting the parent type first. + * @param type Type to project. + * @param parentType Parent type that should be projected first. + * @returns Projected type + */ + function projectViaParent(type: Type, parentType: Type): Type { + projectType(parentType); + const projectedProp = projectedTypes.get(type); + compilerAssert( + projectedProp, + `Type "${program.checker.getTypeName(type)}" should have been projected by now.` + ); + return projectedProp; + } } diff --git a/packages/compiler/test/checker/projection.test.ts b/packages/compiler/test/checker/projection.test.ts index cb2b175e6a..e4a456287a 100644 --- a/packages/compiler/test/checker/projection.test.ts +++ b/packages/compiler/test/checker/projection.test.ts @@ -8,6 +8,7 @@ import { EnumMember, Interface, Model, + ModelProperty, Namespace, NumericLiteral, Operation, @@ -226,21 +227,54 @@ describe("compiler: projections", () => { it("link projected property with sourceProperty", async () => { const code = ` - model Bar { - name: string; - } @test model Foo { ...Bar } + + model Bar { + name: string; + } + #suppress "projections-are-experimental" - projection model#test {to {}}`; + projection Foo#test {to {}}`; const Foo = (await testProjection(code)) as Model; ok(Foo.projectionBase); - ok(Foo.properties.get("name")?.sourceProperty); - strictEqual( - Foo.properties.get("name")?.sourceProperty, - Foo.namespace!.models.get("Bar")?.properties.get("name") - ); + const sourceProperty = Foo.properties.get("name")?.sourceProperty; + ok(sourceProperty); + strictEqual(sourceProperty, Foo.namespace!.models.get("Bar")?.properties.get("name")); + strictEqual(sourceProperty.model, Foo.namespace!.models.get("Bar")); + }); + + it("project all properties first", async () => { + const keySym = Symbol("key"); + testHost.addJsFile("lib.js", { + $tagProp({ program }: DecoratorContext, t: ModelProperty) { + program.stateSet(keySym).add(t); + }, + $tagModel({ program }: DecoratorContext, t: Model) { + for (const prop of t.properties.values()) { + ok( + program.stateSet(keySym).has(prop), + `Prop ${prop.name} should have run @key decorator by this time.` + ); + } + }, + }); + + const code = ` + import "./lib.js"; + + @test @tagModel model Foo { + ...Bar; + } + + model Bar { + @tagProp name: string; + } + + #suppress "projections-are-experimental" + projection Foo#test {to {}}`; + await testProjection(code); }); it("works for versioning", async () => { From 75c188e8e46ef97842b0273f19c3571fba818651 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Mon, 3 Oct 2022 14:26:09 -0700 Subject: [PATCH 07/12] Copy keys point to right model --- packages/rest/src/resource.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/rest/src/resource.ts b/packages/rest/src/resource.ts index 8c725d1acf..05f437ea2c 100644 --- a/packages/rest/src/resource.ts +++ b/packages/rest/src/resource.ts @@ -115,6 +115,7 @@ function cloneKeyProperties(context: DecoratorContext, target: Model, resourceTy // become an optional path parameter const newProp = program.checker.cloneType(keyProperty, { name: keyName, + model: target, decorators, optional: false, }); From 26fe15f1b4b4184202c6c494daa248dc4506c670 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Mon, 3 Oct 2022 14:45:09 -0700 Subject: [PATCH 08/12] Fix extracted params --- packages/openapi/src/helpers.ts | 3 +- packages/rest/src/resource.ts | 3 +- .../test/output/rest/petstore/openapi.json | 56 +++++++++---------- 3 files changed, 31 insertions(+), 31 deletions(-) diff --git a/packages/openapi/src/helpers.ts b/packages/openapi/src/helpers.ts index 5bbc6e10d6..440ae81cc3 100644 --- a/packages/openapi/src/helpers.ts +++ b/packages/openapi/src/helpers.ts @@ -95,8 +95,7 @@ export function getParameterKey( key += `.${property.name}`; } - // JSON check is workaround for https://github.com/microsoft/cadl/issues/462 - if (existingParams[key] && JSON.stringify(newParam) !== JSON.stringify(existingParams[key])) { + if (existingParams[key]) { reportDiagnostic(program, { code: "duplicate-type-name", messageId: "parameter", diff --git a/packages/rest/src/resource.ts b/packages/rest/src/resource.ts index 05f437ea2c..f8fd411449 100644 --- a/packages/rest/src/resource.ts +++ b/packages/rest/src/resource.ts @@ -115,9 +115,10 @@ function cloneKeyProperties(context: DecoratorContext, target: Model, resourceTy // become an optional path parameter const newProp = program.checker.cloneType(keyProperty, { name: keyName, - model: target, decorators, optional: false, + model: target, + sourceProperty: keyProperty, }); // Add the key property to the target type diff --git a/packages/samples/test/output/rest/petstore/openapi.json b/packages/samples/test/output/rest/petstore/openapi.json index b69a018e81..4d101404f0 100644 --- a/packages/samples/test/output/rest/petstore/openapi.json +++ b/packages/samples/test/output/rest/petstore/openapi.json @@ -12,7 +12,7 @@ "description": "Gets an instance of the resource.", "parameters": [ { - "$ref": "#/components/parameters/Pet.petId" + "$ref": "#/components/parameters/Pet.id" } ], "responses": { @@ -43,7 +43,7 @@ "description": "Updates an existing instance of the resource.", "parameters": [ { - "$ref": "#/components/parameters/Pet.petId" + "$ref": "#/components/parameters/Pet.id" } ], "responses": { @@ -83,7 +83,7 @@ "description": "Deletes an existing instance of the resource.", "parameters": [ { - "$ref": "#/components/parameters/Pet.petId" + "$ref": "#/components/parameters/Pet.id" } ], "responses": { @@ -184,10 +184,10 @@ "description": "Creates or update a instance of the extension resource.", "parameters": [ { - "$ref": "#/components/parameters/Pet.petId" + "$ref": "#/components/parameters/Pet.id" }, { - "$ref": "#/components/parameters/Checkup.checkupId" + "$ref": "#/components/parameters/Checkup.id" } ], "responses": { @@ -239,7 +239,7 @@ "description": "Lists all instances of the extension resource.", "parameters": [ { - "$ref": "#/components/parameters/Pet.petId" + "$ref": "#/components/parameters/Pet.id" } ], "responses": { @@ -272,7 +272,7 @@ "description": "Gets the singleton resource.", "parameters": [ { - "$ref": "#/components/parameters/Pet.petId" + "$ref": "#/components/parameters/Pet.id" } ], "responses": { @@ -303,7 +303,7 @@ "description": "Updates the singleton resource.", "parameters": [ { - "$ref": "#/components/parameters/Pet.petId" + "$ref": "#/components/parameters/Pet.id" } ], "responses": { @@ -345,10 +345,10 @@ "description": "Gets an instance of the resource.", "parameters": [ { - "$ref": "#/components/parameters/Pet.petId" + "$ref": "#/components/parameters/Pet.id" }, { - "$ref": "#/components/parameters/Toy.toyId" + "$ref": "#/components/parameters/Toy.id" } ], "responses": { @@ -380,7 +380,7 @@ "operationId": "Toys_list", "parameters": [ { - "$ref": "#/components/parameters/Pet.petId" + "$ref": "#/components/parameters/Pet.id" }, { "name": "nameFilter", @@ -421,10 +421,10 @@ "description": "Gets the singleton resource.", "parameters": [ { - "$ref": "#/components/parameters/Pet.petId" + "$ref": "#/components/parameters/Pet.id" }, { - "$ref": "#/components/parameters/Toy.toyId" + "$ref": "#/components/parameters/Toy.id" } ], "responses": { @@ -455,10 +455,10 @@ "description": "Updates the singleton resource.", "parameters": [ { - "$ref": "#/components/parameters/Pet.petId" + "$ref": "#/components/parameters/Pet.id" }, { - "$ref": "#/components/parameters/Toy.toyId" + "$ref": "#/components/parameters/Toy.id" } ], "responses": { @@ -500,7 +500,7 @@ "description": "Creates or update a instance of the resource.", "parameters": [ { - "$ref": "#/components/parameters/Checkup.checkupId" + "$ref": "#/components/parameters/Checkup.id" } ], "responses": { @@ -581,7 +581,7 @@ "description": "Gets an instance of the resource.", "parameters": [ { - "$ref": "#/components/parameters/Owner.ownerId" + "$ref": "#/components/parameters/Owner.id" } ], "responses": { @@ -612,7 +612,7 @@ "description": "Updates an existing instance of the resource.", "parameters": [ { - "$ref": "#/components/parameters/Owner.ownerId" + "$ref": "#/components/parameters/Owner.id" } ], "responses": { @@ -652,7 +652,7 @@ "description": "Deletes an existing instance of the resource.", "parameters": [ { - "$ref": "#/components/parameters/Owner.ownerId" + "$ref": "#/components/parameters/Owner.id" } ], "responses": { @@ -753,10 +753,10 @@ "description": "Creates or update a instance of the extension resource.", "parameters": [ { - "$ref": "#/components/parameters/Owner.ownerId" + "$ref": "#/components/parameters/Owner.id" }, { - "$ref": "#/components/parameters/Checkup.checkupId" + "$ref": "#/components/parameters/Checkup.id" } ], "responses": { @@ -808,7 +808,7 @@ "description": "Lists all instances of the extension resource.", "parameters": [ { - "$ref": "#/components/parameters/Owner.ownerId" + "$ref": "#/components/parameters/Owner.id" } ], "responses": { @@ -841,7 +841,7 @@ "description": "Gets the singleton resource.", "parameters": [ { - "$ref": "#/components/parameters/Owner.ownerId" + "$ref": "#/components/parameters/Owner.id" } ], "responses": { @@ -872,7 +872,7 @@ "description": "Updates the singleton resource.", "parameters": [ { - "$ref": "#/components/parameters/Owner.ownerId" + "$ref": "#/components/parameters/Owner.id" } ], "responses": { @@ -911,7 +911,7 @@ }, "components": { "parameters": { - "Pet.petId": { + "Pet.id": { "name": "petId", "in": "path", "required": true, @@ -920,7 +920,7 @@ "format": "int32" } }, - "Checkup.checkupId": { + "Checkup.id": { "name": "checkupId", "in": "path", "required": true, @@ -929,7 +929,7 @@ "format": "int32" } }, - "Toy.toyId": { + "Toy.id": { "name": "toyId", "in": "path", "required": true, @@ -938,7 +938,7 @@ "format": "int64" } }, - "Owner.ownerId": { + "Owner.id": { "name": "ownerId", "in": "path", "required": true, From 0115c27711e7ed6e13a749be1ae061518d9233ab Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Mon, 3 Oct 2022 14:56:13 -0700 Subject: [PATCH 09/12] Same for enums --- packages/compiler/core/projector.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/compiler/core/projector.ts b/packages/compiler/core/projector.ts index a7fa526d88..2df9f2a858 100644 --- a/packages/compiler/core/projector.ts +++ b/packages/compiler/core/projector.ts @@ -456,7 +456,7 @@ export function createProjector( projectedTypes.set(e, projectedEnum); for (const member of e.members.values()) { - const projectedMember = projectType(member); + const projectedMember = projectEnumMember(member, projectedEnum); if (projectedMember.kind === "EnumMember") { members.set(projectedMember.name, projectedMember); } @@ -466,7 +466,10 @@ export function createProjector( return applyProjection(e, projectedEnum); } - function projectEnumMember(e: EnumMember) { + function projectEnumMember(e: EnumMember, projectingEnum?: Enum) { + if (projectingEnum === undefined) { + return projectViaParent(e, e.enum); + } const decorators = projectDecorators(e.decorators); const projectedMember = shallowClone(e, { decorators, From 1b2c89844a0014340a308e4f3cd76e0e03cbcd6f Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Mon, 3 Oct 2022 15:05:05 -0700 Subject: [PATCH 10/12] Workaround --- ...fix-projections-parent-source_2022-10-03-22-04.json | 10 ++++++++++ ...fix-projections-parent-source_2022-10-03-22-04.json | 10 ++++++++++ 2 files changed, 20 insertions(+) create mode 100644 common/changes/@cadl-lang/openapi/fix-projections-parent-source_2022-10-03-22-04.json create mode 100644 common/changes/@cadl-lang/rest/fix-projections-parent-source_2022-10-03-22-04.json diff --git a/common/changes/@cadl-lang/openapi/fix-projections-parent-source_2022-10-03-22-04.json b/common/changes/@cadl-lang/openapi/fix-projections-parent-source_2022-10-03-22-04.json new file mode 100644 index 0000000000..1e7448c896 --- /dev/null +++ b/common/changes/@cadl-lang/openapi/fix-projections-parent-source_2022-10-03-22-04.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@cadl-lang/openapi", + "comment": "Remove workaround for issue with `KeysOf` https://github.com/microsoft/cadl/issues/462", + "type": "patch" + } + ], + "packageName": "@cadl-lang/openapi" +} \ No newline at end of file diff --git a/common/changes/@cadl-lang/rest/fix-projections-parent-source_2022-10-03-22-04.json b/common/changes/@cadl-lang/rest/fix-projections-parent-source_2022-10-03-22-04.json new file mode 100644 index 0000000000..e356c60360 --- /dev/null +++ b/common/changes/@cadl-lang/rest/fix-projections-parent-source_2022-10-03-22-04.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@cadl-lang/rest", + "comment": "Remove workaround for issue https://github.com/microsoft/cadl/issues/1069", + "type": "patch" + } + ], + "packageName": "@cadl-lang/rest" +} \ No newline at end of file From 1c398ec1d67a299ab268dd5c65e20724c2803217 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Mon, 3 Oct 2022 15:36:16 -0700 Subject: [PATCH 11/12] Remove sourceProperty and keep params inline --- packages/rest/src/resource.ts | 1 - .../test/output/rest/petstore/openapi.json | 230 +++++++++++++----- 2 files changed, 168 insertions(+), 63 deletions(-) diff --git a/packages/rest/src/resource.ts b/packages/rest/src/resource.ts index f8fd411449..0db818bd87 100644 --- a/packages/rest/src/resource.ts +++ b/packages/rest/src/resource.ts @@ -118,7 +118,6 @@ function cloneKeyProperties(context: DecoratorContext, target: Model, resourceTy decorators, optional: false, model: target, - sourceProperty: keyProperty, }); // Add the key property to the target type diff --git a/packages/samples/test/output/rest/petstore/openapi.json b/packages/samples/test/output/rest/petstore/openapi.json index 4d101404f0..34870360c7 100644 --- a/packages/samples/test/output/rest/petstore/openapi.json +++ b/packages/samples/test/output/rest/petstore/openapi.json @@ -12,7 +12,13 @@ "description": "Gets an instance of the resource.", "parameters": [ { - "$ref": "#/components/parameters/Pet.id" + "name": "petId", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int32" + } } ], "responses": { @@ -43,7 +49,13 @@ "description": "Updates an existing instance of the resource.", "parameters": [ { - "$ref": "#/components/parameters/Pet.id" + "name": "petId", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int32" + } } ], "responses": { @@ -83,7 +95,13 @@ "description": "Deletes an existing instance of the resource.", "parameters": [ { - "$ref": "#/components/parameters/Pet.id" + "name": "petId", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int32" + } } ], "responses": { @@ -184,10 +202,22 @@ "description": "Creates or update a instance of the extension resource.", "parameters": [ { - "$ref": "#/components/parameters/Pet.id" + "name": "petId", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int32" + } }, { - "$ref": "#/components/parameters/Checkup.id" + "name": "checkupId", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int32" + } } ], "responses": { @@ -239,7 +269,13 @@ "description": "Lists all instances of the extension resource.", "parameters": [ { - "$ref": "#/components/parameters/Pet.id" + "name": "petId", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int32" + } } ], "responses": { @@ -272,7 +308,13 @@ "description": "Gets the singleton resource.", "parameters": [ { - "$ref": "#/components/parameters/Pet.id" + "name": "petId", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int32" + } } ], "responses": { @@ -303,7 +345,13 @@ "description": "Updates the singleton resource.", "parameters": [ { - "$ref": "#/components/parameters/Pet.id" + "name": "petId", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int32" + } } ], "responses": { @@ -345,10 +393,22 @@ "description": "Gets an instance of the resource.", "parameters": [ { - "$ref": "#/components/parameters/Pet.id" + "name": "petId", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int32" + } }, { - "$ref": "#/components/parameters/Toy.id" + "name": "toyId", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int64" + } } ], "responses": { @@ -380,7 +440,13 @@ "operationId": "Toys_list", "parameters": [ { - "$ref": "#/components/parameters/Pet.id" + "name": "petId", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int32" + } }, { "name": "nameFilter", @@ -421,10 +487,22 @@ "description": "Gets the singleton resource.", "parameters": [ { - "$ref": "#/components/parameters/Pet.id" + "name": "petId", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int32" + } }, { - "$ref": "#/components/parameters/Toy.id" + "name": "toyId", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int64" + } } ], "responses": { @@ -455,10 +533,22 @@ "description": "Updates the singleton resource.", "parameters": [ { - "$ref": "#/components/parameters/Pet.id" + "name": "petId", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int32" + } }, { - "$ref": "#/components/parameters/Toy.id" + "name": "toyId", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int64" + } } ], "responses": { @@ -500,7 +590,13 @@ "description": "Creates or update a instance of the resource.", "parameters": [ { - "$ref": "#/components/parameters/Checkup.id" + "name": "checkupId", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int32" + } } ], "responses": { @@ -581,7 +677,13 @@ "description": "Gets an instance of the resource.", "parameters": [ { - "$ref": "#/components/parameters/Owner.id" + "name": "ownerId", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int64" + } } ], "responses": { @@ -612,7 +714,13 @@ "description": "Updates an existing instance of the resource.", "parameters": [ { - "$ref": "#/components/parameters/Owner.id" + "name": "ownerId", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int64" + } } ], "responses": { @@ -652,7 +760,13 @@ "description": "Deletes an existing instance of the resource.", "parameters": [ { - "$ref": "#/components/parameters/Owner.id" + "name": "ownerId", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int64" + } } ], "responses": { @@ -753,10 +867,22 @@ "description": "Creates or update a instance of the extension resource.", "parameters": [ { - "$ref": "#/components/parameters/Owner.id" + "name": "ownerId", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int64" + } }, { - "$ref": "#/components/parameters/Checkup.id" + "name": "checkupId", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int32" + } } ], "responses": { @@ -808,7 +934,13 @@ "description": "Lists all instances of the extension resource.", "parameters": [ { - "$ref": "#/components/parameters/Owner.id" + "name": "ownerId", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int64" + } } ], "responses": { @@ -841,7 +973,13 @@ "description": "Gets the singleton resource.", "parameters": [ { - "$ref": "#/components/parameters/Owner.id" + "name": "ownerId", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int64" + } } ], "responses": { @@ -872,7 +1010,13 @@ "description": "Updates the singleton resource.", "parameters": [ { - "$ref": "#/components/parameters/Owner.id" + "name": "ownerId", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int64" + } } ], "responses": { @@ -910,44 +1054,6 @@ } }, "components": { - "parameters": { - "Pet.id": { - "name": "petId", - "in": "path", - "required": true, - "schema": { - "type": "integer", - "format": "int32" - } - }, - "Checkup.id": { - "name": "checkupId", - "in": "path", - "required": true, - "schema": { - "type": "integer", - "format": "int32" - } - }, - "Toy.id": { - "name": "toyId", - "in": "path", - "required": true, - "schema": { - "type": "integer", - "format": "int64" - } - }, - "Owner.id": { - "name": "ownerId", - "in": "path", - "required": true, - "schema": { - "type": "integer", - "format": "int64" - } - } - }, "schemas": { "Pet": { "type": "object", From a17ebd20898777aa3044dd7fa0778d80d785a620 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Mon, 3 Oct 2022 16:10:30 -0700 Subject: [PATCH 12/12] set sourceproperty to undefined explicitly --- packages/compiler/core/projector.ts | 5 +---- packages/rest/src/resource.ts | 1 + 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/compiler/core/projector.ts b/packages/compiler/core/projector.ts index 2df9f2a858..661c67e10a 100644 --- a/packages/compiler/core/projector.ts +++ b/packages/compiler/core/projector.ts @@ -456,7 +456,7 @@ export function createProjector( projectedTypes.set(e, projectedEnum); for (const member of e.members.values()) { - const projectedMember = projectEnumMember(member, projectedEnum); + const projectedMember = projectType(member); if (projectedMember.kind === "EnumMember") { members.set(projectedMember.name, projectedMember); } @@ -467,9 +467,6 @@ export function createProjector( } function projectEnumMember(e: EnumMember, projectingEnum?: Enum) { - if (projectingEnum === undefined) { - return projectViaParent(e, e.enum); - } const decorators = projectDecorators(e.decorators); const projectedMember = shallowClone(e, { decorators, diff --git a/packages/rest/src/resource.ts b/packages/rest/src/resource.ts index 0db818bd87..b43fec07db 100644 --- a/packages/rest/src/resource.ts +++ b/packages/rest/src/resource.ts @@ -118,6 +118,7 @@ function cloneKeyProperties(context: DecoratorContext, target: Model, resourceTy decorators, optional: false, model: target, + sourceProperty: undefined, }); // Add the key property to the target type