From bdc638404cc14ea7a35af986c79a6f02f8fcbe55 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Fri, 19 Jul 2024 11:47:40 -0700 Subject: [PATCH 1/3] Allow spreading a model that has props added in previous version --- packages/versioning/src/validate.ts | 29 +++++++++++++++++-- .../test/incompatible-versioning.test.ts | 28 ++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/packages/versioning/src/validate.ts b/packages/versioning/src/validate.ts index 57e1ca178a..8afefe1ca2 100644 --- a/packages/versioning/src/validate.ts +++ b/packages/versioning/src/validate.ts @@ -6,6 +6,7 @@ import { isTemplateInstance, isType, navigateProgram, + type ModelProperty, type Namespace, type Program, type Type, @@ -772,6 +773,28 @@ function validateAvailabilityForRef( } } +function canIgnoreDependentVersioning(type: Type, versioning: "added" | "removed") { + if (type.kind === "ModelProperty") { + return canIgnoreVersioningOnProperty(type, versioning); + } + return false; +} + +function canIgnoreVersioningOnProperty( + prop: ModelProperty, + versioning: "added" | "removed" +): boolean { + if (prop.sourceProperty === undefined) { + return false; + } + // Check if the decorator was defined on this property or a source property. If source property ignore. + const selfDecorators = prop.decorators.filter((x) => x.definition?.name === `@${versioning}`); + const sourceDecorators = prop.sourceProperty.decorators.filter( + (x) => x.definition?.name === `@${versioning}` + ); + return !selfDecorators.some((x) => !sourceDecorators.some((y) => x.node === y.node)); +} + function validateAvailabilityForContains( program: Program, sourceAvail: Map | undefined, @@ -791,7 +814,8 @@ function validateAvailabilityForContains( if (sourceVal === targetVal) continue; if ( [Availability.Added].includes(targetVal) && - [Availability.Removed, Availability.Unavailable].includes(sourceVal) + [Availability.Removed, Availability.Unavailable].includes(sourceVal) && + !canIgnoreDependentVersioning(target, "added") ) { const sourceAddedOn = findAvailabilityOnOrBeforeVersion(key, Availability.Added, sourceAvail); reportDiagnostic(program, { @@ -808,7 +832,8 @@ function validateAvailabilityForContains( } if ( [Availability.Removed].includes(sourceVal) && - [Availability.Added, Availability.Available].includes(targetVal) + [Availability.Added, Availability.Available].includes(targetVal) && + !canIgnoreDependentVersioning(target, "removed") ) { const targetRemovedOn = findAvailabilityAfterVersion(key, Availability.Removed, targetAvail); reportDiagnostic(program, { diff --git a/packages/versioning/test/incompatible-versioning.test.ts b/packages/versioning/test/incompatible-versioning.test.ts index 6495d6b86c..c5bdf569df 100644 --- a/packages/versioning/test/incompatible-versioning.test.ts +++ b/packages/versioning/test/incompatible-versioning.test.ts @@ -412,6 +412,34 @@ describe("versioning: validate incompatible references", () => { expectDiagnosticEmpty(diagnostics); }); + it("succeed when spreading a model that might have add properties added in previous versions", async () => { + const diagnostics = await runner.diagnose(` + model Base { + @added(Versions.v1) name: string; + } + + @added(Versions.v2) + model Child { + ...Base; + } + `); + expectDiagnosticEmpty(diagnostics); + }); + + it("succeed when spreading a model that might have add properties removed after the model", async () => { + const diagnostics = await runner.diagnose(` + model Base { + @removed(Versions.v3) name: string; + } + + @removed(Versions.v2) + model Child { + ...Base; + } + `); + expectDiagnosticEmpty(diagnostics); + }); + it("emit diagnostic when model property was added before model itself", async () => { const diagnostics = await runner.diagnose(` @added(Versions.v3) From 113aaede07c0b740ed88606716691968ebeb5077 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Fri, 19 Jul 2024 11:52:19 -0700 Subject: [PATCH 2/3] Create allow-spreading-model-previous-version-2024-6-19-18-50-40.md --- ...spreading-model-previous-version-2024-6-19-18-50-40.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .chronus/changes/allow-spreading-model-previous-version-2024-6-19-18-50-40.md diff --git a/.chronus/changes/allow-spreading-model-previous-version-2024-6-19-18-50-40.md b/.chronus/changes/allow-spreading-model-previous-version-2024-6-19-18-50-40.md new file mode 100644 index 0000000000..ab7ab21daf --- /dev/null +++ b/.chronus/changes/allow-spreading-model-previous-version-2024-6-19-18-50-40.md @@ -0,0 +1,8 @@ +--- +# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking +changeKind: fix +packages: + - "@typespec/versioning" +--- + +Allow spreading a model that has props added in previous version From dcc27337f0741a4a5e5bdeb0c643972abac4f188 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 23 Jul 2024 14:19:52 -0700 Subject: [PATCH 3/3] use more reliable --- packages/versioning/src/validate.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/versioning/src/validate.ts b/packages/versioning/src/validate.ts index 8afefe1ca2..4d7461025b 100644 --- a/packages/versioning/src/validate.ts +++ b/packages/versioning/src/validate.ts @@ -13,6 +13,8 @@ import { type TypeNameOptions, } from "@typespec/compiler"; import { + $added, + $removed, findVersionedNamespace, getMadeOptionalOn, getMadeRequiredOn, @@ -787,10 +789,12 @@ function canIgnoreVersioningOnProperty( if (prop.sourceProperty === undefined) { return false; } + + const decoratorFn = versioning === "added" ? $added : $removed; // Check if the decorator was defined on this property or a source property. If source property ignore. - const selfDecorators = prop.decorators.filter((x) => x.definition?.name === `@${versioning}`); + const selfDecorators = prop.decorators.filter((x) => x.decorator === decoratorFn); const sourceDecorators = prop.sourceProperty.decorators.filter( - (x) => x.definition?.name === `@${versioning}` + (x) => x.decorator === decoratorFn ); return !selfDecorators.some((x) => !sourceDecorators.some((y) => x.node === y.node)); }