diff --git a/.chronus/changes/versioning-code-fix-incompatible-versions-2024-11-27-17-5-24.md b/.chronus/changes/versioning-code-fix-incompatible-versions-2024-11-27-17-5-24.md new file mode 100644 index 0000000000..0164463306 --- /dev/null +++ b/.chronus/changes/versioning-code-fix-incompatible-versions-2024-11-27-17-5-24.md @@ -0,0 +1,7 @@ +--- +changeKind: feature +packages: + - "@typespec/versioning" +--- + +add code fixes for incompatible version errors \ No newline at end of file diff --git a/packages/versioning/src/lib.ts b/packages/versioning/src/lib.ts index 9128cccd15..6183adbeff 100644 --- a/packages/versioning/src/lib.ts +++ b/packages/versioning/src/lib.ts @@ -71,8 +71,8 @@ export const $lib = createTypeSpecLibrary({ dependentAddedAfter: paramMessage`'${"sourceName"}' was added in version '${"sourceAddedOn"}' but contains type '${"targetName"}' added in version '${"targetAddedOn"}'.`, removedBefore: paramMessage`'${"sourceName"}' was removed in version '${"sourceRemovedOn"}' but referencing type '${"targetName"}' removed in version '${"targetRemovedOn"}'.`, dependentRemovedBefore: paramMessage`'${"sourceName"}' was removed in 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"}.`, + 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"}'.`, }, }, diff --git a/packages/versioning/src/validate.codefix.ts b/packages/versioning/src/validate.codefix.ts new file mode 100644 index 0000000000..15418d33c2 --- /dev/null +++ b/packages/versioning/src/validate.codefix.ts @@ -0,0 +1,93 @@ +import { + getSourceLocation, + getTypeName, + type CodeFix, + type Program, + type SourceLocation, + type Type, + type TypeNameOptions, +} from "@typespec/compiler"; +import type { Version } from "./types.js"; +import { getAllVersions } from "./versioning.js"; + +export function getVersionAdditionCodefixes( + version: string | Version, + type: Type, + program: Program, + typeOptions?: TypeNameOptions, +): CodeFix[] | undefined { + if (typeof version === "string") { + return getVersionAdditionCodeFixFromString(version, type, program, typeOptions); + } + + return getVersionAdditionCodeFixFromVersion(version, type, typeOptions); +} + +function getVersionAdditionCodeFixFromVersion( + version: Version, + type: Type, + typeOptions?: TypeNameOptions, +): CodeFix[] | undefined { + if (type.node === undefined) return undefined; + + const enumMember = version.enumMember; + const decoratorDeclaration = `@added(${enumMember.enum.name}.${enumMember.name})`; + return [ + getDecorationAdditionCodeFix( + "add-version-to-type", + decoratorDeclaration, + getTypeName(type, typeOptions), + getSourceLocation(type.node), + ), + ]; +} + +function getVersionAdditionCodeFixFromString( + version: string, + type: Type, + program: Program, + typeOptions?: TypeNameOptions, +): CodeFix[] | undefined { + const targetVersion = getAllVersions(program, type)?.find((v) => v.value === version); + if (targetVersion === undefined) return undefined; + + return getVersionAdditionCodeFixFromVersion(targetVersion, type, typeOptions); +} + +export function getVersionRemovalCodeFixes( + version: string, + type: Type, + program: Program, + typeOptions?: TypeNameOptions, +): CodeFix[] | undefined { + if (type.node === undefined) return undefined; + + const targetVersion = getAllVersions(program, type)?.find((v) => v.value === version); + if (targetVersion === undefined) return; + + const enumMember = targetVersion.enumMember; + const decoratorDeclaration = `@removed(${enumMember.enum.name}.${enumMember.name})`; + return [ + getDecorationAdditionCodeFix( + "remove-version-from-type", + decoratorDeclaration, + getTypeName(type, typeOptions), + getSourceLocation(type.node), + ), + ]; +} + +function getDecorationAdditionCodeFix( + id: string, + decoratorDeclaration: string, + typeName: string, + location: SourceLocation, +): CodeFix { + return { + id: id, + label: `Add '${decoratorDeclaration}' to '${typeName}'`, + fix: (context) => { + return context.prependText(location, `${decoratorDeclaration}\n`); + }, + }; +} diff --git a/packages/versioning/src/validate.ts b/packages/versioning/src/validate.ts index b96e53eeb3..cd110e36c8 100644 --- a/packages/versioning/src/validate.ts +++ b/packages/versioning/src/validate.ts @@ -22,12 +22,13 @@ import { getReturnTypeChangedFrom, getTypeChangedFrom, getUseDependencies, - getVersion, } from "./decorators.js"; import { reportDiagnostic } from "./lib.js"; import type { Version } from "./types.js"; +import { getVersionAdditionCodefixes, getVersionRemovalCodeFixes } from "./validate.codefix.js"; import { Availability, + getAllVersions, getAvailabilityMap, getVersionDependencies, getVersions, @@ -208,13 +209,6 @@ 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. */ @@ -254,6 +248,7 @@ function validateTypeAvailability( version: prettyVersion(version), }, target: source, + codefixes: getVersionAdditionCodefixes(version, type, program, options), }); } @@ -665,9 +660,10 @@ function validateTargetVersionCompatible( const [targetNamespace] = getVersions(program, targetAvailability.type); if (!targetAvailability.map || !targetNamespace) return; + let versionMap: Map | Version | undefined; if (sourceNamespace !== targetNamespace) { const dependencies = sourceNamespace && getVersionDependencies(program, sourceNamespace); - const versionMap = dependencies?.get(targetNamespace); + versionMap = dependencies?.get(targetNamespace); if (versionMap === undefined) return; targetAvailability.map = translateAvailability( @@ -697,6 +693,7 @@ function validateTargetVersionCompatible( targetAvailability.map, sourceAvailability.type, targetAvailability.type, + versionMap instanceof Map ? versionMap : undefined, ); } } @@ -728,6 +725,7 @@ function translateAvailability( targetAddedOn: addedAfter, }, target: source, + codefixes: getVersionAdditionCodefixes(version, target, program), }); } if (removedBefore) { @@ -741,6 +739,7 @@ function translateAvailability( targetAddedOn: removedBefore, }, target: source, + codefixes: getVersionAdditionCodefixes(version, target, program), }); } } @@ -799,12 +798,16 @@ function validateAvailabilityForRef( targetAvail: Map, source: Type, target: Type, - sourceOptions?: TypeNameOptions, - targetOptions?: TypeNameOptions, + versionMap?: Map, ) { // if source is unversioned and target is versioned if (sourceAvail === undefined) { if (!isAvailableInAllVersion(targetAvail)) { + const firstAvailableVersion = Array.from(targetAvail.entries()) + .filter(([_, val]) => val === Availability.Available || val === Availability.Added) + .map(([key, _]) => key) + .sort() + .shift(); reportDiagnostic(program, { code: "incompatible-versioned-reference", messageId: "default", @@ -813,6 +816,9 @@ function validateAvailabilityForRef( targetName: getTypeName(target), }, target: source, + codefixes: firstAvailableVersion + ? getVersionAdditionCodefixes(firstAvailableVersion, source, program) + : undefined, }); } return; @@ -840,6 +846,12 @@ function validateAvailabilityForRef( [Availability.Removed, Availability.Unavailable].includes(targetVal) ) { const targetAddedOn = findAvailabilityAfterVersion(key, Availability.Added, targetAvail); + let targetVersion: Version | string = key; + if (versionMap) { + // the `key` here could have already been converted to source version string, thus we need to find the + // original target version so that we can provide the correct codefix + targetVersion = findMatchingTargetVersion(key, versionMap) ?? key; + } reportDiagnostic(program, { code: "incompatible-versioned-reference", @@ -851,6 +863,7 @@ function validateAvailabilityForRef( targetAddedOn: targetAddedOn!, }, target: source, + codefixes: getVersionAdditionCodefixes(targetVersion, target, program), }); } if ( @@ -862,16 +875,23 @@ function validateAvailabilityForRef( Availability.Removed, targetAvail, ); + + let targetVersion: Version | string = key; + if (versionMap) { + targetVersion = findMatchingTargetVersion(key, versionMap) ?? key; + } + reportDiagnostic(program, { code: "incompatible-versioned-reference", messageId: "removedBefore", format: { - sourceName: getTypeName(source, sourceOptions), - targetName: getTypeName(target, targetOptions), + sourceName: getTypeName(source), + targetName: getTypeName(target), sourceRemovedOn: key, targetRemovedOn: targetRemovedOn!, }, target: source, + codefixes: getVersionAdditionCodefixes(targetVersion, target, program), }); } } @@ -934,6 +954,7 @@ function validateAvailabilityForContains( targetAddedOn: key, }, target: target, + codefixes: getVersionAdditionCodefixes(key, source, program, targetOptions), }); } if ( @@ -952,6 +973,7 @@ function validateAvailabilityForContains( targetRemovedOn: targetRemovedOn!, }, target: target, + codefixes: getVersionRemovalCodeFixes(key, target, program, targetOptions), }); } } @@ -967,3 +989,15 @@ function isAvailableInAllVersion(avail: Map): boolean { function prettyVersion(version: Version | undefined): string { return version?.value ?? ""; } + +function findMatchingTargetVersion( + sourceVersion: string, + versionMap: Map, +): Version | undefined { + for (const [source, target] of versionMap.entries()) { + if (source.value === sourceVersion) { + return target; + } + } + return undefined; +} diff --git a/packages/versioning/src/versioning.ts b/packages/versioning/src/versioning.ts index 69fe7d7bf6..0dd31f7da5 100644 --- a/packages/versioning/src/versioning.ts +++ b/packages/versioning/src/versioning.ts @@ -181,7 +181,7 @@ function resolveVersionsForNamespace( } } -function getAllVersions(p: Program, t: Type): Version[] | undefined { +export function getAllVersions(p: Program, t: Type): Version[] | undefined { const [namespace, _] = getVersions(p, t); if (namespace === undefined) return undefined; diff --git a/packages/versioning/test/incompatible-versioning.test.ts b/packages/versioning/test/incompatible-versioning.test.ts index 9469888f5a..d65fd6edda 100644 --- a/packages/versioning/test/incompatible-versioning.test.ts +++ b/packages/versioning/test/incompatible-versioning.test.ts @@ -117,7 +117,7 @@ describe("versioning: validate incompatible references", () => { expectDiagnostics(diagnostics, { code: "@typespec/versioning/incompatible-versioned-reference", message: - "'TestService.test' is referencing versioned type 'TestService.Foo' but is not versioned itself.", + "'TestService.{ param: TestService.Foo }.param' is referencing versioned type 'TestService.Foo' but is not versioned itself.", }); }); @@ -162,7 +162,7 @@ describe("versioning: validate incompatible references", () => { expectDiagnostics(diagnostics, { code: "@typespec/versioning/incompatible-versioned-reference", message: - "'TestService.test' is referencing versioned type 'TestService.Foo' but is not versioned itself.", + "'TestService.{ param: TestService.Foo }.param' is referencing versioned type 'TestService.Foo' but is not versioned itself.", }); }); @@ -715,6 +715,8 @@ describe("versioning: validate incompatible references", () => { `); expectDiagnostics(diagnostics, { code: "@typespec/versioning/incompatible-versioned-reference", + message: + "'TestService.{ param: TestService.Foo }.param' is referencing versioned type 'TestService.Foo' but is not versioned itself.", }); }); }); @@ -771,6 +773,8 @@ describe("versioning: validate incompatible references", () => { `); expectDiagnostics(diagnostics, { code: "@typespec/versioning/incompatible-versioned-reference", + message: + "'TestService.test' is referencing versioned type 'TestService.Versioned' but is not versioned itself.", }); }); @@ -915,7 +919,7 @@ describe("versioning: validate incompatible references", () => { runner = await createVersioningTestRunner(); }); - it("emit diagnostic when referencing incompatible version via version dependency", async () => { + it("emit diagnostic when referencing incompatible version addition via version dependency", async () => { // Here Foo was added in v2 which makes it only available in 1 & 2. const diagnostics = await runner.diagnose(` @versioned(Versions) @@ -949,6 +953,40 @@ describe("versioning: validate incompatible references", () => { }); }); + it("emit diagnostic when referencing incompatible version removal via version dependency", async () => { + // Here Foo was added in v2 which makes it only available in 1 & 2. + const diagnostics = await runner.diagnose(` + @versioned(Versions) + namespace VersionedLib { + enum Versions {l1, l2, l3} + @removed(Versions.l2) + model Foo {} + } + + @versioned(Versions) + namespace TestService { + enum Versions { + @useDependency(VersionedLib.Versions.l1) + v1, + @useDependency(VersionedLib.Versions.l1) + v2, + @useDependency(VersionedLib.Versions.l2) + v3, + @useDependency(VersionedLib.Versions.l3) + v4 + } + + @removed(Versions.v4) + op test(): VersionedLib.Foo; + } + `); + expectDiagnostics(diagnostics, { + code: "@typespec/versioning/incompatible-versioned-reference", + message: + "'TestService.test' was removed in version 'v4' but referencing type 'VersionedLib.Foo' removed in version 'v3'.", + }); + }); + it("doesn't emit diagnostic if all version use the same one", async () => { // Here Foo was added in v2 which makes it only available in 1 & 2. const diagnostics = await runner.diagnose(` @@ -995,7 +1033,7 @@ describe("versioning: validate incompatible references", () => { expectDiagnostics(diagnostics, { code: "@typespec/versioning/incompatible-versioned-reference", message: - "'TestService.test' is referencing type 'VersionedLib.Foo' added in version 'l2' but version used is l1.", + "'TestService.test' is referencing type 'VersionedLib.Foo' added in version 'l2' but version used is 'l1'.", }); }); @@ -1017,7 +1055,7 @@ describe("versioning: validate incompatible references", () => { expectDiagnostics(diagnostics, { code: "@typespec/versioning/incompatible-versioned-reference", message: - "'TestService.test' is referencing type 'VersionedLib.Foo' removed in version 'l2' but version used is l2.", + "'TestService.test' is referencing type 'VersionedLib.Foo' removed in version 'l2' but version used is 'l2'.", }); }); });