-
Notifications
You must be signed in to change notification settings - Fork 387
Emit experimental diagnostics in C# clients #11685
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,13 +14,65 @@ import type { | |
| import { setTypeSpecNamespace } from "@typespec/compiler"; | ||
| import type { DynamicModelDecorator } from "../../../generated-defs/TypeSpec.HttpClient.CSharp.js"; | ||
| import type { ExternalDocs } from "../type/external-docs.js"; | ||
| import type { InputExperimentalDetails } from "../type/input-operation.js"; | ||
|
|
||
| /** | ||
| * The fully qualified decorator name pattern for the dynamicModel decorator. | ||
| * This is used in SDK context options to ensure the decorator is properly recognized. | ||
| * @beta | ||
| */ | ||
| export const DYNAMIC_MODEL_DECORATOR_PATTERN = "TypeSpec\\.HttpClient\\.CSharp\\.@dynamicModel"; | ||
| export const EXPERIMENTAL_DECORATOR_PATTERN = "TypeSpec\\.HttpClient\\.@experimental"; | ||
| const experimentalDecoratorName = "TypeSpec.HttpClient.@experimental"; | ||
| const csharpEmitterName = "@typespec/http-client-csharp"; | ||
|
|
||
| interface ExperimentalDecoratorOptions { | ||
| emitterScope?: string; | ||
| diagnosticId?: string; | ||
| dependsOn?: unknown[]; | ||
| } | ||
|
|
||
| export function getExperimentalDetails( | ||
| decorators: readonly { name: string; arguments: Record<string, unknown> }[], | ||
| ): InputExperimentalDetails | undefined { | ||
| const decorator = decorators.find((item) => item.name === experimentalDecoratorName); | ||
| if (!decorator) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const options = decorator.arguments.options as ExperimentalDecoratorOptions | undefined; | ||
| // TCGC filters a top-level `scope` argument, but @experimental carries | ||
| // `emitterScope` inside its options object. | ||
| if (!isEmitterScopeApplicable(options?.emitterScope)) { | ||
| return undefined; | ||
| } | ||
|
|
||
| return { | ||
| diagnosticId: typeof options?.diagnosticId === "string" ? options.diagnosticId : undefined, | ||
| dependsOn: (options?.dependsOn ?? []).filter( | ||
| (diagnosticId): diagnosticId is string => typeof diagnosticId === "string", | ||
| ), | ||
| }; | ||
| } | ||
|
|
||
| function isEmitterScopeApplicable(emitterScope: string | undefined): boolean { | ||
| if (!emitterScope) { | ||
| return true; | ||
| } | ||
|
|
||
| const scopes = emitterScope | ||
| .split(",") | ||
| .map((scope) => scope.trim()) | ||
| .filter((scope) => scope.length > 0); | ||
| const excludedScopes = scopes | ||
| .filter((scope) => scope.startsWith("!")) | ||
| .map((scope) => scope.slice(1)); | ||
| if (excludedScopes.length > 0) { | ||
| return !excludedScopes.includes(csharpEmitterName); | ||
| } | ||
|
|
||
| return scopes.includes(csharpEmitterName); | ||
|
Comment on lines
+63
to
+74
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm wondering if tcgc already exports some util function we can use here instead ? |
||
| } | ||
|
|
||
| const externalDocsKey = Symbol("externalDocs"); | ||
| export function getExternalDocs(context: SdkContext, entity: Type): ExternalDocs | undefined { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,4 +29,10 @@ export interface InputOperation { | |
| crossLanguageDefinitionId: string; | ||
| decorators?: DecoratorInfo[]; | ||
| namespace?: string; | ||
| experimental?: InputExperimentalDetails; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are we intionally scoping this only for operations at the moment? I'm assuming we'll want to use this for other types in the future ? |
||
| } | ||
|
|
||
| export interface InputExperimentalDetails { | ||
| diagnosticId?: string; | ||
| dependsOn: string[]; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| import { deepStrictEqual, strictEqual } from "assert"; | ||
| import { describe, it } from "vitest"; | ||
| import { getExperimentalDetails } from "../../src/lib/decorators.js"; | ||
|
|
||
| describe("experimental decorator metadata", () => { | ||
| it("extracts diagnostic and dependency identifiers", () => { | ||
| const details = getExperimentalDetails([ | ||
| { | ||
| name: "TypeSpec.HttpClient.@experimental", | ||
| arguments: { | ||
| options: { | ||
| emitterScope: "@typespec/http-client-csharp", | ||
| diagnosticId: "C", | ||
| dependsOn: ["A", "B"], | ||
| }, | ||
| }, | ||
| }, | ||
| ]); | ||
|
|
||
| deepStrictEqual(details, { | ||
| diagnosticId: "C", | ||
| dependsOn: ["A", "B"], | ||
| }); | ||
| }); | ||
|
|
||
| it("ignores metadata scoped to another emitter", () => { | ||
| const details = getExperimentalDetails([ | ||
| { | ||
| name: "TypeSpec.HttpClient.@experimental", | ||
| arguments: { | ||
| options: { | ||
| emitterScope: "other-emitter", | ||
| diagnosticId: "C", | ||
| }, | ||
| }, | ||
| }, | ||
| ]); | ||
|
|
||
| strictEqual(details, undefined); | ||
| }); | ||
|
|
||
| it("applies unscoped metadata", () => { | ||
| const details = getExperimentalDetails([ | ||
| { | ||
| name: "TypeSpec.HttpClient.@experimental", | ||
| arguments: { | ||
| options: { | ||
| diagnosticId: "C", | ||
| }, | ||
| }, | ||
| }, | ||
| ]); | ||
|
|
||
| deepStrictEqual(details, { | ||
| diagnosticId: "C", | ||
| dependsOn: [], | ||
| }); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Diagnostics.CodeAnalysis; | ||
| using System.Linq; | ||
| using Microsoft.TypeSpec.Generator.Input; | ||
| using Microsoft.TypeSpec.Generator.Providers; | ||
| using Microsoft.TypeSpec.Generator.Statements; | ||
| using static Microsoft.TypeSpec.Generator.Snippets.Snippet; | ||
|
|
||
| namespace Microsoft.TypeSpec.Generator.ClientModel.Utilities | ||
| { | ||
| internal static class ExperimentalApiHelpers | ||
| { | ||
| private const string DependencySuppressionJustification = | ||
| "This method depends on experimental functionality."; | ||
|
|
||
| public static IReadOnlyList<AttributeStatement> BuildAttributes(InputOperation operation) | ||
| { | ||
| var diagnosticId = operation.Experimental?.DiagnosticId; | ||
| return string.IsNullOrWhiteSpace(diagnosticId) | ||
| ? [] | ||
| : [new AttributeStatement(typeof(ExperimentalAttribute), [Literal(diagnosticId)])]; | ||
| } | ||
|
|
||
| public static void AddDependencySuppressions(MethodProvider method, InputOperation operation) | ||
| { | ||
| var dependencies = operation.Experimental?.DependsOn; | ||
| if (dependencies is null || dependencies.Count == 0) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| method.Update(suppressions: | ||
| [ | ||
| .. dependencies | ||
| .Where(diagnosticId => !string.IsNullOrWhiteSpace(diagnosticId)) | ||
| .Distinct(StringComparer.Ordinal) | ||
| .Select(diagnosticId => new SuppressionStatement( | ||
| null, | ||
| Literal(diagnosticId), | ||
| DependencySuppressionJustification)), | ||
| .. method.Suppressions | ||
| ]); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this already handled by TCGC?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TCGC does filter scope, but it currently checks only a top-level
decoratorInfo.arguments["scope"].@experimentalreceivesClientDecoratorOptionsthrough itsoptionsparameter, so this value arrives asdecoratorInfo.arguments.options.emitterScopeand is not filtered by TCGC. I kept the local check, added a comment explaining the distinction, and retained coverage verifying metadata scoped to another emitter is ignored.--generated by Copilot