diff --git a/common/changes/@typespec/http/split-shared-route_2023-04-06-12-41.json b/common/changes/@typespec/http/split-shared-route_2023-04-06-12-41.json new file mode 100644 index 00000000000..aab447a5c81 --- /dev/null +++ b/common/changes/@typespec/http/split-shared-route_2023-04-06-12-41.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@typespec/http", + "comment": "Add `@sharedRoute` decorator for marking operations as sharing a route with other operations", + "type": "none" + } + ], + "packageName": "@typespec/http" +} \ No newline at end of file diff --git a/common/changes/@typespec/http/split-shared-route_2023-04-18-14-06.json b/common/changes/@typespec/http/split-shared-route_2023-04-18-14-06.json new file mode 100644 index 00000000000..c7b42480d16 --- /dev/null +++ b/common/changes/@typespec/http/split-shared-route_2023-04-18-14-06.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@typespec/http", + "comment": "Deprecate the `shared` option in the `@route` decorator. `@sharedRoute` is the new way to accomplish the same behavior.", + "type": "none" + } + ], + "packageName": "@typespec/http" +} \ No newline at end of file diff --git a/common/changes/@typespec/openapi3/split-shared-route_2023-04-18-14-27.json b/common/changes/@typespec/openapi3/split-shared-route_2023-04-18-14-27.json new file mode 100644 index 00000000000..d9a70151de5 --- /dev/null +++ b/common/changes/@typespec/openapi3/split-shared-route_2023-04-18-14-27.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@typespec/openapi3", + "comment": "", + "type": "none" + } + ], + "packageName": "@typespec/openapi3" +} \ No newline at end of file diff --git a/common/changes/@typespec/rest/split-shared-route_2023-04-07-14-49.json b/common/changes/@typespec/rest/split-shared-route_2023-04-07-14-49.json new file mode 100644 index 00000000000..64135d54e31 --- /dev/null +++ b/common/changes/@typespec/rest/split-shared-route_2023-04-07-14-49.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@typespec/rest", + "comment": "", + "type": "none" + } + ], + "packageName": "@typespec/rest" +} \ No newline at end of file diff --git a/common/changes/@typespec/rest/split-shared-route_2023-04-18-14-06.json b/common/changes/@typespec/rest/split-shared-route_2023-04-18-14-06.json new file mode 100644 index 00000000000..3dbd1229d14 --- /dev/null +++ b/common/changes/@typespec/rest/split-shared-route_2023-04-18-14-06.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@typespec/rest", + "comment": "Add validation to ensure that @action or @collectionAction operations have a specified name when used with @sharedRoute", + "type": "none" + } + ], + "packageName": "@typespec/rest" +} \ No newline at end of file diff --git a/docs/standard-library/http/reference/decorators.md b/docs/standard-library/http/reference/decorators.md index 4274400390b..5076c235dcb 100644 --- a/docs/standard-library/http/reference/decorators.md +++ b/docs/standard-library/http/reference/decorators.md @@ -237,7 +237,7 @@ dec TypeSpec.Http.query(target: ModelProperty, queryNameOrOptions?: string | Typ ### `@route` {#@TypeSpec.Http.route} -`@route` defines the relative route URI for the target operation +Defines the relative route URI for the target operation The first argument should be a URI fragment that may contain one or more path parameter fields. If the namespace or interface that contains the operation is also marked with a `@route` decorator, @@ -246,19 +246,19 @@ it will be used as a prefix to the route URI of the operation. `@route` can only be applied to operations, namespaces, and interfaces. ```typespec -dec TypeSpec.Http.route(entity: unknown, path: string, parameters?: object) +dec TypeSpec.Http.route(target: Namespace | Interface | Operation, path: string, options?: (anonymous model)) ``` #### Target -`(intrinsic) unknown` +`union Namespace | Interface | Operation` #### Parameters -| Name | Type | Description | -| ---------- | --------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------- | -| path | `scalar string` | Relative route path. Cannot include query parameters. | -| parameters | `model object` | Optional set of parameters used to configure the route. Supports `{shared: true}` which indicates that the route may be shared by several operations. | +| Name | Type | Description | +| ------- | ------------------------- | ----------------------------------------------------- | +| path | `scalar string` | Relative route path. Cannot include query parameters. | +| options | `model (anonymous model)` | | ### `@server` {#@TypeSpec.Http.server} @@ -280,6 +280,33 @@ dec TypeSpec.Http.server(target: Namespace, url: string, description: string, pa | description | `scalar string` | Description of the endpoint | | parameters | `model object` | Optional set of parameters used to interpolate the url. | +### `@sharedRoute` {#@TypeSpec.Http.sharedRoute} + +`@sharedRoute` marks the operation as sharing a route path with other operations. + +When an operation is marked with `@sharedRoute`, it enables other operations to share the same +route path as long as those operations are also marked with `@sharedRoute`. + +`@sharedRoute` can only be applied directly to operations. + +```typespec +@sharedRoute +@route("/widgets") +op getWidget(@path id: string): Widget; +``` + +```typespec +dec TypeSpec.Http.sharedRoute(target: Operation) +``` + +#### Target + +`Operation` + +#### Parameters + +None + ### `@statusCode` {#@TypeSpec.Http.statusCode} Specify the status code for this response. Property type must be a status code integer or a union of status code integer. diff --git a/docs/standard-library/http/reference/index.md b/docs/standard-library/http/reference/index.md index 075c04fe0be..7ac4a1f2e1f 100644 --- a/docs/standard-library/http/reference/index.md +++ b/docs/standard-library/http/reference/index.md @@ -22,6 +22,7 @@ toc_max_heading_level: 3 - [`@query`](./decorators.md#@TypeSpec.Http.query) - [`@route`](./decorators.md#@TypeSpec.Http.route) - [`@server`](./decorators.md#@TypeSpec.Http.server) +- [`@sharedRoute`](./decorators.md#@TypeSpec.Http.sharedRoute) - [`@statusCode`](./decorators.md#@TypeSpec.Http.statusCode) - [`@useAuth`](./decorators.md#@TypeSpec.Http.useAuth) diff --git a/packages/http/lib/http-decorators.tsp b/packages/http/lib/http-decorators.tsp index b1893be79de..61da390f27f 100644 --- a/packages/http/lib/http-decorators.tsp +++ b/packages/http/lib/http-decorators.tsp @@ -206,7 +206,7 @@ extern dec useAuth(target: Namespace, auth: object | Union | object[]); extern dec includeInapplicableMetadataInPayload(target: unknown, value: boolean); /** - * `@route` defines the relative route URI for the target operation + * Defines the relative route URI for the target operation * * The first argument should be a URI fragment that may contain one or more path parameter fields. * If the namespace or interface that contains the operation is also marked with a `@route` decorator, @@ -216,5 +216,34 @@ extern dec includeInapplicableMetadataInPayload(target: unknown, value: boolean) * * @param path Relative route path. Cannot include query parameters. * @param parameters Optional set of parameters used to configure the route. Supports `{shared: true}` which indicates that the route may be shared by several operations. + * + * @example + * + * ```typespec + * @route("/widgets") + * op getWidget(@path id: string): Widget; + * ``` + */ +extern dec route( + target: Namespace | Interface | Operation, + path: string, + options?: { + shared?: boolean, + } +); + +/** + * `@sharedRoute` marks the operation as sharing a route path with other operations. + * + * When an operation is marked with `@sharedRoute`, it enables other operations to share the same + * route path as long as those operations are also marked with `@sharedRoute`. + * + * `@sharedRoute` can only be applied directly to operations. + * + * ```typespec + * @sharedRoute + * @route("/widgets") + * op getWidget(@path id: string): Widget; + * ``` */ -extern dec route(entity: unknown, path: string, parameters?: object); +extern dec sharedRoute(target: Operation); diff --git a/packages/http/src/decorators.ts b/packages/http/src/decorators.ts index bfc5d4523a5..1fb5942f544 100644 --- a/packages/http/src/decorators.ts +++ b/packages/http/src/decorators.ts @@ -1,25 +1,26 @@ import { - createDiagnosticCollector, DecoratorContext, Diagnostic, DiagnosticTarget, - getDoc, - isArrayModelType, Model, ModelProperty, Namespace, Operation, Program, - setTypeSpecNamespace, Tuple, Type, - typespecTypeToJson, Union, + createDiagnosticCollector, + getDoc, + isArrayModelType, + reportDeprecated, + setTypeSpecNamespace, + typespecTypeToJson, validateDecoratorTarget, validateDecoratorUniqueOnNode, } from "@typespec/compiler"; import { createDiagnostic, createStateSymbol, reportDiagnostic } from "./lib.js"; -import { setRoute } from "./route.js"; +import { setRoute, setSharedRoute } from "./route.js"; import { AuthenticationOption, HeaderFieldOptions, @@ -559,23 +560,6 @@ export function getAuthentication( return program.stateMap(authenticationKey).get(namespace); } -function extractSharedValue(context: DecoratorContext, parameters?: Model): boolean { - const sharedType = parameters?.properties.get("shared")?.type; - if (sharedType === undefined) { - return false; - } - switch (sharedType.kind) { - case "Boolean": - return sharedType.value; - default: - reportDiagnostic(context.program, { - code: "shared-boolean", - target: sharedType, - }); - return false; - } -} - /** * `@route` defines the relative route URI for the target operation * @@ -588,12 +572,40 @@ function extractSharedValue(context: DecoratorContext, parameters?: Model): bool export function $route(context: DecoratorContext, entity: Type, path: string, parameters?: Model) { validateDecoratorUniqueOnNode(context, entity, $route); + // Handle the deprecated `shared` option + let shared = false; + const sharedValue = parameters?.properties.get("shared")?.type; + if (sharedValue !== undefined) { + reportDeprecated( + context.program, + "The `shared` option is deprecated, use the `@sharedRoute` decorator instead.", + entity + ); + + // The type checker should have raised a diagnostic if the value isn't boolean + if (sharedValue.kind === "Boolean") { + shared = sharedValue.value; + } + } + setRoute(context, entity, { path, - shared: extractSharedValue(context, parameters), + shared, }); } +/** + * `@sharedRoute` marks the operation as sharing a route path with other operations. + * + * When an operation is marked with `@sharedRoute`, it enables other operations to share the same + * route path as long as those operations are also marked with `@sharedRoute`. + * + * `@sharedRoute` can only be applied directly to operations. + */ +export function $sharedRoute(context: DecoratorContext, entity: Operation) { + setSharedRoute(context.program, entity); +} + const includeInapplicableMetadataInPayloadKey = createStateSymbol( "includeInapplicableMetadataInPayload" ); diff --git a/packages/http/src/lib.ts b/packages/http/src/lib.ts index e0542bf3b14..931d53a3634 100644 --- a/packages/http/src/lib.ts +++ b/packages/http/src/lib.ts @@ -100,12 +100,6 @@ const libDefinition = { default: paramMessage`@useAuth ${"kind"} only accept Auth model, Tuple of auth model or union of auth model.`, }, }, - "shared-boolean": { - severity: "error", - messages: { - default: "shared parameter must be a boolean.", - }, - }, "shared-inconsistency": { severity: "error", messages: { diff --git a/packages/http/src/route.ts b/packages/http/src/route.ts index a226dccf98b..d8b9d9d77a8 100644 --- a/packages/http/src/route.ts +++ b/packages/http/src/route.ts @@ -216,8 +216,8 @@ export function setRoute(context: DecoratorContext, entity: Type, details: Route const state = context.program.stateMap(routesKey); if (state.has(entity) && entity.kind === "Namespace") { - const existingValue: RoutePath = state.get(entity); - if (existingValue.path !== details.path) { + const existingPath: string | undefined = state.get(entity); + if (existingPath !== details.path) { reportDiagnostic(context.program, { code: "duplicate-route-decorator", messageId: "namespace", @@ -225,19 +225,34 @@ export function setRoute(context: DecoratorContext, entity: Type, details: Route }); } } else { - state.set(entity, details); + state.set(entity, details.path); + if (entity.kind === "Operation" && details.shared) { + setSharedRoute(context.program, entity as Operation); + } } } +const sharedRoutesKey = createStateSymbol("sharedRoutes"); + +export function setSharedRoute(program: Program, operation: Operation) { + program.stateMap(sharedRoutesKey).set(operation, true); +} + export function isSharedRoute(program: Program, operation: Operation): boolean { - return program.stateMap(routesKey).get(operation)?.shared; + return program.stateMap(sharedRoutesKey).get(operation) === true; } export function getRoutePath( program: Program, entity: Namespace | Interface | Operation ): RoutePath | undefined { - return program.stateMap(routesKey).get(entity); + const path = program.stateMap(routesKey).get(entity); + return path + ? { + path, + shared: entity.kind === "Operation" && isSharedRoute(program, entity as Operation), + } + : undefined; } const routeOptionsKey = createStateSymbol("routeOptions"); diff --git a/packages/http/test/http-decorators.test.ts b/packages/http/test/http-decorators.test.ts index c43eae583ef..7991aa559be 100644 --- a/packages/http/test/http-decorators.test.ts +++ b/packages/http/test/http-decorators.test.ts @@ -245,10 +245,23 @@ describe("http: decorators", () => { ]); }); - it("emit diagnostics when not all duplicated routes are declared shared", async () => { + it("emits diagnostic when deprecated `shared` option is used", async () => { const diagnostics = await runner.diagnose(` @route("/test", { shared: true }) op test(): string; - @route("/test", { shared: true }) op test2(): string; + `); + expectDiagnostics(diagnostics, [ + { + code: "deprecated", + message: + "Deprecated: The `shared` option is deprecated, use the `@sharedRoute` decorator instead.", + }, + ]); + }); + + it("emit diagnostics when not all duplicated routes are declared shared", async () => { + const diagnostics = await runner.diagnose(` + @route("/test") @sharedRoute op test(): string; + @route("/test") @sharedRoute op test2(): string; @route("/test") op test3(): string; `); expectDiagnostics(diagnostics, [ @@ -261,8 +274,8 @@ describe("http: decorators", () => { it("do not emit diagnostics when duplicated shared routes are applied", async () => { const diagnostics = await runner.diagnose(` - @route("/test", {shared: true}) op test(): string; - @route("/test", {shared: true}) op test2(): string; + @route("/test") @sharedRoute op test(): string; + @route("/test") @sharedRoute op test2(): string; `); expectDiagnosticEmpty(diagnostics); @@ -274,8 +287,8 @@ describe("http: decorators", () => { `); expectDiagnostics(diagnostics, [ { - code: "@typespec/http/shared-boolean", - message: `shared parameter must be a boolean.`, + code: "invalid-argument", + message: `Argument '(anonymous model)' is not assignable to parameter of type '(anonymous model)'`, }, ]); }); diff --git a/packages/http/test/routes.test.ts b/packages/http/test/routes.test.ts index 35c556eee02..5f12ade3370 100644 --- a/packages/http/test/routes.test.ts +++ b/packages/http/test/routes.test.ts @@ -1,7 +1,13 @@ +import { Operation } from "@typespec/compiler"; import { expectDiagnosticEmpty, expectDiagnostics } from "@typespec/compiler/testing"; import { deepStrictEqual, strictEqual } from "assert"; -import { HttpOperation } from "../src/index.js"; -import { compileOperations, getOperations, getRoutesFor } from "./test-host.js"; +import { HttpOperation, getRoutePath } from "../src/index.js"; +import { + compileOperations, + createHttpTestRunner, + getOperations, + getRoutesFor, +} from "./test-host.js"; describe("http: routes", () => { // Describe how routes should be included. @@ -545,4 +551,53 @@ describe("http: routes", () => { { verb: "put", path: "/things/{thingId}", params: ["thingId"] }, ]); }); + + describe("shared routes", () => { + it("@sharedRoute decorator makes routes shared", async () => { + const runner = await createHttpTestRunner(); + const { get1, get2 } = (await runner.compile(` + @route("/test") + namespace Foo { + @test + @sharedRoute + @route("/get1") + op get1(): string; + } + + @route("/test") + namespace Foo { + @test + @route("/get2") + op get2(): string; + } + `)) as { get1: Operation; get2: Operation }; + + strictEqual(getRoutePath(runner.program, get1)?.shared, true); + strictEqual(getRoutePath(runner.program, get2)?.shared, false); + }); + + it("legacy `shared: true parameter` still works", async () => { + const runner = await createHttpTestRunner(); + const { get1, get2 } = (await runner.compile(` + @route("/test") + namespace Foo { + #suppress "deprecated" + @test + @route("/get1", { shared: true }) + op get1(): string; + } + + @route("/test") + namespace Foo { + #suppress "deprecated" + @test + @route("/get2", { shared: false }) + op get2(): string; + } + `)) as { get1: Operation; get2: Operation }; + + strictEqual(getRoutePath(runner.program, get1)?.shared, true); + strictEqual(getRoutePath(runner.program, get2)?.shared, false); + }); + }); }); diff --git a/packages/openapi3/test/shared-routes.test.ts b/packages/openapi3/test/shared-routes.test.ts index 81523e44b16..49893304096 100644 --- a/packages/openapi3/test/shared-routes.test.ts +++ b/packages/openapi3/test/shared-routes.test.ts @@ -41,12 +41,14 @@ describe("openapi3: shared routes", () => { id: string; } + @sharedRoute @operationId("List_ResourceGroup") - @route("/sharedroutes/resources", { shared: true }) + @route("/sharedroutes/resources") op listByResourceGroup(...Resource, @query resourceGroup: string, @query foo: string): Resource[]; + @sharedRoute @operationId("List_Subscription") - @route("/sharedroutes/resources", { shared: true }) + @route("/sharedroutes/resources") op listBySubscription(...Resource, @query subscription: string, @query foo: string): Resource[]; } ` @@ -96,10 +98,12 @@ describe("openapi3: shared routes", () => { id: string; } - @route("/sharedroutes/resources", { shared: true }) + @sharedRoute + @route("/sharedroutes/resources") op listByResourceGroup(...Resource, @query filter: "resourceGroup"): Resource[]; - @route("/sharedroutes/resources", { shared: true }) + @sharedRoute + @route("/sharedroutes/resources") op listBySubscription(...Resource, @query filter: "subscription"): Resource[]; } ` @@ -135,10 +139,12 @@ describe("openapi3: shared routes", () => { id: string; } - @route("/sharedroutes/resources", { shared: true }) + @sharedRoute + @route("/sharedroutes/resources") op listByResourceGroup(...Resource, @query filter: "resourceGroup"): Resource[]; - @route("/sharedroutes/resources", { shared: true }) + @sharedRoute + @route("/sharedroutes/resources") op listBySubscription(...Resource, @header filter: "subscription"): Resource[]; } ` @@ -183,10 +189,12 @@ describe("openapi3: shared routes", () => { id: string; } - @route("/sharedroutes/resources", { shared: true }) + @sharedRoute + @route("/sharedroutes/resources") op returnsInt(...Resource, @query options: string): int32; - @route("/sharedroutes/resources", { shared: true }) + @sharedRoute + @route("/sharedroutes/resources") op returnsString(...Resource, @query options: string): string; } ` @@ -224,10 +232,12 @@ describe("openapi3: shared routes", () => { ` @service({title: "My Service"}) namespace Foo { - @route("/sharedroutes/resources", { shared: true }) + @sharedRoute + @route("/sharedroutes/resources") op processInt(@body body: int32, @query options: string): void; - @route("/sharedroutes/resources", { shared: true }) + @sharedRoute + @route("/sharedroutes/resources") op processString(@body body: string, @query options: string): void; } ` @@ -261,10 +271,12 @@ describe("openapi3: shared routes", () => { ` @service({title: "My Service"}) namespace Foo { - @route("/process", { shared: true }) + @sharedRoute + @route("/process") op processInt(@body body: int32, @query options: string): int32; - @route("/process", { shared: true }) + @sharedRoute + @route("/process") op processString(@body body: string, @query options: string): string; } ` diff --git a/packages/rest/src/lib.ts b/packages/rest/src/lib.ts index f46d2003e86..0f5882c5993 100644 --- a/packages/rest/src/lib.ts +++ b/packages/rest/src/lib.ts @@ -39,6 +39,12 @@ const libDefinition = { default: "Action name cannot be empty string.", }, }, + "shared-route-unspecified-action-name": { + severity: "error", + messages: { + default: paramMessage`An operation marked as '@sharedRoute' must have an explicit collection action name passed to '${"decoratorName"}'.`, + }, + }, }, } as const; diff --git a/packages/rest/src/rest.ts b/packages/rest/src/rest.ts index 6ad7837fd4f..73ab672b8c3 100644 --- a/packages/rest/src/rest.ts +++ b/packages/rest/src/rest.ts @@ -79,7 +79,9 @@ function getResourceOperationHttpVerb( return ( getOperationVerb(program, operation) ?? (resourceOperation && resourceOperationToVerb[resourceOperation.operation]) ?? - (getAction(program, operation) || getCollectionAction(program, operation) ? "post" : undefined) + (getActionDetails(program, operation) || getCollectionActionDetails(program, operation) + ? "post" + : undefined) ); } @@ -421,8 +423,11 @@ function lowerCaseFirstChar(str: string): string { return str[0].toLocaleLowerCase() + str.substring(1); } -function makeActionName(op: Operation, name: string | undefined): string { - return lowerCaseFirstChar(name || op.name); +function makeActionName(op: Operation, name: string | undefined): ActionDetails { + return { + name: lowerCaseFirstChar(name || op.name), + kind: name ? "specified" : "automatic", + }; } const actionsSegmentKey = createStateSymbol("actionSegment"); @@ -435,6 +440,22 @@ export function getActionSegment(program: Program, entity: Type): string | undef return program.stateMap(actionsSegmentKey).get(entity); } +/** + * Provides details about an action or collection action. + */ +export interface ActionDetails { + /** + * The name of the action + */ + name: string; + + /** + * Identifies whether the action's name was generated from the original + * operation name or if it was explicitly specified. + */ + kind: "automatic" | "specified"; +} + const actionsKey = createStateSymbol("actions"); export function $action(context: DecoratorContext, entity: Operation, name?: string) { if (name === "") { @@ -444,15 +465,29 @@ export function $action(context: DecoratorContext, entity: Operation, name?: str }); return; } + const action = makeActionName(entity, name); - context.call($actionSegment, entity, action); + context.call($actionSegment, entity, action.name); context.program.stateMap(actionsKey).set(entity, action); } -export function getAction(program: Program, operation: Operation): string | null | undefined { +/** + * Gets the ActionDetails for the specified operation if it has previously been marked with @action. + */ +export function getActionDetails( + program: Program, + operation: Operation +): ActionDetails | undefined { return program.stateMap(actionsKey).get(operation); } +/** + * @deprecated Use getActionDetails instead. + */ +export function getAction(program: Program, operation: Operation): string | null | undefined { + return getActionDetails(program, operation)?.name; +} + const collectionActionsKey = createStateSymbol("collectionActions"); export function $collectionAction( @@ -473,16 +508,31 @@ export function $collectionAction( } const action = makeActionName(entity, name); - context.call($actionSegment, entity, action); + context.call($actionSegment, entity, action.name); - context.program.stateMap(collectionActionsKey).set(entity, `${segment}/${action}`); + // Update the final name and store it + action.name = `${segment}/${action.name}`; + context.program.stateMap(collectionActionsKey).set(entity, action); } +/** + * Gets the ActionDetails for the specified operation if it has previously been marked with @collectionAction. + */ +export function getCollectionActionDetails( + program: Program, + operation: Operation +): ActionDetails | undefined { + return program.stateMap(collectionActionsKey).get(operation); +} + +/** + * @deprecated Use getCollectionActionDetails instead. + */ export function getCollectionAction( program: Program, operation: Operation ): string | null | undefined { - return program.stateMap(collectionActionsKey).get(operation); + return getCollectionActionDetails(program, operation)?.name; } const resourceLocationsKey = createStateSymbol("resourceLocations"); diff --git a/packages/rest/src/validate.ts b/packages/rest/src/validate.ts index 48f6e09c3e1..3da0f5af67d 100644 --- a/packages/rest/src/validate.ts +++ b/packages/rest/src/validate.ts @@ -7,8 +7,10 @@ import { navigateTypesInNamespace, Program, } from "@typespec/compiler"; +import { isSharedRoute } from "@typespec/http"; import { reportDiagnostic } from "./lib.js"; import { getParentResource, getResourceTypeKey, ResourceKey } from "./resource.js"; +import { getActionDetails, getCollectionActionDetails } from "./rest.js"; function checkForDuplicateResourceKeyNames(program: Program) { const seenTypes = new Set(); @@ -60,8 +62,38 @@ function checkForDuplicateResourceKeyNames(program: Program) { } } +function checkForSharedRouteUnnamedActions(program: Program) { + for (const service of listServices(program)) { + // If the model type is defined under the service namespace, check that the + // parent resource type(s) don't have the same key name as the + // current resource type. + navigateTypesInNamespace(service.type, { + operation: (op) => { + const actionDetails = getActionDetails(program, op); + if ( + isSharedRoute(program, op) && + (actionDetails?.kind === "automatic" || + getCollectionActionDetails(program, op)?.kind === "automatic") + ) { + reportDiagnostic(program, { + code: "shared-route-unspecified-action-name", + target: op, + format: { + decoratorName: actionDetails ? "@action" : "@collectionAction", + }, + }); + } + }, + }); + } +} + export function $onValidate(program: Program) { // Make sure any defined resource types don't have any conflicts with parent // resource type key names checkForDuplicateResourceKeyNames(program); + + // Ensure that all shared routes with an `@action` decorator have an + // explicitly named action + checkForSharedRouteUnnamedActions(program); } diff --git a/packages/rest/test/routes.test.ts b/packages/rest/test/routes.test.ts index 459aef33019..7c49f11f1e0 100644 --- a/packages/rest/test/routes.test.ts +++ b/packages/rest/test/routes.test.ts @@ -1,7 +1,13 @@ import { ModelProperty, Operation } from "@typespec/compiler"; import { expectDiagnostics } from "@typespec/compiler/testing"; +import { isSharedRoute } from "@typespec/http"; import { deepStrictEqual, strictEqual } from "assert"; -import { compileOperations, getOperations, getRoutesFor } from "./test-host.js"; +import { + compileOperations, + createRestTestRunner, + getOperations, + getRoutesFor, +} from "./test-host.js"; describe("rest: routes", () => { it("always produces a route starting with /", async () => { @@ -441,4 +447,66 @@ describe("rest: routes", () => { { verb: "put", path: "/things/{thingId}", params: ["thingId"] }, ]); }); + + it("@autoRoute operations can also be shared routes", async () => { + const runner = await createRestTestRunner(); + const { get1, get2 } = (await runner.compile(` + @test + @autoRoute + @sharedRoute + op get1(@segment("get1") @path name: string): string; + + @test + @autoRoute + op get2(@segment("get2") @path name: string): string; + `)) as { get1: Operation; get2: Operation }; + + strictEqual(isSharedRoute(runner.program, get1), true); + strictEqual(isSharedRoute(runner.program, get2), false); + }); + + it("emits a diagnostic when @sharedRoute is used on action without explicit name", async () => { + const [_, diagnostics] = await compileOperations( + ` + model Thing { + @key + @segment("things") + thingId: string; + } + + @action + @autoRoute + @sharedRoute + op badAction(): {}; + + @action("good") + @autoRoute + @sharedRoute + op goodAction(): {}; + + @autoRoute + @sharedRoute + @collectionAction(Thing) + op badCollectionAction(): {}; + + @autoRoute + @sharedRoute + @collectionAction(Thing, "goodCollection") + op goodCollectionAction(): {}; + ` + ); + + expectDiagnostics(diagnostics, [ + { + code: "@typespec/rest/shared-route-unspecified-action-name", + message: + "An operation marked as '@sharedRoute' must have an explicit collection action name passed to '@action'.", + }, + { + code: "@typespec/rest/shared-route-unspecified-action-name", + message: + "An operation marked as '@sharedRoute' must have an explicit collection action name passed to '@collectionAction'.", + }, + ]); + }); });