Skip to content

Fix nested options #6711

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.

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 20 commits into from
Mar 28, 2025
Merged
Show file tree
Hide file tree
Changes from 19 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
7 changes: 7 additions & 0 deletions .chronus/changes/fix-nested-options-2025-2-26-17-11-41.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
changeKind: fix
packages:
- "@typespec/compiler"
---

Fix extra properties not validated in nested entries of the config
8 changes: 8 additions & 0 deletions .chronus/changes/fix-nested-options-2025-2-26-22-18-28.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
changeKind: fix
packages:
- "@typespec/compiler"
---

Fix passing nested emitter options with `--option`
10 changes: 10 additions & 0 deletions .chronus/changes/fix-nested-options-2025-2-28-17-33-1.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
changeKind: internal
packages:
- "@typespec/http-server-js"
- "@typespec/http-server-csharp"
- "@typespec/http-specs"
---

Fix nested options
12 changes: 2 additions & 10 deletions packages/compiler/src/config/config-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export const TypeSpecConfigJsonSchema: JSONSchemaType<TypeSpecRawConfig> = {
default: { type: "string" },
},
required: ["default"],
additionalProperties: false,
},
},
parameters: {
Expand All @@ -40,6 +41,7 @@ export const TypeSpecConfigJsonSchema: JSONSchemaType<TypeSpecRawConfig> = {
default: { type: "string" },
},
required: ["default"],
additionalProperties: false,
},
},

Expand Down Expand Up @@ -76,16 +78,6 @@ export const TypeSpecConfigJsonSchema: JSONSchemaType<TypeSpecRawConfig> = {
required: [],
additionalProperties: emitterOptionsSchema,
},
emitters: {
type: "object",
nullable: true,
deprecated: true,
required: [],
additionalProperties: {
oneOf: [{ type: "boolean" }, emitterOptionsSchema],
},
},

linter: {
type: "object",
nullable: true,
Expand Down
32 changes: 16 additions & 16 deletions packages/compiler/src/config/config-to-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,28 +142,28 @@ function mergeOptions(
overrides: Record<string, Record<string, unknown>> | undefined,
): Record<string, EmitterOptions> {
const configuredEmitters: Record<string, Record<string, unknown>> = deepClone(base ?? {});
function deepMerge(emitter: any, options: any, name: string): any {
if (typeof emitter !== "object") {
return emitter;
}
for (const key of Object.keys(emitter)) {
if (hasNestedObjects(emitter[key])) {
emitter[key] = deepMerge(emitter[key], options, `${name}.${key}`);
} else if (typeof options[name] === "object") {
emitter[key] = options[name][key] ?? emitter[key];
function isObject(item: unknown): item is Record<string, unknown> {
return item && typeof item === "object" && (!Array.isArray(item) as any);
}
function deepMerge(target: any, source: any): any {
if (isObject(target) && isObject(source)) {
for (const key in source) {
if (isObject(source[key])) {
if (!target[key]) Object.assign(target, { [key]: {} });
deepMerge(target[key], source[key]);
} else {
Object.assign(target, { [key]: source[key] });
}
}
}
return emitter;

return target;
}

for (const [emitterName, cliOptionOverride] of Object.entries(overrides ?? {})) {
configuredEmitters[emitterName] = deepMerge(
{
...(configuredEmitters[emitterName] ?? {}),
...cliOptionOverride,
},
overrides,
emitterName,
configuredEmitters[emitterName] ?? {},
cliOptionOverride,
);
}

Expand Down
1 change: 0 additions & 1 deletion packages/compiler/src/config/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ export interface TypeSpecRawConfig {

emit?: string[];
options?: Record<string, EmitterOptions>;
emitters?: Record<string, boolean | EmitterOptions>;

linter?: LinterConfig;
}
Expand Down
57 changes: 35 additions & 22 deletions packages/compiler/src/core/cli/actions/compile/args.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { resolveCompilerOptions } from "../../../../config/config-to-options.js";
import { omitUndefined } from "../../../../utils/misc.js";
import { createDiagnosticCollector } from "../../../diagnostics.js";
import { createDiagnostic } from "../../../messages.js";
import { CompilerOptions } from "../../../options.js";
import { resolvePath } from "../../../path-utils.js";
import { CompilerHost, Diagnostic } from "../../../types.js";
import { CompilerHost, Diagnostic, NoTarget } from "../../../types.js";

export interface CompileCliArgs {
path?: string;
Expand Down Expand Up @@ -42,7 +43,7 @@ export async function getCompilerOptions(
: resolvePath(cwd, pathArg)
: undefined;

const cliOptions = resolveCliOptions(args);
const cliOptions = diagnostics.pipe(resolveCliOptions(args));
const resolvedOptions = diagnostics.pipe(
await resolveCompilerOptions(host, {
entrypoint,
Expand Down Expand Up @@ -89,40 +90,52 @@ function resolveConfigArgs(args: CompileCliArgs): Record<string, string> {

return map;
}
function resolveCliOptions(args: CompileCliArgs): {
options: Record<string, Record<string, unknown>>;
miscOptions: Record<string, string> | undefined;
} {

function resolveCliOptions(args: CompileCliArgs): [
{
options: Record<string, Record<string, unknown>>;
miscOptions: Record<string, string> | undefined;
},
readonly Diagnostic[],
] {
const diagnostics: Diagnostic[] = [];
let miscOptions: Record<string, string> | undefined;
const options: Record<string, Record<string, string>> = {};
const options: Record<string, any> = {};
for (const option of args.options ?? []) {
const optionParts = option.split("=");
if (optionParts.length !== 2) {
throw new Error(
`The --option parameter value "${option}" must be in the format: <emitterName>.some-options=value`,
diagnostics.push(
createDiagnostic({
code: "invalid-option-flag",
target: NoTarget,
format: { value: option },
}),
);
continue;
}
let optionKeyParts = optionParts[0].split(".");
const optionKeyParts = optionParts[0].split(".");
if (optionKeyParts.length === 1) {
const key = optionKeyParts[0];
if (miscOptions === undefined) {
miscOptions = {};
}
miscOptions[key] = optionParts[1];
continue;
} else if (optionKeyParts.length > 2) {
// support emitter/path/file.js.option=xyz
optionKeyParts = [
optionKeyParts.slice(0, -1).join("."),
optionKeyParts[optionKeyParts.length - 1],
];
}
const emitterName = optionKeyParts[0];
const key = optionKeyParts[1];
if (!(emitterName in options)) {
options[emitterName] = {};

let current: any = options;
for (let i = 0; i < optionKeyParts.length; i++) {
const part = optionKeyParts[i];
if (i === optionKeyParts.length - 1) {
current[part] = optionParts[1];
} else {
if (!current[part]) {
current[part] = {};
}
current = current[part];
}
}
options[emitterName][key] = optionParts[1];
}
return { options, miscOptions };

return [{ options, miscOptions }, diagnostics];
}
24 changes: 18 additions & 6 deletions packages/compiler/src/core/cli/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,23 @@ export interface CliHostArgs {
}

export function withCliHost<T extends CliHostArgs>(
fn: (host: CliCompilerHost, args: T) => void | Promise<void>,
): (args: T) => void | Promise<void> {
return (args: T) => {
fn: (host: CliCompilerHost, args: T) => Promise<void>,
): (args: T) => Promise<void> {
return withFailsafe((args: T) => {
const host = createCLICompilerHost(args);
return fn(host, args);
});
}

function withFailsafe<T extends unknown[], R>(
fn: (...args: T) => Promise<R>,
): (...args: T) => Promise<R> {
return async (...args: T) => {
try {
return await fn(...args);
} catch (error) {
handleInternalCompilerError(error);
}
};
}

Expand All @@ -42,15 +54,15 @@ export function withCliHost<T extends CliHostArgs>(
export function withCliHostAndDiagnostics<T extends CliHostArgs>(
fn: (host: CliCompilerHost, args: T) => readonly Diagnostic[] | Promise<readonly Diagnostic[]>,
): (args: T) => void | Promise<void> {
return async (args: T) => {
return withFailsafe(async (args: T) => {
const host = createCLICompilerHost(args);
const diagnostics = await fn(host, args);
logDiagnostics(diagnostics, host.logSink);
logDiagnosticCount(diagnostics);
if (diagnostics.some((d) => d.severity === "error")) {
process.exit(1);
}
};
});
}

export function createCLICompilerHost(options: CliHostArgs): CliCompilerHost {
Expand Down Expand Up @@ -169,7 +181,7 @@ export function logInternalCompilerError(error: unknown) {
*
* @param error error thrown
*/
export function handleInternalCompilerError(error: unknown) {
export function handleInternalCompilerError(error: unknown): never {
logInternalCompilerError(error);
process.exit(1);
}
Expand Down
6 changes: 6 additions & 0 deletions packages/compiler/src/core/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1059,6 +1059,12 @@ const diagnostics = {
osx: "Couldn't find VS Code 'code' command in PATH. Make sure you have the VS Code executable added to the system PATH.\nSee instruction for Mac OS here https://code.visualstudio.com/docs/setup/mac",
},
},
"invalid-option-flag": {
severity: "error",
messages: {
default: paramMessage`The --option parameter value "${"value"}" must be in the format: <emitterName>.some-options=value`,
},
},
// #endregion CLI
} as const;

Expand Down
48 changes: 37 additions & 11 deletions packages/compiler/test/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,23 @@ describe("compiler: cli", () => {
});
});

it("error if option doesn't have =", async () => {
const [_, diagnostics] = await getCompilerOptions(
host.compilerHost,
"ws/main.tsp",
cwd,
{
options: [`my-emitter.bar`],
},
{},
);

expectDiagnostics(diagnostics, {
code: "invalid-option-flag",
message: `The --option parameter value "my-emitter.bar" must be in the format: <emitterName>.some-options=value`,
});
});

it("--option without an emitter are moved to miscOptions", async () => {
const options = await resolveCompilerOptions({
options: [`test-debug=true`],
Expand All @@ -54,6 +71,26 @@ describe("compiler: cli", () => {
deepStrictEqual(options?.options, {});
});

it("--option allows root level options", async () => {
const options = await resolveCompilerOptions({
options: [`my-emitter.foo=abc`],
});

deepStrictEqual(options?.options, {
"my-emitter": { foo: "abc" },
});
});

it("--option allows nested options", async () => {
const options = await resolveCompilerOptions({
options: [`my-emitter.foo.bar=abc`],
});

deepStrictEqual(options?.options, {
"my-emitter": { foo: { bar: "abc" } },
});
});

describe("config file with emitters", () => {
beforeEach(() => {
host.addTypeSpecFile(
Expand Down Expand Up @@ -106,17 +143,6 @@ describe("compiler: cli", () => {
);
});

it("override emitter-output-dir from cli args using path to emitter with extension", async () => {
const options = await resolveCompilerOptions({
options: [`path/to/emitter.js.emitter-output-dir={cwd}/relative-to-cwd`],
});

strictEqual(
options?.options?.["path/to/emitter.js"]?.["emitter-output-dir"],
`${cwd}/relative-to-cwd`,
);
});

describe("arg interpolation", () => {
it("use default arg value", async () => {
const options = await resolveCompilerOptions({});
Expand Down
6 changes: 3 additions & 3 deletions packages/compiler/test/config/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,18 +153,18 @@ describe("compiler: config file loading", () => {
});

it("fails if passing the wrong type", () => {
deepStrictEqual(validate({ emitters: true } as any), [
deepStrictEqual(validate({ emit: true } as any), [
{
code: "invalid-schema",
target: { file, pos: 0, end: 0 },
severity: "error",
message: "Schema violation: must be object (/emitters)",
message: "Schema violation: must be array (/emit)",
},
]);
});

it("succeeds if config is valid", () => {
deepStrictEqual(validate({ emitters: { openapi: {} } }), []);
deepStrictEqual(validate({ options: { openapi: {} } }), []);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ describe("compiler: resolve compiler options", () => {
options: {
description: {
name: "Testing name: Sphere",
details: { one: "Type: Bar", two: { "two-one": "Default: Metadata default" } },
details: { one: "Type: Microsoft", two: { "two-one": "Default: Microsoft" } },
},
},
outputDir: tspOutputPath,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,12 @@ emit:
parameters:
service-name:
default: "Sphere"
metadata:
bar: "Bar"
Owner: "Microsoft"
default: "Metadata default"
owner:
default: "Microsoft"
options:
description:
name: "Testing name: {service-name}"
details:
one: "Type: {metadata.bar}"
one: "Type: {owner}"
two:
two-one: "Default: {metadata.default}"
two-one: "Default: {owner}"
Loading
Loading