Skip to content

Commit 5e51bb5

Browse files
authored
Merge pull request #2098 from github/koesie10/refactor-cli-arguments
Use options objects instead of multiple optional arguments in CLI methods
2 parents 396fdb8 + 0f90319 commit 5e51bb5

File tree

1 file changed

+73
-47
lines changed
  • extensions/ql-vscode/src

1 file changed

+73
-47
lines changed

extensions/ql-vscode/src/cli.ts

Lines changed: 73 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -124,14 +124,6 @@ export interface SourceInfo {
124124
*/
125125
export type ResolvedTests = string[];
126126

127-
/**
128-
* Options for `codeql test run`.
129-
*/
130-
export interface TestRunOptions {
131-
cancellationToken?: CancellationToken;
132-
logger?: Logger;
133-
}
134-
135127
/**
136128
* Event fired by `codeql test run`.
137129
*/
@@ -485,8 +477,13 @@ export class CodeQLCliServer implements Disposable {
485477
command: string[],
486478
commandArgs: string[],
487479
description: string,
488-
cancellationToken?: CancellationToken,
489-
logger?: Logger,
480+
{
481+
cancellationToken,
482+
logger,
483+
}: {
484+
cancellationToken?: CancellationToken;
485+
logger?: Logger;
486+
} = {},
490487
): AsyncGenerator<EventType, void, unknown> {
491488
for await (const event of this.runAsyncCodeQlCliCommandInternal(
492489
command,
@@ -519,8 +516,13 @@ export class CodeQLCliServer implements Disposable {
519516
command: string[],
520517
commandArgs: string[],
521518
description: string,
522-
progressReporter?: ProgressReporter,
523-
onLine?: OnLineCallback,
519+
{
520+
progressReporter,
521+
onLine,
522+
}: {
523+
progressReporter?: ProgressReporter;
524+
onLine?: OnLineCallback;
525+
} = {},
524526
): Promise<string> {
525527
if (progressReporter) {
526528
progressReporter.report({ message: description });
@@ -564,22 +566,25 @@ export class CodeQLCliServer implements Disposable {
564566
command: string[],
565567
commandArgs: string[],
566568
description: string,
567-
addFormat = true,
568-
progressReporter?: ProgressReporter,
569-
onLine?: OnLineCallback,
569+
{
570+
addFormat = true,
571+
progressReporter,
572+
onLine,
573+
}: {
574+
addFormat?: boolean;
575+
progressReporter?: ProgressReporter;
576+
onLine?: OnLineCallback;
577+
} = {},
570578
): Promise<OutputType> {
571579
let args: string[] = [];
572580
if (addFormat)
573581
// Add format argument first, in case commandArgs contains positional parameters.
574582
args = args.concat(["--format", "json"]);
575583
args = args.concat(commandArgs);
576-
const result = await this.runCodeQlCliCommand(
577-
command,
578-
args,
579-
description,
584+
const result = await this.runCodeQlCliCommand(command, args, description, {
580585
progressReporter,
581586
onLine,
582-
);
587+
});
583588
try {
584589
return JSON.parse(result) as OutputType;
585590
} catch (err) {
@@ -617,8 +622,13 @@ export class CodeQLCliServer implements Disposable {
617622
command: string[],
618623
commandArgs: string[],
619624
description: string,
620-
addFormat = true,
621-
progressReporter?: ProgressReporter,
625+
{
626+
addFormat,
627+
progressReporter,
628+
}: {
629+
addFormat?: boolean;
630+
progressReporter?: ProgressReporter;
631+
} = {},
622632
): Promise<OutputType> {
623633
const accessToken = await this.app.credentials.getExistingAccessToken();
624634

@@ -628,24 +638,26 @@ export class CodeQLCliServer implements Disposable {
628638
command,
629639
[...extraArgs, ...commandArgs],
630640
description,
631-
addFormat,
632-
progressReporter,
633-
async (line) => {
634-
if (line.startsWith("Enter value for --github-auth-stdin")) {
635-
try {
636-
return await this.app.credentials.getAccessToken();
637-
} catch (e) {
638-
// If the user cancels the authentication prompt, we still need to give a value to the CLI.
639-
// By giving a potentially invalid value, the user will just get a 401/403 when they try to access a
640-
// private package and the access token is invalid.
641-
// This code path is very rare to hit. It would only be hit if the user is logged in when
642-
// starting the command, then logging out before the getAccessToken() is called again and
643-
// then cancelling the authentication prompt.
644-
return accessToken;
641+
{
642+
addFormat,
643+
progressReporter,
644+
onLine: async (line) => {
645+
if (line.startsWith("Enter value for --github-auth-stdin")) {
646+
try {
647+
return await this.app.credentials.getAccessToken();
648+
} catch (e) {
649+
// If the user cancels the authentication prompt, we still need to give a value to the CLI.
650+
// By giving a potentially invalid value, the user will just get a 401/403 when they try to access a
651+
// private package and the access token is invalid.
652+
// This code path is very rare to hit. It would only be hit if the user is logged in when
653+
// starting the command, then logging out before the getAccessToken() is called again and
654+
// then cancelling the authentication prompt.
655+
return accessToken;
656+
}
645657
}
646-
}
647658

648-
return undefined;
659+
return undefined;
660+
},
649661
},
650662
);
651663
}
@@ -714,7 +726,9 @@ export class CodeQLCliServer implements Disposable {
714726
["resolve", "qlref"],
715727
subcommandArgs,
716728
"Resolving qlref",
717-
false,
729+
{
730+
addFormat: false,
731+
},
718732
);
719733
}
720734

@@ -742,7 +756,13 @@ export class CodeQLCliServer implements Disposable {
742756
public async *runTests(
743757
testPaths: string[],
744758
workspaces: string[],
745-
options: TestRunOptions,
759+
{
760+
cancellationToken,
761+
logger,
762+
}: {
763+
cancellationToken?: CancellationToken;
764+
logger?: Logger;
765+
},
746766
): AsyncGenerator<TestCompleted, void, unknown> {
747767
const subcommandArgs = this.cliConfig.additionalTestArguments.concat([
748768
...this.getAdditionalPacksArg(workspaces),
@@ -755,8 +775,10 @@ export class CodeQLCliServer implements Disposable {
755775
["test", "run"],
756776
subcommandArgs,
757777
"Run CodeQL Tests",
758-
options.cancellationToken,
759-
options.logger,
778+
{
779+
cancellationToken,
780+
logger,
781+
},
760782
)) {
761783
yield event;
762784
}
@@ -787,7 +809,9 @@ export class CodeQLCliServer implements Disposable {
787809
["resolve", "ml-models"],
788810
args,
789811
"Resolving ML models",
790-
false,
812+
{
813+
addFormat: false,
814+
},
791815
);
792816
}
793817

@@ -811,8 +835,9 @@ export class CodeQLCliServer implements Disposable {
811835
["resolve", "ram"],
812836
args,
813837
"Resolving RAM settings",
814-
true,
815-
progressReporter,
838+
{
839+
progressReporter,
840+
},
816841
);
817842
}
818843
/**
@@ -1229,12 +1254,13 @@ export class CodeQLCliServer implements Disposable {
12291254
async packAdd(dir: string, queryLanguage: QueryLanguage) {
12301255
const args = ["--dir", dir];
12311256
args.push(`codeql/${queryLanguage}-all`);
1232-
const addFormat = false;
12331257
return this.runJsonCodeQlCliCommandWithAuthentication(
12341258
["pack", "add"],
12351259
args,
12361260
`Adding and installing ${queryLanguage} pack dependency.`,
1237-
addFormat,
1261+
{
1262+
addFormat: false,
1263+
},
12381264
);
12391265
}
12401266

0 commit comments

Comments
 (0)