Skip to content
Merged
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@cadl-lang/compiler",
"comment": "Add option to Checker.getTypeName to filter namespaces",
"type": "minor"
}
],
"packageName": "@cadl-lang/compiler"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@cadl-lang/openapi",
"comment": "Add shared helpers for OpenAPI 2 and 3 emit",
"type": "minor"
}
],
"packageName": "@cadl-lang/openapi"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@cadl-lang/openapi3",
"comment": "URI-encode refs",
"type": "patch"
}
],
"packageName": "@cadl-lang/openapi3"
}
46 changes: 29 additions & 17 deletions packages/compiler/core/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ import {
} from "./types.js";
import { isArray } from "./util.js";

export interface TypeNameOptions {
namespaceFilter: (ns: NamespaceType) => boolean;
}

export interface Checker {
getTypeForNode(node: Node): Type;
setUsingsForFile(file: CadlScriptNode): void;
Expand All @@ -107,8 +111,8 @@ export interface Checker {
getLiteralType(node: NumericLiteralNode): NumericLiteralType;
getLiteralType(node: BooleanLiteralNode): BooleanLiteralType;
getLiteralType(node: LiteralNode): LiteralType;
getTypeName(type: Type): string;
getNamespaceString(type: NamespaceType | undefined): string;
getTypeName(type: Type, options?: TypeNameOptions): string;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really in the scope of this PR but couldn't getTypeName be moved out of the checker. Doesn't feel like they depend on it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, I can look at doing that as part of some other work I'm doing nearby next.

getNamespaceString(type: NamespaceType | undefined, options?: TypeNameOptions): string;
cloneType<T extends Type>(type: T): T;
evalProjection(node: ProjectionNode, target: Type, args: Type[]): Type;
project(
Expand Down Expand Up @@ -407,18 +411,18 @@ export function createChecker(program: Program): Checker {
return errorType;
}

function getTypeName(type: Type): string {
function getTypeName(type: Type, options?: TypeNameOptions): string {
switch (type.kind) {
case "Model":
return getModelName(type);
return getModelName(type, options);
case "Enum":
return getEnumName(type);
return getEnumName(type, options);
case "Union":
return type.name || type.options.map(getTypeName).join(" | ");
return type.name || type.options.map((x) => getTypeName(x, options)).join(" | ");
case "UnionVariant":
return getTypeName(type.type);
return getTypeName(type.type, options);
case "Array":
return getTypeName(type.elementType) + "[]";
return getTypeName(type.elementType, options) + "[]";
case "String":
case "Number":
case "Boolean":
Expand All @@ -428,10 +432,18 @@ export function createChecker(program: Program): Checker {
return "(unnamed type)";
}

function getNamespaceString(type: NamespaceType | undefined): string {
if (!type) return "";
const parent = type.namespace;
return parent && parent.name !== "" ? `${getNamespaceString(parent)}.${type.name}` : type.name;
function getNamespaceString(type: NamespaceType | undefined, options?: TypeNameOptions): string {
if (!type || !type.name) {
return "";
}

const filter = options?.namespaceFilter;
if (filter && !filter(type)) {
return "";
}

const parent = getNamespaceString(type.namespace, options);
return parent ? `${parent}.${type.name}` : type.name;
}

function getFullyQualifiedSymbolName(sym: Sym | undefined): string {
Expand All @@ -442,8 +454,8 @@ export function createChecker(program: Program): Checker {
: sym.name;
}

function getEnumName(e: EnumType): string {
const nsName = getNamespaceString(e.namespace);
function getEnumName(e: EnumType, options: TypeNameOptions | undefined): string {
const nsName = getNamespaceString(e.namespace, options);
return nsName ? `${nsName}.${e.name}` : e.name;
}

Expand All @@ -456,12 +468,12 @@ export function createChecker(program: Program): Checker {
return node.symbol!.id!;
}

function getModelName(model: ModelType) {
const nsName = getNamespaceString(model.namespace);
function getModelName(model: ModelType, options: TypeNameOptions | undefined) {
const nsName = getNamespaceString(model.namespace, options);
const modelName = (nsName ? nsName + "." : "") + (model.name || "(anonymous model)");
if (model.templateArguments && model.templateArguments.length > 0) {
// template instantiation
const args = model.templateArguments.map(getTypeName);
const args = model.templateArguments.map((x) => getTypeName(x, options));
return `${modelName}<${args.join(", ")}>`;
} else if ((model.node as ModelStatementNode).templateParameters?.length > 0) {
// template
Expand Down
1 change: 1 addition & 0 deletions packages/compiler/core/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ export * as decorators from "../lib/decorators.js";
export * from "../lib/service.js";
export * as service from "../lib/service.js";
export * from "../server/serverlib.js";
export * from "./checker.js";
export * from "./decorator-utils.js";
export * from "./diagnostics.js";
export * from "./library.js";
Expand Down
146 changes: 146 additions & 0 deletions packages/openapi/src/helpers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
import {
getFriendlyName as getAssignedFriendlyName,
ModelType,
ModelTypeProperty,
Program,
Type,
TypeNameOptions,
} from "@cadl-lang/compiler";
import { reportDiagnostic } from "./lib.js";

/**
* Determines whether a type will be inlined in OpenAPI rather than defined
* as a schema and referenced.
*
* All anonymous types (anonymous models, arrays, tuples, etc.) are inlined.
*
* Template instantiations are inlined unless they have a friendly name.
*
* A friendly name can be provided by the user using `@friendlyName`
* decorator, or chosen by default in simple cases.
*/
export function shouldInline(program: Program, type: Type): boolean {
if (hasFriendlyName(program, type)) {
return false;
}

switch (type.kind) {
case "Model":
return !type.name || hasTemplateArguments(type);
case "Enum":
case "Union":
return !type.name;
default:
return true;
}
}

/**
* Gets the name of a type to be used in OpenAPI.
*
* For inlined types: this is the Cadl-native name written to `x-cadl-name`.
*
* For non-inlined types: this is either the friendly name or the Cadl-native name.
*
* Cadl-native names are shortened to exclude root `Cadl` namespace and service
* namespace using the provided `TypeNameOptions`.
*/
export function getTypeName(
program: Program,
type: Type,
options: TypeNameOptions,
existing?: Record<string, any>
): string {
const name =
getFriendlyName(program, type, options) ?? program.checker!.getTypeName(type, options);

if (existing && existing[name]) {
reportDiagnostic(program, {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we report both instances conflicting. That's what we have for duplicate-symbol or duplicate routes.

Copy link
Contributor Author

@nguerrera nguerrera Apr 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should. It will take some more refactoring to do the bookkeeping. If you don't mind, I'd rather add this to the list to review on #464 because it will be easier to review more refactoring separately, I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We weren't handling the collisions at all before so this is a strict improvement already.

code: "duplicate-type-name",
format: {
value: name,
},
target: type,
});
}

return name;
}

/**
* Gets the key that is used to define a parameter in OpenAPI.
*/
export function getParameterKey(
program: Program,
propery: ModelTypeProperty,
newParam: unknown,
existingParams: Record<string, unknown>,
options: TypeNameOptions
): string {
const parent = propery.model!;
let key = getTypeName(program, parent, options);

if (parent.properties.size > 1) {
key += `.${propery.name}`;
}

// JSON check is workaround for https://github.com/microsoft/cadl/issues/462
if (existingParams[key] && JSON.stringify(newParam) !== JSON.stringify(existingParams[key])) {
reportDiagnostic(program, {
code: "duplicate-type-name",
messageId: "parameter",
format: {
value: key,
},
target: propery,
});
}

return key;
}

function hasTemplateArguments(type: Type): type is ModelType & { templateArguments: Type[] } {
return type.kind === "Model" && !!type.templateArguments && type.templateArguments.length > 0;
}

function hasFriendlyName(program: Program, type: Type): boolean {
return !!getAssignedFriendlyName(program, type) || hasDefaultFriendlyName(program, type);
}

function getFriendlyName(program: Program, type: Type, options: TypeNameOptions): string {
return getAssignedFriendlyName(program, type) ?? getDefaultFriendlyName(program, type, options);
}

/**
* A template instantiation has a default friendly name if none if its type
* arguments are nested template instantiations or inlined types.
*/
function hasDefaultFriendlyName(
program: Program,
type: Type
): type is ModelType & { name: string; templateArguments: Type[] } {
return (
type.kind === "Model" &&
!!type.name &&
hasTemplateArguments(type) &&
!type.templateArguments.some((arg) => hasTemplateArguments(arg) || shouldInline(program, arg))
);
}

/**
* Gets the default friendly name of the form Type_Arg1_..._ArgN when applicable as described
* by `hasDefaultFriendlyName`. Returns undefined when not applicable.
*/
function getDefaultFriendlyName(
program: Program,
type: Type,
options: TypeNameOptions
): string | undefined {
if (!hasDefaultFriendlyName(program, type)) {
return undefined;
}
const ns = program.checker!.getNamespaceString(type.namespace, options);
const model = (ns ? ns + "." : "") + type.name;
const args = type.templateArguments.map((arg) => getTypeName(program, arg, options));
return `${model}_${args.join("_")}`;
}
1 change: 1 addition & 0 deletions packages/openapi/src/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export * from "./decorators.js";
export * from "./helpers.js";
7 changes: 7 additions & 0 deletions packages/openapi/src/lib.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ export const libDef = {
default: paramMessage`OpenAPI extension must start with 'x-' but was '${"value"}'`,
},
},
"duplicate-type-name": {
severity: "error",
messages: {
default: paramMessage`Duplicate type name: '${"value"}'. Check @friendlyName decorators and overlap with types in Cadl or service namespace.`,
parameter: paramMessage`Duplicate parameter key: '${"value"}'. Check @friendlyName decorators and overlap with types in Cadl or service namespace.`,
},
},
},
} as const;
export const { reportDiagnostic } = createCadlLibrary(libDef);
Loading