From 1672b8317e4ad69c7b29190cb653c50e8d74c591 Mon Sep 17 00:00:00 2001 From: Will Temple Date: Wed, 18 Dec 2024 16:52:11 -0500 Subject: [PATCH 01/22] Additional Delete/Query visibilities and filter versions of returntype/parameter visibility decorator metadata. --- packages/compiler/lib/std/visibility.tsp | 103 +++++++++++++++++- .../compiler/src/core/visibility/lifecycle.ts | 8 ++ packages/compiler/src/lib/visibility.ts | 61 +++++++++++ .../test/decorators/visibility.test.ts | 10 +- packages/compiler/test/visibility.test.ts | 2 +- packages/openapi/src/helpers.ts | 9 +- .../standard-library/built-in-data-types.md | 98 ++++++++++++++++- 7 files changed, 278 insertions(+), 13 deletions(-) diff --git a/packages/compiler/lib/std/visibility.tsp b/packages/compiler/lib/std/visibility.tsp index e6b7641f1c4..3de21ddf924 100644 --- a/packages/compiler/lib/std/visibility.tsp +++ b/packages/compiler/lib/std/visibility.tsp @@ -176,8 +176,7 @@ extern dec defaultVisibility(target: Enum, ...visibilities: valueof EnumMember[] /** * A visibility class for resource lifecycle phases. * - * These visibilities control whether a property is visible during the create, read, and update phases of a resource's - * lifecycle. + * These visibilities control whether a property is visible during the various phases of a resource's lifecycle. * * @example * ```typespec @@ -195,9 +194,32 @@ extern dec defaultVisibility(target: Enum, ...visibilities: valueof EnumMember[] * therefore visible in all phases. */ enum Lifecycle { + /** + * The property is visible when a resource is being created. + */ Create, + + /** + * The property is visible when a resource is being read. + */ Read, + + /** + * The property is visible when a resource is being updated. + */ Update, + + /** + * The property is visible when a resource is being deleted. + */ + Delete, + + /** + * The property is visible when a resource is being queried. + * + * In HTTP APIs, this visibility applies to parameters of GET or HEAD operations. + */ + Query, } /** @@ -306,6 +328,9 @@ model Create { ...T; } + +/** + * A copy of the input model `T` with only the properties that are visible during the + * "Delete" resource lifecycle phase. + * + * The "Delete" lifecycle phase is used for properties passed as parameters to operations + * that delete data, like HTTP DELETE operations. + * + * This transformation is recursive, and will include only properties that have the + * `Lifecycle.Delete` visibility modifier. + * + * If a `NameTemplate` is provided, the new model will be named according to the template. + * The template uses the same syntax as the `@friendlyName` decorator. + * + * @template T The model to transform. + * @template NameTemplate The name template to use for the new model. + * + * * @example + * ```typespec + * model Dog { + * @visibility(Lifecycle.Read) + * id: int32; + * + * name: string; + * } + * + * model DeleteDog is Delete; + * ``` + */ +@friendlyName(NameTemplate, T) +@withVisibilityFilter(#{ all: #[Lifecycle.Delete] }) +model Delete { + ...T; +} + +/** + * A copy of the input model `T` with only the properties that are visible during the + * "Query" resource lifecycle phase. + * + * The "Query" lifecycle phase is used for properties passed as parameters to operations + * that read data, like HTTP GET or HEAD operations. + * + * This transformation is recursive, and will include only properties that have the + * `Lifecycle.Query` visibility modifier. + * + * If a `NameTemplate` is provided, the new model will be named according to the template. + * The template uses the same syntax as the `@friendlyName` decorator. + * + * @template T The model to transform. + * @template NameTemplate The name template to use for the new model. + * + * * @example + * ```typespec + * model Dog { + * @visibility(Lifecycle.Read) + * id: int32; + * + * name: string; + * } + * + * model QueryDog is Query; + * ``` + */ +@friendlyName(NameTemplate, T) +@withVisibilityFilter(#{ all: #[Lifecycle.Query] }) +model Query { + ...T; +} diff --git a/packages/compiler/src/core/visibility/lifecycle.ts b/packages/compiler/src/core/visibility/lifecycle.ts index 0d6addf6273..2660c14d174 100644 --- a/packages/compiler/src/core/visibility/lifecycle.ts +++ b/packages/compiler/src/core/visibility/lifecycle.ts @@ -54,6 +54,10 @@ export function normalizeLegacyLifecycleVisibilityString( return lifecycle.members.get("Read")!; case "update": return lifecycle.members.get("Update")!; + case "delete": + return lifecycle.members.get("Delete")!; + case "query": + return lifecycle.members.get("Query")!; default: return undefined; } @@ -83,6 +87,10 @@ export function normalizeVisibilityToLegacyLifecycleString( return "read"; case "Update": return "update"; + case "Delete": + return "delete"; + case "Query": + return "query"; default: return undefined; } diff --git a/packages/compiler/src/lib/visibility.ts b/packages/compiler/src/lib/visibility.ts index 5d8f8245a8d..51ef36ef8ed 100644 --- a/packages/compiler/src/lib/visibility.ts +++ b/packages/compiler/src/lib/visibility.ts @@ -46,6 +46,7 @@ import { } from "../core/visibility/core.js"; import { getLifecycleVisibilityEnum, + normalizeLegacyLifecycleVisibilityString, normalizeVisibilityToLegacyLifecycleString, } from "../core/visibility/lifecycle.js"; import { isMutableType, mutateSubgraph, Mutator, MutatorFlow } from "../experimental/mutators.js"; @@ -180,6 +181,9 @@ export const $parameterVisibility: ParameterVisibilityDecorator = ( /** * Returns the visibilities of the parameters of the given operation, if provided with `@parameterVisibility`. * + * @deprecated Use `getParameterVisibilityFilter` instead. + * + * @see {@link getParameterVisibilityFilter} * @see {@link $parameterVisibility} */ export function getParameterVisibility(program: Program, entity: Operation): string[] | undefined { @@ -190,6 +194,33 @@ export function getParameterVisibility(program: Program, entity: Operation): str .filter((p) => !!p) as string[]; } +/** + * Get the visibility filter that should apply to the parameters of the given operation, or `undefined` if no parameter + * visibility is set. + * + * @param program - the Program in which the operation is defined + * @param operation - the Operation to get the parameter visibility filter for + * @returns a visibility filter for the parameters of the operation, or `undefined` if no parameter visibility is set + */ +export function getParameterVisibilityFilter( + program: Program, + operation: Operation, +): VisibilityFilter | undefined { + const visibilityConfig = getOperationVisibilityConfig(program, operation); + + if (!visibilityConfig.parameters) return undefined; + + return { + any: new Set( + visibilityConfig.parameters + .map((v) => + typeof v === "string" ? normalizeLegacyLifecycleVisibilityString(program, v) : v, + ) + .filter((v) => !!v), + ), + }; +} + export const $returnTypeVisibility: ReturnTypeVisibilityDecorator = ( context: DecoratorContext, operation: Operation, @@ -218,6 +249,9 @@ export const $returnTypeVisibility: ReturnTypeVisibilityDecorator = ( /** * Returns the visibilities of the return type of the given operation, if provided with `@returnTypeVisibility`. * + * @deprecated Use `getReturnTypeVisibilityFilter` instead. + * + * @see {@link getReturnTypeVisibilityFilter} * @see {@link $returnTypeVisibility} */ export function getReturnTypeVisibility(program: Program, entity: Operation): string[] | undefined { @@ -228,6 +262,33 @@ export function getReturnTypeVisibility(program: Program, entity: Operation): st .filter((p) => !!p) as string[]; } +/** + * Get the visibility filter that should apply to the return type of the given operation, or `undefined` if no return + * type visibility is set. + * + * @param program - the Program in which the operation is defined + * @param operation - the Operation to get the return type visibility filter for + * @returns a visibility filter for the return type of the operation, or `undefined` if no return type visibility is set + */ +export function getReturnTypeVisibilityFilter( + program: Program, + operation: Operation, +): VisibilityFilter | undefined { + const visibilityConfig = getOperationVisibilityConfig(program, operation); + + if (!visibilityConfig.returnType) return undefined; + + return { + any: new Set( + visibilityConfig.returnType + .map((v) => + typeof v === "string" ? normalizeLegacyLifecycleVisibilityString(program, v) : v, + ) + .filter((v) => !!v), + ), + }; +} + // #endregion // #region Core Visibility Decorators diff --git a/packages/compiler/test/decorators/visibility.test.ts b/packages/compiler/test/decorators/visibility.test.ts index af06ede963e..737d33427e4 100644 --- a/packages/compiler/test/decorators/visibility.test.ts +++ b/packages/compiler/test/decorators/visibility.test.ts @@ -43,11 +43,19 @@ describe("visibility", function () { Read: LifecycleEnum.members.get("Read")!, Create: LifecycleEnum.members.get("Create")!, Update: LifecycleEnum.members.get("Update")!, + Delete: LifecycleEnum.members.get("Delete")!, + Query: LifecycleEnum.members.get("Query")!, }; assertSetsEqual( getVisibilityForClass(runner.program, name, LifecycleEnum), - new Set([Lifecycle.Read, Lifecycle.Create, Lifecycle.Update]), + new Set([ + Lifecycle.Read, + Lifecycle.Create, + Lifecycle.Update, + Lifecycle.Delete, + Lifecycle.Query, + ]), ); assertSetsEqual( diff --git a/packages/compiler/test/visibility.test.ts b/packages/compiler/test/visibility.test.ts index 837464c1fe9..b15d1a3fa92 100644 --- a/packages/compiler/test/visibility.test.ts +++ b/packages/compiler/test/visibility.test.ts @@ -299,7 +299,7 @@ describe("compiler: visibility core", () => { const resetVisibility = getVisibilityForClass(runner.program, x, Lifecycle); - strictEqual(resetVisibility.size, 3); + strictEqual(resetVisibility.size, 5); for (const member of Lifecycle.members.values()) { ok(resetVisibility.has(member)); diff --git a/packages/openapi/src/helpers.ts b/packages/openapi/src/helpers.ts index 411de4eceb2..e49a0566087 100644 --- a/packages/openapi/src/helpers.ts +++ b/packages/openapi/src/helpers.ts @@ -2,9 +2,10 @@ import { Diagnostic, DiagnosticTarget, getFriendlyName, + getLifecycleVisibilityEnum, getProperty, getTypeName, - getVisibility, + getVisibilityForClass, isGlobalNamespace, isService, isTemplateInstance, @@ -164,11 +165,11 @@ export function resolveOperationId(program: Program, operation: Operation) { * designate a read-only property. */ export function isReadonlyProperty(program: Program, property: ModelProperty) { - // eslint-disable-next-line @typescript-eslint/no-deprecated - const visibility = getVisibility(program, property); + const Lifecycle = getLifecycleVisibilityEnum(program); + const visibility = getVisibilityForClass(program, property, getLifecycleVisibilityEnum(program)); // note: multiple visibilities that include read are not handled using // readonly: true, but using separate schemas. - return visibility?.length === 1 && visibility[0] === "read"; + return visibility.size === 1 && visibility.has(Lifecycle.members.get("Read")!); } /** diff --git a/website/src/content/docs/docs/standard-library/built-in-data-types.md b/website/src/content/docs/docs/standard-library/built-in-data-types.md index 80aba54b417..5eb71cd6be1 100644 --- a/website/src/content/docs/docs/standard-library/built-in-data-types.md +++ b/website/src/content/docs/docs/standard-library/built-in-data-types.md @@ -60,6 +60,9 @@ None A copy of the input model `T` with only the properties that are visible during the "Create" or "Update" resource lifecycle phases. +The "CreateOrUpdate" lifecycle phase is used by default for properties passed as parameters to operations +that can create _or_ update data, like HTTP PUT operations. + This transformation is recursive, and will include only properties that have the `Lifecycle.Create` or `Lifecycle.Update` visibility modifier. @@ -105,6 +108,45 @@ model DefaultKeyVisibility | Visibility | The visibility to apply to all properties. | +#### Properties +None + +### `Delete` {#Delete} + +A copy of the input model `T` with only the properties that are visible during the +"Delete" resource lifecycle phase. + +The "Delete" lifecycle phase is used for properties passed as parameters to operations +that delete data, like HTTP DELETE operations. + +This transformation is recursive, and will include only properties that have the +`Lifecycle.Delete` visibility modifier. + +If a `NameTemplate` is provided, the new model will be named according to the template. +The template uses the same syntax as the `@friendlyName` decorator. +```typespec +model Delete +``` + +#### Template Parameters +| Name | Description | +|------|-------------| +| T | The model to transform. | +| NameTemplate | The name template to use for the new model.

* | + +#### Examples + +```typespec +model Dog { + @visibility(Lifecycle.Read) + id: int32; + + name: string; +} + +model DeleteDog is Delete; +``` + #### Properties None @@ -213,6 +255,45 @@ model PickProperties | Keys | The property keys to include. | +#### Properties +None + +### `Query` {#Query} + +A copy of the input model `T` with only the properties that are visible during the +"Query" resource lifecycle phase. + +The "Query" lifecycle phase is used for properties passed as parameters to operations +that read data, like HTTP GET or HEAD operations. + +This transformation is recursive, and will include only properties that have the +`Lifecycle.Query` visibility modifier. + +If a `NameTemplate` is provided, the new model will be named according to the template. +The template uses the same syntax as the `@friendlyName` decorator. +```typespec +model Query +``` + +#### Template Parameters +| Name | Description | +|------|-------------| +| T | The model to transform. | +| NameTemplate | The name template to use for the new model.

* | + +#### Examples + +```typespec +model Dog { + @visibility(Lifecycle.Read) + id: int32; + + name: string; +} + +model QueryDog is Query; +``` + #### Properties None @@ -221,6 +302,9 @@ None A copy of the input model `T` with only the properties that are visible during the "Read" resource lifecycle phase. +The "Read" lifecycle phase is used for properties returned by operations that read data, like +HTTP GET operations. + This transformation is recursive, and will include only properties that have the `Lifecycle.Read` visibility modifier. @@ -288,6 +372,9 @@ model ServiceOptions A copy of the input model `T` with only the properties that are visible during the "Update" resource lifecycle phase. +The "Update" lifecycle phase is used for properties passed as parameters to operations +that update data, like HTTP PATCH operations. + This transformation will include only the properties that have the `Lifecycle.Update` visibility modifier, and the types of all properties will be replaced with the equivalent `CreateOrUpdate` transformation. @@ -414,17 +501,18 @@ enum DurationKnownEncoding A visibility class for resource lifecycle phases. -These visibilities control whether a property is visible during the create, read, and update phases of a resource's -lifecycle. +These visibilities control whether a property is visible during the various phases of a resource's lifecycle. ```typespec enum Lifecycle ``` | Name | Value | Description | |------|-------|-------------| -| Create | | | -| Read | | | -| Update | | | +| Create | | The property is visible when a resource is being created. | +| Read | | The property is visible when a resource is being read. | +| Update | | The property is visible when a resource is being updated. | +| Delete | | The property is visible when a resource is being deleted. | +| Query | | The property is visible when a resource is being queried.

In HTTP APIs, this visibility applies to parameters of GET or HEAD operations. | #### Examples ```typespec From 3721fa3c70c08cc5217aded39845cee3dceeb267 Mon Sep 17 00:00:00 2001 From: Will Temple Date: Thu, 19 Dec 2024 00:45:39 -0500 Subject: [PATCH 02/22] Rework HTTP to use new visibility APIs --- packages/compiler/src/core/visibility/core.ts | 41 ++-- .../compiler/src/core/visibility/index.ts | 1 + packages/compiler/src/lib/visibility.ts | 56 ++++- packages/compiler/test/visibility.test.ts | 80 +++++++ packages/http/src/metadata.ts | 224 +++++++++++++++--- 5 files changed, 347 insertions(+), 55 deletions(-) diff --git a/packages/compiler/src/core/visibility/core.ts b/packages/compiler/src/core/visibility/core.ts index 796705a8784..b68ebd58097 100644 --- a/packages/compiler/src/core/visibility/core.ts +++ b/packages/compiler/src/core/visibility/core.ts @@ -603,18 +603,31 @@ export function hasVisibility( * AND * * - NONE of the visibilities in the `none` set. + * + * Note: The constraints behave similarly to the `every` and `some` methods of the Array prototype in JavaScript. If the + * `any` constraint is set to an empty set, it will _NEVER_ be satisfied (similarly, `Array.prototype.some` will always + * return `false` for an empty array). If the `none` constraint is set to an empty set, it will _ALWAYS_ be satisfied. + * If the `all` constraint is set to an empty set, it will be satisfied (similarly, `Array.prototype.every` will always + * return `true` for an empty array). + * */ export interface VisibilityFilter { /** * If set, the filter considers a property visible if it has ALL of these visibility modifiers. + * + * If this set is empty, the filter will be satisfied if the other constraints are satisfied. */ all?: Set; /** * If set, the filter considers a property visible if it has ANY of these visibility modifiers. + * + * If this set is empty, the filter will _NEVER_ be satisfied. */ any?: Set; /** * If set, the filter considers a property visible if it has NONE of these visibility modifiers. + * + * If this set is empty, the filter will be satisfied if the other constraints are satisfied. */ none?: Set; } @@ -690,30 +703,30 @@ export function isVisible( return isVisibleLegacy(_filterOrLegacyVisibilities); } - const filter = { ...(_filterOrLegacyVisibilities as VisibilityFilter) }; - filter.all ??= new Set(); - filter.any ??= new Set(); - filter.none ??= new Set(); + const filter = _filterOrLegacyVisibilities as VisibilityFilter; // Validate that property has ALL of the required visibilities of filter.all - for (const modifier of filter.all) { - if (!hasVisibility(program, property, modifier)) return false; + if (filter.all) { + for (const modifier of filter.all) { + if (!hasVisibility(program, property, modifier)) return false; + } } - // Validate that property has ANY of the required visibilities of filter.any - outer: while (filter.any.size > 0) { + // Validate that property has NONE of the excluded visibilities of filter.none + if (filter.none) { + for (const modifier of filter.none) { + if (hasVisibility(program, property, modifier)) return false; + } + } + + if (filter.any) { for (const modifier of filter.any) { - if (hasVisibility(program, property, modifier)) break outer; + if (hasVisibility(program, property, modifier)) return true; } return false; } - // Validate that property has NONE of the excluded visibilities of filter.none - for (const modifier of filter.none) { - if (hasVisibility(program, property, modifier)) return false; - } - return true; function isVisibleLegacy(visibilities: readonly string[]) { diff --git a/packages/compiler/src/core/visibility/index.ts b/packages/compiler/src/core/visibility/index.ts index 97ae71a0f7c..2f4ac16c600 100644 --- a/packages/compiler/src/core/visibility/index.ts +++ b/packages/compiler/src/core/visibility/index.ts @@ -4,6 +4,7 @@ export { getLifecycleVisibilityEnum } from "./lifecycle.js"; export { + VisibilityFilter, addVisibilityModifiers, clearVisibilityModifiersForClass, getVisibility, diff --git a/packages/compiler/src/lib/visibility.ts b/packages/compiler/src/lib/visibility.ts index 51ef36ef8ed..03010ea36a0 100644 --- a/packages/compiler/src/lib/visibility.ts +++ b/packages/compiler/src/lib/visibility.ts @@ -194,25 +194,62 @@ export function getParameterVisibility(program: Program, entity: Operation): str .filter((p) => !!p) as string[]; } +/** + * A context-specific provider for visibility information that applies when parameter or return type visibility + * constraints are not explicitly specified. Visibility providers are provided by libraries that define implied + * visibility semantics, such as `@typespec/http`. + * + * If you are not working in a protocol that has specific visibility semantics, you can use the + * {@link EmptyVisibilityProvider} from this package as a default provider. It will consider all properties visible by + * default unless otherwise explicitly specified. + */ +export interface VisibilityProvider { + parameters(program: Program, operation: Operation): VisibilityFilter; + returnType(program: Program, operation: Operation): VisibilityFilter; +} + +/** + * An empty visibility provider. This provider returns an empty filter that considers all properties visible. This filter + * is used when no context-specific visibility provider is available. + * + * When working with an HTTP specification, use the `HttpVisibilityProvider` from the `@typespec/http` library instead. + */ +export const EmptyVisibilityProvider: VisibilityProvider = { + parameters: () => ({}), + returnType: () => ({}), +}; + /** * Get the visibility filter that should apply to the parameters of the given operation, or `undefined` if no parameter * visibility is set. * + * If you are not working in a protocol that has specific implicit visibility semantics, you can use the + * {@link EmptyVisibilityProvider} as a default provider. If you working in a protocol or context where parameters have + * implicit visibility transformations (like HTTP), you should use the visibility provider from that library (for HTTP, + * use the `HttpVisibilityProvider` from the `@typespec/http` library). + * * @param program - the Program in which the operation is defined * @param operation - the Operation to get the parameter visibility filter for + * @param defaultProvider - a provider for visibility filters that apply when no visibility constraints are explicitly + * set. Defaults to an empty provider that returns an empty filter if not provided. * @returns a visibility filter for the parameters of the operation, or `undefined` if no parameter visibility is set */ export function getParameterVisibilityFilter( program: Program, operation: Operation, -): VisibilityFilter | undefined { - const visibilityConfig = getOperationVisibilityConfig(program, operation); + defaultProvider: VisibilityProvider, +): VisibilityFilter { + const operationVisibilityConfig = getOperationVisibilityConfig(program, operation); - if (!visibilityConfig.parameters) return undefined; + if (!operationVisibilityConfig.parameters) return defaultProvider.parameters(program, operation); return { + // WARNING: the HTTP library depends on `any` being the only key in the filter object returned by this method. + // if you change this logic, you will need to update the HTTP library to account for differences in the + // returned object. HTTP does not currently have a way to express `all` or `none` constraints in the same + // way that the core visibility system does. any: new Set( - visibilityConfig.parameters + operationVisibilityConfig.parameters .map((v) => typeof v === "string" ? normalizeLegacyLifecycleVisibilityString(program, v) : v, ) @@ -268,17 +305,24 @@ export function getReturnTypeVisibility(program: Program, entity: Operation): st * * @param program - the Program in which the operation is defined * @param operation - the Operation to get the return type visibility filter for + * @param defaultProvider - a provider for visibility filters that apply when no visibility constraints are explicitly + * set. Defaults to an empty provider that returns an empty filter if not provided. * @returns a visibility filter for the return type of the operation, or `undefined` if no return type visibility is set */ export function getReturnTypeVisibilityFilter( program: Program, operation: Operation, -): VisibilityFilter | undefined { + defaultProvider: VisibilityProvider, +): VisibilityFilter { const visibilityConfig = getOperationVisibilityConfig(program, operation); - if (!visibilityConfig.returnType) return undefined; + if (!visibilityConfig.returnType) return defaultProvider.returnType(program, operation); return { + // WARNING: the HTTP library depends on `any` being the only key in the filter object returned by this method. + // if you change this logic, you will need to update the HTTP library to account for differences in the + // returned object. HTTP does not currently have a way to express `all` or `none` constraints in the same + // way that the core visibility system does. any: new Set( visibilityConfig.returnType .map((v) => diff --git a/packages/compiler/test/visibility.test.ts b/packages/compiler/test/visibility.test.ts index b15d1a3fa92..987a60d1840 100644 --- a/packages/compiler/test/visibility.test.ts +++ b/packages/compiler/test/visibility.test.ts @@ -9,14 +9,17 @@ import { addVisibilityModifiers, clearVisibilityModifiersForClass, DecoratorContext, + EmptyVisibilityProvider, Enum, getLifecycleVisibilityEnum, + getParameterVisibilityFilter, getVisibilityForClass, hasVisibility, isSealed, isVisible, Model, ModelProperty, + Operation, removeVisibilityModifiers, resetVisibilityModifiersForClass, sealVisibilityModifiers, @@ -569,6 +572,83 @@ describe("compiler: visibility core", () => { true, ); }); + + describe("parameter visibility filters", () => { + it("correctly provides empty default visibility filter", async () => { + const { Example, foo } = (await runner.compile(` + @test model Example { + @visibility(Lifecycle.Create) + x: string; + } + + @test op foo(example: Example): void; + `)) as { Example: Model; foo: Operation }; + + const x = Example.properties.get("x")!; + + const filter = getParameterVisibilityFilter(runner.program, foo, EmptyVisibilityProvider); + + strictEqual(filter.all, undefined); + strictEqual(filter.any, undefined); + strictEqual(filter.none, undefined); + + strictEqual(isVisible(runner.program, x, filter), true); + }); + + it("correctly provides visibility filter from operation", async () => { + const { Example, foo } = (await runner.compile(` + @test model Example { + @visibility(Lifecycle.Create) + x: string; + } + + @parameterVisibility(Lifecycle.Update) + @test op foo( + example: Example + ): void; + `)) as { Example: Model; foo: Operation }; + + const x = Example.properties.get("x")!; + + const filter = getParameterVisibilityFilter(runner.program, foo, EmptyVisibilityProvider); + + const Lifecycle = getLifecycleVisibilityEnum(runner.program); + + strictEqual(filter.all, undefined); + strictEqual(filter.any?.size, 1); + strictEqual(filter.any.has(Lifecycle.members.get("Update")!), true); + strictEqual(filter.none, undefined); + + strictEqual(isVisible(runner.program, x, filter), false); + }); + + it("correctly coerces legacy string in visibility filter", async () => { + const { Example, foo } = (await runner.compile(` + @test model Example { + @visibility(Lifecycle.Create) + x: string; + } + + @parameterVisibility("update") + @test op foo( + example: Example + ): void; + `)) as { Example: Model; foo: Operation }; + + const x = Example.properties.get("x")!; + + const filter = getParameterVisibilityFilter(runner.program, foo, EmptyVisibilityProvider); + + const Lifecycle = getLifecycleVisibilityEnum(runner.program); + + strictEqual(filter.all, undefined); + strictEqual(filter.any?.size, 1); + strictEqual(filter.any.has(Lifecycle.members.get("Update")!), true); + strictEqual(filter.none, undefined); + + strictEqual(isVisible(runner.program, x, filter), false); + }); + }); }); describe("legacy compatibility", () => { diff --git a/packages/http/src/metadata.ts b/packages/http/src/metadata.ts index 2f789815744..c092bdf548d 100644 --- a/packages/http/src/metadata.ts +++ b/packages/http/src/metadata.ts @@ -1,7 +1,9 @@ import { compilerAssert, + EnumMember, getEffectiveModelType, - getParameterVisibility, + getLifecycleVisibilityEnum, + getParameterVisibilityFilter, isVisible as isVisibleCore, Model, ModelProperty, @@ -9,9 +11,12 @@ import { Program, Type, Union, + type VisibilityFilter, + VisibilityProvider, } from "@typespec/compiler"; import { TwoLevelMap } from "@typespec/compiler/utils"; import { + getOperationVerb, includeInapplicableMetadataInPayload, isBody, isBodyIgnore, @@ -23,7 +28,7 @@ import { isQueryParam, isStatusCode, } from "./decorators.js"; -import { HttpVerb } from "./types.js"; +import { HttpVerb, OperationParameterOptions } from "./types.js"; /** * Flags enum representation of well-known visibilities that are used in @@ -84,33 +89,117 @@ function visibilityToArray(visibility: Visibility): readonly string[] { return result; } -function arrayToVisibility(array: readonly string[] | undefined): Visibility | undefined { - if (!array) { - return undefined; - } + +function filterToVisibility(program: Program, filter: VisibilityFilter): Visibility { + const Lifecycle = getLifecycleVisibilityEnum(program); let value = Visibility.None; - for (const item of array) { - switch (item) { - case "read": - value |= Visibility.Read; - break; - case "create": - value |= Visibility.Create; - break; - case "update": - value |= Visibility.Update; - break; - case "delete": - value |= Visibility.Delete; - break; - case "query": - value |= Visibility.Query; - break; - default: - return undefined; + + compilerAssert( + !filter.all, + "Unexpected: `all` constraint in visibility filter passed to filterToVisibility", + ); + compilerAssert( + !filter.none, + "Unexpected: `none` constraint in visibility filter passed to filterToVisibility", + ); + + if (filter.any) { + for (const item of filter.any ?? []) { + if (item.enum !== Lifecycle) continue; + switch (item.name) { + case "Read": + value |= Visibility.Read; + break; + case "Create": + value |= Visibility.Create; + break; + case "Update": + value |= Visibility.Update; + break; + case "Delete": + value |= Visibility.Delete; + break; + case "Query": + value |= Visibility.Query; + break; + default: + compilerAssert( + false, + `Unreachable: unrecognized Lifecycle visibility member: '${item.name}'`, + ); + } } + + return value; + } else { + return Visibility.All; } - return value; +} + +const VISIBILITY_FILTER_CACHE_MAP = new WeakMap>(); + +function getVisibilityFilterCache(program: Program): Map { + let cache = VISIBILITY_FILTER_CACHE_MAP.get(program); + if (!cache) { + cache = new Map(); + VISIBILITY_FILTER_CACHE_MAP.set(program, cache); + } + return cache; +} + +/** + * Convert an HTTP visibility to a visibility filter that can be used to test core visibility and applied to a model. + * + * The Item and Patch visibility flags are ignored. + * + * @param program - the Program we're working in + * @param visibility - the visibility to convert to a filter + * @returns a VisibilityFilter object that selects properties having any of the given visibility flags + */ +function visibilityToFilter(program: Program, visibility: Visibility): VisibilityFilter { + const cache = getVisibilityFilterCache(program); + let filter = cache.get(visibility); + + if (!filter) { + // Item and Patch flags are not real visibilities. + visibility &= ~(Visibility.Item | Visibility.Patch); + + const LifecycleEnum = getLifecycleVisibilityEnum(program); + + const Lifecycle = { + Create: LifecycleEnum.members.get("Create")!, + Read: LifecycleEnum.members.get("Read")!, + Update: LifecycleEnum.members.get("Update")!, + Delete: LifecycleEnum.members.get("Delete")!, + Query: LifecycleEnum.members.get("Query")!, + } as const; + + const any = new Set(); + + if (visibility & Visibility.Read) { + any.add(Lifecycle.Read); + } + if (visibility & Visibility.Create) { + any.add(Lifecycle.Create); + } + if (visibility & Visibility.Update) { + any.add(Lifecycle.Update); + } + if (visibility & Visibility.Delete) { + any.add(Lifecycle.Delete); + } + if (visibility & Visibility.Query) { + any.add(Lifecycle.Query); + } + + compilerAssert(any.size > 0 || visibility === Visibility.None, "invalid visibility"); + + filter = { any }; + + cache.set(visibility, filter); + } + + return filter; } /** @@ -130,7 +219,7 @@ function arrayToVisibility(array: readonly string[] | undefined): Visibility | u * */ export function getVisibilitySuffix( visibility: Visibility, - canonicalVisibility: Visibility | undefined = Visibility.None, + canonicalVisibility: Visibility = Visibility.None, ) { let suffix = ""; @@ -168,8 +257,7 @@ function getDefaultVisibilityForVerb(verb: HttpVerb): Visibility { case "delete": return Visibility.Delete; default: - const _assertNever: never = verb; - compilerAssert(false, "unreachable"); + compilerAssert(false, `Unreachable: unrecognized HTTP verb: '${verb satisfies never}'`); } } @@ -195,6 +283,71 @@ export function getRequestVisibility(verb: HttpVerb): Visibility { return visibility; } +/** + * A visibility provider for HTTP operations. Pass this value as a provider to the `getParameterVisibilityFilter` and + * `getReturnTypeVisibilityFilter` functions in the TypeSpec core to get the applicable parameter and return type + * visibility filters for an HTTP operation. + * + * When created with a verb, this provider will use the default visibility for that verb. + * + * @param verb - the HTTP verb for the operation + * + * @see {@link VisibilityProvider} + * @see {@link getParameterVisibilityFilter} + * @see {@link getReturnTypeVisibilityFilter} + */ +export function HttpVisibilityProvider(verb: HttpVerb): VisibilityProvider; +/** + * A visibility provider for HTTP operations. Pass this value as a provider to the `getParameterVisibilityFilter` and + * `getReturnTypeVisibilityFilter` functions in the TypeSpec core to get the applicable parameter and return type + * visibility filters for an HTTP operation. + * + * When created with an options object, this provider will use the `verbSelector` function to determine the verb for the + * operation and use the default visibility for that verb, or the configured HTTP verb for the operation, and finally + * the GET verb if the verbSelector function is not defined and no HTTP verb is configured. + * + * @param options - an options object with a `verbSelector` function that returns the HTTP verb for the operation + * + * @see {@link VisibilityProvider} + * @see {@link getParameterVisibilityFilter} + * @see {@link getReturnTypeVisibilityFilter} + */ +export function HttpVisibilityProvider(options: OperationParameterOptions): VisibilityProvider; +/** + * A visibility provider for HTTP operations. Pass this value as a provider to the `getParameterVisibilityFilter` and + * `getReturnTypeVisibilityFilter` functions in the TypeSpec core to get the applicable parameter and return type + * visibility filters for an HTTP operation. + * + * When created without any arguments, this provider will use the configured verb for the operation or the GET verb if + * no HTTP verb is configured and use the default visibility for that selected verb. + * + * @see {@link VisibilityProvider} + * @see {@link getParameterVisibilityFilter} + * @see {@link getReturnTypeVisibilityFilter} + */ +export function HttpVisibilityProvider(): VisibilityProvider; +export function HttpVisibilityProvider( + verbOrParameterOptions?: HttpVerb | OperationParameterOptions, +): VisibilityProvider { + const hasVerb = typeof verbOrParameterOptions === "string"; + + return { + parameters: (program, operation) => { + const verb = hasVerb + ? (verbOrParameterOptions as HttpVerb) + : (verbOrParameterOptions?.verbSelector?.(program, operation) ?? + getOperationVerb(program, operation) ?? + "get"); + return visibilityToFilter(program, getDefaultVisibilityForVerb(verb)); + }, + returnType: (program, _) => { + const Read = getLifecycleVisibilityEnum(program).members.get("Read")!; + // For return types, we always use Read visibility in HTTP. + return { any: new Set([Read]) }; + }, + }; +} + /** * Returns the applicable parameter visibility or visibilities for the request if `@requestVisibility` was used. * Otherwise, returns the default visibility based on the HTTP verb for the operation. @@ -207,10 +360,12 @@ export function resolveRequestVisibility( operation: Operation, verb: HttpVerb, ): Visibility { - const parameterVisibility = getParameterVisibility(program, operation); - const parameterVisibilityArray = arrayToVisibility(parameterVisibility); - const defaultVisibility = getDefaultVisibilityForVerb(verb); - let visibility = parameterVisibilityArray ?? defaultVisibility; + const parameterVisibilityFilter = getParameterVisibilityFilter( + program, + operation, + HttpVisibilityProvider(verb), + ); + let visibility = filterToVisibility(program, parameterVisibilityFilter); // If the verb is PATCH, then we need to add the patch flag to the visibility in order for // later processes to properly apply it if (verb === "patch") { @@ -237,8 +392,7 @@ export function isMetadata(program: Program, property: ModelProperty) { * Determines if the given property is visible with the given visibility. */ export function isVisible(program: Program, property: ModelProperty, visibility: Visibility) { - // eslint-disable-next-line @typescript-eslint/no-deprecated - return isVisibleCore(program, property, visibilityToArray(visibility)); + return isVisibleCore(program, property, visibilityToFilter(program, visibility)); } /** From a0b2361a46745e420cebfe67aab04ff3ab614e0b Mon Sep 17 00:00:00 2001 From: Will Temple Date: Thu, 19 Dec 2024 01:04:27 -0500 Subject: [PATCH 03/22] [hsc] fix a null visibility constraint in effective type calculation --- packages/http-server-csharp/src/service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/http-server-csharp/src/service.ts b/packages/http-server-csharp/src/service.ts index 2eefa32289b..537e224c77a 100644 --- a/packages/http-server-csharp/src/service.ts +++ b/packages/http-server-csharp/src/service.ts @@ -811,7 +811,7 @@ export async function $onEmit(context: EmitContext) code`${this.emitter.emitTypeReference( this.#metaInfo.getEffectivePayloadType( bodyParam.type, - Visibility.Create & Visibility.Update, + Visibility.Create | Visibility.Update, ), )} body`, ); From 42242b2d273ae08e73cb30da51fd661d72d8dfb5 Mon Sep 17 00:00:00 2001 From: Will Temple Date: Thu, 19 Dec 2024 13:37:03 -0500 Subject: [PATCH 04/22] Make fallback HttpVisibilityProvider logic more accurate --- packages/http/src/metadata.ts | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/packages/http/src/metadata.ts b/packages/http/src/metadata.ts index c092bdf548d..0194060d0d8 100644 --- a/packages/http/src/metadata.ts +++ b/packages/http/src/metadata.ts @@ -28,6 +28,7 @@ import { isQueryParam, isStatusCode, } from "./decorators.js"; +import { getHttpOperation } from "./operations.js"; import { HttpVerb, OperationParameterOptions } from "./types.js"; /** @@ -333,11 +334,17 @@ export function HttpVisibilityProvider( return { parameters: (program, operation) => { - const verb = hasVerb + let verb = hasVerb ? (verbOrParameterOptions as HttpVerb) : (verbOrParameterOptions?.verbSelector?.(program, operation) ?? - getOperationVerb(program, operation) ?? - "get"); + getOperationVerb(program, operation)); + + if (!verb) { + const [httpOperation] = getHttpOperation(program, operation); + + verb = httpOperation.verb; + } + return visibilityToFilter(program, getDefaultVisibilityForVerb(verb)); }, returnType: (program, _) => { @@ -360,6 +367,10 @@ export function resolveRequestVisibility( operation: Operation, verb: HttpVerb, ): Visibility { + // WARNING: This is the only place where we call HttpVisibilityProvider _WITHIN_ the HTTP implementation itself. We + // _must_ provide the verb directly to the function as the first argument. If the verb is not provided directly, the + // provider calls getHttpOperation to resolve the verb. Since the current function is called from getHttpOperation, it + // will cause a stack overflow if the version of HttpVisibilityProvider we use here has to resolve the verb itself. const parameterVisibilityFilter = getParameterVisibilityFilter( program, operation, From d8516850988db272854a096cf2b991c9a8bd1534 Mon Sep 17 00:00:00 2001 From: Will Temple Date: Thu, 19 Dec 2024 13:37:17 -0500 Subject: [PATCH 05/22] Chronus --- ...itemple-msft-http-visibility-enum-2024-11-19-13-8-4.md | 8 ++++++++ ...temple-msft-http-visibility-enum-2024-11-19-13-8-52.md | 7 +++++++ ...temple-msft-http-visibility-enum-2024-11-19-13-9-45.md | 7 +++++++ 3 files changed, 22 insertions(+) create mode 100644 .chronus/changes/witemple-msft-http-visibility-enum-2024-11-19-13-8-4.md create mode 100644 .chronus/changes/witemple-msft-http-visibility-enum-2024-11-19-13-8-52.md create mode 100644 .chronus/changes/witemple-msft-http-visibility-enum-2024-11-19-13-9-45.md diff --git a/.chronus/changes/witemple-msft-http-visibility-enum-2024-11-19-13-8-4.md b/.chronus/changes/witemple-msft-http-visibility-enum-2024-11-19-13-8-4.md new file mode 100644 index 00000000000..eb762a58a83 --- /dev/null +++ b/.chronus/changes/witemple-msft-http-visibility-enum-2024-11-19-13-8-4.md @@ -0,0 +1,8 @@ +--- +changeKind: internal +packages: + - "@typespec/http" + - "@typespec/openapi3" +--- + +Updated the OpenAPI3 and HTTP libraries to use the new visibility analysis APIs. \ No newline at end of file diff --git a/.chronus/changes/witemple-msft-http-visibility-enum-2024-11-19-13-8-52.md b/.chronus/changes/witemple-msft-http-visibility-enum-2024-11-19-13-8-52.md new file mode 100644 index 00000000000..7efb81f9153 --- /dev/null +++ b/.chronus/changes/witemple-msft-http-visibility-enum-2024-11-19-13-8-52.md @@ -0,0 +1,7 @@ +--- +changeKind: feature +packages: + - "@typespec/compiler" +--- + +Added APIs for getting parameterVisibility and returnTypeVisibility as VisibilityFilter objects. \ No newline at end of file diff --git a/.chronus/changes/witemple-msft-http-visibility-enum-2024-11-19-13-9-45.md b/.chronus/changes/witemple-msft-http-visibility-enum-2024-11-19-13-9-45.md new file mode 100644 index 00000000000..4f9e22eb7b8 --- /dev/null +++ b/.chronus/changes/witemple-msft-http-visibility-enum-2024-11-19-13-9-45.md @@ -0,0 +1,7 @@ +--- +changeKind: fix +packages: + - "@typespec/http-server-csharp" +--- + +Fixed a null visibility constraint when calculating effective type. \ No newline at end of file From be4057f0f43416d362b11aa8cfab7d992e8f1f19 Mon Sep 17 00:00:00 2001 From: Will Temple Date: Thu, 19 Dec 2024 13:53:42 -0500 Subject: [PATCH 06/22] Update .chronus/changes/witemple-msft-http-visibility-enum-2024-11-19-13-8-4.md --- .../witemple-msft-http-visibility-enum-2024-11-19-13-8-4.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.chronus/changes/witemple-msft-http-visibility-enum-2024-11-19-13-8-4.md b/.chronus/changes/witemple-msft-http-visibility-enum-2024-11-19-13-8-4.md index eb762a58a83..0a004cf70e2 100644 --- a/.chronus/changes/witemple-msft-http-visibility-enum-2024-11-19-13-8-4.md +++ b/.chronus/changes/witemple-msft-http-visibility-enum-2024-11-19-13-8-4.md @@ -2,7 +2,7 @@ changeKind: internal packages: - "@typespec/http" - - "@typespec/openapi3" + - "@typespec/openapi" --- Updated the OpenAPI3 and HTTP libraries to use the new visibility analysis APIs. \ No newline at end of file From f03067da72d0b7d71933c581f5036f6cf7fffd18 Mon Sep 17 00:00:00 2001 From: Will Temple Date: Thu, 19 Dec 2024 14:47:07 -0500 Subject: [PATCH 07/22] Docs --- .../docs/docs/language-basics/visibility.md | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/website/src/content/docs/docs/language-basics/visibility.md b/website/src/content/docs/docs/language-basics/visibility.md index dd523a840d8..18f44970ef1 100644 --- a/website/src/content/docs/docs/language-basics/visibility.md +++ b/website/src/content/docs/docs/language-basics/visibility.md @@ -7,6 +7,11 @@ title: Visibility properties of the model are "visible." Visibility is a very powerful feature that allows you to define different "views" of a model within different operations or contexts. +**Note** ⚠️: Enum-based visibility as described in this document _replaces_ visibility strings that you may have used +in the past. The system is backwards-compatible with visibility strings, but you should use enum-based visibility for +new specifications. String-based visibility (e.g. `@visibility("create")`) may be deprecated and removed in future +versions of TypeSpec. + ## Basic concepts - Visibility applies to _model properties_ only. It is used to determine when an emitter should include or exclude a @@ -19,7 +24,7 @@ of a model within different operations or contexts. ## Lifecycle visibility TypeSpec provides a built-in visibility called "resource lifecycle visibility." This visibility allows you to declare -whether properties are visible when a creating, updating, or reading a resource from an API endpoint. For example: +whether properties are visible when passing a resource to or reading a resource from an API endpoint. For example: ```typespec model Example { @@ -43,8 +48,8 @@ model Example { /** * The description of this resource. * - * By default, properties are visible in all three lifecycle phases, so this - * property can be set when the resource is created, updated, and read. + * By default, properties are visible in all lifecycle phases, so this property + * is present in all lifecycle phases. */ description: string; } @@ -166,6 +171,22 @@ Notice: - The TypeSpec model is only defined _once_, and any changes in the output schemas are derived from the lifecycle visibility of the properties in the model. +### Lifecycle modifiers + +The following visibility modifiers are available in the `Lifecycle` visibility class: + +- `Create`: The property is visible when the resource is created. This visibility is checked, for example, when a property + is a parameter in an HTTP `POST` operation. +- `Read`: The property is visible when the resource is read. This visibility is checked, for example, when a property is + returned in an HTTP `GET` operation. +- `Update`: The property is visible when the resource is updated. This visibility is checked, for example, when a property + is a parameter in an HTTP `PATCH` or `PUT` operation. +- `Delete`: The property is visible when a resource is deleted. This visibility is checked, for example, when a property + is a parameter in an HTTP `DELETE` operation. +- `Query`: The property is visible when a resource is passed as a parameter in a query. This visibility is checked, for + example, when a property is a parameter in an HTTP `GET` operation (**this should not be confused with an HTTP query + parameter defined using `@query`**). + ### Lifecycle visibility transforms You can explicitly compute the shape of a model within a _specific_ lifecycle phase by using the four built-in @@ -179,6 +200,10 @@ templates for lifecycle transforms: phase, with the types of the properties set to `CreateOrUpdate`, recursively. - `CreateOrUpdate`: creates a copy of `T` with only the properties that have _either_ the `Create` or `Update` visibility modifiers enabled, recursively. +- `Delete`: creates a copy of `T` with only the properties that have the `Lifecycle.Delete` modifier enabled, + recursively. +- `Query`: creates a copy of `T` with only the properties that have the `Lifecycle.Query` modifier enabled, + recursively. For example: From d81b712486e12af5d6eb7d56a2e56467ccc0c998 Mon Sep 17 00:00:00 2001 From: Will Temple Date: Wed, 8 Jan 2025 14:55:33 -0500 Subject: [PATCH 08/22] Fix bug where hasVisibility was improperly initializing visibility store. --- packages/compiler/src/core/visibility/core.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/compiler/src/core/visibility/core.ts b/packages/compiler/src/core/visibility/core.ts index b68ebd58097..3bbb69d5ffd 100644 --- a/packages/compiler/src/core/visibility/core.ts +++ b/packages/compiler/src/core/visibility/core.ts @@ -579,12 +579,12 @@ export function hasVisibility( property: ModelProperty, modifier: EnumMember, ): boolean { - const activeSet = getOrInitializeActiveModifierSetForClass( - program, - property, - modifier.enum, - /* defaultSet: */ getDefaultModifierSetForClass(program, modifier.enum), - ); + const visibilityClass = modifier.enum; + + const store = getVisibilityStore(program, property); + + const activeSet = + store?.get(visibilityClass) ?? getDefaultModifierSetForClass(program, visibilityClass); return activeSet?.has(modifier) ?? false; } From b6d2de0a59a50680cb5e3da15e510ae7b7b390b8 Mon Sep 17 00:00:00 2001 From: Will Temple Date: Wed, 8 Jan 2025 18:26:41 -0500 Subject: [PATCH 09/22] Prevent hasVisibility from eagerly initializing visibility --- packages/compiler/src/core/visibility/core.ts | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/packages/compiler/src/core/visibility/core.ts b/packages/compiler/src/core/visibility/core.ts index 3bbb69d5ffd..b6e88b68d38 100644 --- a/packages/compiler/src/core/visibility/core.ts +++ b/packages/compiler/src/core/visibility/core.ts @@ -555,12 +555,9 @@ export function getVisibilityForClass( property: ModelProperty, visibilityClass: Enum, ): Set { - return getOrInitializeActiveModifierSetForClass( - program, - property, - visibilityClass, - /* defaultSet: */ getDefaultModifierSetForClass(program, visibilityClass), - ); + const store = getVisibilityStore(program, property); + + return store?.get(visibilityClass) ?? getDefaultModifierSetForClass(program, visibilityClass); } /** @@ -581,12 +578,7 @@ export function hasVisibility( ): boolean { const visibilityClass = modifier.enum; - const store = getVisibilityStore(program, property); - - const activeSet = - store?.get(visibilityClass) ?? getDefaultModifierSetForClass(program, visibilityClass); - - return activeSet?.has(modifier) ?? false; + return getVisibilityForClass(program, property, visibilityClass).has(modifier) ?? false; } /** From 0747732d9caa5c82e1a658983d4795ede02db203 Mon Sep 17 00:00:00 2001 From: Will Temple Date: Mon, 13 Jan 2025 11:09:10 -0500 Subject: [PATCH 10/22] Made visibility filters empty when returnType/parameter visibility args are empty --- packages/compiler/src/lib/visibility.ts | 8 +++++ packages/compiler/test/visibility.test.ts | 36 +++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/packages/compiler/src/lib/visibility.ts b/packages/compiler/src/lib/visibility.ts index 03010ea36a0..7bb30737125 100644 --- a/packages/compiler/src/lib/visibility.ts +++ b/packages/compiler/src/lib/visibility.ts @@ -243,6 +243,10 @@ export function getParameterVisibilityFilter( if (!operationVisibilityConfig.parameters) return defaultProvider.parameters(program, operation); + // If there are no parameters, return an empty filter. This has the effect of allowing `@parameterVisibility()` to + // disable the default visibility provider for an operation. + if (operationVisibilityConfig.parameters.length === 0) return {}; + return { // WARNING: the HTTP library depends on `any` being the only key in the filter object returned by this method. // if you change this logic, you will need to update the HTTP library to account for differences in the @@ -318,6 +322,10 @@ export function getReturnTypeVisibilityFilter( if (!visibilityConfig.returnType) return defaultProvider.returnType(program, operation); + // If there are no return type visibilities, return an empty filter. This has the effect of allowing + // `@returnTypeVisibility()` to disable the default visibility provider for an operation. + if (visibilityConfig.returnType.length === 0) return {}; + return { // WARNING: the HTTP library depends on `any` being the only key in the filter object returned by this method. // if you change this logic, you will need to update the HTTP library to account for differences in the diff --git a/packages/compiler/test/visibility.test.ts b/packages/compiler/test/visibility.test.ts index f54289043c6..c523079e98d 100644 --- a/packages/compiler/test/visibility.test.ts +++ b/packages/compiler/test/visibility.test.ts @@ -13,6 +13,7 @@ import { Enum, getLifecycleVisibilityEnum, getParameterVisibilityFilter, + getReturnTypeVisibilityFilter, getVisibilityForClass, hasVisibility, isSealed, @@ -25,6 +26,7 @@ import { resetVisibilityModifiersForClass, sealVisibilityModifiers, sealVisibilityModifiersForProgram, + VisibilityProvider, } from "../src/index.js"; import { BasicTestRunner, createTestRunner, expectDiagnostics } from "../src/testing/index.js"; @@ -649,6 +651,40 @@ describe("compiler: visibility core", () => { strictEqual(isVisible(runner.program, x, filter), false); }); + + it("allows properties when no arguments provided to parameter/returnType visibility", async () => { + const { Example, foo } = (await runner.compile(` + @test model Example { + @visibility(Lifecycle.Create) + x: string; + } + + @parameterVisibility + @returnTypeVisibility + @test op foo( + example: Example + ): Example; + `)) as { Example: Model; foo: Operation }; + + const x = Example.properties.get("x")!; + + const provider: VisibilityProvider = { + parameters: (program) => { + const Lifecycle = getLifecycleVisibilityEnum(program); + return { all: new Set([Lifecycle.members.get("Read")!]) }; + }, + returnType: (program) => { + const Lifecycle = getLifecycleVisibilityEnum(program); + return { all: new Set([Lifecycle.members.get("Read")!]) }; + }, + }; + + const parameterFilter = getParameterVisibilityFilter(runner.program, foo, provider); + const returnTypeFilter = getReturnTypeVisibilityFilter(runner.program, foo, provider); + + strictEqual(isVisible(runner.program, x, parameterFilter), true); + strictEqual(isVisible(runner.program, x, returnTypeFilter), true); + }); }); }); From ebb3f5de2dd926bff8c244c50580ef222c624054 Mon Sep 17 00:00:00 2001 From: Will Temple Date: Tue, 14 Jan 2025 18:57:12 -0500 Subject: [PATCH 11/22] Updated documentation of returnType/parameter visibility decorators --- packages/compiler/generated-defs/TypeSpec.ts | 14 ++++++++++---- packages/compiler/lib/std/visibility.tsp | 15 +++++++++++---- .../docs/standard-library/built-in-decorators.md | 14 ++++++++++---- 3 files changed, 31 insertions(+), 12 deletions(-) diff --git a/packages/compiler/generated-defs/TypeSpec.ts b/packages/compiler/generated-defs/TypeSpec.ts index 85973225679..24e54904bb7 100644 --- a/packages/compiler/generated-defs/TypeSpec.ts +++ b/packages/compiler/generated-defs/TypeSpec.ts @@ -927,9 +927,12 @@ export type WithVisibilityDecorator = ( ) => void; /** - * Sets which visibilities apply to parameters for the given operation. + * Declares the visibility constraint of the parameters of a given operation. * - * @param visibilities List of visibility strings which apply to this operation. + * A parameter or property nested within a parameter will be visible if it has _any_ of the visibilities + * in the list, or if the list is empty (in which case the parameter or property is always visible). + * + * @param visibilities List of visibility modifiers that apply to the parameters of this operation. */ export type ParameterVisibilityDecorator = ( context: DecoratorContext, @@ -938,9 +941,12 @@ export type ParameterVisibilityDecorator = ( ) => void; /** - * Sets which visibilities apply to the return type for the given operation. + * Declares the visibility constraint of the return type of a given operation. + * + * A property within the return type of the operation will be visible if it has _any_ of the visibilities + * in the list, or if the list is empty (in which case the property is always visible). * - * @param visibilities List of visibility strings which apply to this operation. + * @param visibilities List of visibility modifiers that apply to the return type of this operation. */ export type ReturnTypeVisibilityDecorator = ( context: DecoratorContext, diff --git a/packages/compiler/lib/std/visibility.tsp b/packages/compiler/lib/std/visibility.tsp index 3de21ddf924..aeee125b1ea 100644 --- a/packages/compiler/lib/std/visibility.tsp +++ b/packages/compiler/lib/std/visibility.tsp @@ -141,15 +141,22 @@ extern dec withVisibility(target: Model, ...visibilities: valueof (string | Enum extern dec withDefaultKeyVisibility(target: Model, visibility: valueof string | EnumMember); /** - * Sets which visibilities apply to parameters for the given operation. + * Declares the visibility constraint of the parameters of a given operation. * - * @param visibilities List of visibility strings which apply to this operation. + * A parameter or property nested within a parameter will be visible if it has _any_ of the visibilities + * in the list, or if the list is empty (in which case the parameter or property is always visible). + * + * @param visibilities List of visibility modifiers that apply to the parameters of this operation. */ extern dec parameterVisibility(target: Operation, ...visibilities: valueof (string | EnumMember)[]); /** - * Sets which visibilities apply to the return type for the given operation. - * @param visibilities List of visibility strings which apply to this operation. + * Declares the visibility constraint of the return type of a given operation. + * + * A property within the return type of the operation will be visible if it has _any_ of the visibilities + * in the list, or if the list is empty (in which case the property is always visible). + * + * @param visibilities List of visibility modifiers that apply to the return type of this operation. */ extern dec returnTypeVisibility( target: Operation, diff --git a/website/src/content/docs/docs/standard-library/built-in-decorators.md b/website/src/content/docs/docs/standard-library/built-in-decorators.md index 531888f1ced..20b1d50257d 100644 --- a/website/src/content/docs/docs/standard-library/built-in-decorators.md +++ b/website/src/content/docs/docs/standard-library/built-in-decorators.md @@ -928,7 +928,10 @@ model Page { ### `@parameterVisibility` {#@parameterVisibility} -Sets which visibilities apply to parameters for the given operation. +Declares the visibility constraint of the parameters of a given operation. + +A parameter or property nested within a parameter will be visible if it has _any_ of the visibilities +in the list, or if the list is empty (in which case the parameter or property is always visible). ```typespec @parameterVisibility(...visibilities: valueof string | EnumMember[]) ``` @@ -940,7 +943,7 @@ Sets which visibilities apply to parameters for the given operation. #### Parameters | Name | Type | Description | |------|------|-------------| -| visibilities | `valueof string \| EnumMember[]` | List of visibility strings which apply to this operation. | +| visibilities | `valueof string \| EnumMember[]` | List of visibility modifiers that apply to the parameters of this operation. | @@ -1097,7 +1100,10 @@ op get(): Pet | NotFound; ### `@returnTypeVisibility` {#@returnTypeVisibility} -Sets which visibilities apply to the return type for the given operation. +Declares the visibility constraint of the return type of a given operation. + +A property within the return type of the operation will be visible if it has _any_ of the visibilities +in the list, or if the list is empty (in which case the property is always visible). ```typespec @returnTypeVisibility(...visibilities: valueof string | EnumMember[]) ``` @@ -1109,7 +1115,7 @@ Sets which visibilities apply to the return type for the given operation. #### Parameters | Name | Type | Description | |------|------|-------------| -| visibilities | `valueof string \| EnumMember[]` | List of visibility strings which apply to this operation. | +| visibilities | `valueof string \| EnumMember[]` | List of visibility modifiers that apply to the return type of this operation. | From 91891389a57c87a617d9c160f3b396b5ba206ad1 Mon Sep 17 00:00:00 2001 From: Will Temple Date: Tue, 14 Jan 2025 18:58:12 -0500 Subject: [PATCH 12/22] Fixed a subtle bug in invocation of .map --- .../specs/rest-metadata-emitter/rest-metadata-emitter-sample.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/samples/specs/rest-metadata-emitter/rest-metadata-emitter-sample.ts b/packages/samples/specs/rest-metadata-emitter/rest-metadata-emitter-sample.ts index a979932f9d5..54052ae92bb 100644 --- a/packages/samples/specs/rest-metadata-emitter/rest-metadata-emitter-sample.ts +++ b/packages/samples/specs/rest-metadata-emitter/rest-metadata-emitter-sample.ts @@ -240,7 +240,7 @@ export async function $onEmit(context: EmitContext): Promise { // it's always in the payload, common case. } else { // it's in the payload for certain visibilities only. - remarks.push(`${inPayloadVisibilities.map(getVisibilitySuffix).join(",")} only`); + remarks.push(`${inPayloadVisibilities.map((v) => getVisibilitySuffix(v)).join(",")} only`); } return remarks.length === 0 ? "" : ` (${remarks.join(", ")})`; From 1e53aeb4071eb8b26becb2c3684d9e804c19d9b6 Mon Sep 17 00:00:00 2001 From: Will Temple Date: Tue, 14 Jan 2025 18:59:33 -0500 Subject: [PATCH 13/22] Changed body-type detection to use payload disposition instead of visibility --- packages/http/src/http-property.ts | 36 ++++++++++------- packages/http/src/parameters.ts | 4 +- packages/http/src/payload.ts | 62 ++++++++++++++++++++++-------- packages/http/src/responses.ts | 4 +- 4 files changed, 72 insertions(+), 34 deletions(-) diff --git a/packages/http/src/http-property.ts b/packages/http/src/http-property.ts index 04bfdd88483..ce7ba0bc604 100644 --- a/packages/http/src/http-property.ts +++ b/packages/http/src/http-property.ts @@ -1,9 +1,9 @@ import { + compilerAssert, + createDiagnosticCollector, DiagnosticResult, Model, Type, - compilerAssert, - createDiagnosticCollector, walkPropertiesInherited, type Diagnostic, type ModelProperty, @@ -20,7 +20,8 @@ import { isStatusCode, } from "./decorators.js"; import { createDiagnostic } from "./lib.js"; -import { Visibility, isVisible } from "./metadata.js"; +import { isVisible, Visibility } from "./metadata.js"; +import { HttpPayloadDisposition } from "./payload.js"; import { CookieParameterOptions, HeaderFieldOptions, @@ -216,6 +217,7 @@ export function resolvePayloadProperties( program: Program, type: Type, visibility: Visibility, + disposition: HttpPayloadDisposition, options: GetHttpPropertyOptions = {}, ): DiagnosticResult { const diagnostics = createDiagnosticCollector(); @@ -238,13 +240,13 @@ export function resolvePayloadProperties( } let httpProperty = diagnostics.pipe(getHttpProperty(program, property, propPath, options)); - if (shouldTreatAsBodyProperty(httpProperty, visibility)) { + if (shouldTreatAsBodyProperty(httpProperty, disposition)) { httpProperty = { kind: "bodyProperty", property, path: propPath }; } // Ignore cookies in response to avoid future breaking changes to @cookie. // https://github.com/microsoft/typespec/pull/4761#discussion_r1805082132 - if (httpProperty.kind === "cookie" && visibility & Visibility.Read) { + if (httpProperty.kind === "cookie" && disposition === HttpPayloadDisposition.Response) { diagnostics.add( createDiagnostic({ code: "response-cookie-not-supported", @@ -290,13 +292,21 @@ function isModelWithProperties(type: Type): type is Model { return type.kind === "Model" && !type.indexer && type.properties.size > 0; } -function shouldTreatAsBodyProperty(property: HttpProperty, visibility: Visibility): boolean { - if (visibility & Visibility.Read) { - return property.kind === "query" || property.kind === "path"; - } - - if (!(visibility & Visibility.Read)) { - return property.kind === "statusCode"; +function shouldTreatAsBodyProperty( + property: HttpProperty, + disposition: HttpPayloadDisposition, +): boolean { + switch (disposition) { + case HttpPayloadDisposition.Request: + return property.kind === "statusCode"; + case HttpPayloadDisposition.Response: + return property.kind === "query" || property.kind === "path"; + case HttpPayloadDisposition.Multipart: + return ( + property.kind === "path" || property.kind === "query" || property.kind === "statusCode" + ); + default: + void (disposition satisfies never); + return false; } - return false; } diff --git a/packages/http/src/parameters.ts b/packages/http/src/parameters.ts index b41de87acbf..9e8b2b1bbd4 100644 --- a/packages/http/src/parameters.ts +++ b/packages/http/src/parameters.ts @@ -8,7 +8,7 @@ import { import { getOperationVerb } from "./decorators.js"; import { createDiagnostic } from "./lib.js"; import { resolveRequestVisibility } from "./metadata.js"; -import { resolveHttpPayload } from "./payload.js"; +import { HttpPayloadDisposition, resolveHttpPayload } from "./payload.js"; import { HttpOperation, HttpOperationParameter, @@ -65,7 +65,7 @@ function getOperationParametersForVerb( const parameters: HttpOperationParameter[] = []; const { body: resolvedBody, metadata } = diagnostics.pipe( - resolveHttpPayload(program, operation.parameters, visibility, "request", { + resolveHttpPayload(program, operation.parameters, visibility, HttpPayloadDisposition.Request, { implicitParameter: ( param: ModelProperty, ): QueryParameterOptions | PathParameterOptions | undefined => { diff --git a/packages/http/src/payload.ts b/packages/http/src/payload.ts index c647bff7927..78819e15017 100644 --- a/packages/http/src/payload.ts +++ b/packages/http/src/payload.ts @@ -31,19 +31,41 @@ export interface HttpPayload { readonly body?: HttpOperationBody | HttpOperationMultipartBody; readonly metadata: HttpProperty[]; } + export interface ExtractBodyAndMetadataOptions extends GetHttpPropertyOptions {} + +/** + * The disposition of a payload in an HTTP operation. + */ +export enum HttpPayloadDisposition { + /** + * The payload appears in a request. + */ + Request, + /** + * The payload appears in a response. + */ + Response, + /** + * The payload appears in a multipart part. + */ + Multipart, +} + export function resolveHttpPayload( program: Program, type: Type, visibility: Visibility, - usedIn: "request" | "response" | "multipart", + disposition: HttpPayloadDisposition, options: ExtractBodyAndMetadataOptions = {}, ): [HttpPayload, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); - const metadata = diagnostics.pipe(resolvePayloadProperties(program, type, visibility, options)); + const metadata = diagnostics.pipe( + resolvePayloadProperties(program, type, visibility, disposition, options), + ); - const body = diagnostics.pipe(resolveBody(program, type, metadata, visibility, usedIn)); + const body = diagnostics.pipe(resolveBody(program, type, metadata, visibility, disposition)); if (body) { if ( @@ -69,10 +91,12 @@ function resolveBody( requestOrResponseType: Type, metadata: HttpProperty[], visibility: Visibility, - usedIn: "request" | "response" | "multipart", + disposition: HttpPayloadDisposition, ): [HttpOperationBody | HttpOperationMultipartBody | undefined, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); - const resolvedContentTypes = diagnostics.pipe(resolveContentTypes(program, metadata, usedIn)); + const resolvedContentTypes = diagnostics.pipe( + resolveContentTypes(program, metadata, disposition), + ); const file = getHttpFileModel(program, requestOrResponseType); if (file !== undefined) { @@ -100,7 +124,7 @@ function resolveBody( // look for explicit body const resolvedBody: HttpOperationBody | HttpOperationMultipartBody | undefined = diagnostics.pipe( - resolveExplicitBodyProperty(program, metadata, resolvedContentTypes, visibility, usedIn), + resolveExplicitBodyProperty(program, metadata, resolvedContentTypes, visibility, disposition), ); if (resolvedBody === undefined) { @@ -172,7 +196,7 @@ interface ResolvedContentType { function resolveContentTypes( program: Program, metadata: HttpProperty[], - usedIn: "request" | "response" | "multipart", + disposition: HttpPayloadDisposition, ): [ResolvedContentType, readonly Diagnostic[]] { for (const prop of metadata) { if (prop.kind === "contentType") { @@ -180,8 +204,8 @@ function resolveContentTypes( return [{ contentTypes, contentTypeProperty: prop.property }, diagnostics]; } } - switch (usedIn) { - case "multipart": + switch (disposition) { + case HttpPayloadDisposition.Multipart: // Figure this out later return [{ contentTypes: [] }, []]; default: @@ -194,7 +218,7 @@ function resolveExplicitBodyProperty( metadata: HttpProperty[], resolvedContentTypes: ResolvedContentType, visibility: Visibility, - usedIn: "request" | "response" | "multipart", + disposition: HttpPayloadDisposition, ): [HttpOperationBody | HttpOperationMultipartBody | undefined, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); let resolvedBody: HttpOperationBody | HttpOperationMultipartBody | undefined; @@ -210,7 +234,7 @@ function resolveExplicitBodyProperty( case "bodyRoot": let containsMetadataAnnotations = false; if (item.kind === "body") { - const valid = diagnostics.pipe(validateBodyProperty(program, item.property, usedIn)); + const valid = diagnostics.pipe(validateBodyProperty(program, item.property, disposition)); containsMetadataAnnotations = !valid; } if (resolvedBody === undefined) { @@ -250,7 +274,7 @@ function resolveExplicitBodyProperty( function validateBodyProperty( program: Program, property: ModelProperty, - usedIn: "request" | "response" | "multipart", + disposition: HttpPayloadDisposition, ): [boolean, readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); navigateType( @@ -260,13 +284,17 @@ function validateBodyProperty( const kind = isHeader(program, prop) ? "header" : // also emit metadata-ignored for response cookie - (usedIn === "request" || usedIn === "response") && isCookieParam(program, prop) + (disposition === HttpPayloadDisposition.Request || + disposition === HttpPayloadDisposition.Response) && + isCookieParam(program, prop) ? "cookie" - : (usedIn === "request" || usedIn === "multipart") && isQueryParam(program, prop) + : (disposition === HttpPayloadDisposition.Request || + disposition === HttpPayloadDisposition.Multipart) && + isQueryParam(program, prop) ? "query" - : usedIn === "request" && isPathParam(program, prop) + : disposition === HttpPayloadDisposition.Request && isPathParam(program, prop) ? "path" - : usedIn === "response" && isStatusCode(program, prop) + : disposition === HttpPayloadDisposition.Response && isStatusCode(program, prop) ? "statusCode" : undefined; @@ -412,7 +440,7 @@ function resolvePart( program, part.type, visibility, - "multipart", + HttpPayloadDisposition.Multipart, ); if (body === undefined) { return [undefined, diagnostics]; diff --git a/packages/http/src/responses.ts b/packages/http/src/responses.ts index 841652d51c8..a0d07f72a58 100644 --- a/packages/http/src/responses.ts +++ b/packages/http/src/responses.ts @@ -18,7 +18,7 @@ import { getStatusCodeDescription, getStatusCodesWithDiagnostics } from "./decor import { HttpProperty } from "./http-property.js"; import { HttpStateKeys, reportDiagnostic } from "./lib.js"; import { Visibility } from "./metadata.js"; -import { resolveHttpPayload } from "./payload.js"; +import { HttpPayloadDisposition, resolveHttpPayload } from "./payload.js"; import { HttpOperationResponse, HttpStatusCodes, HttpStatusCodesEntry } from "./types.js"; /** @@ -82,7 +82,7 @@ function processResponseType( ) { // Get body let { body: resolvedBody, metadata } = diagnostics.pipe( - resolveHttpPayload(program, responseType, Visibility.Read, "response"), + resolveHttpPayload(program, responseType, Visibility.Read, HttpPayloadDisposition.Response), ); // Get explicity defined status codes const statusCodes: HttpStatusCodes = diagnostics.pipe( From c92604306968bb737cdf60b83494ec92e492c19a Mon Sep 17 00:00:00 2001 From: Will Temple Date: Tue, 14 Jan 2025 18:59:50 -0500 Subject: [PATCH 14/22] Changed default canonicalVisibility to Visibility.All --- packages/http/src/metadata.ts | 10 ++++++---- packages/http/test/http-decorators.test.ts | 12 ++++++++++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/packages/http/src/metadata.ts b/packages/http/src/metadata.ts index 0194060d0d8..3539bfde6ea 100644 --- a/packages/http/src/metadata.ts +++ b/packages/http/src/metadata.ts @@ -93,7 +93,6 @@ function visibilityToArray(visibility: Visibility): readonly string[] { function filterToVisibility(program: Program, filter: VisibilityFilter): Visibility { const Lifecycle = getLifecycleVisibilityEnum(program); - let value = Visibility.None; compilerAssert( !filter.all, @@ -105,6 +104,8 @@ function filterToVisibility(program: Program, filter: VisibilityFilter): Visibil ); if (filter.any) { + let value = Visibility.None; + for (const item of filter.any ?? []) { if (item.enum !== Lifecycle) continue; switch (item.name) { @@ -220,7 +221,7 @@ function visibilityToFilter(program: Program, visibility: Visibility): Visibilit * */ export function getVisibilitySuffix( visibility: Visibility, - canonicalVisibility: Visibility = Visibility.None, + canonicalVisibility: Visibility = Visibility.All, ) { let suffix = ""; @@ -376,6 +377,7 @@ export function resolveRequestVisibility( operation, HttpVisibilityProvider(verb), ); + let visibility = filterToVisibility(program, parameterVisibilityFilter); // If the verb is PATCH, then we need to add the patch flag to the visibility in order for // later processes to properly apply it @@ -528,7 +530,7 @@ export interface MetadataInfoOptions { /** * The visibility to be used as the baseline against which * {@link MetadataInfo.isEmptied} and {@link MetadataInfo.isTransformed} - * are computed. If not specified, {@link Visibility.None} is used, which + * are computed. If not specified, {@link Visibility.All} is used, which * will consider that any model that has fields that are only visible to * some visibilities as transformed. */ @@ -546,7 +548,7 @@ export interface MetadataInfoOptions { } export function createMetadataInfo(program: Program, options?: MetadataInfoOptions): MetadataInfo { - const canonicalVisibility = options?.canonicalVisibility ?? Visibility.None; + const canonicalVisibility = options?.canonicalVisibility ?? Visibility.All; const enum State { NotTransformed, Transformed, diff --git a/packages/http/test/http-decorators.test.ts b/packages/http/test/http-decorators.test.ts index a54cedfd9d7..692422daee2 100644 --- a/packages/http/test/http-decorators.test.ts +++ b/packages/http/test/http-decorators.test.ts @@ -1272,5 +1272,17 @@ describe("http: decorators", () => { Visibility.Update | Visibility.Create | Visibility.Patch, ); }); + + it("ensures all properties visible when parameterVisibility has no arguments", async () => { + const { test } = (await runner.compile(` + @parameterVisibility + @test op test(@path id: string, @body example: {}): void; + `)) as { test: Operation }; + deepStrictEqual( + resolveRequestVisibility(runner.program, test, "patch"), + Visibility.All | Visibility.Patch, + ); + deepStrictEqual(resolveRequestVisibility(runner.program, test, "get"), Visibility.All); + }); }); }); From 48c88bb30f4c6954b0538ddf766334afb2ac8be7 Mon Sep 17 00:00:00 2001 From: Will Temple Date: Wed, 15 Jan 2025 15:15:56 -0500 Subject: [PATCH 15/22] Consolidated synthetic visibility metadata, added SkipEffectiveOptionality --- packages/http/src/metadata.ts | 87 ++++++++++++++++------ packages/http/test/http-decorators.test.ts | 9 ++- 2 files changed, 69 insertions(+), 27 deletions(-) diff --git a/packages/http/src/metadata.ts b/packages/http/src/metadata.ts index 3539bfde6ea..50024d3fbaa 100644 --- a/packages/http/src/metadata.ts +++ b/packages/http/src/metadata.ts @@ -48,22 +48,47 @@ export enum Visibility { /** * Additional flag to indicate when something is nested in a collection * and therefore no metadata is applicable. + * + * Never use this flag. It is used internally by the HTTP core. + * + * @internal */ Item = 1 << 20, /** * Additional flag to indicate when the verb is path and therefore * will have fields made optional if request visibility includes update. + * + * Never use this flag. It is used internally by the HTTP core. + * + * @internal */ Patch = 1 << 21, + + /** + * Additional flag to indicate that effective-optionality analysis should be skipped, + * allowing the shape of PATCH request bodies to be represented exactly. + * + * Never use this flag. It is used internally by the HTTP core. + * + * @internal + */ + SkipEffectiveOptionality = 1 << 22, + + /** + * Additional flags to indicate the treatment of properties in specific contexts. + * + * Never use these flags. They are used internally by the HTTP core. + * + * @internal + */ + Synthetic = Visibility.Item | Visibility.Patch | Visibility.SkipEffectiveOptionality, } const visibilityToArrayMap: Map = new Map(); function visibilityToArray(visibility: Visibility): readonly string[] { - // Item and Patch flags are not real visibilities. - visibility &= ~Visibility.Item; - visibility &= ~Visibility.Patch; - + // Synthetic flags are not real visibilities. + visibility &= ~Visibility.Synthetic; let result = visibilityToArrayMap.get(visibility); if (!result) { result = []; @@ -103,38 +128,38 @@ function filterToVisibility(program: Program, filter: VisibilityFilter): Visibil "Unexpected: `none` constraint in visibility filter passed to filterToVisibility", ); - if (filter.any) { - let value = Visibility.None; + if (!filter.any) { + return Visibility.All; + } else { + let visibility = Visibility.None; - for (const item of filter.any ?? []) { - if (item.enum !== Lifecycle) continue; - switch (item.name) { + for (const modifierConstraint of filter.any ?? []) { + if (modifierConstraint.enum !== Lifecycle) continue; + switch (modifierConstraint.name) { case "Read": - value |= Visibility.Read; + visibility |= Visibility.Read; break; case "Create": - value |= Visibility.Create; + visibility |= Visibility.Create; break; case "Update": - value |= Visibility.Update; + visibility |= Visibility.Update; break; case "Delete": - value |= Visibility.Delete; + visibility |= Visibility.Delete; break; case "Query": - value |= Visibility.Query; + visibility |= Visibility.Query; break; default: compilerAssert( false, - `Unreachable: unrecognized Lifecycle visibility member: '${item.name}'`, + `Unreachable: unrecognized Lifecycle visibility member: '${modifierConstraint.name}'`, ); } } - return value; - } else { - return Visibility.All; + return visibility; } } @@ -159,13 +184,15 @@ function getVisibilityFilterCache(program: Program): Map v[0].toUpperCase() + v.slice(1)).join("Or"); } @@ -356,6 +383,10 @@ export function HttpVisibilityProvider( }; } +// Hidden internal symbol to mark that parameter visibility was empty. This is used by HTTP +// to set the SkipEffectiveOptionality flag. This is a hack. +const parameterVisibilityIsEmpty = Symbol.for("TypeSpec::Visibility::ParameterVisibilityIsEmpty"); + /** * Returns the applicable parameter visibility or visibilities for the request if `@requestVisibility` was used. * Otherwise, returns the default visibility based on the HTTP verb for the operation. @@ -380,10 +411,17 @@ export function resolveRequestVisibility( let visibility = filterToVisibility(program, parameterVisibilityFilter); // If the verb is PATCH, then we need to add the patch flag to the visibility in order for - // later processes to properly apply it + // later processes to properly apply it. if (verb === "patch") { visibility |= Visibility.Patch; } + + const isEmptyParameterVisibility = program.stateSet(parameterVisibilityIsEmpty).has(operation); + + if (isEmptyParameterVisibility) { + visibility |= Visibility.SkipEffectiveOptionality; + } + return visibility; } @@ -672,7 +710,8 @@ export function createMetadataInfo(program: Program, options?: MetadataInfoOptio const hasUpdate = (visibility & Visibility.Update) !== 0; const isPatch = (visibility & Visibility.Patch) !== 0; const isItem = (visibility & Visibility.Item) !== 0; - return property.optional || (hasUpdate && isPatch && !isItem); + const skipEffectiveOptionality = (visibility & Visibility.SkipEffectiveOptionality) !== 0; + return property.optional || (!skipEffectiveOptionality && hasUpdate && isPatch && !isItem); } function isPayloadProperty( diff --git a/packages/http/test/http-decorators.test.ts b/packages/http/test/http-decorators.test.ts index 692422daee2..9dbcb5a964b 100644 --- a/packages/http/test/http-decorators.test.ts +++ b/packages/http/test/http-decorators.test.ts @@ -1273,16 +1273,19 @@ describe("http: decorators", () => { ); }); - it("ensures all properties visible when parameterVisibility has no arguments", async () => { + it("ensures all properties visible ane effective optionality skipped when parameterVisibility has no arguments", async () => { const { test } = (await runner.compile(` @parameterVisibility @test op test(@path id: string, @body example: {}): void; `)) as { test: Operation }; deepStrictEqual( resolveRequestVisibility(runner.program, test, "patch"), - Visibility.All | Visibility.Patch, + Visibility.All | Visibility.Patch | Visibility.SkipEffectiveOptionality, + ); + deepStrictEqual( + resolveRequestVisibility(runner.program, test, "get"), + Visibility.All | Visibility.SkipEffectiveOptionality, ); - deepStrictEqual(resolveRequestVisibility(runner.program, test, "get"), Visibility.All); }); }); }); From e7c3e62b8ca98b7e792fa27e492a99edcb567e74 Mon Sep 17 00:00:00 2001 From: Will Temple Date: Wed, 15 Jan 2025 15:16:14 -0500 Subject: [PATCH 16/22] Added underbar protocol for detecting empty parameterVisibility for HTTP --- packages/compiler/src/lib/visibility.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/compiler/src/lib/visibility.ts b/packages/compiler/src/lib/visibility.ts index 7bb30737125..201cc6664a0 100644 --- a/packages/compiler/src/lib/visibility.ts +++ b/packages/compiler/src/lib/visibility.ts @@ -153,6 +153,10 @@ function getOperationVisibilityConfig( return config; } +// Hidden internal symbol to mark that parameter visibility was empty. This is used by HTTP +// to set the SkipEffectiveOptionality flag. This is a hack. +const parameterVisibilityIsEmpty = Symbol.for("TypeSpec::Visibility::ParameterVisibilityIsEmpty"); + export const $parameterVisibility: ParameterVisibilityDecorator = ( context: DecoratorContext, operation: Operation, @@ -160,6 +164,10 @@ export const $parameterVisibility: ParameterVisibilityDecorator = ( ) => { validateDecoratorUniqueOnNode(context, operation, $parameterVisibility); + if (visibilities.length === 0) { + context.program.stateSet(parameterVisibilityIsEmpty).add(operation); + } + const [modifiers, legacyVisibilities] = splitLegacyVisibility(visibilities); if (modifiers.length > 0 && legacyVisibilities.length > 0) { From 942b0a18ca6516f751c962f8636a26edc309c21c Mon Sep 17 00:00:00 2001 From: Will Temple Date: Wed, 15 Jan 2025 15:19:51 -0500 Subject: [PATCH 17/22] Un-internal visibility flags --- packages/http/src/metadata.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/packages/http/src/metadata.ts b/packages/http/src/metadata.ts index 50024d3fbaa..d39ed6fb50d 100644 --- a/packages/http/src/metadata.ts +++ b/packages/http/src/metadata.ts @@ -50,8 +50,6 @@ export enum Visibility { * and therefore no metadata is applicable. * * Never use this flag. It is used internally by the HTTP core. - * - * @internal */ Item = 1 << 20, @@ -60,8 +58,6 @@ export enum Visibility { * will have fields made optional if request visibility includes update. * * Never use this flag. It is used internally by the HTTP core. - * - * @internal */ Patch = 1 << 21, @@ -70,8 +66,6 @@ export enum Visibility { * allowing the shape of PATCH request bodies to be represented exactly. * * Never use this flag. It is used internally by the HTTP core. - * - * @internal */ SkipEffectiveOptionality = 1 << 22, @@ -79,8 +73,6 @@ export enum Visibility { * Additional flags to indicate the treatment of properties in specific contexts. * * Never use these flags. They are used internally by the HTTP core. - * - * @internal */ Synthetic = Visibility.Item | Visibility.Patch | Visibility.SkipEffectiveOptionality, } From 451ead4f1c96aa04a7ae56b3245c1533120541df Mon Sep 17 00:00:00 2001 From: Will Temple Date: Fri, 17 Jan 2025 14:30:41 -0500 Subject: [PATCH 18/22] Add horrible workaround for empty parameterVisibility --- packages/http/src/metadata.ts | 24 +++++++++++++++------- packages/http/test/http-decorators.test.ts | 4 ++-- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/packages/http/src/metadata.ts b/packages/http/src/metadata.ts index d39ed6fb50d..873cdecde54 100644 --- a/packages/http/src/metadata.ts +++ b/packages/http/src/metadata.ts @@ -62,19 +62,23 @@ export enum Visibility { Patch = 1 << 21, /** - * Additional flag to indicate that effective-optionality analysis should be skipped, - * allowing the shape of PATCH request bodies to be represented exactly. + * Additional flag to indicate that legacy parameter visibility behavior + * should be used. This disables effective optionality for the body of + * PATCH operations, and considers only properties that do not have _explicit_ + * visibility visible. This flag is activated by default when an operation + * uses `@parameterVisibility` without arguments, and should not be enabled + * in any other circumstances. * * Never use this flag. It is used internally by the HTTP core. */ - SkipEffectiveOptionality = 1 << 22, + LegacyParameterVisibility = 1 << 22, /** * Additional flags to indicate the treatment of properties in specific contexts. * * Never use these flags. They are used internally by the HTTP core. */ - Synthetic = Visibility.Item | Visibility.Patch | Visibility.SkipEffectiveOptionality, + Synthetic = Visibility.Item | Visibility.Patch | Visibility.LegacyParameterVisibility, } const visibilityToArrayMap: Map = new Map(); @@ -411,7 +415,7 @@ export function resolveRequestVisibility( const isEmptyParameterVisibility = program.stateSet(parameterVisibilityIsEmpty).has(operation); if (isEmptyParameterVisibility) { - visibility |= Visibility.SkipEffectiveOptionality; + visibility |= Visibility.LegacyParameterVisibility; } return visibility; @@ -435,7 +439,13 @@ export function isMetadata(program: Program, property: ModelProperty) { * Determines if the given property is visible with the given visibility. */ export function isVisible(program: Program, property: ModelProperty, visibility: Visibility) { - return isVisibleCore(program, property, visibilityToFilter(program, visibility)); + if (visibility & Visibility.LegacyParameterVisibility) { + // This is a hack that preserves the behavior of `@parameterVisibility()` with no arguments for now. + // eslint-disable-next-line @typescript-eslint/no-deprecated + return isVisibleCore(program, property, []); + } else { + return isVisibleCore(program, property, visibilityToFilter(program, visibility)); + } } /** @@ -702,7 +712,7 @@ export function createMetadataInfo(program: Program, options?: MetadataInfoOptio const hasUpdate = (visibility & Visibility.Update) !== 0; const isPatch = (visibility & Visibility.Patch) !== 0; const isItem = (visibility & Visibility.Item) !== 0; - const skipEffectiveOptionality = (visibility & Visibility.SkipEffectiveOptionality) !== 0; + const skipEffectiveOptionality = (visibility & Visibility.LegacyParameterVisibility) !== 0; return property.optional || (!skipEffectiveOptionality && hasUpdate && isPatch && !isItem); } diff --git a/packages/http/test/http-decorators.test.ts b/packages/http/test/http-decorators.test.ts index 9dbcb5a964b..76e990a39de 100644 --- a/packages/http/test/http-decorators.test.ts +++ b/packages/http/test/http-decorators.test.ts @@ -1280,11 +1280,11 @@ describe("http: decorators", () => { `)) as { test: Operation }; deepStrictEqual( resolveRequestVisibility(runner.program, test, "patch"), - Visibility.All | Visibility.Patch | Visibility.SkipEffectiveOptionality, + Visibility.All | Visibility.Patch | Visibility.LegacyParameterVisibility, ); deepStrictEqual( resolveRequestVisibility(runner.program, test, "get"), - Visibility.All | Visibility.SkipEffectiveOptionality, + Visibility.All | Visibility.LegacyParameterVisibility, ); }); }); From 64572efdb318aa46a02bbc79ec13de5b3190ee3c Mon Sep 17 00:00:00 2001 From: Will Temple Date: Tue, 21 Jan 2025 12:42:44 -0500 Subject: [PATCH 19/22] Update packages/http/test/http-decorators.test.ts Co-authored-by: Mark Cowlishaw --- packages/http/test/http-decorators.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/http/test/http-decorators.test.ts b/packages/http/test/http-decorators.test.ts index 76e990a39de..21276e10fbb 100644 --- a/packages/http/test/http-decorators.test.ts +++ b/packages/http/test/http-decorators.test.ts @@ -1273,7 +1273,7 @@ describe("http: decorators", () => { ); }); - it("ensures all properties visible ane effective optionality skipped when parameterVisibility has no arguments", async () => { + it("ensures all properties visible and effective optionality skipped when parameterVisibility has no arguments", async () => { const { test } = (await runner.compile(` @parameterVisibility @test op test(@path id: string, @body example: {}): void; From 3211758b83158220ce67353a646f4e1e3999ac57 Mon Sep 17 00:00:00 2001 From: Will Temple Date: Tue, 21 Jan 2025 18:15:06 -0500 Subject: [PATCH 20/22] Improve documentation --- packages/compiler/generated-defs/TypeSpec.ts | 86 ++++++++---- packages/compiler/lib/std/visibility.tsp | 132 +++++++++++++----- .../standard-library/built-in-data-types.md | 46 +++++- .../standard-library/built-in-decorators.md | 86 ++++++++---- 4 files changed, 257 insertions(+), 93 deletions(-) diff --git a/packages/compiler/generated-defs/TypeSpec.ts b/packages/compiler/generated-defs/TypeSpec.ts index 24e54904bb7..b150b76075e 100644 --- a/packages/compiler/generated-defs/TypeSpec.ts +++ b/packages/compiler/generated-defs/TypeSpec.ts @@ -800,19 +800,31 @@ export type InspectTypeNameDecorator = ( ) => void; /** - * Indicates that a property is only considered to be present or applicable ("visible") with - * the in the given named contexts ("visibilities"). When a property has no visibilities applied - * to it, it is implicitly visible always. + * Sets the visibility modifiers that are active on a property, indicating that it is only considered to be present + * (or "visible") in contexts that select for the given modifiers. * - * As far as the TypeSpec core library is concerned, visibilities are open-ended and can be arbitrary - * strings, but the following visibilities are well-known to standard libraries and should be used - * with standard emitters that interpret them as follows: + * A property without any visibility settings applied for any visibility class (e.g. `Lifecycle`) is considered to have + * the default visibility settings for that class. * - * - "read": output of any operation. - * - "create": input to operations that create an entity.. - * - "query": input to operations that read data. - * - "update": input to operations that update data. - * - "delete": input to operations that delete data. + * If visibility for the property has already been set for a visibility class (for example, using `@invisible` or + * `@removeVisibility`), this decorator will **add** the specified visibility modifiers to the property. + * + * See: [Visibility](https://typespec.io/docs/language-basics/visibility) + * + * The `@typespec/http` library uses `Lifecycle` visibility to determine which properties are included in the request or + * response bodies of HTTP operations. By default, it uses the following visibility settings: + * + * - For the return type of operations, properties are included if they have `Lifecycle.Read` visibility. + * - For POST operation parameters, properties are included if they have `Lifecycle.Create` visibility. + * - For PUT operation parameters, properties are included if they have `Lifecycle.Create` or `Lifecycle.Update` visibility. + * - For PATCH operation parameters, properties are included if they have `Lifecycle.Update` visibility. + * - For DELETE operation parameters, properties are included if they have `Lifecycle.Delete` visibility. + * - For GET or HEAD operation parameters, properties are included if they have `Lifecycle.Query` visibility. + * + * By default, properties have all five Lifecycle visibility modifiers enabled, so a property is visible in all contexts + * by default. + * + * The default settings may be overridden using the `@returnTypeVisibility` and `@parameterVisibility` decorators. * * See also: [Automatic visibility](https://typespec.io/docs/libraries/http/operations#automatic-visibility) * @@ -820,11 +832,15 @@ export type InspectTypeNameDecorator = ( * @example * ```typespec * model Dog { - * // the service will generate an ID, so you don't need to send it. - * @visibility(Lifecycle.Read) id: int32; - * // the service will store this secret name, but won't ever return it - * @visibility(Lifecycle.Create, Lifecycle.Update) secretName: string; - * // the regular name is always present + * // The service will generate an ID, so you don't need to send it. + * @visibility(Lifecycle.Read) + * id: int32; + * + * // The service will store this secret name, but won't ever return it. + * @visibility(Lifecycle.Create, Lifecycle.Update) + * secretName: string; + * + * // The regular name has all vi * name: string; * } * ``` @@ -839,7 +855,8 @@ export type VisibilityDecorator = ( * Indicates that a property is not visible in the given visibility class. * * This decorator removes all active visibility modifiers from the property within - * the given visibility class. + * the given visibility class, making it invisible to any context that selects for + * visibility modifiers within that class. * * @param visibilityClass The visibility class to make the property invisible within. * @example @@ -868,8 +885,8 @@ export type InvisibleDecorator = ( * @example * ```typespec * model Example { - * // This property will have the Create and Update visibilities, but not the - * // Read visibility, since it is removed. + * // This property will have all Lifecycle visibilities except the Read + * // visibility, since it is removed. * @removeVisibility(Lifecycle.Read) * secret_property: string; * } @@ -882,22 +899,26 @@ export type RemoveVisibilityDecorator = ( ) => void; /** - * Removes properties that are not considered to be present or applicable - * ("visible") in the given named contexts ("visibilities"). Can be used - * together with spread to effectively spread only visible properties into - * a new model. + * Removes properties that do not have at least one of the given visibility modifiers + * active. + * + * If no visibility modifiers are supplied, this decorator has no effect. * * See also: [Automatic visibility](https://typespec.io/docs/libraries/http/operations#automatic-visibility) * * When using an emitter that applies visibility automatically, it is generally * not necessary to use this decorator. * - * @param visibilities List of visibilities which apply to this property. + * @param visibilities List of visibilities that apply to this property. * @example * ```typespec * model Dog { - * @visibility("read") id: int32; - * @visibility("create", "update") secretName: string; + * @visibility(Lifecycle.Read) + * id: int32; + * + * @visibility(Lifecycle.Create, Lifecycle.Update) + * secretName: string; + * * name: string; * } * @@ -907,14 +928,14 @@ export type RemoveVisibilityDecorator = ( * // * // In this case, the id property is removed, and the name and secretName * // properties are kept. - * @withVisibility("create", "update") + * @withVisibility(Lifecycle.Create, Lifecycle.Update) * model DogCreateOrUpdate { * ...Dog; * } * * // In this case the id and name properties are kept and the secretName property * // is removed. - * @withVisibility("read") + * @withVisibility(Lifecycle.Read) * model DogRead { * ...Dog; * } @@ -930,7 +951,14 @@ export type WithVisibilityDecorator = ( * Declares the visibility constraint of the parameters of a given operation. * * A parameter or property nested within a parameter will be visible if it has _any_ of the visibilities - * in the list, or if the list is empty (in which case the parameter or property is always visible). + * in the list. + * + * WARNING: If no arguments are provided to this decorator, the `@typespec/http` library considers only properties + * that do not have visibility modifiers _explicitly_ configured to be visible. Additionally, the HTTP library will + * disable the feature of `@patch` operations that causes the properties of the request body to become effectively + * optional. Some specifications have used this configuration in the past to describe exact PATCH bodies, but using this + * decorator with no arguments in that manner is not recommended. The legacy behavior of `@parameterVisibility` with no + * arguments is preserved for backwards compatibility pending a future review and possible deprecation. * * @param visibilities List of visibility modifiers that apply to the parameters of this operation. */ diff --git a/packages/compiler/lib/std/visibility.tsp b/packages/compiler/lib/std/visibility.tsp index aeee125b1ea..7a5d639fbe8 100644 --- a/packages/compiler/lib/std/visibility.tsp +++ b/packages/compiler/lib/std/visibility.tsp @@ -8,19 +8,31 @@ using TypeSpec.Reflection; namespace TypeSpec; /** - * Indicates that a property is only considered to be present or applicable ("visible") with - * the in the given named contexts ("visibilities"). When a property has no visibilities applied - * to it, it is implicitly visible always. + * Sets the visibility modifiers that are active on a property, indicating that it is only considered to be present + * (or "visible") in contexts that select for the given modifiers. * - * As far as the TypeSpec core library is concerned, visibilities are open-ended and can be arbitrary - * strings, but the following visibilities are well-known to standard libraries and should be used - * with standard emitters that interpret them as follows: + * A property without any visibility settings applied for any visibility class (e.g. `Lifecycle`) is considered to have + * the default visibility settings for that class. * - * - "read": output of any operation. - * - "create": input to operations that create an entity.. - * - "query": input to operations that read data. - * - "update": input to operations that update data. - * - "delete": input to operations that delete data. + * If visibility for the property has already been set for a visibility class (for example, using `@invisible` or + * `@removeVisibility`), this decorator will **add** the specified visibility modifiers to the property. + * + * See: [Visibility](https://typespec.io/docs/language-basics/visibility) + * + * The `@typespec/http` library uses `Lifecycle` visibility to determine which properties are included in the request or + * response bodies of HTTP operations. By default, it uses the following visibility settings: + * + * - For the return type of operations, properties are included if they have `Lifecycle.Read` visibility. + * - For POST operation parameters, properties are included if they have `Lifecycle.Create` visibility. + * - For PUT operation parameters, properties are included if they have `Lifecycle.Create` or `Lifecycle.Update` visibility. + * - For PATCH operation parameters, properties are included if they have `Lifecycle.Update` visibility. + * - For DELETE operation parameters, properties are included if they have `Lifecycle.Delete` visibility. + * - For GET or HEAD operation parameters, properties are included if they have `Lifecycle.Query` visibility. + * + * By default, properties have all five Lifecycle visibility modifiers enabled, so a property is visible in all contexts + * by default. + * + * The default settings may be overridden using the `@returnTypeVisibility` and `@parameterVisibility` decorators. * * See also: [Automatic visibility](https://typespec.io/docs/libraries/http/operations#automatic-visibility) * @@ -30,11 +42,15 @@ namespace TypeSpec; * * ```typespec * model Dog { - * // the service will generate an ID, so you don't need to send it. - * @visibility(Lifecycle.Read) id: int32; - * // the service will store this secret name, but won't ever return it - * @visibility(Lifecycle.Create, Lifecycle.Update) secretName: string; - * // the regular name is always present + * // The service will generate an ID, so you don't need to send it. + * @visibility(Lifecycle.Read) + * id: int32; + * + * // The service will store this secret name, but won't ever return it. + * @visibility(Lifecycle.Create, Lifecycle.Update) + * secretName: string; + * + * // The regular name has all vi * name: string; * } * ``` @@ -45,7 +61,8 @@ extern dec visibility(target: ModelProperty, ...visibilities: valueof (string | * Indicates that a property is not visible in the given visibility class. * * This decorator removes all active visibility modifiers from the property within - * the given visibility class. + * the given visibility class, making it invisible to any context that selects for + * visibility modifiers within that class. * * @param visibilityClass The visibility class to make the property invisible within. * @@ -72,8 +89,8 @@ extern dec invisible(target: ModelProperty, visibilityClass: Enum); * @example * ```typespec * model Example { - * // This property will have the Create and Update visibilities, but not the - * // Read visibility, since it is removed. + * // This property will have all Lifecycle visibilities except the Read + * // visibility, since it is removed. * @removeVisibility(Lifecycle.Read) * secret_property: string; * } @@ -82,23 +99,27 @@ extern dec invisible(target: ModelProperty, visibilityClass: Enum); extern dec removeVisibility(target: ModelProperty, ...visibilities: valueof EnumMember[]); /** - * Removes properties that are not considered to be present or applicable - * ("visible") in the given named contexts ("visibilities"). Can be used - * together with spread to effectively spread only visible properties into - * a new model. + * Removes properties that do not have at least one of the given visibility modifiers + * active. + * + * If no visibility modifiers are supplied, this decorator has no effect. * * See also: [Automatic visibility](https://typespec.io/docs/libraries/http/operations#automatic-visibility) * * When using an emitter that applies visibility automatically, it is generally * not necessary to use this decorator. * - * @param visibilities List of visibilities which apply to this property. + * @param visibilities List of visibilities that apply to this property. * * @example * ```typespec * model Dog { - * @visibility("read") id: int32; - * @visibility("create", "update") secretName: string; + * @visibility(Lifecycle.Read) + * id: int32; + * + * @visibility(Lifecycle.Create, Lifecycle.Update) + * secretName: string; + * * name: string; * } * @@ -108,14 +129,14 @@ extern dec removeVisibility(target: ModelProperty, ...visibilities: valueof Enum * // * // In this case, the id property is removed, and the name and secretName * // properties are kept. - * @withVisibility("create", "update") + * @withVisibility(Lifecycle.Create, Lifecycle.Update) * model DogCreateOrUpdate { * ...Dog; * } * * // In this case the id and name properties are kept and the secretName property * // is removed. - * @withVisibility("read") + * @withVisibility(Lifecycle.Read) * model DogRead { * ...Dog; * } @@ -144,7 +165,14 @@ extern dec withDefaultKeyVisibility(target: Model, visibility: valueof string | * Declares the visibility constraint of the parameters of a given operation. * * A parameter or property nested within a parameter will be visible if it has _any_ of the visibilities - * in the list, or if the list is empty (in which case the parameter or property is always visible). + * in the list. + * + * WARNING: If no arguments are provided to this decorator, the `@typespec/http` library considers only properties + * that do not have visibility modifiers _explicitly_ configured to be visible. Additionally, the HTTP library will + * disable the feature of `@patch` operations that causes the properties of the request body to become effectively + * optional. Some specifications have used this configuration in the past to describe exact PATCH bodies, but using this + * decorator with no arguments in that manner is not recommended. The legacy behavior of `@parameterVisibility` with no + * arguments is preserved for backwards compatibility pending a future review and possible deprecation. * * @param visibilities List of visibility modifiers that apply to the parameters of this operation. */ @@ -188,8 +216,12 @@ extern dec defaultVisibility(target: Enum, ...visibilities: valueof EnumMember[] * @example * ```typespec * model Dog { - * @visibility(Lifecycle.Read) id: int32; - * @visibility(Lifecycle.Create, Lifecycle.Update) secretName: string; + * @visibility(Lifecycle.Read) + * id: int32; + * + * @visibility(Lifecycle.Create, Lifecycle.Update) + * secretName: string; + * * name: string; * } * ``` @@ -322,6 +354,7 @@ extern dec withLifecycleUpdate(target: Model); * name: string; * } * + * // This model has only the `name` field. * model CreateDog is Create; * ``` */ @@ -353,9 +386,13 @@ model Create; * ``` */ @@ -422,9 +463,16 @@ model Update; ``` @@ -327,9 +355,13 @@ model Dog { @visibility(Lifecycle.Read) id: int32; + @visibility(Lifecycle.Create, Lifecycle.Update) + secretName: string; + name: string; } +// This model has the `id` and `name` fields, but not `secretName`. model ReadDog is Read; ``` @@ -398,9 +430,13 @@ model Dog { @visibility(Lifecycle.Read) id: int32; + @visibility(Lifecycle.Create, Lifecycle.Update) + secretName: string; + name: string; } +// This model will have the `secretName` and `name` fields, but not the `id` field. model UpdateDog is Update; ``` @@ -517,8 +553,12 @@ enum Lifecycle ```typespec model Dog { - @visibility(Lifecycle.Read) id: int32; - @visibility(Lifecycle.Create, Lifecycle.Update) secretName: string; + @visibility(Lifecycle.Read) + id: int32; + + @visibility(Lifecycle.Create, Lifecycle.Update) + secretName: string; + name: string; } ``` diff --git a/website/src/content/docs/docs/standard-library/built-in-decorators.md b/website/src/content/docs/docs/standard-library/built-in-decorators.md index 20b1d50257d..45fa2cd5de8 100644 --- a/website/src/content/docs/docs/standard-library/built-in-decorators.md +++ b/website/src/content/docs/docs/standard-library/built-in-decorators.md @@ -427,7 +427,8 @@ A debugging decorator used to inspect a type name. Indicates that a property is not visible in the given visibility class. This decorator removes all active visibility modifiers from the property within -the given visibility class. +the given visibility class, making it invisible to any context that selects for +visibility modifiers within that class. ```typespec @invisible(visibilityClass: Enum) ``` @@ -931,7 +932,14 @@ model Page { Declares the visibility constraint of the parameters of a given operation. A parameter or property nested within a parameter will be visible if it has _any_ of the visibilities -in the list, or if the list is empty (in which case the parameter or property is always visible). +in the list. + +WARNING: If no arguments are provided to this decorator, the `@typespec/http` library considers only properties +that do not have visibility modifiers _explicitly_ configured to be visible. Additionally, the HTTP library will +disable the feature of `@patch` operations that causes the properties of the request body to become effectively +optional. Some specifications have used this configuration in the past to describe exact PATCH bodies, but using this +decorator with no arguments in that manner is not recommended. The legacy behavior of `@parameterVisibility` with no +arguments is preserved for backwards compatibility pending a future review and possible deprecation. ```typespec @parameterVisibility(...visibilities: valueof string | EnumMember[]) ``` @@ -1065,8 +1073,8 @@ The property to remove visibility from. ```typespec model Example { - // This property will have the Create and Update visibilities, but not the - // Read visibility, since it is removed. + // This property will have all Lifecycle visibilities except the Read + // visibility, since it is removed. @removeVisibility(Lifecycle.Read) secret_property: string; } @@ -1223,19 +1231,31 @@ Attaches a tag to an operation, interface, or namespace. Multiple `@tag` decorat ### `@visibility` {#@visibility} -Indicates that a property is only considered to be present or applicable ("visible") with -the in the given named contexts ("visibilities"). When a property has no visibilities applied -to it, it is implicitly visible always. +Sets the visibility modifiers that are active on a property, indicating that it is only considered to be present +(or "visible") in contexts that select for the given modifiers. + +A property without any visibility settings applied for any visibility class (e.g. `Lifecycle`) is considered to have +the default visibility settings for that class. + +If visibility for the property has already been set for a visibility class (for example, using `@invisible` or +`@removeVisibility`), this decorator will **add** the specified visibility modifiers to the property. + +See: [Visibility](https://typespec.io/docs/language-basics/visibility) + +The `@typespec/http` library uses `Lifecycle` visibility to determine which properties are included in the request or +response bodies of HTTP operations. By default, it uses the following visibility settings: -As far as the TypeSpec core library is concerned, visibilities are open-ended and can be arbitrary -strings, but the following visibilities are well-known to standard libraries and should be used -with standard emitters that interpret them as follows: +- For the return type of operations, properties are included if they have `Lifecycle.Read` visibility. +- For POST operation parameters, properties are included if they have `Lifecycle.Create` visibility. +- For PUT operation parameters, properties are included if they have `Lifecycle.Create` or `Lifecycle.Update` visibility. +- For PATCH operation parameters, properties are included if they have `Lifecycle.Update` visibility. +- For DELETE operation parameters, properties are included if they have `Lifecycle.Delete` visibility. +- For GET or HEAD operation parameters, properties are included if they have `Lifecycle.Query` visibility. -- "read": output of any operation. -- "create": input to operations that create an entity.. -- "query": input to operations that read data. -- "update": input to operations that update data. -- "delete": input to operations that delete data. +By default, properties have all five Lifecycle visibility modifiers enabled, so a property is visible in all contexts +by default. + +The default settings may be overridden using the `@returnTypeVisibility` and `@parameterVisibility` decorators. See also: [Automatic visibility](https://typespec.io/docs/libraries/http/operations#automatic-visibility) ```typespec @@ -1255,11 +1275,15 @@ See also: [Automatic visibility](https://typespec.io/docs/libraries/http/operati ```typespec model Dog { - // the service will generate an ID, so you don't need to send it. - @visibility(Lifecycle.Read) id: int32; - // the service will store this secret name, but won't ever return it - @visibility(Lifecycle.Create, Lifecycle.Update) secretName: string; - // the regular name is always present + // The service will generate an ID, so you don't need to send it. + @visibility(Lifecycle.Read) + id: int32; + + // The service will store this secret name, but won't ever return it. + @visibility(Lifecycle.Create, Lifecycle.Update) + secretName: string; + + // The regular name has all vi name: string; } ``` @@ -1418,10 +1442,10 @@ None ### `@withVisibility` {#@withVisibility} -Removes properties that are not considered to be present or applicable -("visible") in the given named contexts ("visibilities"). Can be used -together with spread to effectively spread only visible properties into -a new model. +Removes properties that do not have at least one of the given visibility modifiers +active. + +If no visibility modifiers are supplied, this decorator has no effect. See also: [Automatic visibility](https://typespec.io/docs/libraries/http/operations#automatic-visibility) @@ -1438,14 +1462,18 @@ not necessary to use this decorator. #### Parameters | Name | Type | Description | |------|------|-------------| -| visibilities | `valueof string \| EnumMember[]` | List of visibilities which apply to this property. | +| visibilities | `valueof string \| EnumMember[]` | List of visibilities that apply to this property. | #### Examples ```typespec model Dog { - @visibility("read") id: int32; - @visibility("create", "update") secretName: string; + @visibility(Lifecycle.Read) + id: int32; + + @visibility(Lifecycle.Create, Lifecycle.Update) + secretName: string; + name: string; } @@ -1455,14 +1483,14 @@ model Dog { // // In this case, the id property is removed, and the name and secretName // properties are kept. -@withVisibility("create", "update") +@withVisibility(Lifecycle.Create, Lifecycle.Update) model DogCreateOrUpdate { ...Dog; } // In this case the id and name properties are kept and the secretName property // is removed. -@withVisibility("read") +@withVisibility(Lifecycle.Read) model DogRead { ...Dog; } From 827e42fb635bb2a43642fb14131c8558304ba139 Mon Sep 17 00:00:00 2001 From: Will Temple Date: Tue, 21 Jan 2025 18:15:30 -0500 Subject: [PATCH 21/22] Change shared symbol for IsParameterVisibilityEmpty --- packages/compiler/src/lib/visibility.ts | 4 ++-- packages/http/src/metadata.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/compiler/src/lib/visibility.ts b/packages/compiler/src/lib/visibility.ts index 201cc6664a0..811b4e87bd9 100644 --- a/packages/compiler/src/lib/visibility.ts +++ b/packages/compiler/src/lib/visibility.ts @@ -154,8 +154,8 @@ function getOperationVisibilityConfig( } // Hidden internal symbol to mark that parameter visibility was empty. This is used by HTTP -// to set the SkipEffectiveOptionality flag. This is a hack. -const parameterVisibilityIsEmpty = Symbol.for("TypeSpec::Visibility::ParameterVisibilityIsEmpty"); +// to set the LegacyParameterVisibility flag. This is a hack. +const parameterVisibilityIsEmpty = Symbol.for("TypeSpec.Visibility.ParameterVisibilityIsEmpty"); export const $parameterVisibility: ParameterVisibilityDecorator = ( context: DecoratorContext, diff --git a/packages/http/src/metadata.ts b/packages/http/src/metadata.ts index 873cdecde54..88897fd6873 100644 --- a/packages/http/src/metadata.ts +++ b/packages/http/src/metadata.ts @@ -381,7 +381,7 @@ export function HttpVisibilityProvider( // Hidden internal symbol to mark that parameter visibility was empty. This is used by HTTP // to set the SkipEffectiveOptionality flag. This is a hack. -const parameterVisibilityIsEmpty = Symbol.for("TypeSpec::Visibility::ParameterVisibilityIsEmpty"); +const parameterVisibilityIsEmpty = Symbol.for("TypeSpec.Visibility.ParameterVisibilityIsEmpty"); /** * Returns the applicable parameter visibility or visibilities for the request if `@requestVisibility` was used. From 9eb32a093578b1115653c0ca260b312e5e68f1e4 Mon Sep 17 00:00:00 2001 From: Will Temple Date: Tue, 21 Jan 2025 18:15:38 -0500 Subject: [PATCH 22/22] More thorough test --- packages/http/test/http-decorators.test.ts | 61 ++++++++++++++++++++-- 1 file changed, 57 insertions(+), 4 deletions(-) diff --git a/packages/http/test/http-decorators.test.ts b/packages/http/test/http-decorators.test.ts index 21276e10fbb..75cc525329f 100644 --- a/packages/http/test/http-decorators.test.ts +++ b/packages/http/test/http-decorators.test.ts @@ -27,7 +27,13 @@ import { isQueryParam, isStatusCode, } from "../src/decorators.js"; -import { Visibility, getRequestVisibility, resolveRequestVisibility } from "../src/metadata.js"; +import { + Visibility, + createMetadataInfo, + getRequestVisibility, + resolveRequestVisibility, +} from "../src/metadata.js"; +import { getHttpOperation } from "../src/operations.js"; import { createHttpTestRunner } from "./test-host.js"; describe("http: decorators", () => { let runner: BasicTestRunner; @@ -1273,19 +1279,66 @@ describe("http: decorators", () => { ); }); - it("ensures all properties visible and effective optionality skipped when parameterVisibility has no arguments", async () => { + it("ensures legacy behavior of parameterVisibility with no arguments", async () => { const { test } = (await runner.compile(` + model Example { + @visibility(Lifecycle.Read) + @path + id: string; + + // @parameterVisibility with no args activates a hidden mode that hides all properties with explicit + // visibility. + @visibility(Lifecycle.Read, Lifecycle.Create, Lifecycle.Update, Lifecycle.Delete, Lifecycle.Query) + stillNotVisible: string; + + // This is the only property that will be visible in the payload. + name: string + } + @parameterVisibility - @test op test(@path id: string, @body example: {}): void; + @test + @route("/test") + @patch op test(@path id: Example.id, @bodyRoot example: Example): void; `)) as { test: Operation }; + const requestVisibility = resolveRequestVisibility(runner.program, test, "patch"); deepStrictEqual( - resolveRequestVisibility(runner.program, test, "patch"), + requestVisibility, Visibility.All | Visibility.Patch | Visibility.LegacyParameterVisibility, ); deepStrictEqual( resolveRequestVisibility(runner.program, test, "get"), Visibility.All | Visibility.LegacyParameterVisibility, ); + + const [httpOperation, diagnostics] = getHttpOperation(runner.program, test); + + // Ensure id parameter is not duplicated in route + strictEqual(httpOperation.uriTemplate, "/test/{id}"); + + const metadataInfo = createMetadataInfo(runner.program); + + const { body: requestBody } = httpOperation.parameters; + + strictEqual(diagnostics.length, 0); + ok(requestBody); + strictEqual(requestBody.bodyKind, "single"); + strictEqual(requestBody.type.kind, "Model"); + strictEqual(requestBody.type.name, "Example"); + + const properties = { + id: requestBody.type.properties.get("id")!, + stillNotVisible: requestBody.type.properties.get("stillNotVisible")!, + name: requestBody.type.properties.get("name")!, + } as const; + + strictEqual(metadataInfo.isPayloadProperty(properties.id, requestVisibility), false); + strictEqual( + metadataInfo.isPayloadProperty(properties.stillNotVisible, requestVisibility), + false, + ); + strictEqual(metadataInfo.isPayloadProperty(properties.name, requestVisibility), true); + + strictEqual(metadataInfo.isOptional(properties.name, requestVisibility), false); }); }); });