Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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,10 @@
{
"changes": [
{
"packageName": "@typespec/versioning",
"comment": "Fixed issue with `@typeChangedFrom` complaining about incorrect versioned references.",
"type": "none"
}
],
"packageName": "@typespec/versioning"
}
1 change: 1 addition & 0 deletions packages/versioning/src/lib.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ const libDef = {
dependentRemovedBefore: paramMessage`'${"sourceName"}' was removed on version '${"sourceRemovedOn"}' but contains type '${"targetName"}' removed in version '${"targetRemovedOn"}'.`,
versionedDependencyAddedAfter: paramMessage`'${"sourceName"}' is referencing type '${"targetName"}' added in version '${"targetAddedOn"}' but version used is ${"dependencyVersion"}.`,
versionedDependencyRemovedBefore: paramMessage`'${"sourceName"}' is referencing type '${"targetName"}' removed in version '${"targetAddedOn"}' but version used is ${"dependencyVersion"}.`,
doesNotExist: paramMessage`'${"sourceName"}' is referencing type '${"targetName"}' which does not exist in version '${"version"}'.`,
},
},
"incompatible-versioned-namespace-use-dependency": {
Expand Down
122 changes: 115 additions & 7 deletions packages/versioning/src/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ import {
findVersionedNamespace,
getAvailabilityMap,
getMadeOptionalOn,
getReturnTypeChangedFrom,
getTypeChangedFrom,
getUseDependencies,
getVersion,
getVersionDependencies,
getVersions,
} from "./versioning.js";
Expand Down Expand Up @@ -52,7 +55,12 @@ export function $onValidate(program: Program) {
validateTargetVersionCompatible(program, model, prop, { isTargetADependent: true });

// Validate model property -> type have correct versioning
validateReference(program, prop, prop.type);
const typeChangedFrom = getTypeChangedFrom(program, prop);
if (typeChangedFrom !== undefined) {
validateMultiTypeReference(program, prop);
} else {
validateReference(program, prop, prop.type);
}

// Validate model property type is correct when madeOptional
validateMadeOptional(program, prop);
Expand Down Expand Up @@ -164,6 +172,95 @@ export function $onValidate(program: Program) {
validateVersionedNamespaceUsage(program, namespaceDependencies);
}

function getAllVersions(p: Program, t: Type): Version[] | undefined {
const [namespace, _] = getVersions(p, t);
if (namespace === undefined) return undefined;

return getVersion(p, namespace)?.getVersions();
}

/**
* Ensures that properties whose type has changed with versioning are valid.
*/
function validateMultiTypeReference(program: Program, source: Type) {
const versionTypeMap = getVersionedTypeMap(program, source);
if (versionTypeMap === undefined) return;
for (const [version, type] of versionTypeMap!) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for (const [version, type] of versionTypeMap!) {
for (const [version, type] of versionTypeMap) {

shouldn't need that as you already check for it above

if (type === undefined) continue;
const availMap = getAvailabilityMap(program, type);
const availability = availMap?.get(version.name) ?? Availability.Available;
if ([Availability.Added, Availability.Available].includes(availability)) {
continue;
}
reportDiagnostic(program, {
code: "incompatible-versioned-reference",
messageId: "doesNotExist",
format: {
sourceName: getTypeName(source),
targetName: getTypeName(type),
version: prettyVersion(version),
},
target: source,
});
}
}

/**
* Constructs a map of version to type for the the source.
*/
function getVersionedTypeMap(
program: Program,
source: Type
): Map<Version, Type | undefined> | undefined {
const allVersions = getAllVersions(program, source);
if (allVersions === undefined) return undefined;

const map: Map<Version, Type | undefined> = new Map(allVersions.map((v) => [v, undefined]));
const availMap = getAvailabilityMap(program, source);
const alwaysAvail = availMap === undefined;

// Populate the map with any typeChangedFrom data, which may have holes.
// We will fill these holes in a later pass.
const typeChangedFrom = getTypeChangedFrom(program, source);
if (typeChangedFrom !== undefined) {
for (const [version, type] of typeChangedFrom) {
const versionIndex = allVersions.indexOf(version);
if (versionIndex !== -1) {
map.set(allVersions[versionIndex - 1], type);
}
}
}
let lastType: Type | undefined = undefined;
switch (source.kind) {
case "ModelProperty":
lastType = source.type;
break;
default:
throw new Error(`Not implemented '${source.kind}'.`);
}
for (const version of allVersions.reverse()) {
const isAvail =
alwaysAvail ||
[Availability.Added, Availability.Available].includes(availMap.get(version.name)!);

// If property is unavailable in this version, it can't have a type
if (!isAvail) {
map.set(version, undefined);
continue;
}

// Working backwards, we fill in any holes from the last type we encountered. Since we expect
// to encounter a hole at the start, we use the raw property type
const mapType = map.get(version);
if (mapType !== undefined) {
lastType = mapType;
} else {
map.set(version, lastType);
}
}
return map;
}

/**
* Ensures that the version enum for a @versioned namespace has unique values.
*/
Expand Down Expand Up @@ -292,15 +389,13 @@ function getAvailabilityMapWithParentInfo(
case "ModelProperty":
const parentModel = type.model;
if (parentModel) {
parentMap = getAvailabilityMap(program, parentModel);
parentMap = getAvailabilityMapWithParentInfo(program, parentModel);
}
break;
default:
break;
}
if (!base && !parentMap) return undefined;
else if (!base && parentMap) return parentMap;
else return base;
return base ?? parentMap;
}

/**
Expand Down Expand Up @@ -466,8 +561,20 @@ function validateAvailabilityForRef(
}
return;
}

const keySet = new Set([...sourceAvail.keys(), ...targetAvail.keys()]);
let keyValSource: string[] = [...sourceAvail.keys(), ...targetAvail.keys()];
const sourceTypeChanged = getTypeChangedFrom(program, source);
if (sourceTypeChanged !== undefined) {
const sourceTypeChangedKeys = [...sourceTypeChanged.keys()].map((item) => item.name);
keyValSource = [...keyValSource, ...sourceTypeChangedKeys];
}
const sourceReturnTypeChanged = getReturnTypeChangedFrom(program, source);
if (sourceReturnTypeChanged !== undefined) {
const sourceReturnTypeChangedKeys = [...sourceReturnTypeChanged.keys()].map(
(item) => item.name
);
keyValSource = [...keyValSource, ...sourceReturnTypeChangedKeys];
}
const keySet = new Set(keyValSource);

for (const key of keySet) {
const sourceVal = sourceAvail.get(key)!;
Expand All @@ -477,6 +584,7 @@ function validateAvailabilityForRef(
[Availability.Removed, Availability.Unavailable].includes(targetVal)
) {
const targetAddedOn = findAvailabilityAfterVersion(key, Availability.Added, targetAvail);

reportDiagnostic(program, {
code: "incompatible-versioned-reference",
messageId: "addedAfter",
Expand Down
20 changes: 18 additions & 2 deletions packages/versioning/src/versioning.ts
Original file line number Diff line number Diff line change
Expand Up @@ -695,10 +695,18 @@ export function getAvailabilityMap(

const added = getAddedOnVersions(program, type) ?? [];
const removed = getRemovedOnVersions(program, type) ?? [];
const typeChanged = getTypeChangedFrom(program, type);
const returnTypeChanged = getReturnTypeChangedFrom(program, type);
// if there's absolutely no versioning information, return undefined
// contextually, this might mean it inherits its versioning info from a parent
// or that it is treated as unversioned
if (!added.length && !removed.length) return undefined;
if (
!added.length &&
!removed.length &&
typeChanged === undefined &&
returnTypeChanged === undefined
)
return undefined;

// implicitly, all versioned things are assumed to have been added at
// v1 if not specified
Expand Down Expand Up @@ -735,10 +743,18 @@ export function getAvailabilityMapInTimeline(

const added = getAddedOnVersions(program, type) ?? [];
const removed = getRemovedOnVersions(program, type) ?? [];
const typeChanged = getTypeChangedFrom(program, type);
const returnTypeChanged = getReturnTypeChangedFrom(program, type);
// if there's absolutely no versioning information, return undefined
// contextually, this might mean it inherits its versioning info from a parent
// or that it is treated as unversioned
if (!added.length && !removed.length) return undefined;
if (
!added.length &&
!removed.length &&
typeChanged === undefined &&
returnTypeChanged === undefined
)
return undefined;

// implicitly, all versioned things are assumed to have been added at
// v1 if not specified
Expand Down
47 changes: 47 additions & 0 deletions packages/versioning/test/incompatible-versioning.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,53 @@ describe("versioning: validate incompatible references", () => {
});
});

it("emit diagnostic when using @typeChangedFrom with a type parameter that does not yet exist", async () => {
const diagnostics = await runner.diagnose(`
@test
model Original {}

@test
@added(Versions.v3)
model Updated {}

@test
model Test {
@typeChangedFrom(Versions.v2, Original)
prop: Updated;
}
`);
expectDiagnostics(diagnostics, {
code: "@typespec/versioning/incompatible-versioned-reference",
severity: "error",
message:
"'TestService.Test.prop' is referencing type 'TestService.Updated' which does not exist in version 'v2'.",
});
});

it("emit diagnostic when using @typeChangedFrom with a base parameter that does not yet exist", async () => {
const diagnostics = await runner.diagnose(`
@test
@added(Versions.v2)
model Original {}

@test
@added(Versions.v3)
model Updated {}

@test
model Test {
@typeChangedFrom(Versions.v3, Original)
prop: Updated;
}
`);
expectDiagnostics(diagnostics, {
code: "@typespec/versioning/incompatible-versioned-reference",
severity: "error",
message:
"'TestService.Test.prop' is referencing type 'TestService.Original' which does not exist in version 'v1'.",
});
});

it("succeed if version are compatible in model", async () => {
const diagnostics = await runner.diagnose(`
@added(Versions.v2)
Expand Down
29 changes: 28 additions & 1 deletion packages/versioning/test/versioning.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,34 @@ describe("versioning: logic", () => {
ok(v2.properties.get("b")!.optional === true);
});

it("can change type", async () => {
it("can change type to versioned models", async () => {
const {
projections: [v1, v2, v3],
} = await versionedModel(
["v1", "v2", "v3"],
`
@test
model Original {}

@test
@added(Versions.v2)
model Updated {}

@test
model Test {
@added(Versions.v2)
@typeChangedFrom(Versions.v3, Original)
prop: Updated;
}
`
);

ok(v1.properties.get("prop") === undefined);
ok((v2.properties.get("prop")!.type as Model).name === "Original");
ok((v3.properties.get("prop")!.type as Model).name === "Updated");
});

it("can change type over multiple versions", async () => {
const {
projections: [v1, v2, v3],
} = await versionedModel(
Expand Down