Skip to content

Commit 0945571

Browse files
authored
fix(dotnet): prevent "false" being incorrectly passed to dotnet command (#818)
* fix(dotnet): prevent "false" being incorrectly passed to dotnet command * chore(dotnet): refactor how command line flags are built
1 parent d9fff67 commit 0945571

File tree

12 files changed

+368
-100
lines changed

12 files changed

+368
-100
lines changed

packages/dotnet/src/lib/core/dotnet.client.spec.ts

Lines changed: 150 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ import { DotNetClient } from './dotnet.client';
44
import { dotnetFactory, mockDotnetFactory } from './dotnet.factory';
55

66
describe('dotnet client', () => {
7+
afterEach(() => {
8+
jest.resetAllMocks();
9+
});
10+
711
describe('publish', () => {
812
describe('extra parameters', () => {
913
const dotnetClient = new DotNetClient(mockDotnetFactory());
@@ -13,13 +17,7 @@ describe('dotnet client', () => {
1317
beforeEach(() => {
1418
spawnSyncSpy = jest
1519
.spyOn(cp, 'spawnSync')
16-
.mockReturnValue({ status: 0 } as Partial<
17-
cp.SpawnSyncReturns<Buffer>
18-
> as cp.SpawnSyncReturns<Buffer>);
19-
});
20-
21-
afterEach(() => {
22-
jest.resetAllMocks();
20+
.mockReturnValue({ status: 0 } as cp.SpawnSyncReturns<Buffer>);
2321
});
2422

2523
it('should handle multiple parameters', () => {
@@ -78,6 +76,27 @@ describe('dotnet client', () => {
7876
expect(spawnSyncSpy.mock.calls[0][1]).toContain('-p:Name=bar');
7977
});
8078
});
79+
it('should convert options to flags', () => {
80+
const dotnetClient = new DotNetClient(mockDotnetFactory());
81+
const spawnSyncSpy = jest
82+
.spyOn(cp, 'spawnSync')
83+
.mockReturnValue({ status: 0 } as cp.SpawnSyncReturns<Buffer>);
84+
dotnetClient.publish('my-project', {
85+
noBuild: false,
86+
noRestore: true,
87+
configuration: 'Release',
88+
});
89+
expect(spawnSyncSpy).toBeCalledTimes(1);
90+
expect(spawnSyncSpy.mock.calls[0][1]).toMatchInlineSnapshot(`
91+
[
92+
"publish",
93+
""my-project"",
94+
"--no-restore",
95+
"--configuration",
96+
"Release",
97+
]
98+
`);
99+
});
81100
});
82101

83102
describe('listInstalledTemplates', () => {
@@ -484,4 +503,128 @@ ASP.NET Core gRPC Service grpc [C#]
484503
`);
485504
});
486505
});
506+
507+
describe('test', () => {
508+
it('should convert options to flags', () => {
509+
const dotnetClient = new DotNetClient(mockDotnetFactory());
510+
const spawnSyncSpy = jest
511+
.spyOn(cp, 'spawnSync')
512+
.mockReturnValue({ status: 0 } as cp.SpawnSyncReturns<Buffer>);
513+
dotnetClient.test('my-project', false, {
514+
blame: false,
515+
noRestore: true,
516+
blameHang: true,
517+
blameHangDump: 'dump.file',
518+
});
519+
expect(spawnSyncSpy).toHaveBeenCalledTimes(1);
520+
expect(spawnSyncSpy.mock.calls[0][1]).toMatchInlineSnapshot(`
521+
[
522+
"test",
523+
"my-project",
524+
"--no-restore",
525+
"--blame-hang",
526+
"--blame-hang-dump",
527+
"dump.file",
528+
]
529+
`);
530+
});
531+
});
532+
533+
describe('build', () => {
534+
it('should convert options to flags', () => {
535+
const dotnetClient = new DotNetClient(mockDotnetFactory());
536+
const spawnSyncSpy = jest
537+
.spyOn(cp, 'spawnSync')
538+
.mockReturnValue({ status: 0 } as cp.SpawnSyncReturns<Buffer>);
539+
dotnetClient.build('my-project', {
540+
noDependencies: false,
541+
noRestore: true,
542+
configuration: 'Release',
543+
});
544+
expect(spawnSyncSpy).toHaveBeenCalledTimes(1);
545+
expect(spawnSyncSpy.mock.calls[0][1]).toMatchInlineSnapshot(`
546+
[
547+
"build",
548+
"my-project",
549+
"--no-restore",
550+
"--configuration",
551+
"Release",
552+
]
553+
`);
554+
});
555+
});
556+
557+
describe('addPackageReference', () => {
558+
it('should convert options to flags', () => {
559+
const dotnetClient = new DotNetClient(mockDotnetFactory());
560+
const spawnSyncSpy = jest
561+
.spyOn(cp, 'spawnSync')
562+
.mockReturnValue({ status: 0 } as cp.SpawnSyncReturns<Buffer>);
563+
dotnetClient.addPackageReference('my-project', 'other-package', {
564+
noRestore: true,
565+
version: '1.2.3',
566+
});
567+
expect(spawnSyncSpy).toHaveBeenCalledTimes(1);
568+
expect(spawnSyncSpy.mock.calls[0][1]).toMatchInlineSnapshot(`
569+
[
570+
"add",
571+
"my-project",
572+
"package",
573+
"other-package",
574+
"--no-restore",
575+
"--version",
576+
"1.2.3",
577+
]
578+
`);
579+
});
580+
});
581+
582+
describe('new', () => {
583+
it('should convert options to flags', () => {
584+
const dotnetClient = new DotNetClient(mockDotnetFactory());
585+
const spawnSyncSpy = jest
586+
.spyOn(cp, 'spawnSync')
587+
.mockReturnValue({ status: 0 } as cp.SpawnSyncReturns<Buffer>);
588+
dotnetClient.new('my-template', {
589+
dryRun: true,
590+
force: false,
591+
name: 'my-project',
592+
});
593+
expect(spawnSyncSpy).toHaveBeenCalledTimes(1);
594+
expect(spawnSyncSpy.mock.calls[0][1]).toMatchInlineSnapshot(`
595+
[
596+
"new",
597+
"my-template",
598+
"--dry-run",
599+
"--name",
600+
"my-project",
601+
]
602+
`);
603+
});
604+
});
605+
606+
describe('run', () => {
607+
it('should convert options to flags', () => {
608+
const dotnetClient = new DotNetClient(mockDotnetFactory());
609+
const spawnSpy = jest
610+
.spyOn(cp, 'spawn')
611+
.mockReturnValue({ exitCode: 0 } as cp.ChildProcess);
612+
dotnetClient.run('my-project', false, {
613+
noRestore: true,
614+
noDependencies: false,
615+
configuration: 'Release',
616+
});
617+
expect(spawnSpy).toHaveBeenCalledTimes(1);
618+
expect(spawnSpy.mock.calls[0][1]).toMatchInlineSnapshot(`
619+
[
620+
"run",
621+
"--project",
622+
"my-project",
623+
"--no-restore",
624+
"--configuration",
625+
"Release",
626+
]
627+
`);
628+
});
629+
});
487630
});

packages/dotnet/src/lib/core/dotnet.client.ts

Lines changed: 43 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
import { ChildProcess, spawn, spawnSync } from 'child_process';
22
import * as semver from 'semver';
33

4-
import { getSpawnParameterArray, swapKeysUsingMap } from '@nx-dotnet/utils';
4+
import {
5+
convertOptionsToParams,
6+
getSpawnParameterArray,
7+
} from '@nx-dotnet/utils';
58

69
import {
7-
addPackageKeyMap,
8-
buildKeyMap,
10+
addPackageCommandLineParamFixes,
11+
buildCommandLineParamFixes,
912
dotnetAddPackageOptions,
1013
dotnetBuildOptions,
1114
dotnetFormatOptions,
@@ -14,18 +17,21 @@ import {
1417
dotnetRunOptions,
1518
DotnetTemplate,
1619
dotnetTestOptions,
17-
formatKeyMap,
20+
formatCommandLineParamFixes,
1821
KnownDotnetTemplates,
19-
newKeyMap,
20-
publishKeyMap,
21-
runKeyMap,
22-
testKeyMap,
22+
newCommandLineParamFixes,
23+
publishCommandLineParamFixes,
24+
runCommandLineParamFixes,
25+
testCommandLineParamFixes,
2326
} from '../models';
2427
import { parseDotnetNewListOutput } from '../utils/parse-dotnet-new-list-output';
2528
import { LoadedCLI } from './dotnet.factory';
2629

2730
export class DotNetClient {
28-
constructor(private cliCommand: LoadedCLI, public cwd?: string) {}
31+
constructor(
32+
private cliCommand: LoadedCLI,
33+
public cwd?: string,
34+
) {}
2935

3036
new(
3137
template: KnownDotnetTemplates,
@@ -34,8 +40,9 @@ export class DotNetClient {
3440
): void {
3541
const params = [`new`, template];
3642
if (parameters) {
37-
parameters = swapKeysUsingMap(parameters, newKeyMap);
38-
params.push(...getSpawnParameterArray(parameters));
43+
params.push(
44+
...convertOptionsToParams(parameters, newCommandLineParamFixes),
45+
);
3946
}
4047
params.push(...(additionalArguments ?? []));
4148
return this.logAndExecute(params);
@@ -72,8 +79,9 @@ export class DotNetClient {
7279
): void {
7380
const params = [`build`, project];
7481
if (parameters) {
75-
parameters = swapKeysUsingMap(parameters, buildKeyMap);
76-
params.push(...getSpawnParameterArray(parameters));
82+
params.push(
83+
...convertOptionsToParams(parameters, buildCommandLineParamFixes),
84+
);
7785
}
7886
if (extraParameters) {
7987
const matches = extraParameters.match(EXTRA_PARAMS_REGEX);
@@ -91,8 +99,9 @@ export class DotNetClient {
9199
? [`watch`, `--project`, project, `run`]
92100
: [`run`, `--project`, project];
93101
if (parameters) {
94-
parameters = swapKeysUsingMap(parameters, runKeyMap);
95-
params.push(...getSpawnParameterArray(parameters));
102+
params.push(
103+
...convertOptionsToParams(parameters, runCommandLineParamFixes),
104+
);
96105
}
97106

98107
return this.logAndSpawn(params);
@@ -109,8 +118,9 @@ export class DotNetClient {
109118
: [`test`, project];
110119

111120
if (parameters) {
112-
parameters = swapKeysUsingMap(parameters, testKeyMap);
113-
params.push(...getSpawnParameterArray(parameters));
121+
params.push(
122+
...convertOptionsToParams(parameters, testCommandLineParamFixes),
123+
);
114124
}
115125
if (extraParameters) {
116126
const matches = extraParameters.match(EXTRA_PARAMS_REGEX);
@@ -130,8 +140,9 @@ export class DotNetClient {
130140
): void {
131141
const params = [`add`, project, `package`, pkg];
132142
if (parameters) {
133-
parameters = swapKeysUsingMap(parameters, addPackageKeyMap);
134-
params.push(...getSpawnParameterArray(parameters));
143+
params.push(
144+
...convertOptionsToParams(parameters, addPackageCommandLineParamFixes),
145+
);
135146
}
136147
return this.logAndExecute(params);
137148
}
@@ -148,8 +159,9 @@ export class DotNetClient {
148159
): void {
149160
const params = [`publish`, `"${project}"`];
150161
if (parameters) {
151-
parameters = swapKeysUsingMap(parameters, publishKeyMap);
152-
params.push(...getSpawnParameterArray(parameters));
162+
params.push(
163+
...convertOptionsToParams(parameters, publishCommandLineParamFixes),
164+
);
153165
}
154166
if (publishProfile) {
155167
params.push(`-p:PublishProfile=${publishProfile}`);
@@ -216,8 +228,9 @@ export class DotNetClient {
216228
...parameters,
217229
};
218230
subcommandParams.push(
219-
...getSpawnParameterArray(
220-
swapKeysUsingMap(subcommandParameterObject, formatKeyMap),
231+
...convertOptionsToParams(
232+
subcommandParameterObject,
233+
formatCommandLineParamFixes,
221234
),
222235
);
223236
this.logAndExecute(subcommandParams);
@@ -233,8 +246,9 @@ export class DotNetClient {
233246
subcommandParameterObject.severity = style;
234247
}
235248
subcommandParams.push(
236-
...getSpawnParameterArray(
237-
swapKeysUsingMap(subcommandParameterObject, formatKeyMap),
249+
...convertOptionsToParams(
250+
subcommandParameterObject,
251+
formatCommandLineParamFixes,
238252
),
239253
);
240254
this.logAndExecute(subcommandParams);
@@ -250,8 +264,9 @@ export class DotNetClient {
250264
subcommandParameterObject.severity = analyzers;
251265
}
252266
subcommandParams.push(
253-
...getSpawnParameterArray(
254-
swapKeysUsingMap(subcommandParameterObject, formatKeyMap),
267+
...convertOptionsToParams(
268+
subcommandParameterObject,
269+
formatCommandLineParamFixes,
255270
),
256271
);
257272
this.logAndExecute(subcommandParams);
@@ -260,7 +275,7 @@ export class DotNetClient {
260275
params.push(project);
261276
if (parameters) {
262277
params.push(
263-
...getSpawnParameterArray(swapKeysUsingMap(parameters, formatKeyMap)),
278+
...convertOptionsToParams(parameters, formatCommandLineParamFixes),
264279
);
265280
}
266281
return this.logAndExecute(params);
Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { CommandLineParamFixes } from '@nx-dotnet/utils';
2+
13
export type dotnetAddPackageFlags =
24
| 'version'
35
| 'framework'
@@ -6,9 +8,11 @@ export type dotnetAddPackageFlags =
68
| 'noRestore'
79
| 'source';
810

9-
export const addPackageKeyMap: Partial<{
10-
[key in dotnetAddPackageFlags]: string;
11-
}> = {
12-
packageDirectory: 'package-directory',
13-
noRestore: 'no-restore',
14-
};
11+
export const addPackageCommandLineParamFixes: CommandLineParamFixes<dotnetAddPackageFlags> =
12+
{
13+
keyMap: {
14+
packageDirectory: 'package-directory',
15+
noRestore: 'no-restore',
16+
},
17+
explicitFalseKeys: [],
18+
};

packages/dotnet/src/lib/models/dotnet-build/dotnet-build-flags.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { CommandLineParamFixes } from '@nx-dotnet/utils';
2+
13
export type dotnetBuildFlags =
24
| 'configuration'
35
| 'framework'
@@ -12,9 +14,13 @@ export type dotnetBuildFlags =
1214
| 'versionSuffix'
1315
| 'runtime';
1416

15-
export const buildKeyMap: Partial<{ [key in dotnetBuildFlags]: string }> = {
16-
noRestore: 'no-restore',
17-
noIncremental: 'no-incremental',
18-
noDependencies: 'no-dependencies',
19-
versionSuffix: 'version-suffix',
20-
};
17+
export const buildCommandLineParamFixes: CommandLineParamFixes<dotnetBuildFlags> =
18+
{
19+
keyMap: {
20+
noRestore: 'no-restore',
21+
noIncremental: 'no-incremental',
22+
noDependencies: 'no-dependencies',
23+
versionSuffix: 'version-suffix',
24+
},
25+
explicitFalseKeys: [],
26+
};

0 commit comments

Comments
 (0)