Skip to content

Commit

Permalink
Refine the client-namespace-conflict diagnostic (#6161)
Browse files Browse the repository at this point in the history
Fixes Azure/autorest.csharp#5241

I removed the report diagnostic in our emitter - because not every
generator would have this issue
and move the diagnostic reporting into our C# generator, when we build
the namespace for a client type.

I also added a few test cases for the `Emitter` class in our C#
generator, which leads to series of other changes about the disposing
issue.
  • Loading branch information
ArcturusZhang authored Mar 3, 2025
1 parent 737af9e commit 4abea21
Showing 87 changed files with 741 additions and 318 deletions.
Original file line number Diff line number Diff line change
@@ -92,20 +92,11 @@ export function createModel(sdkContext: CSharpEmitterContext): CodeModel {
const uri = getMethodUri(endpointParameter);
const clientParameters = fromSdkEndpointParameter(endpointParameter);
const clientName = getClientName(client, parentNames);
// see if this namespace is a sub-namespace of an existing bad namespace
const segments = client.namespace.split(".");
const lastSegment = segments[segments.length - 1];
if (lastSegment === clientName) {
// we report diagnostics when the last segment of the namespace is the same as the client name
// because in our design, a sub namespace will be generated as a sub client with exact the same name as the namespace
// in csharp, this will cause a conflict between the namespace and the class name
sdkContext.logger.reportDiagnostic({
code: "client-namespace-conflict",
format: { namespace: client.namespace, clientName },
target: client.__raw.type ?? NoTarget,
});
}

sdkContext.__typeCache.crossLanguageDefinitionIds.set(
client.crossLanguageDefinitionId,
client.__raw.type,
);
return {
Name: clientName,
Namespace: client.namespace,
@@ -125,6 +116,7 @@ export function createModel(sdkContext: CSharpEmitterContext): CodeModel {
Parent: parentNames.length > 0 ? parentNames[parentNames.length - 1] : undefined,
Parameters: clientParameters,
Decorators: client.decorators,
CrossLanguageDefinitionId: client.crossLanguageDefinitionId,
};
}

2 changes: 1 addition & 1 deletion packages/http-client-csharp/emitter/src/lib/lib.ts
Original file line number Diff line number Diff line change
@@ -63,7 +63,7 @@ const diags: { [code: string]: DiagnosticDefinition<DiagnosticMessages> } = {
"client-namespace-conflict": {
severity: "warning",
messages: {
default: paramMessage`namespace ${"clientNamespace"} conflicts with client ${"clientName"}, please use @clientName to specify a different name for the client.`,
default: paramMessage`${"message"}`,
},
},
"unsupported-endpoint-url": {
Original file line number Diff line number Diff line change
@@ -125,7 +125,7 @@ function updateSdkTypeReferences(
if ("crossLanguageDefinitionId" in sdkType) {
sdkContext.__typeCache.crossLanguageDefinitionIds.set(
sdkType.crossLanguageDefinitionId,
sdkType,
sdkType.__raw,
);
}
}
6 changes: 1 addition & 5 deletions packages/http-client-csharp/emitter/src/lib/utils.ts
Original file line number Diff line number Diff line change
@@ -90,11 +90,7 @@ function processJsonRpc(context: CSharpEmitterContext, message: string) {
if (crossLanguageDefinitionId === undefined) {
return undefined;
}
const target = context.__typeCache.crossLanguageDefinitionIds.get(crossLanguageDefinitionId);
if (target) {
return target.__raw;
}
return undefined;
return context.__typeCache.crossLanguageDefinitionIds.get(crossLanguageDefinitionId);
}
}

3 changes: 2 additions & 1 deletion packages/http-client-csharp/emitter/src/sdk-context.ts
Original file line number Diff line number Diff line change
@@ -2,6 +2,7 @@
// Licensed under the MIT License. See License.txt in the project root for license information.

import { SdkContext, SdkType } from "@azure-tools/typespec-client-generator-core";
import { Type } from "@typespec/compiler";
import { Logger } from "./lib/logger.js";
import { CSharpEmitterOptions } from "./options.js";
import { InputEnumType, InputModelType, InputType } from "./type/input-type.js";
@@ -16,7 +17,7 @@ export interface CSharpEmitterContext extends SdkContext<CSharpEmitterOptions> {
}

export interface SdkTypeMap {
crossLanguageDefinitionIds: Map<string, SdkType>;
crossLanguageDefinitionIds: Map<string, Type | undefined>;
types: Map<SdkType, InputType>;
models: Map<string, InputModelType>;
enums: Map<string, InputEnumType>;
Original file line number Diff line number Diff line change
@@ -16,4 +16,5 @@ export interface InputClient {
Parent?: string;
Parameters?: InputParameter[];
Decorators?: DecoratorInfo[];
CrossLanguageDefinitionId: string;
}
Original file line number Diff line number Diff line change
@@ -7,6 +7,7 @@
using System.Linq;
using System.Threading;
using Microsoft.TypeSpec.Generator.ClientModel.Primitives;
using Microsoft.TypeSpec.Generator.EmitterRpc;
using Microsoft.TypeSpec.Generator.Expressions;
using Microsoft.TypeSpec.Generator.Input;
using Microsoft.TypeSpec.Generator.Primitives;
@@ -147,9 +148,29 @@ public ClientProvider(InputClient inputClient)
_subClients = new(GetSubClients);
}

protected override string BuildNamespace() => string.IsNullOrEmpty(_inputClient.Namespace) ?
base.BuildNamespace() :
ScmCodeModelPlugin.Instance.TypeFactory.GetCleanNameSpace(_inputClient.Namespace);
private const string namespaceConflictCode = "client-namespace-conflict";

private string? _namespace;
// This `BuildNamespace` method has been called twice - one when building the `Type`, the other is trying to find the CustomCodeView, both of them are required.
// therefore here to avoid this being called twice because this method now reports a diagnostic, we cache the result.
protected override string BuildNamespace() => _namespace ??= BuildNamespaceCore();

private string BuildNamespaceCore()
{
// if namespace is empty, we fallback to the root namespace
if (string.IsNullOrEmpty(_inputClient.Namespace))
{
return base.BuildNamespace();
}
var ns = ScmCodeModelPlugin.Instance.TypeFactory.GetCleanNameSpace(_inputClient.Namespace);

// figure out if this namespace has been changed for this client
if (!StringExtensions.IsLastNamespaceSegmentTheSame(ns, _inputClient.Namespace))
{
ScmCodeModelPlugin.Instance.Emitter.ReportDiagnostic(namespaceConflictCode, $"namespace {_inputClient.Namespace} conflicts with client {_inputClient.Name}, please use `@clientName` to specify a different name for the client.", _inputClient.CrossLanguageDefinitionId);
}
return ns;
}

private IReadOnlyList<ParameterProvider> GetSubClientInternalConstructorParameters()
{
Original file line number Diff line number Diff line change
@@ -11,21 +11,23 @@ public class InputClient
private readonly string? _key;
private IReadOnlyDictionary<string, InputClientExample>? _examples;

public InputClient(string name, string @namespace, string? summary, string? doc, IReadOnlyList<InputOperation> operations, IReadOnlyList<InputParameter> parameters, string? parent)
public InputClient(string name, string @namespace, string crossLanguageDefinitionId, string? summary, string? doc, IReadOnlyList<InputOperation> operations, IReadOnlyList<InputParameter> parameters, string? parent)
{
Name = name;
Namespace = @namespace;
CrossLanguageDefinitionId = crossLanguageDefinitionId;
Summary = summary;
Doc = doc;
Operations = operations;
Parameters = parameters;
Parent = parent;
}

public InputClient() : this(string.Empty, string.Empty, string.Empty, string.Empty, Array.Empty<InputOperation>(), Array.Empty<InputParameter>(), null) { }
public InputClient() : this(string.Empty, string.Empty, string.Empty, string.Empty, string.Empty, Array.Empty<InputOperation>(), Array.Empty<InputParameter>(), null) { }

public string Name { get; internal set; }
public string Namespace { get; internal set; }
public string CrossLanguageDefinitionId { get; internal set; }
public string? Summary { get; internal set; }
public string? Doc { get; internal set; }
public IReadOnlyList<InputOperation> Operations { get; internal set; }
Original file line number Diff line number Diff line change
@@ -23,7 +23,7 @@ public TypeSpecInputClientConverter(TypeSpecReferenceHandler referenceHandler)
public override void Write(Utf8JsonWriter writer, InputClient value, JsonSerializerOptions options)
=> throw new NotSupportedException("Writing not supported");

private static InputClient? CreateInputClient(ref Utf8JsonReader reader, string? id, JsonSerializerOptions options, ReferenceResolver resolver)
private InputClient? CreateInputClient(ref Utf8JsonReader reader, string? id, JsonSerializerOptions options, ReferenceResolver resolver)
{
if (id == null)
{
@@ -42,6 +42,7 @@ public override void Write(Utf8JsonWriter writer, InputClient value, JsonSeriali
IReadOnlyList<InputParameter>? parameters = null;
IReadOnlyList<InputDecoratorInfo>? decorators = null;
string? parent = null;
string? crossLanguageDefinitionId = null;

while (reader.TokenType != JsonTokenType.EndObject)
{
@@ -52,7 +53,8 @@ public override void Write(Utf8JsonWriter writer, InputClient value, JsonSeriali
|| reader.TryReadWithConverter(nameof(InputClient.Operations), options, ref operations)
|| reader.TryReadWithConverter(nameof(InputClient.Parameters), options, ref parameters)
|| reader.TryReadString(nameof(InputClient.Parent), ref parent)
|| reader.TryReadWithConverter(nameof(InputClient.Decorators), options, ref decorators);
|| reader.TryReadWithConverter(nameof(InputClient.Decorators), options, ref decorators)
|| reader.TryReadString("CrossLanguageDefinitionId", ref crossLanguageDefinitionId);

if (!isKnownProperty)
{
@@ -62,18 +64,18 @@ public override void Write(Utf8JsonWriter writer, InputClient value, JsonSeriali

client.Name = name ?? throw new JsonException("InputClient must have name");
client.Namespace = @namespace ?? string.Empty;
client.CrossLanguageDefinitionId = crossLanguageDefinitionId ?? string.Empty;
client.Summary = summary;
client.Doc = doc;
client.Operations = operations ?? Array.Empty<InputOperation>();
client.Parameters = parameters ?? Array.Empty<InputParameter>();
client.Operations = operations ?? [];
client.Parameters = parameters ?? [];
client.Parent = parent;
client.Decorators = decorators ?? [];

if (GetLastSegment(client.Namespace) == client.Name)
var lastSegment = GetLastSegment(client.Namespace);
if (lastSegment == client.Name)
{
// invalid namespace segment found
// check if the list is already there
// get the list out
// invalid namespace segment found, add it into the list
var invalidNamespaceSegments = (List<string>)resolver.ResolveReference(TypeSpecSerialization.InvalidNamespaceSegmentsKey);
invalidNamespaceSegments.Add(client.Name);
}
Original file line number Diff line number Diff line change
@@ -6,9 +6,6 @@
using System.IO;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.TypeSpec.Generator.EmitterRpc;
using Microsoft.CodeAnalysis;
using Microsoft.TypeSpec.Generator.Providers;
using Microsoft.TypeSpec.Generator.SourceInput;

namespace Microsoft.TypeSpec.Generator
@@ -88,7 +85,7 @@ public async Task ExecuteAsync()
continue;
}
var filename = Path.Combine(outputPath, file.Name);
Emitter.Instance.Info($"Writing {Path.GetFullPath(filename)}");
CodeModelPlugin.Instance.Emitter.Info($"Writing {Path.GetFullPath(filename)}");
Directory.CreateDirectory(Path.GetDirectoryName(filename)!);
await File.WriteAllTextAsync(filename, file.Text);
}
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@
using System.Collections.Generic;
using System.ComponentModel.Composition;
using Microsoft.CodeAnalysis;
using Microsoft.TypeSpec.Generator.EmitterRpc;
using Microsoft.TypeSpec.Generator.Input;
using Microsoft.TypeSpec.Generator.Primitives;
using Microsoft.TypeSpec.Generator.Providers;
@@ -46,6 +47,7 @@ public CodeModelPlugin(GeneratorContext context)
Configuration = context.Configuration;
_inputLibrary = new InputLibrary(Configuration.OutputDirectory);
TypeFactory = new TypeFactory();
Emitter = new Emitter(Console.OpenStandardOutput());
}

// for mocking
@@ -58,6 +60,8 @@ protected CodeModelPlugin()
internal bool IsNewProject { get; set; }
private InputLibrary _inputLibrary;

public virtual Emitter Emitter { get; }

// Extensibility points to be implemented by a plugin
public virtual TypeFactory TypeFactory { get; }

Original file line number Diff line number Diff line change
@@ -13,18 +13,15 @@ public sealed class Emitter : IDisposable
private const string Trace = "trace";
private const string Diagnostic = "diagnostic";

private static Emitter? _emitter;
private bool _disposed;

private readonly StreamWriter _writer;

private Emitter()
internal Emitter(Stream stream)
{
_writer = new StreamWriter(Console.OpenStandardOutput()) { AutoFlush = true };
_writer = new StreamWriter(stream) { AutoFlush = true };
}

public static Emitter Instance => _emitter ??= new Emitter();

private void SendNotification(string method, object content)
{
var paramsContent = JsonSerializer.Serialize(content);
Original file line number Diff line number Diff line change
@@ -110,7 +110,7 @@ public string? Deprecated
public CSharpType Type => _type ??=
new(
_name ??= CustomCodeView?.Name ?? BuildName(),
CustomCodeView?.BuildNamespace() ?? BuildNamespace(),
CustomCodeView?.Type.Namespace ?? BuildNamespace(),
this is EnumProvider ||
DeclarationModifiers.HasFlag(TypeSignatureModifiers.Struct) ||
DeclarationModifiers.HasFlag(TypeSignatureModifiers.Enum),
Original file line number Diff line number Diff line change
@@ -244,5 +244,50 @@ public static string ToApiVersionMemberName(this string version)

return CultureInfo.InvariantCulture.TextInfo.ToTitleCase(sb.ToString());
}

/// <summary>
/// Checks if two namespaces share the same last segment
/// </summary>
/// <param name="left">the first namespace</param>
/// <param name="right">the second namespace</param>
/// <returns></returns>
public static bool IsLastNamespaceSegmentTheSame(string left, string right)
{
// finish this via Span API
var leftSpan = left.AsSpan();
var rightSpan = right.AsSpan();
// swap if left is longer, we ensure left is the shorter one
if (leftSpan.Length > rightSpan.Length)
{
var temp = leftSpan;
leftSpan = rightSpan;
rightSpan = temp;
}
for (int i = 1; i <= leftSpan.Length; i++)
{
var lc = leftSpan[^i];
var rc = rightSpan[^i];
// check if each char is the same from the right-most side
// if both of them are dot, we finished scanning the last segment - and if we could be here, meaning all of them are the same, return true.
if (lc == '.' && rc == '.')
{
return true;
}
// if these are different - there is one different character, return false.
if (lc != rc)
{
return false;
}
}

// we come here because we run out of characters in left - which means left does not have a dot.
// if they have the same length, they are identical, return true
if (leftSpan.Length == rightSpan.Length)
{
return true;
}
// otherwise, right is longer, we check its next character, if it is the dot, return true, otherwise return false.
return rightSpan[^(leftSpan.Length + 1)] == '.';
}
}
}
Original file line number Diff line number Diff line change
@@ -2,6 +2,7 @@
// Licensed under the MIT License.

using System.Threading.Tasks;
using Microsoft.TypeSpec.Generator.EmitterRpc;

namespace Microsoft.TypeSpec.Generator
{
Original file line number Diff line number Diff line change
@@ -31,11 +31,9 @@ public static async Task<int> Main(string[] args)

private static async Task<int> Run(CommandLineOptions options, GeneratorRunner runner)
{
using var emitter = Emitter.Instance;

if (options.ShouldDebug)
{
emitter.Debug("Attempting to attach debugger..");
Console.Error.WriteLine("Attempting to attach debugger..");
Debugger.Launch();
}

@@ -50,6 +48,8 @@ private static async Task<int> Run(CommandLineOptions options, GeneratorRunner r
return 1;
}

(CodeModelPlugin.Instance.Emitter as IDisposable)?.Dispose();

return 0;
}
}
Loading

0 comments on commit 4abea21

Please sign in to comment.