Skip to content

Commit 8346751

Browse files
Fix nested options (#6711)
fix #6674 Also fix a crash when passing option without `=` and stopped showing the yargs help on crash
1 parent 2d89d3f commit 8346751

File tree

19 files changed

+164
-98
lines changed

19 files changed

+164
-98
lines changed
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
changeKind: fix
3+
packages:
4+
- "@typespec/compiler"
5+
---
6+
7+
Fix extra properties not validated in nested entries of the config
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
3+
changeKind: fix
4+
packages:
5+
- "@typespec/compiler"
6+
---
7+
8+
Fix passing nested emitter options with `--option`
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
3+
changeKind: internal
4+
packages:
5+
- "@typespec/http-client-js"
6+
- "@typespec/http-server-js"
7+
- "@typespec/http-server-csharp"
8+
- "@typespec/http-specs"
9+
---
10+
11+
Fix nested options

packages/compiler/src/config/config-schema.ts

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ export const TypeSpecConfigJsonSchema: JSONSchemaType<TypeSpecRawConfig> = {
2828
default: { type: "string" },
2929
},
3030
required: ["default"],
31+
additionalProperties: false,
3132
},
3233
},
3334
parameters: {
@@ -40,6 +41,7 @@ export const TypeSpecConfigJsonSchema: JSONSchemaType<TypeSpecRawConfig> = {
4041
default: { type: "string" },
4142
},
4243
required: ["default"],
44+
additionalProperties: false,
4345
},
4446
},
4547

@@ -76,16 +78,6 @@ export const TypeSpecConfigJsonSchema: JSONSchemaType<TypeSpecRawConfig> = {
7678
required: [],
7779
additionalProperties: emitterOptionsSchema,
7880
},
79-
emitters: {
80-
type: "object",
81-
nullable: true,
82-
deprecated: true,
83-
required: [],
84-
additionalProperties: {
85-
oneOf: [{ type: "boolean" }, emitterOptionsSchema],
86-
},
87-
},
88-
8981
linter: {
9082
type: "object",
9183
nullable: true,

packages/compiler/src/config/config-to-options.ts

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -142,28 +142,28 @@ function mergeOptions(
142142
overrides: Record<string, Record<string, unknown>> | undefined,
143143
): Record<string, EmitterOptions> {
144144
const configuredEmitters: Record<string, Record<string, unknown>> = deepClone(base ?? {});
145-
function deepMerge(emitter: any, options: any, name: string): any {
146-
if (typeof emitter !== "object") {
147-
return emitter;
148-
}
149-
for (const key of Object.keys(emitter)) {
150-
if (hasNestedObjects(emitter[key])) {
151-
emitter[key] = deepMerge(emitter[key], options, `${name}.${key}`);
152-
} else if (typeof options[name] === "object") {
153-
emitter[key] = options[name][key] ?? emitter[key];
145+
function isObject(item: unknown): item is Record<string, unknown> {
146+
return item && typeof item === "object" && (!Array.isArray(item) as any);
147+
}
148+
function deepMerge(target: any, source: any): any {
149+
if (isObject(target) && isObject(source)) {
150+
for (const key in source) {
151+
if (isObject(source[key])) {
152+
if (!target[key]) Object.assign(target, { [key]: {} });
153+
deepMerge(target[key], source[key]);
154+
} else {
155+
Object.assign(target, { [key]: source[key] });
156+
}
154157
}
155158
}
156-
return emitter;
159+
160+
return target;
157161
}
158162

159163
for (const [emitterName, cliOptionOverride] of Object.entries(overrides ?? {})) {
160164
configuredEmitters[emitterName] = deepMerge(
161-
{
162-
...(configuredEmitters[emitterName] ?? {}),
163-
...cliOptionOverride,
164-
},
165-
overrides,
166-
emitterName,
165+
configuredEmitters[emitterName] ?? {},
166+
cliOptionOverride,
167167
);
168168
}
169169

packages/compiler/src/config/types.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,6 @@ export interface TypeSpecRawConfig {
8686

8787
emit?: string[];
8888
options?: Record<string, EmitterOptions>;
89-
emitters?: Record<string, boolean | EmitterOptions>;
9089

9190
linter?: LinterConfig;
9291
}

packages/compiler/src/core/cli/actions/compile/args.ts

Lines changed: 35 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import { resolveCompilerOptions } from "../../../../config/config-to-options.js";
22
import { omitUndefined } from "../../../../utils/misc.js";
33
import { createDiagnosticCollector } from "../../../diagnostics.js";
4+
import { createDiagnostic } from "../../../messages.js";
45
import { CompilerOptions } from "../../../options.js";
56
import { resolvePath } from "../../../path-utils.js";
6-
import { CompilerHost, Diagnostic } from "../../../types.js";
7+
import { CompilerHost, Diagnostic, NoTarget } from "../../../types.js";
78

89
export interface CompileCliArgs {
910
path?: string;
@@ -42,7 +43,7 @@ export async function getCompilerOptions(
4243
: resolvePath(cwd, pathArg)
4344
: undefined;
4445

45-
const cliOptions = resolveCliOptions(args);
46+
const cliOptions = diagnostics.pipe(resolveCliOptions(args));
4647
const resolvedOptions = diagnostics.pipe(
4748
await resolveCompilerOptions(host, {
4849
entrypoint,
@@ -89,40 +90,52 @@ function resolveConfigArgs(args: CompileCliArgs): Record<string, string> {
8990

9091
return map;
9192
}
92-
function resolveCliOptions(args: CompileCliArgs): {
93-
options: Record<string, Record<string, unknown>>;
94-
miscOptions: Record<string, string> | undefined;
95-
} {
93+
94+
function resolveCliOptions(args: CompileCliArgs): [
95+
{
96+
options: Record<string, Record<string, unknown>>;
97+
miscOptions: Record<string, string> | undefined;
98+
},
99+
readonly Diagnostic[],
100+
] {
101+
const diagnostics: Diagnostic[] = [];
96102
let miscOptions: Record<string, string> | undefined;
97-
const options: Record<string, Record<string, string>> = {};
103+
const options: Record<string, any> = {};
98104
for (const option of args.options ?? []) {
99105
const optionParts = option.split("=");
100106
if (optionParts.length !== 2) {
101-
throw new Error(
102-
`The --option parameter value "${option}" must be in the format: <emitterName>.some-options=value`,
107+
diagnostics.push(
108+
createDiagnostic({
109+
code: "invalid-option-flag",
110+
target: NoTarget,
111+
format: { value: option },
112+
}),
103113
);
114+
continue;
104115
}
105-
let optionKeyParts = optionParts[0].split(".");
116+
const optionKeyParts = optionParts[0].split(".");
106117
if (optionKeyParts.length === 1) {
107118
const key = optionKeyParts[0];
108119
if (miscOptions === undefined) {
109120
miscOptions = {};
110121
}
111122
miscOptions[key] = optionParts[1];
112123
continue;
113-
} else if (optionKeyParts.length > 2) {
114-
// support emitter/path/file.js.option=xyz
115-
optionKeyParts = [
116-
optionKeyParts.slice(0, -1).join("."),
117-
optionKeyParts[optionKeyParts.length - 1],
118-
];
119124
}
120-
const emitterName = optionKeyParts[0];
121-
const key = optionKeyParts[1];
122-
if (!(emitterName in options)) {
123-
options[emitterName] = {};
125+
126+
let current: any = options;
127+
for (let i = 0; i < optionKeyParts.length; i++) {
128+
const part = optionKeyParts[i];
129+
if (i === optionKeyParts.length - 1) {
130+
current[part] = optionParts[1];
131+
} else {
132+
if (!current[part]) {
133+
current[part] = {};
134+
}
135+
current = current[part];
136+
}
124137
}
125-
options[emitterName][key] = optionParts[1];
126138
}
127-
return { options, miscOptions };
139+
140+
return [{ options, miscOptions }, diagnostics];
128141
}

packages/compiler/src/core/cli/utils.ts

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,23 @@ export interface CliHostArgs {
2828
}
2929

3030
export function withCliHost<T extends CliHostArgs>(
31-
fn: (host: CliCompilerHost, args: T) => void | Promise<void>,
32-
): (args: T) => void | Promise<void> {
33-
return (args: T) => {
31+
fn: (host: CliCompilerHost, args: T) => Promise<void>,
32+
): (args: T) => Promise<void> {
33+
return withFailsafe((args: T) => {
3434
const host = createCLICompilerHost(args);
3535
return fn(host, args);
36+
});
37+
}
38+
39+
function withFailsafe<T extends unknown[], R>(
40+
fn: (...args: T) => Promise<R>,
41+
): (...args: T) => Promise<R> {
42+
return async (...args: T) => {
43+
try {
44+
return await fn(...args);
45+
} catch (error) {
46+
handleInternalCompilerError(error);
47+
}
3648
};
3749
}
3850

@@ -42,15 +54,15 @@ export function withCliHost<T extends CliHostArgs>(
4254
export function withCliHostAndDiagnostics<T extends CliHostArgs>(
4355
fn: (host: CliCompilerHost, args: T) => readonly Diagnostic[] | Promise<readonly Diagnostic[]>,
4456
): (args: T) => void | Promise<void> {
45-
return async (args: T) => {
57+
return withFailsafe(async (args: T) => {
4658
const host = createCLICompilerHost(args);
4759
const diagnostics = await fn(host, args);
4860
logDiagnostics(diagnostics, host.logSink);
4961
logDiagnosticCount(diagnostics);
5062
if (diagnostics.some((d) => d.severity === "error")) {
5163
process.exit(1);
5264
}
53-
};
65+
});
5466
}
5567

5668
export function createCLICompilerHost(options: CliHostArgs): CliCompilerHost {
@@ -169,7 +181,7 @@ export function logInternalCompilerError(error: unknown) {
169181
*
170182
* @param error error thrown
171183
*/
172-
export function handleInternalCompilerError(error: unknown) {
184+
export function handleInternalCompilerError(error: unknown): never {
173185
logInternalCompilerError(error);
174186
process.exit(1);
175187
}

packages/compiler/src/core/messages.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1059,6 +1059,12 @@ const diagnostics = {
10591059
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",
10601060
},
10611061
},
1062+
"invalid-option-flag": {
1063+
severity: "error",
1064+
messages: {
1065+
default: paramMessage`The --option parameter value "${"value"}" must be in the format: <emitterName>.some-options=value`,
1066+
},
1067+
},
10621068
// #endregion CLI
10631069
} as const;
10641070

packages/compiler/test/cli.test.ts

Lines changed: 37 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,23 @@ describe("compiler: cli", () => {
4545
});
4646
});
4747

48+
it("error if option doesn't have =", async () => {
49+
const [_, diagnostics] = await getCompilerOptions(
50+
host.compilerHost,
51+
"ws/main.tsp",
52+
cwd,
53+
{
54+
options: [`my-emitter.bar`],
55+
},
56+
{},
57+
);
58+
59+
expectDiagnostics(diagnostics, {
60+
code: "invalid-option-flag",
61+
message: `The --option parameter value "my-emitter.bar" must be in the format: <emitterName>.some-options=value`,
62+
});
63+
});
64+
4865
it("--option without an emitter are moved to miscOptions", async () => {
4966
const options = await resolveCompilerOptions({
5067
options: [`test-debug=true`],
@@ -54,6 +71,26 @@ describe("compiler: cli", () => {
5471
deepStrictEqual(options?.options, {});
5572
});
5673

74+
it("--option allows root level options", async () => {
75+
const options = await resolveCompilerOptions({
76+
options: [`my-emitter.foo=abc`],
77+
});
78+
79+
deepStrictEqual(options?.options, {
80+
"my-emitter": { foo: "abc" },
81+
});
82+
});
83+
84+
it("--option allows nested options", async () => {
85+
const options = await resolveCompilerOptions({
86+
options: [`my-emitter.foo.bar=abc`],
87+
});
88+
89+
deepStrictEqual(options?.options, {
90+
"my-emitter": { foo: { bar: "abc" } },
91+
});
92+
});
93+
5794
describe("config file with emitters", () => {
5895
beforeEach(() => {
5996
host.addTypeSpecFile(
@@ -106,17 +143,6 @@ describe("compiler: cli", () => {
106143
);
107144
});
108145

109-
it("override emitter-output-dir from cli args using path to emitter with extension", async () => {
110-
const options = await resolveCompilerOptions({
111-
options: [`path/to/emitter.js.emitter-output-dir={cwd}/relative-to-cwd`],
112-
});
113-
114-
strictEqual(
115-
options?.options?.["path/to/emitter.js"]?.["emitter-output-dir"],
116-
`${cwd}/relative-to-cwd`,
117-
);
118-
});
119-
120146
describe("arg interpolation", () => {
121147
it("use default arg value", async () => {
122148
const options = await resolveCompilerOptions({});

0 commit comments

Comments
 (0)