Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Various issue around parent types and sourceProperty in checker and projections #1115

Merged
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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"
}
23 changes: 5 additions & 18 deletions packages/compiler/core/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import {
IntrinsicModelName,
isIntrinsic,
isNeverType,
isProjectedProgram,
isUnknownType,
isVoidType,
JsSourceFileNode,
Expand Down Expand Up @@ -1921,8 +1920,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,
})
);
}
Expand Down Expand Up @@ -4179,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
Expand Down Expand Up @@ -4243,7 +4243,7 @@ export function getEffectiveModelType(
}
}

return match ? getProjectedEffectiveModelType(program, match) : model;
return match ?? model;
}

/**
Expand Down Expand Up @@ -4295,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;

Expand Down
50 changes: 42 additions & 8 deletions packages/compiler/core/projector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,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) {
Expand Down Expand Up @@ -201,7 +201,7 @@ export function createProjector(
}

projectedNamespaces.push(ns);
return applyProjection(ns, projectedNs);
return applyProjection(ns, projectedNs) as Namespace;
}

/**
Expand Down Expand Up @@ -279,7 +279,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),
};
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -320,7 +320,11 @@ 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);

Expand All @@ -329,6 +333,15 @@ export function createProjector(
decorators: projectedDecs,
});

if (projectedModel) {
projectedProp.model = projectedModel;
}

if (prop.sourceProperty) {
const sourceProperty = projectType(prop.sourceProperty) as ModelProperty;
projectedProp.sourceProperty = sourceProperty;
}

if (shouldFinishType(prop)) {
finishTypeForProgram(projectedProgram, projectedProp);
}
Expand Down Expand Up @@ -388,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);
}
Expand All @@ -401,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);

Expand All @@ -410,6 +426,8 @@ export function createProjector(
decorators: projectedDecs,
});

const parentUnion = projectType(variant.union) as Union;
projectedVariant.union = parentUnion;
finishTypeForProgram(projectedProgram, projectedVariant);
return projectedVariant;
}
Expand Down Expand Up @@ -557,7 +575,7 @@ export function createProjector(
return projectedType;
}

function shallowClone<T extends Type>(type: T, additionalProps: Partial<T>) {
function shallowClone<T extends Type>(type: T, additionalProps: Partial<T>): T {
const scopeProps: any = {};
if ("namespace" in type && type.namespace !== undefined) {
scopeProps.namespace = projectedNamespaceScope();
Expand Down Expand Up @@ -587,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;
}
}
116 changes: 66 additions & 50 deletions packages/compiler/test/checker/model.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down
Loading