Skip to content

Commit

Permalink
fix: consolidate toolchain building
Browse files Browse the repository at this point in the history
  • Loading branch information
kristof-mattei committed Nov 18, 2023
1 parent 9381f01 commit 52ca270
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 35 deletions.
38 changes: 22 additions & 16 deletions dist/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -91162,25 +91162,24 @@ async function buildContext(program, toolchain) {
clippy: "",
rustc: "",
};
toolchain = `+${toolchain}` ?? 0;
await Promise.all([
await exec.exec("rustc", [toolchain, "-V"], {
await exec.exec("rustc", buildToolchainArguments(toolchain, ["-V"]), {
silent: false,
listeners: {
stdout: (buffer) => {
return (context.rustc = buffer.toString().trim());
},
},
}),
await program.call([toolchain, "-V"], {
await program.call(buildToolchainArguments(toolchain, ["-V"]), {
silent: false,
listeners: {
stdout: (buffer) => {
return (context.cargo = buffer.toString().trim());
},
},
}),
await program.call([toolchain, "clippy", "-V"], {
await program.call(buildToolchainArguments(toolchain, ["clippy", "-V"]), {
silent: false,
listeners: {
stdout: (buffer) => {
Expand All @@ -91192,7 +91191,7 @@ async function buildContext(program, toolchain) {
return context;
}
async function runClippy(actionInput, program) {
const args = buildArgs(actionInput);
const args = buildClippyArguments(actionInput);
const outputParser = new outputParser_1.OutputParser();
const options = {
ignoreReturnCode: true,
Expand Down Expand Up @@ -91238,18 +91237,25 @@ async function run(actionInput) {
}
}
exports.run = run;
function buildArgs(actionInput) {
function buildToolchainArguments(toolchain, after) {
const args = [];
// Toolchain selection MUST go first in any condition
if (actionInput.toolchain) {
args.push(`+${actionInput.toolchain}`);
}
args.push("clippy");
// `--message-format=json` should just right after the `cargo clippy`
// because usually people are adding the `-- -D warnings` at the end
// of arguments and it will mess up the output.
args.push("--message-format=json");
return args.concat(actionInput.args);
if (toolchain) {
args.push(`+${toolchain}`);
}
args.push(...after);
return args;
}
function buildClippyArguments(actionInput) {
// Toolchain selection MUST go first in any condition!
return buildToolchainArguments(actionInput.toolchain, [
"clippy",
// `--message-format=json` should just right after the `cargo clippy`
// because usually people are adding the `-- -D warnings` at the end
// of arguments and it will mess up the output.
"--message-format=json",
// and the rest
...actionInput.args,
]);
}


Expand Down
2 changes: 1 addition & 1 deletion dist/index.js.map

Large diffs are not rendered by default.

41 changes: 24 additions & 17 deletions src/clippy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,26 +24,24 @@ async function buildContext(program: Program, toolchain: string | undefined): Pr
rustc: "",
};

toolchain = `+${toolchain}` ?? "";

await Promise.all([
await exec.exec("rustc", [toolchain, "-V"], {
await exec.exec("rustc", buildToolchainArguments(toolchain, ["-V"]), {
silent: false,
listeners: {
stdout: (buffer: Buffer) => {
return (context.rustc = buffer.toString().trim());
},
},
}),
await program.call([toolchain, "-V"], {
await program.call(buildToolchainArguments(toolchain, ["-V"]), {
silent: false,
listeners: {
stdout: (buffer: Buffer) => {
return (context.cargo = buffer.toString().trim());
},
},
}),
await program.call([toolchain, "clippy", "-V"], {
await program.call(buildToolchainArguments(toolchain, ["clippy", "-V"]), {
silent: false,
listeners: {
stdout: (buffer: Buffer) => {
Expand All @@ -57,7 +55,7 @@ async function buildContext(program: Program, toolchain: string | undefined): Pr
}

async function runClippy(actionInput: input.ParsedInput, program: Program): Promise<ClippyResult> {
const args = buildArgs(actionInput);
const args = buildClippyArguments(actionInput);
const outputParser = new OutputParser();

const options: exec.ExecOptions = {
Expand Down Expand Up @@ -112,20 +110,29 @@ export async function run(actionInput: input.ParsedInput): Promise<void> {
}
}

function buildArgs(actionInput: input.ParsedInput): string[] {
const args: string[] = [];
function buildToolchainArguments(toolchain: string | undefined, after: string[]): string[] {
const args = [];

// Toolchain selection MUST go first in any condition
if (actionInput.toolchain) {
args.push(`+${actionInput.toolchain}`);
if (toolchain) {
args.push(`+${toolchain}`);
}

args.push("clippy");
args.push(...after);

return args;
}

// `--message-format=json` should just right after the `cargo clippy`
// because usually people are adding the `-- -D warnings` at the end
// of arguments and it will mess up the output.
args.push("--message-format=json");
function buildClippyArguments(actionInput: input.ParsedInput): string[] {
// Toolchain selection MUST go first in any condition!
return buildToolchainArguments(actionInput.toolchain, [
"clippy",

return args.concat(actionInput.args);
// `--message-format=json` should just right after the `cargo clippy`
// because usually people are adding the `-- -D warnings` at the end
// of arguments and it will mess up the output.
"--message-format=json",

// and the rest
...actionInput.args,
]);
}
29 changes: 28 additions & 1 deletion src/tests/clippy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,33 @@ describe("clippy", () => {
await expect(run(actionInput)).rejects.toThrow(/Clippy had exited with the (\d)+ exit code/);
});

it("records versions with toolchain", async () => {
const reportSpy = jest.spyOn(Reporter.prototype, "report");
jest.spyOn(exec, "exec").mockImplementation((commandline: string, args?: string[], options?: exec.ExecOptions) => {
if (commandline.endsWith("cargo")) {
if (args?.[0] === "+nightly" && args?.[1] === "-V") {
options?.listeners?.stdout?.(Buffer.from("cargo version"));
} else if (args?.[0] === "+nightly" && args?.[1] === "clippy" && args?.[2] === "-V") {
options?.listeners?.stdout?.(Buffer.from("clippy version"));
}
} else if (commandline === "rustc" && args?.[0] === "+nightly" && args?.[1] === "-V") {
options?.listeners?.stdout?.(Buffer.from("rustc version"));
}
return Promise.resolve(0);
});

const actionInput: ParsedInput = {
toolchain: "nightly",
args: [],
useCross: false,
workingDirectory: undefined,
};

await expect(run(actionInput)).resolves.toBeUndefined();

expect(reportSpy).toBeCalledWith({ error: 0, help: 0, ice: 0, note: 0, warning: 0 }, [], { cargo: "cargo version", clippy: "clippy version", rustc: "rustc version" });
});

it("records versions", async () => {
const reportSpy = jest.spyOn(Reporter.prototype, "report");
jest.spyOn(exec, "exec").mockImplementation((commandline: string, args?: string[], options?: exec.ExecOptions) => {
Expand All @@ -78,7 +105,7 @@ describe("clippy", () => {
});

const actionInput: ParsedInput = {
toolchain: "stable",
toolchain: undefined,
args: [],
useCross: false,
workingDirectory: undefined,
Expand Down

0 comments on commit 52ca270

Please sign in to comment.