Skip to content

Fix the reserved word issue in Modular for API layer #3191

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.

Sign up for GitHub

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

Merged
merged 14 commits into from
May 21, 2025
Merged

Fix the reserved word issue in Modular for API layer #3191

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
b77ebb4
add test
v-jiaodi May 12, 2025
72b85ef
update
v-jiaodi May 12, 2025
a7bf0ff
fix
v-jiaodi May 13, 2025
635efaa
Merge branch 'main' into test512
MaryGao May 19, 2025
8303f94
Add the UTs for the reserved word
MaryGao May 19, 2025
3a38e67
Format codes
MaryGao May 19, 2025
285624b
Fix the issue in sample gen
MaryGao May 19, 2025
c710415
Fix the UTs
MaryGao May 19, 2025
0c41bcc
remove help test
v-jiaodi May 20, 2025
8df4fdb
add test case for reserved word operation
v-jiaodi May 20, 2025
81e4676
fix ci
v-jiaodi May 20, 2025
df899b7
fix ci
v-jiaodi May 20, 2025
51bc20d
update reserved words list
v-jiaodi May 21, 2025
2e7ce44
Merge branch 'main' into test512
MaryGao May 21, 2025
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
87 changes: 50 additions & 37 deletions packages/rlc-common/src/helpers/nameUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,69 +32,81 @@ export const ReservedModelNames: ReservedName[] = [
{ name: "as", reservedFor: [NameType.Parameter] },
{ name: "assert", reservedFor: [NameType.Parameter] },
{ name: "async", reservedFor: [NameType.Parameter] },
{ name: "await", reservedFor: [NameType.Parameter] },
{ name: "await", reservedFor: [NameType.Parameter, NameType.Method] },
{ name: "boolean", reservedFor: [NameType.Parameter, ...Newable] },
{ name: "break", reservedFor: [NameType.Parameter] },
{ name: "case", reservedFor: [NameType.Parameter] },
{ name: "catch", reservedFor: [NameType.Parameter] },
{ name: "class", reservedFor: [NameType.Parameter] },
{ name: "break", reservedFor: [NameType.Parameter, NameType.Method] },
{ name: "case", reservedFor: [NameType.Parameter, NameType.Method] },
{ name: "catch", reservedFor: [NameType.Parameter, NameType.Method] },
{ name: "class", reservedFor: [NameType.Parameter, NameType.Method] },
{ name: "const", reservedFor: [NameType.Parameter] },
{ name: "constructor", reservedFor: [NameType.Parameter] },
{ name: "continue", reservedFor: [NameType.Parameter] },
{ name: "continue", reservedFor: [NameType.Parameter, NameType.Method] },
{ name: "date", reservedFor: [NameType.Parameter, ...Newable] },
{ name: "debugger", reservedFor: [NameType.Parameter] },
{ name: "debugger", reservedFor: [NameType.Parameter, NameType.Method] },
{ name: "declare", reservedFor: [NameType.Parameter] },
{ name: "default", reservedFor: [NameType.Parameter] },
{ name: "delete", reservedFor: [NameType.Parameter, NameType.Operation] },
{ name: "do", reservedFor: [NameType.Parameter] },
{ name: "default", reservedFor: [NameType.Parameter, NameType.Method] },
{
name: "delete",
reservedFor: [NameType.Parameter, NameType.Operation, NameType.Method]
},
{ name: "do", reservedFor: [NameType.Parameter, NameType.Method] },
{ name: "else", reservedFor: [NameType.Parameter] },
{ name: "enum", reservedFor: [NameType.Parameter] },
{ name: "error", reservedFor: [NameType.Parameter, ...Newable] },
{ name: "export", reservedFor: [NameType.Parameter, NameType.Operation] },
{
name: "export",
reservedFor: [NameType.Parameter, NameType.Operation, NameType.Method]
},
{ name: "extends", reservedFor: [NameType.Parameter] },
{ name: "false", reservedFor: [NameType.Parameter] },
{ name: "finally", reservedFor: [NameType.Parameter] },
{ name: "for", reservedFor: [NameType.Parameter] },
{ name: "finally", reservedFor: [NameType.Parameter, NameType.Method] },
{ name: "for", reservedFor: [NameType.Parameter, NameType.Method] },
{ name: "from", reservedFor: [NameType.Parameter] },
{ name: "function", reservedFor: [NameType.Parameter, ...Newable] },
{
name: "function",
reservedFor: [NameType.Parameter, ...Newable, NameType.Method]
},
{ name: "get", reservedFor: [NameType.Parameter] },
{ name: "if", reservedFor: [NameType.Parameter] },
{ name: "implements", reservedFor: [NameType.Parameter] },
{ name: "import", reservedFor: [NameType.Parameter] },
{ name: "in", reservedFor: [NameType.Parameter] },
{ name: "instanceof", reservedFor: [NameType.Parameter] },
{ name: "in", reservedFor: [NameType.Parameter, NameType.Method] },
{ name: "instanceof", reservedFor: [NameType.Parameter, NameType.Method] },
{ name: "interface", reservedFor: [NameType.Parameter] },
{ name: "let", reservedFor: [NameType.Parameter] },
{ name: "let", reservedFor: [NameType.Parameter, NameType.Method] },
{ name: "module", reservedFor: [NameType.Parameter] },
{ name: "new", reservedFor: [NameType.Parameter] },
{ name: "null", reservedFor: [NameType.Parameter] },
{ name: "new", reservedFor: [NameType.Parameter, NameType.Method] },
{ name: "null", reservedFor: [NameType.Parameter, NameType.Method] },
{ name: "number", reservedFor: [NameType.Parameter, ...Newable] },
{ name: "of", reservedFor: [NameType.Parameter] },
{ name: "package", reservedFor: [NameType.Parameter] },
{ name: "private", reservedFor: [NameType.Parameter] },
{ name: "protected", reservedFor: [NameType.Parameter] },
{ name: "public", reservedFor: [NameType.Parameter, NameType.Operation] },
{
name: "public",
reservedFor: [NameType.Parameter, NameType.Operation, NameType.Method]
},
{ name: "requestoptions", reservedFor: [NameType.Parameter] },
{ name: "require", reservedFor: [NameType.Parameter] },
{ name: "require", reservedFor: [NameType.Parameter, NameType.Method] },
{ name: "return", reservedFor: [NameType.Parameter] },
{ name: "set", reservedFor: [NameType.Parameter, ...Newable] },
{ name: "static", reservedFor: [NameType.Parameter] },
{ name: "static", reservedFor: [NameType.Parameter, NameType.Method] },
{ name: "string", reservedFor: [NameType.Parameter, ...Newable] },
{ name: "super", reservedFor: [NameType.Parameter] },
{ name: "switch", reservedFor: [NameType.Parameter] },
{ name: "switch", reservedFor: [NameType.Parameter, NameType.Method] },
{ name: "symbol", reservedFor: [NameType.Parameter, ...Newable] },
{ name: "this", reservedFor: [NameType.Parameter] },
{ name: "throw", reservedFor: [NameType.Parameter] },
{ name: "throw", reservedFor: [NameType.Parameter, NameType.Method] },
{ name: "true", reservedFor: [NameType.Parameter] },
{ name: "try", reservedFor: [NameType.Parameter] },
{ name: "type", reservedFor: [NameType.Parameter] },
{ name: "typeof", reservedFor: [NameType.Parameter] },
{ name: "var", reservedFor: [NameType.Parameter] },
{ name: "var", reservedFor: [NameType.Parameter, NameType.Method] },
{ name: "void", reservedFor: [NameType.Parameter] },
{ name: "while", reservedFor: [NameType.Parameter] },
{ name: "while", reservedFor: [NameType.Parameter, NameType.Method] },
{ name: "with", reservedFor: [NameType.Parameter] },
{ name: "yield", reservedFor: [NameType.Parameter] },
{ name: "arguments", reservedFor: [NameType.Parameter] },
{ name: "yield", reservedFor: [NameType.Parameter, NameType.Method] },
{ name: "arguments", reservedFor: [NameType.Parameter, NameType.Method] },
{ name: "global", reservedFor: [...Newable] },
// reserve client for codegen
{ name: "client", reservedFor: [NameType.Parameter] },
Expand All @@ -112,30 +124,31 @@ export function guardReservedNames(
nameType: NameType,
customReservedNames: ReservedName[] = []
): string {
const suffix = getSuffix(nameType);
const [prefix, suffix] = getAffix(nameType);
return [...ReservedModelNames, ...customReservedNames]
.filter((r) => r.reservedFor.includes(nameType))
.find((r) => r.name === name.toLowerCase())
? `${name}${suffix}`
? `${prefix}${name}${suffix}`
: name;
}

function getSuffix(nameType?: NameType) {
function getAffix(nameType?: NameType): [string, string] {
switch (nameType) {
case NameType.File:
case NameType.Operation:
return "";
return ["", ""];
case NameType.Property:
return "Property";
return ["", "Property"];
case NameType.OperationGroup:
return "Operations";
return ["", "Operations"];
case NameType.Parameter:
return "Param";
return ["", "Param"];
case NameType.Method:
return ["$", ""];
case NameType.Class:
case NameType.Interface:
case NameType.Method:
default:
return "Model";
return ["", "Model"];
}
}

Expand Down
29 changes: 29 additions & 0 deletions packages/rlc-common/test/helpers/nameUtils.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,35 @@ describe("#normalizeName", () => {
});
});

describe("for method", () => {
it("should return the name with prefix $ for the method name is a reserved name", () => {
expect(normalizeName("continue", NameType.Method, true)).to.equal(
"$continue"
);
expect(normalizeName("break", NameType.Method, true)).to.equal(
"$break"
);
Copy link
Member

@MaryGao MaryGao May 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@v-jiaodi

  • pls help add more UTs in rlc-common.
  • Also include UTs in classical client and api operations.
  • Also add one test case in smoke.
  • Help verify the list among our reserved words and spector operations and help add missing cases in spector

Copy link
Member Author

Choose a reason for hiding this comment

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

add missing cases in spector : microsoft/typespec#7413

Copy link
Member

Choose a reason for hiding this comment

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

could @v-jiaodi try to enableOperationGroup as false to see if it works for special words case?

Copy link
Member

Choose a reason for hiding this comment

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

@qiaozha we would have generation issue then and create an issue to track this #3206!

expect(normalizeName("case", NameType.Method, true)).to.equal(
"$case"
);
expect(normalizeName("break", NameType.Method, true)).to.equal(
"$break"
);
expect(normalizeName("class", NameType.Method, true)).to.equal(
"$class"
);
expect(normalizeName("default", NameType.Method, true)).to.equal(
"$default"
);
expect(normalizeName("do", NameType.Method, true)).to.equal(
"$do"
);
expect(normalizeName("function", NameType.Method, true)).to.equal(
"$function"
);
});
});

describe("for operation", () => {
it("should return the name with the suffix 'Operation' if the name is a reserved name", () => {
expect(normalizeName("export", NameType.Operation,)).to.equal(
Expand Down
5 changes: 5 additions & 0 deletions ...es/typespec-test/test/widget_dpg/generated/typespec-ts/review/widget_dpg.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ export interface AnalyzeResult {
summary: string;
}

// @public
export interface BudgetsContinueOptionalParams extends OperationOptions {
}

// @public
export interface BudgetsCreateOrReplaceOptionalParams extends OperationOptions {
updateIntervalInMs?: number;
Expand All @@ -30,6 +34,7 @@ export interface BudgetsGetBudgetsOptionalParams extends OperationOptions {

// @public
export interface BudgetsOperations {
continue: (options?: BudgetsContinueOptionalParams) => Promise<void>;
createOrReplace: (name: string, resource: SAPUser, options?: BudgetsCreateOrReplaceOptionalParams) => PollerLike<OperationState<SAPUser>, SAPUser>;
// (undocumented)
getBudgets: (name: string, options?: BudgetsGetBudgetsOptionalParams) => Promise<Widget[]>;
Expand Down
3 changes: 2 additions & 1 deletion packages/typespec-test/test/widget_dpg/generated/typespec-ts/src/api/budgets/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

export { getBudgets, createOrReplace } from "./operations.js";
export { $continue, getBudgets, createOrReplace } from "./operations.js";
export {
BudgetsContinueOptionalParams,
BudgetsGetBudgetsOptionalParams,
BudgetsCreateOrReplaceOptionalParams,
} from "./options.js";
35 changes: 35 additions & 0 deletions packages/typespec-test/test/widget_dpg/generated/typespec-ts/src/api/budgets/operations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
sapUserDeserializer,
} from "../../models/models.js";
import {
BudgetsContinueOptionalParams,
BudgetsGetBudgetsOptionalParams,
BudgetsCreateOrReplaceOptionalParams,
} from "./options.js";
Expand All @@ -24,6 +25,40 @@ import {
} from "@azure-rest/core-client";
import { PollerLike, OperationState } from "@azure/core-lro";

export function _$continueSend(
context: Client,
options: BudgetsContinueOptionalParams = { requestOptions: {} },
): StreamableMethod {
context.pipeline.removePolicy({ name: "ClientApiVersionPolicy" });
return context
.path("/budgets/widgets/continue")
.get({ ...operationOptionsToRequestParameters(options) });
}

export async function _$continueDeserialize(
result: PathUncheckedResponse,
): Promise<void> {
const expectedStatuses = ["204"];
if (!expectedStatuses.includes(result.status)) {
throw createRestError(result);
}

return;
}

/**
* @fixme continue is a reserved word that cannot be used as an operation name.
* Please add @clientName("clientName") or @clientName("<JS-Specific-Name>", "javascript")
* to the operation to override the generated name.
*/
export async function $continue(
context: Client,
options: BudgetsContinueOptionalParams = { requestOptions: {} },
): Promise<void> {
const result = await _$continueSend(context, options);
return _$continueDeserialize(result);
}

export function _getBudgetsSend(
context: Client,
name: string,
Expand Down
3 changes: 3 additions & 0 deletions packages/typespec-test/test/widget_dpg/generated/typespec-ts/src/api/budgets/options.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@

import { OperationOptions } from "@azure-rest/core-client";

/** Optional parameters. */
export interface BudgetsContinueOptionalParams extends OperationOptions {}

/** Optional parameters. */
export interface BudgetsGetBudgetsOptionalParams extends OperationOptions {}

Expand Down
15 changes: 14 additions & 1 deletion packages/typespec-test/test/widget_dpg/generated/typespec-ts/src/classic/budgets/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,25 @@
import { SAPWidgetServiceContext } from "../../api/sapWidgetServiceContext.js";
import { Widget, SAPUser } from "../../models/models.js";
import {
BudgetsContinueOptionalParams,
BudgetsGetBudgetsOptionalParams,
BudgetsCreateOrReplaceOptionalParams,
} from "../../api/budgets/options.js";
import { getBudgets, createOrReplace } from "../../api/budgets/operations.js";
import {
$continue,
getBudgets,
createOrReplace,
} from "../../api/budgets/operations.js";
import { PollerLike, OperationState } from "@azure/core-lro";

/** Interface representing a Budgets operations. */
export interface BudgetsOperations {
/**
* @fixme continue is a reserved word that cannot be used as an operation name.
* Please add @clientName("clientName") or @clientName("<JS-Specific-Name>", "javascript")
* to the operation to override the generated name.
*/
continue: (options?: BudgetsContinueOptionalParams) => Promise<void>;
getBudgets: (
name: string,
options?: BudgetsGetBudgetsOptionalParams,
Expand All @@ -26,6 +37,8 @@ export interface BudgetsOperations {

function _getBudgets(context: SAPWidgetServiceContext) {
return {
continue: (options?: BudgetsContinueOptionalParams) =>
$continue(context, options),
getBudgets: (name: string, options?: BudgetsGetBudgetsOptionalParams) =>
getBudgets(context, name, options),
createOrReplace: (
Expand Down
1 change: 1 addition & 0 deletions packages/typespec-test/test/widget_dpg/generated/typespec-ts/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export {
} from "./models/index.js";
export { SAPWidgetServiceClientOptionalParams } from "./api/index.js";
export {
BudgetsContinueOptionalParams,
BudgetsGetBudgetsOptionalParams,
BudgetsCreateOrReplaceOptionalParams,
} from "./api/budgets/index.js";
Expand Down
4 changes: 4 additions & 0 deletions packages/typespec-test/test/widget_dpg/spec/main.tsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,4 +179,8 @@ interface Budgets {
@get getBudgets(
@query name: string
): Widget[] | WidgetError;

@route("/widgets/continue")
// test the reserved word "continue" operation
op continue(): void;
}
8 changes: 4 additions & 4 deletions packages/typespec-ts/src/modular/helpers/namingHelpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,19 @@ export interface GuardedName {
}

export function getOperationName(operation: ServiceOperation): GuardedName {
if (isReservedName(operation.name, NameType.Operation)) {
const norm = normalizeName(operation.name, NameType.Method, true);
if (isReservedName(operation.name, NameType.Method)) {
return {
name: `$${operation.name}`,
name: norm,
fixme: [
`${operation.name} is a reserved word that cannot be used as an operation name.
Please add @clientName("clientName") or @clientName("<JS-Specific-Name>", "javascript")
to the operation to override the generated name.`
]
};
}

return {
name: normalizeName(operation.name, NameType.Operation, true)
name: norm
};
}

Expand Down
Loading
Loading