diff --git a/.chronus/changes/fix-nested-options-2025-2-26-17-11-41.md b/.chronus/changes/fix-nested-options-2025-2-26-17-11-41.md new file mode 100644 index 00000000000..186a3b115fc --- /dev/null +++ b/.chronus/changes/fix-nested-options-2025-2-26-17-11-41.md @@ -0,0 +1,7 @@ +--- +changeKind: fix +packages: + - "@typespec/compiler" +--- + +Fix extra properties not validated in nested entries of the config \ No newline at end of file diff --git a/.chronus/changes/fix-nested-options-2025-2-26-22-18-28.md b/.chronus/changes/fix-nested-options-2025-2-26-22-18-28.md new file mode 100644 index 00000000000..537a77e4e42 --- /dev/null +++ b/.chronus/changes/fix-nested-options-2025-2-26-22-18-28.md @@ -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` diff --git a/.chronus/changes/fix-nested-options-2025-2-28-17-33-1.md b/.chronus/changes/fix-nested-options-2025-2-28-17-33-1.md new file mode 100644 index 00000000000..da2e23b20e8 --- /dev/null +++ b/.chronus/changes/fix-nested-options-2025-2-28-17-33-1.md @@ -0,0 +1,11 @@ +--- +# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking +changeKind: internal +packages: + - "@typespec/http-client-js" + - "@typespec/http-server-js" + - "@typespec/http-server-csharp" + - "@typespec/http-specs" +--- + +Fix nested options diff --git a/packages/compiler/src/config/config-schema.ts b/packages/compiler/src/config/config-schema.ts index 8c2b615e750..0df1c8b9333 100644 --- a/packages/compiler/src/config/config-schema.ts +++ b/packages/compiler/src/config/config-schema.ts @@ -28,6 +28,7 @@ export const TypeSpecConfigJsonSchema: JSONSchemaType = { default: { type: "string" }, }, required: ["default"], + additionalProperties: false, }, }, parameters: { @@ -40,6 +41,7 @@ export const TypeSpecConfigJsonSchema: JSONSchemaType = { default: { type: "string" }, }, required: ["default"], + additionalProperties: false, }, }, @@ -76,16 +78,6 @@ export const TypeSpecConfigJsonSchema: JSONSchemaType = { required: [], additionalProperties: emitterOptionsSchema, }, - emitters: { - type: "object", - nullable: true, - deprecated: true, - required: [], - additionalProperties: { - oneOf: [{ type: "boolean" }, emitterOptionsSchema], - }, - }, - linter: { type: "object", nullable: true, diff --git a/packages/compiler/src/config/config-to-options.ts b/packages/compiler/src/config/config-to-options.ts index 322b6b372aa..24687261e52 100644 --- a/packages/compiler/src/config/config-to-options.ts +++ b/packages/compiler/src/config/config-to-options.ts @@ -142,28 +142,28 @@ function mergeOptions( overrides: Record> | undefined, ): Record { const configuredEmitters: Record> = 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 { + 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, ); } diff --git a/packages/compiler/src/config/types.ts b/packages/compiler/src/config/types.ts index b9aaf93af1f..f38d07a1173 100644 --- a/packages/compiler/src/config/types.ts +++ b/packages/compiler/src/config/types.ts @@ -86,7 +86,6 @@ export interface TypeSpecRawConfig { emit?: string[]; options?: Record; - emitters?: Record; linter?: LinterConfig; } diff --git a/packages/compiler/src/core/cli/actions/compile/args.ts b/packages/compiler/src/core/cli/actions/compile/args.ts index 04b9863ac2b..b1e236e4a8e 100644 --- a/packages/compiler/src/core/cli/actions/compile/args.ts +++ b/packages/compiler/src/core/cli/actions/compile/args.ts @@ -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; @@ -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, @@ -89,20 +90,30 @@ function resolveConfigArgs(args: CompileCliArgs): Record { return map; } -function resolveCliOptions(args: CompileCliArgs): { - options: Record>; - miscOptions: Record | undefined; -} { + +function resolveCliOptions(args: CompileCliArgs): [ + { + options: Record>; + miscOptions: Record | undefined; + }, + readonly Diagnostic[], +] { + const diagnostics: Diagnostic[] = []; let miscOptions: Record | undefined; - const options: Record> = {}; + const options: Record = {}; 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: .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) { @@ -110,19 +121,21 @@ function resolveCliOptions(args: CompileCliArgs): { } 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]; } diff --git a/packages/compiler/src/core/cli/utils.ts b/packages/compiler/src/core/cli/utils.ts index c42e35b41bc..a536f0de515 100644 --- a/packages/compiler/src/core/cli/utils.ts +++ b/packages/compiler/src/core/cli/utils.ts @@ -28,11 +28,23 @@ export interface CliHostArgs { } export function withCliHost( - fn: (host: CliCompilerHost, args: T) => void | Promise, -): (args: T) => void | Promise { - return (args: T) => { + fn: (host: CliCompilerHost, args: T) => Promise, +): (args: T) => Promise { + return withFailsafe((args: T) => { const host = createCLICompilerHost(args); return fn(host, args); + }); +} + +function withFailsafe( + fn: (...args: T) => Promise, +): (...args: T) => Promise { + return async (...args: T) => { + try { + return await fn(...args); + } catch (error) { + handleInternalCompilerError(error); + } }; } @@ -42,7 +54,7 @@ export function withCliHost( export function withCliHostAndDiagnostics( fn: (host: CliCompilerHost, args: T) => readonly Diagnostic[] | Promise, ): (args: T) => void | Promise { - return async (args: T) => { + return withFailsafe(async (args: T) => { const host = createCLICompilerHost(args); const diagnostics = await fn(host, args); logDiagnostics(diagnostics, host.logSink); @@ -50,7 +62,7 @@ export function withCliHostAndDiagnostics( if (diagnostics.some((d) => d.severity === "error")) { process.exit(1); } - }; + }); } export function createCLICompilerHost(options: CliHostArgs): CliCompilerHost { @@ -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); } diff --git a/packages/compiler/src/core/messages.ts b/packages/compiler/src/core/messages.ts index 33b68d93cd5..5c42d275d59 100644 --- a/packages/compiler/src/core/messages.ts +++ b/packages/compiler/src/core/messages.ts @@ -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: .some-options=value`, + }, + }, // #endregion CLI } as const; diff --git a/packages/compiler/test/cli.test.ts b/packages/compiler/test/cli.test.ts index 872f2e77977..b4b85ba91d1 100644 --- a/packages/compiler/test/cli.test.ts +++ b/packages/compiler/test/cli.test.ts @@ -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: .some-options=value`, + }); + }); + it("--option without an emitter are moved to miscOptions", async () => { const options = await resolveCompilerOptions({ options: [`test-debug=true`], @@ -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( @@ -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({}); diff --git a/packages/compiler/test/config/config.test.ts b/packages/compiler/test/config/config.test.ts index a30a339b035..abc4fca5b6d 100644 --- a/packages/compiler/test/config/config.test.ts +++ b/packages/compiler/test/config/config.test.ts @@ -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: {} } }), []); }); }); }); diff --git a/packages/compiler/test/config/resolve-compiler-option.test.ts b/packages/compiler/test/config/resolve-compiler-option.test.ts index ac5d318202d..75a64ab0c92 100644 --- a/packages/compiler/test/config/resolve-compiler-option.test.ts +++ b/packages/compiler/test/config/resolve-compiler-option.test.ts @@ -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, diff --git a/packages/compiler/test/config/scenarios/custom/myConfigNested.yaml b/packages/compiler/test/config/scenarios/custom/myConfigNested.yaml index 9d6ce41cf72..8ebf740d4d9 100644 --- a/packages/compiler/test/config/scenarios/custom/myConfigNested.yaml +++ b/packages/compiler/test/config/scenarios/custom/myConfigNested.yaml @@ -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}" diff --git a/packages/compiler/test/e2e/cli/cli.e2e.ts b/packages/compiler/test/e2e/cli/cli.e2e.ts index 43391e70670..e4c0ad4e5a1 100644 --- a/packages/compiler/test/e2e/cli/cli.e2e.ts +++ b/packages/compiler/test/e2e/cli/cli.e2e.ts @@ -215,7 +215,7 @@ describe("compile", () => { "--arg", "custom-dir=custom-dir-name", "--arg", - "metadata.owner=TypeSpec", + "owner=TypeSpec", "--option", "description.name=TypeSpec with options", "--option", @@ -231,7 +231,7 @@ describe("compile", () => { ); expect(file.toString()).toContain(`By Owner: TypeSpec TypeSpec with options -Succeeded: TypeSpec with options with this short example by TypeSpec +Succeeded: TypeSpec with options by TypeSpec Owner: TypeSpec Co-owner is defined by this test`); }); diff --git a/packages/compiler/test/e2e/cli/scenarios/with-option/tspconfig.yaml b/packages/compiler/test/e2e/cli/scenarios/with-option/tspconfig.yaml index 04fffe5aae9..67f759575a9 100644 --- a/packages/compiler/test/e2e/cli/scenarios/with-option/tspconfig.yaml +++ b/packages/compiler/test/e2e/cli/scenarios/with-option/tspconfig.yaml @@ -3,23 +3,19 @@ parameters: default: default-value service-name: default: "TypeSpec" - metadata: - name: "Bar" - owner: "None" - description: "this short example" - default: "Metadata default" + argName: + default: "Bar" + owner: + default: "None" options: description: header: "By {description.by.owners.primary}" - name: "Testing name: {metadata.name}" - details: "Succeeded: {name} with {metadata.description} by {metadata.owner}" + name: "Testing name: {argName}" + details: "Succeeded: {name} by {owner}" by: owners: - primary: "Owner: {metadata.owner}" + primary: "Owner: {owner}" secondary: "Undefined secondary owner" -emit: - - ./emitter.js - output-dir: "{project-root}/tsp-output/{custom-dir}" diff --git a/packages/http-client-js/eng/scripts/tspconfig.yaml b/packages/http-client-js/eng/scripts/tspconfig.yaml index 3734bfa6b4c..a758e05bebd 100644 --- a/packages/http-client-js/eng/scripts/tspconfig.yaml +++ b/packages/http-client-js/eng/scripts/tspconfig.yaml @@ -1,5 +1,5 @@ -emitters: - "@typespec/http-client-js": true +emit: + - "@typespec/http-client-js" options: "@typespec/http-client-js": diff --git a/packages/http-server-csharp/eng/scripts/tspconfig.yaml b/packages/http-server-csharp/eng/scripts/tspconfig.yaml index 7216934b25d..3ae534ee9c1 100644 --- a/packages/http-server-csharp/eng/scripts/tspconfig.yaml +++ b/packages/http-server-csharp/eng/scripts/tspconfig.yaml @@ -3,9 +3,9 @@ parameters: default: "8080" service-port-https: default: "8443" -emitters: - "@typespec/http-server-csharp": true - "@typespec/openapi3": true +emit: + - "@typespec/http-server-csharp" + - "@typespec/openapi3" options: "@typespec/http-server-csharp": emitter-output-dir: "{output-dir}" diff --git a/packages/http-server-js/eng/scripts/tspconfig.yaml b/packages/http-server-js/eng/scripts/tspconfig.yaml index 4b2afedea14..f1a34f59696 100644 --- a/packages/http-server-js/eng/scripts/tspconfig.yaml +++ b/packages/http-server-js/eng/scripts/tspconfig.yaml @@ -1,5 +1,5 @@ -emitters: - "@typespec/http-server-js": true +emit: + - "@typespec/http-server-js" options: "@typespec/http-server-js": diff --git a/packages/http-specs/tspconfig.yaml b/packages/http-specs/tspconfig.yaml index e86cb1f4c20..e69de29bb2d 100644 --- a/packages/http-specs/tspconfig.yaml +++ b/packages/http-specs/tspconfig.yaml @@ -1,2 +0,0 @@ -emitters: - "@typespec/openapi3": true