Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions packages/http-client-csharp/emitter/src/lib/decorators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Copy link
Copy Markdown
Contributor Author

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?

Copy link
Copy Markdown
Contributor Author

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"]. @experimental receives ClientDecoratorOptions through its options parameter, so this value arrives as decoratorInfo.arguments.options.emitterScope and 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

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ import type { OperationResponse } from "../type/operation-response.js";
import { RequestLocation } from "../type/request-location.js";
import { parseHttpRequestMethod } from "../type/request-method.js";
import { ResponseLocation } from "../type/response-location.js";
import { getExternalDocs, getOperationId } from "./decorators.js";
import { getExperimentalDetails, getExternalDocs, getOperationId } from "./decorators.js";
import { fromSdkHttpExamples } from "./example-converter.js";
import { createDiagnostic } from "./lib.js";
import { fromSdkType } from "./type-converter.js";
Expand Down Expand Up @@ -252,6 +252,7 @@ export function fromSdkServiceMethodOperation(
namespace: method.__raw?.namespace
? getClientNamespace(sdkContext, method.__raw.namespace)
: undefined,
experimental: getExperimentalDetails(method.decorators),
};

sdkContext.__typeCache.updateSdkOperationReferences(method.operation, operation);
Expand Down
7 changes: 5 additions & 2 deletions packages/http-client-csharp/emitter/src/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ import type { CreateSdkContextOptions } from "@azure-tools/typespec-client-gener
import { UnbrandedSdkEmitterOptions } from "@azure-tools/typespec-client-generator-core";
import type { EmitContext, JSONSchemaType } from "@typespec/compiler";
import { _defaultGeneratorName } from "./constants.js";
import { DYNAMIC_MODEL_DECORATOR_PATTERN } from "./lib/decorators.js";
import {
DYNAMIC_MODEL_DECORATOR_PATTERN,
EXPERIMENTAL_DECORATOR_PATTERN,
} from "./lib/decorators.js";
import { LoggerLevel } from "./lib/logger-level.js";

/**
Expand Down Expand Up @@ -176,7 +179,7 @@ export const defaultOptions = {
logLevel: LoggerLevel.INFO,
"generator-name": _defaultGeneratorName,
"sdk-context-options": {
additionalDecorators: [DYNAMIC_MODEL_DECORATOR_PATTERN],
additionalDecorators: [DYNAMIC_MODEL_DECORATOR_PATTERN, EXPERIMENTAL_DECORATOR_PATTERN],
},
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,10 @@ export interface InputOperation {
crossLanguageDefinitionId: string;
decorators?: DecoratorInfo[];
namespace?: string;
experimental?: InputExperimentalDetails;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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
Expand Up @@ -8,6 +8,7 @@
using System.Linq;
using Microsoft.TypeSpec.Generator.ClientModel.Primitives;
using Microsoft.TypeSpec.Generator.ClientModel.Snippets;
using Microsoft.TypeSpec.Generator.ClientModel.Utilities;
using Microsoft.TypeSpec.Generator.EmitterRpc;
using Microsoft.TypeSpec.Generator.Expressions;
using Microsoft.TypeSpec.Generator.Input;
Expand Down Expand Up @@ -239,13 +240,15 @@ private ScmMethodProvider BuildCreateRequestMethod(InputServiceMethod serviceMet
// Build message and all request modifications
var messageStatements = BuildMessage(serviceMethod, signature, isNextLinkRequest);

return new ScmMethodProvider(
var method = new ScmMethodProvider(
signature,
messageStatements,
this,
ScmMethodKind.CreateRequest,
xmlDocProvider: XmlDocProvider.Empty,
serviceMethod: serviceMethod);
ExperimentalApiHelpers.AddDependencySuppressions(method, operation);
return method;
}

private MethodBodyStatements BuildMessage(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,10 +229,11 @@ private ScmMethodProvider BuildConvenienceMethod(MethodProvider protocolMethod,
GetConvenienceMethodModifiers(protocolMethod.Signature.Modifiers, signatureParameters),
GetResponseType(ServiceMethod.Operation.Responses, true, isAsync, out _),
null,
signatureParameters,
Attributes: BuildConvenienceMethodAttributes());
signatureParameters);
}

AddMethodAttributes(methodSignature, BuildConvenienceMethodAttributes());

// Recompute the response body type so we can branch the body accordingly.
GetResponseType(ServiceMethod.Operation.Responses, true, isAsync, out var responseBodyType);
var streamingResponse = _streamingResponse.Value;
Expand Down Expand Up @@ -315,6 +316,7 @@ .. GetStackVariablesForReturnValueConversion(result, responseBodyType, isAsync,
}

var convenienceMethod = new ScmMethodProvider(methodSignature, methodBody, EnclosingType, ScmMethodKind.Convenience, collectionDefinition: collection, serviceMethod: ServiceMethod);
ExperimentalApiHelpers.AddDependencySuppressions(convenienceMethod, ServiceMethod.Operation);

if (convenienceMethod.XmlDocs != null)
{
Expand Down Expand Up @@ -938,16 +940,28 @@ private static bool IsConvertibleFromBinaryData(CSharpType type)
type.Equals(typeof(TimeSpan?));
}

private IReadOnlyList<AttributeStatement>? BuildConvenienceMethodAttributes()
private IReadOnlyList<AttributeStatement> BuildConvenienceMethodAttributes()
{
List<AttributeStatement> attributes = [.. ExperimentalApiHelpers.BuildAttributes(ServiceMethod.Operation)];
var bodyInputParam = ServiceMethod.Parameters.FirstOrDefault(p => p.Location == InputRequestLocation.Body);
if (bodyInputParam?.Type is InputModelType bodyModel
if (attributes.Count == 0
&& bodyInputParam?.Type is InputModelType bodyModel
&& bodyModel.Usage.HasFlag(InputModelTypeUsage.MultipartFormData))
{
return [new AttributeStatement(typeof(ExperimentalAttribute), [Literal(ScmModelProvider.FileBinaryContentDiagnosticId)])];
attributes.Add(new AttributeStatement(typeof(ExperimentalAttribute), [Literal(ScmModelProvider.FileBinaryContentDiagnosticId)]));
}

return null;
return attributes;
}

private static void AddMethodAttributes(
MethodSignature signature,
IReadOnlyList<AttributeStatement> attributes)
{
if (attributes.Count > 0)
{
signature.Update(attributes: [.. signature.Attributes, .. attributes]);
}
}

private IReadOnlyList<ValueExpression> GetProtocolMethodArguments(Dictionary<string, ValueExpression> declarations)
Expand Down Expand Up @@ -1260,6 +1274,8 @@ private ScmMethodProvider BuildProtocolMethod(MethodProvider createRequestMethod
bodyParameters = parameters;
}

AddMethodAttributes(methodSignature, ExperimentalApiHelpers.BuildAttributes(ServiceMethod.Operation));

TypeProvider? collection = null;
MethodBodyStatement[] methodBody;
if (_pagingServiceMethod != null)
Expand Down Expand Up @@ -1297,6 +1313,7 @@ .. ServiceMethod.Operation.BufferResponse

var protocolMethod =
new ScmMethodProvider(methodSignature, methodBody, EnclosingType, ScmMethodKind.Protocol, collectionDefinition: collection, serviceMethod: ServiceMethod);
ExperimentalApiHelpers.AddDependencySuppressions(protocolMethod, ServiceMethod.Operation);

if (protocolMethod.XmlDocs != null)
{
Expand Down
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
]);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1767,6 +1767,66 @@ public void TestMethodTypeIdentification()
Assert.AreEqual(ScmMethodKind.CreateRequest, createRequestMethod.Kind);
}

[Test]
public void ExperimentalOperationGeneratesAttributeAndDependencySuppressions()
{
MockHelpers.LoadMockGenerator();

var inputOperation = InputFactory.Operation(
"Bar",
experimental: new InputExperimentalDetails("C", ["A", "B"]));
var inputServiceMethod = InputFactory.BasicServiceMethod("Bar", inputOperation);
var inputClient = InputFactory.Client("TestClient", methods: [inputServiceMethod]);
var client = ScmCodeModelGenerator.Instance.TypeFactory.CreateClient(inputClient);

var methodCollection = new ScmMethodProviderCollection(inputServiceMethod, client!);

foreach (var method in methodCollection)
{
using var writer = new CodeWriter();
writer.WriteMethod(method);
var code = writer.ToString(false);

StringAssert.Contains(
"[global::System.Diagnostics.CodeAnalysis.ExperimentalAttribute(\"C\")]",
code);
StringAssert.Contains("#pragma warning disable A", code);
StringAssert.Contains("#pragma warning disable B", code);
StringAssert.Contains("#pragma warning restore A", code);
StringAssert.Contains("#pragma warning restore B", code);
}

using var createRequestWriter = new CodeWriter();
createRequestWriter.WriteMethod(client!.RestClient.GetCreateRequestMethod(inputOperation));
var createRequestCode = createRequestWriter.ToString(false);
StringAssert.DoesNotContain("ExperimentalAttribute", createRequestCode);
StringAssert.Contains("#pragma warning disable A", createRequestCode);
StringAssert.Contains("#pragma warning disable B", createRequestCode);
}

[Test]
public void OperationWithoutExperimentalMetadataDoesNotGenerateExperimentalCode()
{
MockHelpers.LoadMockGenerator();

var inputOperation = InputFactory.Operation("Bar");
var inputServiceMethod = InputFactory.BasicServiceMethod("Bar", inputOperation);
var inputClient = InputFactory.Client("TestClient", methods: [inputServiceMethod]);
var client = ScmCodeModelGenerator.Instance.TypeFactory.CreateClient(inputClient);

var methodCollection = new ScmMethodProviderCollection(inputServiceMethod, client!);

foreach (var method in methodCollection)
{
using var writer = new CodeWriter();
writer.WriteMethod(method);
var code = writer.ToString(false);

StringAssert.DoesNotContain("ExperimentalAttribute", code);
StringAssert.DoesNotContain("#pragma warning disable A", code);
}
}

[Test]
public async Task CollectionResultDefinitionAddedEvenWhenPagingMethodsCustomized()
{
Expand Down
Loading
Loading