Skip to content

Commit

Permalink
Allow spreading a model that has props added in previous version (#3911)
Browse files Browse the repository at this point in the history
fix [#3639](#3639)

Allow this scenario

```

model Base {
  @added(Versions.v1) name: string;
}

@added(Versions.v2)
model Child {
  ...Base;
}

```
  • Loading branch information
timotheeguerin authored Jul 23, 2024
1 parent 755df74 commit 10488de
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -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
33 changes: 31 additions & 2 deletions packages/versioning/src/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,15 @@ import {
isTemplateInstance,
isType,
navigateProgram,
type ModelProperty,
type Namespace,
type Program,
type Type,
type TypeNameOptions,
} from "@typespec/compiler";
import {
$added,
$removed,
findVersionedNamespace,
getMadeOptionalOn,
getMadeRequiredOn,
Expand Down Expand Up @@ -777,6 +780,30 @@ 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;
}

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.decorator === decoratorFn);
const sourceDecorators = prop.sourceProperty.decorators.filter(
(x) => x.decorator === decoratorFn
);
return !selfDecorators.some((x) => !sourceDecorators.some((y) => x.node === y.node));
}

function validateAvailabilityForContains(
program: Program,
sourceAvail: Map<string, Availability> | undefined,
Expand All @@ -796,7 +823,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, {
Expand All @@ -813,7 +841,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, {
Expand Down
28 changes: 28 additions & 0 deletions packages/versioning/test/incompatible-versioning.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,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)
Expand Down

0 comments on commit 10488de

Please sign in to comment.