diff --git a/.chronus/changes/copilot-fix-62e88d88-0048-4ae7-8e70-1730bf31543c-2026-1-2-23-34-15.md b/.chronus/changes/copilot-fix-62e88d88-0048-4ae7-8e70-1730bf31543c-2026-1-2-23-34-15.md new file mode 100644 index 00000000000..04577f8e247 --- /dev/null +++ b/.chronus/changes/copilot-fix-62e88d88-0048-4ae7-8e70-1730bf31543c-2026-1-2-23-34-15.md @@ -0,0 +1,7 @@ +--- +changeKind: fix +packages: + - "@typespec/rest" +--- + +Fix @actionSeparator decorator to only accept Operation, Interface, and Namespace targets \ No newline at end of file diff --git a/packages/rest/README.md b/packages/rest/README.md index 6b7744b7804..2707bc94df8 100644 --- a/packages/rest/README.md +++ b/packages/rest/README.md @@ -51,13 +51,17 @@ Specify this operation is an action. (Scoped to a resource item /pets/{petId}/my Defines the separator string that is inserted before the action name in auto-generated routes for actions. +When applied to a namespace, the separator applies to all action operations in that namespace and its sub-namespaces. +When applied to an interface, the separator applies to all action operations in that interface and overrides any namespace-level separator. +When applied to an operation, the separator applies only to that operation and overrides any interface or namespace-level separator. + ```typespec @TypeSpec.Rest.actionSeparator(seperator: valueof "/" | ":" | "/:") ``` ##### Target -`Model | ModelProperty | Operation` +`Operation | Interface | Namespace` ##### Parameters diff --git a/packages/rest/generated-defs/TypeSpec.Rest.ts b/packages/rest/generated-defs/TypeSpec.Rest.ts index 14a2b275266..fd1a947b9b9 100644 --- a/packages/rest/generated-defs/TypeSpec.Rest.ts +++ b/packages/rest/generated-defs/TypeSpec.Rest.ts @@ -4,6 +4,7 @@ import type { Interface, Model, ModelProperty, + Namespace, Operation, } from "@typespec/compiler"; @@ -58,11 +59,15 @@ export type SegmentOfDecorator = ( /** * Defines the separator string that is inserted before the action name in auto-generated routes for actions. * + * When applied to a namespace, the separator applies to all action operations in that namespace and its sub-namespaces. + * When applied to an interface, the separator applies to all action operations in that interface and overrides any namespace-level separator. + * When applied to an operation, the separator applies only to that operation and overrides any interface or namespace-level separator. + * * @param seperator Seperator seperating the action segment from the rest of the url */ export type ActionSeparatorDecorator = ( context: DecoratorContext, - target: Model | ModelProperty | Operation, + target: Operation | Interface | Namespace, seperator: "/" | ":" | "/:", ) => DecoratorValidatorCallbacks | void; diff --git a/packages/rest/lib/rest-decorators.tsp b/packages/rest/lib/rest-decorators.tsp index 6f1d96ea5f9..c430c13c6cd 100644 --- a/packages/rest/lib/rest-decorators.tsp +++ b/packages/rest/lib/rest-decorators.tsp @@ -38,10 +38,14 @@ extern dec segmentOf(target: Operation, type: Model); /** * Defines the separator string that is inserted before the action name in auto-generated routes for actions. * + * When applied to a namespace, the separator applies to all action operations in that namespace and its sub-namespaces. + * When applied to an interface, the separator applies to all action operations in that interface and overrides any namespace-level separator. + * When applied to an operation, the separator applies only to that operation and overrides any interface or namespace-level separator. + * * @param seperator Seperator seperating the action segment from the rest of the url */ extern dec actionSeparator( - target: Model | ModelProperty | Operation, + target: Operation | Interface | Namespace, seperator: valueof "/" | ":" | "/:" ); diff --git a/packages/rest/src/rest.ts b/packages/rest/src/rest.ts index 82cae4b25f8..239c5e048c9 100644 --- a/packages/rest/src/rest.ts +++ b/packages/rest/src/rest.ts @@ -5,6 +5,7 @@ import { Interface, Model, ModelProperty, + Namespace, Operation, Program, Scalar, @@ -273,11 +274,11 @@ const actionSeparatorKey = createStateSymbol("actionSeparator"); * `@actionSeparator` defines the separator string that is used to precede the action name * in auto-generated actions. * - * `@actionSeparator` can only be applied to model properties, operation parameters, or operations. + * `@actionSeparator` can only be applied to operations, interfaces, or namespaces. */ export function $actionSeparator( context: DecoratorContext, - entity: Model | ModelProperty | Operation, + entity: Operation | Interface | Namespace, separator: "/" | ":" | "/:", ) { context.program.stateMap(actionSeparatorKey).set(entity, separator); @@ -286,10 +287,64 @@ export function $actionSeparator( /** * @param program the TypeSpec program * @param entity the target entity - * @returns the action separator string + * @returns the action separator string, checking the hierarchy: operation -> interface -> namespace */ export function getActionSeparator(program: Program, entity: Type): string | undefined { - return program.stateMap(actionSeparatorKey).get(entity); + const stateMap = program.stateMap(actionSeparatorKey); + + // First, check if the entity itself has an action separator + const directSeparator = stateMap.get(entity); + if (directSeparator !== undefined) { + return directSeparator; + } + + // If entity is an operation, check its interface, then namespace + if (entity.kind === "Operation") { + // Check the interface + if (entity.interface) { + const interfaceSeparator = stateMap.get(entity.interface); + if (interfaceSeparator !== undefined) { + return interfaceSeparator; + } + + // Check the namespace of the interface + if (entity.interface.namespace) { + return getNamespaceActionSeparator(program, entity.interface.namespace); + } + } + + // Check the namespace directly if no interface + if (entity.namespace) { + return getNamespaceActionSeparator(program, entity.namespace); + } + } + + // If entity is an interface, check its namespace + if (entity.kind === "Interface" && entity.namespace) { + return getNamespaceActionSeparator(program, entity.namespace); + } + + return undefined; +} + +/** + * Helper function to recursively check namespace hierarchy for action separator + */ +function getNamespaceActionSeparator(program: Program, namespace: Namespace): string | undefined { + const stateMap = program.stateMap(actionSeparatorKey); + + // Check current namespace + const separator = stateMap.get(namespace); + if (separator !== undefined) { + return separator; + } + + // Check parent namespace recursively + if (namespace.namespace) { + return getNamespaceActionSeparator(program, namespace.namespace); + } + + return undefined; } /** diff --git a/packages/rest/test/action-separator.test.ts b/packages/rest/test/action-separator.test.ts new file mode 100644 index 00000000000..26a9d3f801e --- /dev/null +++ b/packages/rest/test/action-separator.test.ts @@ -0,0 +1,232 @@ +import { expectDiagnostics } from "@typespec/compiler/testing"; +import { strictEqual } from "assert"; +import { describe, it } from "vitest"; +import { Tester, getRoutesFor } from "./test-host.js"; + +describe("rest: @actionSeparator decorator", () => { + describe("valid targets", () => { + it("works on Operation and affects routing", async () => { + const routes = await getRoutesFor(` + @autoRoute + interface Things { + @action + @TypeSpec.Rest.actionSeparator(":") + @put op customAction(@segment("things") @path thingId: string): string; + } + `); + + strictEqual(routes.length, 1); + strictEqual(routes[0].path, "/things/{thingId}:customAction"); + }); + + it("accepts Interface as target without compilation errors", async () => { + // This test verifies that @actionSeparator can be applied to interfaces without errors + const diagnostics = await Tester.diagnose(` + @TypeSpec.Rest.actionSeparator(":") + interface TestInterface { + op test(): void; + } + `); + + // No diagnostics means the decorator accepts interfaces as valid targets + strictEqual(diagnostics.length, 0); + }); + + it("accepts Namespace as target without compilation errors", async () => { + // This test verifies that @actionSeparator can be applied to namespaces without errors + const diagnostics = await Tester.diagnose(` + @TypeSpec.Rest.actionSeparator(":") + namespace TestNamespace { + op test(): void; + } + `); + + // No diagnostics means the decorator accepts namespaces as valid targets + strictEqual(diagnostics.length, 0); + }); + + it("supports all separator values in routing", async () => { + const routes = await getRoutesFor(` + @autoRoute + interface Things { + @action + @TypeSpec.Rest.actionSeparator("/") + @put op action1(@segment("things") @path thingId: string): string; + + @action + @TypeSpec.Rest.actionSeparator(":") + @put op action2(@segment("things") @path thingId: string): string; + + @action + @TypeSpec.Rest.actionSeparator("/:") + @put op action3(@segment("things") @path thingId: string): string; + } + `); + + strictEqual(routes.length, 3); + strictEqual(routes[0].path, "/things/{thingId}/action1"); + strictEqual(routes[1].path, "/things/{thingId}:action2"); + strictEqual(routes[2].path, "/things/{thingId}/:action3"); + }); + }); + + describe("hierarchy behavior", () => { + it("interface-level separator applies to all actions in interface", async () => { + const routes = await getRoutesFor(` + @autoRoute + @TypeSpec.Rest.actionSeparator(":") + interface Things { + @action + @put op action1(@segment("things") @path thingId: string): string; + + @action + @put op action2(@segment("things") @path thingId: string): string; + } + `); + + strictEqual(routes.length, 2); + strictEqual(routes[0].path, "/things/{thingId}:action1"); + strictEqual(routes[1].path, "/things/{thingId}:action2"); + }); + + it("namespace-level separator applies to all actions in namespace", async () => { + const routes = await getRoutesFor(` + @TypeSpec.Rest.actionSeparator(":") + namespace TestNs { + @autoRoute + interface Things { + @action + @put op action1(@segment("things") @path thingId: string): string; + } + } + `); + + strictEqual(routes.length, 1); + strictEqual(routes[0].path, "/things/{thingId}:action1"); + }); + + it("operation-level separator overrides interface-level separator", async () => { + const routes = await getRoutesFor(` + @autoRoute + @TypeSpec.Rest.actionSeparator(":") + interface Things { + @action + @TypeSpec.Rest.actionSeparator("/") + @put op action1(@segment("things") @path thingId: string): string; + + @action + @put op action2(@segment("things") @path thingId: string): string; + } + `); + + strictEqual(routes.length, 2); + strictEqual(routes[0].path, "/things/{thingId}/action1"); // Uses operation-level "/" + strictEqual(routes[1].path, "/things/{thingId}:action2"); // Uses interface-level ":" + }); + + it("interface-level separator overrides namespace-level separator", async () => { + const routes = await getRoutesFor(` + @TypeSpec.Rest.actionSeparator("/:") + namespace TestNs { + @autoRoute + @TypeSpec.Rest.actionSeparator(":") + interface Things { + @action + @put op action1(@segment("things") @path thingId: string): string; + } + + @autoRoute + interface Other { + @action + @put op action2(@segment("other") @path otherId: string): string; + } + } + `); + + strictEqual(routes.length, 2); + strictEqual(routes[0].path, "/things/{thingId}:action1"); // Uses interface-level ":" + strictEqual(routes[1].path, "/other/{otherId}/:action2"); // Uses namespace-level "/:" + }); + + it("namespace separator applies to subnamespaces", async () => { + const routes = await getRoutesFor(` + @TypeSpec.Rest.actionSeparator(":") + namespace Parent { + namespace Child { + @autoRoute + interface Things { + @action + @put op action1(@segment("things") @path thingId: string): string; + } + } + } + `); + + strictEqual(routes.length, 1); + strictEqual(routes[0].path, "/things/{thingId}:action1"); // Uses parent namespace-level ":" + }); + + it("operation in namespace without interface uses namespace separator", async () => { + const routes = await getRoutesFor(` + @TypeSpec.Rest.actionSeparator(":") + namespace TestNs { + @autoRoute + @action + @put op action1(@segment("things") @path thingId: string): string; + } + `); + + strictEqual(routes.length, 1); + strictEqual(routes[0].path, "/things/{thingId}:action1"); + }); + }); + + describe("invalid targets", () => { + it("rejects Model", async () => { + const diagnostics = await Tester.diagnose(` + @TypeSpec.Rest.actionSeparator(":") + model TestModel { + id: string; + } + `); + + expectDiagnostics(diagnostics, [ + { + code: "decorator-wrong-target", + message: + "Cannot apply @actionSeparator decorator to TestModel since it is not assignable to Operation | Interface | Namespace", + }, + ]); + }); + + it("rejects ModelProperty", async () => { + const diagnostics = await Tester.diagnose(` + model TestModel { + @TypeSpec.Rest.actionSeparator(":") + id: string; + } + `); + + expectDiagnostics(diagnostics, [ + { + code: "decorator-wrong-target", + message: + /Cannot apply @actionSeparator decorator to .* since it is not assignable to Operation \| Interface \| Namespace/, + }, + ]); + }); + + it("rejects invalid separator values", async () => { + const diagnostics = await Tester.diagnose(` + @TypeSpec.Rest.actionSeparator("invalid") + op test(): void; + `); + + expectDiagnostics(diagnostics, [ + { + code: "invalid-argument", + }, + ]); + }); + }); +}); diff --git a/website/src/content/docs/docs/libraries/rest/reference/decorators.md b/website/src/content/docs/docs/libraries/rest/reference/decorators.md index 62a7179c5e1..da4c3f85567 100644 --- a/website/src/content/docs/docs/libraries/rest/reference/decorators.md +++ b/website/src/content/docs/docs/libraries/rest/reference/decorators.md @@ -30,13 +30,17 @@ Specify this operation is an action. (Scoped to a resource item /pets/{petId}/my Defines the separator string that is inserted before the action name in auto-generated routes for actions. +When applied to a namespace, the separator applies to all action operations in that namespace and its sub-namespaces. +When applied to an interface, the separator applies to all action operations in that interface and overrides any namespace-level separator. +When applied to an operation, the separator applies only to that operation and overrides any interface or namespace-level separator. + ```typespec @TypeSpec.Rest.actionSeparator(seperator: valueof "/" | ":" | "/:") ``` #### Target -`Model | ModelProperty | Operation` +`Operation | Interface | Namespace` #### Parameters