Skip to content

Commit

Permalink
Report diagnostics on @clientName conflicts (#1119)
Browse files Browse the repository at this point in the history
Resolves #1096

---------

Co-authored-by: Timothee Guerin <[email protected]>
  • Loading branch information
live1206 and timotheeguerin committed Jul 17, 2024
1 parent a7118e1 commit d635570
Show file tree
Hide file tree
Showing 7 changed files with 547 additions and 16 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
changeKind: feature
packages:
- "@azure-tools/typespec-client-generator-core"
---

Report diagnostics on `@clientName` conflicts
28 changes: 20 additions & 8 deletions packages/typespec-client-generator-core/src/decorators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,19 +41,29 @@ import {
SdkServiceOperation,
UsageFlags,
} from "./interfaces.js";
import { TCGCContext, parseEmitterName } from "./internal-utils.js";
import { AllScopes, TCGCContext, clientNameKey, parseEmitterName } from "./internal-utils.js";
import { createStateSymbol, reportDiagnostic } from "./lib.js";
import { getSdkPackage } from "./package.js";
import { getLibraryName } from "./public-utils.js";
import { getSdkEnum, getSdkModel, getSdkUnion } from "./types.js";

export const namespace = "Azure.ClientGenerator.Core";
const AllScopes = Symbol.for("@azure-core/typespec-client-generator-core/all-scopes");

function getScopedDecoratorData(context: TCGCContext, key: symbol, target: Type): any {
function getScopedDecoratorData(
context: TCGCContext,
key: symbol,
target: Type,
languageScope?: string | typeof AllScopes
): any {
const retval: Record<string | symbol, any> = context.program.stateMap(key).get(target);
if (retval === undefined) return retval;
if (Object.keys(retval).includes(context.emitterName)) return retval[context.emitterName];
if (languageScope === AllScopes) {
return retval[languageScope];
}
if (languageScope === undefined || typeof languageScope === "string") {
const scope = languageScope ?? context.emitterName;
if (Object.keys(retval).includes(scope)) return retval[scope];
}
return retval[AllScopes]; // in this case it applies to all languages
}

Expand Down Expand Up @@ -957,8 +967,6 @@ export function shouldFlattenProperty(context: TCGCContext, target: ModelPropert
return getScopedDecoratorData(context, flattenPropertyKey, target) ?? false;
}

const clientNameKey = createStateSymbol("clientName");

export function $clientName(
context: DecoratorContext,
entity: Type,
Expand Down Expand Up @@ -995,6 +1003,10 @@ export function $clientName(
setScopedDecoratorData(context, $clientName, clientNameKey, entity, value, scope);
}

export function getClientNameOverride(context: TCGCContext, entity: Type): string | undefined {
return getScopedDecoratorData(context, clientNameKey, entity);
export function getClientNameOverride(
context: TCGCContext,
entity: Type,
languageScope?: string | typeof AllScopes
): string | undefined {
return getScopedDecoratorData(context, clientNameKey, entity, languageScope);
}
1 change: 1 addition & 0 deletions packages/typespec-client-generator-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ export * from "./interfaces.js";
export * from "./lib.js";
export * from "./public-utils.js";
export * from "./types.js";
export { $onValidate } from "./validate.js";
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import {
SdkType,
SdkUnionType,
} from "./interfaces.js";
import { createDiagnostic } from "./lib.js";
import { createDiagnostic, createStateSymbol } from "./lib.js";
import {
getCrossLanguageDefinitionId,
getDefaultApiVersion,
Expand All @@ -47,6 +47,10 @@ import {
isApiVersion,
} from "./public-utils.js";

export const AllScopes = Symbol.for("@azure-core/typespec-client-generator-core/all-scopes");

export const clientNameKey = createStateSymbol("clientName");

/**
*
* @param emitterName Full emitter name
Expand Down
7 changes: 7 additions & 0 deletions packages/typespec-client-generator-core/src/lib.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,13 @@ export const $lib = createTypeSpecLibrary({
default: `Cannot pass an empty value to the @clientName decorator`,
},
},
"duplicate-client-name": {
severity: "error",
messages: {
default: paramMessage`Client name: "${"name"}" is duplicated in language scope: "${"scope"}"`,
nonDecorator: paramMessage`Client name: "${"name"}" is defined somewhere causing nameing conflicts in language scope: "${"scope"}"`,
},
},
},
});

Expand Down
160 changes: 153 additions & 7 deletions packages/typespec-client-generator-core/src/validate.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,158 @@
import { Program } from "@typespec/compiler";
import { createTCGCContext } from "./internal-utils.js";
import { getAllModelsWithDiagnostics } from "./types.js";
import {
AugmentDecoratorStatementNode,
DecoratorExpressionNode,
Enum,
EnumMember,
Interface,
Model,
ModelProperty,
Namespace,
Operation,
Program,
Scalar,
SyntaxKind,
Type,
Union,
UnionVariant,
} from "@typespec/compiler";
import { DuplicateTracker } from "@typespec/compiler/utils";
import { getClientNameOverride } from "./decorators.js";
import { AllScopes, clientNameKey, createTCGCContext, TCGCContext } from "./internal-utils.js";
import { reportDiagnostic } from "./lib.js";

export function $onValidate(program: Program) {
// Pass along any diagnostics that might be returned from the HTTP library
const tcgcContext = createTCGCContext(program);
const [_, diagnostics] = getAllModelsWithDiagnostics(tcgcContext);
if (diagnostics.length > 0) {
program.reportDiagnostics(diagnostics);
const languageScopes = getDefinedLanguageScopes(program);
for (const scope of languageScopes) {
validateClientNamesPerNamespace(tcgcContext, scope, program.getGlobalNamespaceType());
}
}

function getDefinedLanguageScopes(program: Program): Set<string | typeof AllScopes> {
const languageScopes = new Set<string | typeof AllScopes>();
for (const value of program.stateMap(clientNameKey).values()) {
if (value[AllScopes]) {
languageScopes.add(AllScopes);
}
for (const languageScope of Object.keys(value)) {
languageScopes.add(languageScope);
}
}
return languageScopes;
}

function validateClientNamesPerNamespace(
tcgcContext: TCGCContext,
scope: string | typeof AllScopes,
namespace: Namespace
) {
// Check for duplicate client names for models, enums, and unions
validateClientNamesCore(tcgcContext, scope, [
...namespace.models.values(),
...namespace.enums.values(),
...namespace.unions.values(),
]);

// Check for duplicate client names for operations
validateClientNamesCore(tcgcContext, scope, namespace.operations.values());

// Check for duplicate client names for interfaces
validateClientNamesCore(tcgcContext, scope, namespace.interfaces.values());

// Check for duplicate client names for scalars
validateClientNamesCore(tcgcContext, scope, namespace.scalars.values());

// Check for duplicate client names for namespaces
validateClientNamesCore(tcgcContext, scope, namespace.namespaces.values());

// Check for duplicate client names for model properties
for (const model of namespace.models.values()) {
validateClientNamesCore(tcgcContext, scope, model.properties.values());
}

// Check for duplicate client names for enum members
for (const item of namespace.enums.values()) {
validateClientNamesCore(tcgcContext, scope, item.members.values());
}

// Check for duplicate client names for union variants
for (const item of namespace.unions.values()) {
validateClientNamesCore(tcgcContext, scope, item.variants.values());
}

// Check for duplicate client names for nested namespaces
for (const item of namespace.namespaces.values()) {
validateClientNamesPerNamespace(tcgcContext, scope, item);
}
}

function validateClientNamesCore(
tcgcContext: TCGCContext,
scope: string | typeof AllScopes,
items: Iterable<
| Namespace
| Scalar
| Operation
| Interface
| Model
| Enum
| Union
| ModelProperty
| EnumMember
| UnionVariant
>
) {
const duplicateTracker = new DuplicateTracker<
string,
Type | DecoratorExpressionNode | AugmentDecoratorStatementNode
>();

for (const item of items) {
const clientName = getClientNameOverride(tcgcContext, item, scope);
if (clientName !== undefined) {
const clientNameDecorator = item.decorators.find((x) => x.definition?.name === "@clientName");
if (clientNameDecorator?.node !== undefined) {
duplicateTracker.track(clientName, clientNameDecorator.node);
}
} else {
if (item.name !== undefined && typeof item.name === "string") {
duplicateTracker.track(item.name, item);
}
}
}

reportDuplicateClientNames(tcgcContext.program, duplicateTracker, scope);
}

function reportDuplicateClientNames(
program: Program,
duplicateTracker: DuplicateTracker<
string,
Type | DecoratorExpressionNode | AugmentDecoratorStatementNode
>,
scope: string | typeof AllScopes
) {
for (const [name, duplicates] of duplicateTracker.entries()) {
for (const item of duplicates) {
const scopeStr = scope === AllScopes ? "AllScopes" : scope;
// If the item is a decorator application node
if (
item.kind === SyntaxKind.DecoratorExpression ||
item.kind === SyntaxKind.AugmentDecoratorStatement
) {
reportDiagnostic(program, {
code: "duplicate-client-name",
format: { name, scope: scopeStr },
target: item,
});
} else {
reportDiagnostic(program, {
code: "duplicate-client-name",
messageId: "nonDecorator",
format: { name, scope: scopeStr },
target: item,
});
}
}
}
}
Loading

0 comments on commit d635570

Please sign in to comment.