Skip to content

Commit

Permalink
fix(build-tools): Windows compatible clean policy (#17874)
Browse files Browse the repository at this point in the history
single quotes `'` become part of argument on Windows
- policy: require double quotes `"` for globs
- fix resolver logic and new quote policy

add generalized argument validation helpers
  • Loading branch information
jason-ha authored Oct 25, 2023
1 parent ea265c9 commit a1fb4e8
Showing 1 changed file with 65 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,61 @@ function ensurePrivatePackagesComputed(): void {
});
}

/**
* Parses command line string into arguments following OS quote rules
*
* @param commandLine - complete command line as a simple string
* @param onlyDoubleQuotes - only consider double quotes for grouping
* @returns array of ordered pairs of resolved `arg` strings (quotes and escapes resolved)
* and `original` corresponding strings.
*/
function parseArgs(commandLine: string, { onlyDoubleQuotes }: { onlyDoubleQuotes: boolean }) {
const regexArg = onlyDoubleQuotes
? /(?<!\S)(?:[^\s"\\]|\\\S)*(?:(").*?(?:(?<=[^\\](?:\\\\)*)\1(?:[^\s"\\]|\\\S)*|$))*(?!\S)/g
: /(?<!\S)(?:[^\s'"\\]|\\\S)*(?:(['"]).*?(?:(?<=[^\\](?:\\\\)*)\1(?:[^\s'"\\]|\\\S)*|$))*(?!\S)/g;
const regexQuotedSegment = onlyDoubleQuotes
? /(?:^|(?<=(?:[^\\]|^)(?:\\\\)*))(")(.*?)(?:(?<=[^\\](?:\\\\)*)\1|$)/g
: /(?:^|(?<=(?:[^\\]|^)(?:\\\\)*))(['"])(.*?)(?:(?<=[^\\](?:\\\\)*)\1|$)/g;
const regexEscapedCharacters = onlyDoubleQuotes ? /\\([\\"])/g : /\\([\\'"])/g;
return [...commandLine.matchAll(regexArg)].map((matches) => ({
arg: matches[0].replace(regexQuotedSegment, "$2").replace(regexEscapedCharacters, "$1"),
original: matches[0],
}));
}

/**
* Applies universally understood grouping and escaping to form a single argument
* text to be used as part of a command line string.
*
* @param resolvedArg - string as it should appear to new process
* @returns preferred string to use within a command line string
*/
function quoteAndEscapeArgsForUniversalCommandLine(resolvedArg: string) {
// Unix shells provide feature where arguments that can be resolved as globs
// are expanded before passed to new process. Detect those and group them
// to ensure consistent arg passing. (Grouping disables glob expansion.)
const regexGlob = /[*?[()]/;
const notAGlob = resolvedArg.startsWith("-") || !regexGlob.test(resolvedArg);
// Double quotes are used for grouping arguments with whitespace and must be
// escaped with \ and \ itself must therefore be escaped.
// Unix shells also use single quotes for grouping. Rather than escape those,
// which Windows command shell would not unescape, those args must be grouped.
const escapedArg = resolvedArg.replace(/([\\"])/g, "\\$1");
return notAGlob && !/[\s']/.test(resolvedArg) ? escapedArg : `"${escapedArg}"`;
}

/**
* Parse command line as if unix shell, then form preferred command line
*
* @param commandLine - unparsed command line
* @returns preferred command line
*/
function getPreferredCommandLine(commandLine: string) {
return parseArgs(commandLine, { onlyDoubleQuotes: false })
.map((a) => quoteAndEscapeArgsForUniversalCommandLine(a.arg))
.join(" ");
}

let privatePackages: Set<string>;

const match = /(^|\/)package\.json/i;
Expand Down Expand Up @@ -1097,31 +1152,28 @@ export const handlers: Handler[] = [
.join("")}`;
}

const clean = scripts["clean"];
if (clean && clean.startsWith("rimraf ")) {
if (clean.includes('"')) {
return "'clean' script using double quotes instead of single quotes";
}

if (!clean.includes("'")) {
return "'clean' script rimraf argument should have single quotes";
if (cleanScript) {
if (cleanScript !== getPreferredCommandLine(cleanScript)) {
return "'clean' script should double quote the globs and only the globs";
}
}
},
resolver: (file) => {
const result: { resolved: boolean; message?: string } = { resolved: true };
updatePackageJsonFile(path.dirname(file), (json) => {
const missing = missingCleanDirectories(json.scripts);
const clean = json.scripts["clean"] ?? "rimraf --glob";
if (clean.startsWith("rimraf --glob")) {
let clean: string = json.scripts["clean"] ?? "rimraf --glob";
if (!clean.startsWith("rimraf --glob")) {
result.resolved = false;
result.message =
"Unable to fix 'clean' script that doesn't start with 'rimraf --glob'";
}
if (missing.length === 0) {
return;
}
json.scripts["clean"] = `${clean} ${missing.map((name) => `'${name}'`).join(" ")}`;
if (missing.length > 0) {
clean += ` ${missing.join(" ")}`;
}
// clean up for grouping
json.scripts["clean"] = getPreferredCommandLine(clean);
});

return result;
Expand Down

0 comments on commit a1fb4e8

Please sign in to comment.