Repository: mheap/pin-github-action (js)

Issue:
Format YAML without conflict with `prettier`
Hi,
Would it be possible to output YAML in a way that doesn't clash with `prettier`?

`pin-github-actions` wants files formatted like this:

```diff
-    needs: [changed-files]
+    needs: [ changed-files ]
```

`prettier` formats them back like this:

```diff
-    needs: [ changed-files ]
+    needs: [changed-files]
```

Pull request:
Update CLI to not reformat input files
Closes #153

Adjust the implementation of the CLI to leverage the `actions` output list from the main library instead of the `workflow` output to rewrite the input file. In particular, this constructs are regular expression to find `uses:` directives to replace and replaces them (with a fixed format). The result is that the input file isn't reformatted, apart from the `uses:` directives.

Currently this is implemented in for the CLI only. It might make sense to lift this into the main library instead and use this rewrite strategy for the `workflow` output instead of using [`workflow.toString()`](https://github.com/mheap/pin-github-action/blob/8e271c1eb28e643569f502d4df589ce5d77add4e/index.js#L52).

Also note that this duplicates the logic for the default comment from [replaceActions.js](https://github.com/mheap/pin-github-action/blob/8e271c1eb28e643569f502d4df589ce5d77add4e/replaceActions.js#L93-L96). This should probably be addressed before this is merged, but I'm unsure how to resolve that within the current project structure.

Patch:
diff --git a/bin.js b/bin.js
index 8208815..290620f 100755
--- a/bin.js
+++ b/bin.js
@@ -27,16 +27,6 @@ const packageDetails = require(path.join(__dirname, "package.json"));
         "-e, --allow-empty",
         "allow workflows that do not contain any actions"
       )
-      .option(
-        "-l, --yaml-line-width <width>",
-        "set maximum output width before a line break",
-        "120"
-      )
-      .option(
-        "-n, --yaml-null-str <string>",
-        "set string representation for null values",
-        "null"
-      )
       .option(
         "-c, --comment <string>",
         "comment to add inline when pinning an action",
@@ -52,8 +42,6 @@ const packageDetails = require(path.join(__dirname, "package.json"));
     allowed = (allowed || "").split(",").filter((r) => r);
     let ignoreShas = program.opts().ignoreShas;
     let allowEmpty = program.opts().allowEmpty;
-    let yamlLineWidth = program.opts().yamlLineWidth;
-    let yamlNullStr = program.opts().yamlNullStr;
     let comment = program.opts().comment;
 
     for (const filename of program.args) {
@@ -70,8 +58,6 @@ const packageDetails = require(path.join(__dirname, "package.json"));
         ignoreShas,
         allowEmpty,
         debug,
-        yamlLineWidth,
-        yamlNullStr,
         comment
       );
       fs.writeFileSync(filename, output.workflow);
diff --git a/index.js b/index.js
index 1ca2aeb..4e24481 100644
--- a/index.js
+++ b/index.js
@@ -12,8 +12,6 @@ module.exports = async function (
   ignoreShas,
   allowEmpty,
   debug,
-  yamlLineWidth,
-  yamlNullStr,
   comment
 ) {
   allowed = allowed || [];
@@ -41,15 +39,11 @@ module.exports = async function (
     actions[i].newVersion = newVersion;
 
     // Rewrite each action, replacing the uses block with a specific sha
-    workflow = replaceActions(workflow, actions[i], comment);
+    workflow = replaceActions(input, actions[i], comment);
   }
 
-  stringOpts = {
-    lineWidth: yamlLineWidth,
-    nullStr: yamlNullStr,
-  };
   return {
-    workflow: workflow.toString(stringOpts),
+    workflow,
     actions,
   };
 };
diff --git a/package-lock.json b/package-lock.json
index d9f51a5..88d6f00 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -12,6 +12,7 @@
         "@octokit/rest": "^18",
         "commander": "^9",
         "debug": "^4.3.4",
+        "escape-string-regexp": "^4.0.0",
         "matcher": "^4.0.0",
         "yaml": "^2.1.3"
       },
@@ -2157,6 +2158,7 @@
       "version": "4.0.0",
       "resolved": "https://registry.npmjs.org/escape-string-regexp/-/escape-string-regexp-4.0.0.tgz",
       "integrity": "sha512-TtpcNJ3XAzx3Gq8sWRzJaVajRs0uVxA2YAkdb1jm2YkPz4G6egUFAyA3n5vtEIZefPk5Wa4UXbKuS5fKkJWdgA==",
+      "license": "MIT",
       "engines": {
         "node": ">=10"
       },
diff --git a/package.json b/package.json
index 2399a0c..dc77880 100644
--- a/package.json
+++ b/package.json
@@ -26,6 +26,7 @@
     "@octokit/rest": "^18",
     "commander": "^9",
     "debug": "^4.3.4",
+    "escape-string-regexp": "^4.0.0",
     "matcher": "^4.0.0",
     "yaml": "^2.1.3"
   },
diff --git a/replaceActions.js b/replaceActions.js
index 56e08b4..dce5d62 100644
--- a/replaceActions.js
+++ b/replaceActions.js
@@ -1,94 +1,38 @@
+const escapeStringRegexp = require("escape-string-regexp");
+
 module.exports = function (input, action, comment) {
-  let runs = input.contents.items.filter((n) => n.key == "runs");
-  if (runs.length) {
-    return replaceInComposite(input, action, comment);
-  }
-  return replaceInWorkflow(input, action, comment);
+  const { currentVersion, owner, newVersion, path, repo } = action;
+
+  const actionId = `${owner}/${repo}${path ? `/${path}` : ""}`;
+  const newComment = generateComment(action, comment);
+
+  const regexpUpdate = new RegExp(
+    `uses(\\s*):(\\s+)${escapeStringRegexp(actionId)}@${escapeStringRegexp(
+      currentVersion
+    )}(\\s*)#[^\\n]*`,
+    "g"
+  );
+
+  if (regexpUpdate.test(input)) {
+    return input.replace(
+      regexpUpdate,
+      `uses$1:$2${actionId}@${newVersion}$3#${newComment}`
+    );
+  }
+
+  const regexpPin = new RegExp(
+    `uses(\\s*):(\\s+)${escapeStringRegexp(actionId)}@${escapeStringRegexp(
+      currentVersion
+    )}`,
+    "g"
+  );
+
+  return input.replace(
+    regexpPin,
+    `uses$1:$2${actionId}@${newVersion} #${newComment}`
+  );
 };
 
-function replaceInReusable(input, action) {}
-
-function replaceInComposite(input, action, comment) {
-  let actionString = `${action.owner}/${action.repo}`;
-  if (action.path) {
-    actionString += `/${action.path}`;
-  }
-
-  const replacement = `${actionString}@${action.newVersion}`;
-
-  actionString += `@${action.currentVersion}`;
-
-  const runs = input.contents.items.filter((n) => n.key == "runs")[0].value
-    .items;
-  const steps = runs.filter((n) => n.key == "steps")[0].value.items;
-
-  for (let step of steps) {
-    const uses = step.items.filter((n) => n.key == "uses");
-    for (let use of uses) {
-      if (use.value.value == actionString) {
-        use.value.value = replacement;
-        use.value.comment = generateComment(action, comment);
-      }
-    }
-  }
-
-  return input;
-}
-
-function replaceInWorkflow(input, action, comment) {
-  let actionString = `${action.owner}/${action.repo}`;
-  if (action.path) {
-    actionString += `/${action.path}`;
-  }
-
-  const replacement = `${actionString}@${action.newVersion}`;
-
-  actionString += `@${action.currentVersion}`;
-
-  const jobs = input.contents.items.filter((n) => n.key == "jobs")[0].value
-    .items;
-
-  for (let job of jobs) {
-    const stepKeys = job.value.items.filter((n) => n.key == "steps");
-    const usesKeys = job.value.items.filter((n) => n.key == "uses");
-
-    if (stepKeys.length) {
-      replaceStandard(
-        stepKeys[0].value.items,
-        actionString,
-        replacement,
-        action,
-        comment
-      );
-    } else if (usesKeys.length) {
-      replaceReusable(usesKeys, actionString, replacement, action, comment);
-    }
-  }
-
-  return input;
-}
-
-function replaceReusable(uses, actionString, replacement, action, comment) {
-  for (let use of uses) {
-    if (use.value.value == actionString) {
-      use.value.value = replacement;
-      use.value.comment = generateComment(action, comment);
-    }
-  }
-}
-
-function replaceStandard(steps, actionString, replacement, action, comment) {
-  for (let step of steps) {
-    const uses = step.items.filter((n) => n.key == "uses");
-    for (let use of uses) {
-      if (use.value.value == actionString) {
-        use.value.value = replacement;
-        use.value.comment = generateComment(action, comment);
-      }
-    }
-  }
-}
-
 function generateComment(action, comment) {
   if (!comment) {
     comment = ` pin@{ref}`;


Test patch:
diff --git a/replaceActions.test.js b/replaceActions.test.js
index 75814a6..3149434 100644
--- a/replaceActions.test.js
+++ b/replaceActions.test.js
@@ -11,7 +11,7 @@ const action = {
 };
 
 test("replaces a single action with a sha (workflow)", () => {
-  const input = convertToAst({
+  const input = convertToYaml({
     name: "PR",
     on: ["pull_request"],
     jobs: {
@@ -27,13 +27,13 @@ test("replaces a single action with a sha (workflow)", () => {
     },
   });
 
-  const actual = replaceActions(input, { ...action }).toString();
+  const actual = replaceActions(input, { ...action });
 
   expect(actual).toContain("uses: mheap/test-action@sha-here # pin@master");
 });
 
 test("supports a custom comment format (workflow)", () => {
-  const input = convertToAst({
+  const input = convertToYaml({
     name: "PR",
     on: ["pull_request"],
     jobs: {
@@ -49,13 +49,13 @@ test("supports a custom comment format (workflow)", () => {
     },
   });
 
-  const actual = replaceActions(input, { ...action }, " {ref}").toString();
+  const actual = replaceActions(input, { ...action }, " {ref}");
 
   expect(actual).toContain("uses: mheap/test-action@sha-here # master");
 });
 
 test("replaces a single action with a sha (composite)", () => {
-  const input = convertToAst({
+  const input = convertToYaml({
     name: "Sample Composite",
     runs: {
       using: "composite",
@@ -68,13 +68,13 @@ test("replaces a single action with a sha (composite)", () => {
     },
   });
 
-  const actual = replaceActions(input, { ...action }).toString();
+  const actual = replaceActions(input, { ...action });
 
   expect(actual).toContain("uses: mheap/test-action@sha-here # pin@master");
 });
 
 test("supports a custom comment format (composite)", () => {
-  const input = convertToAst({
+  const input = convertToYaml({
     name: "Sample Composite",
     runs: {
       using: "composite",
@@ -87,13 +87,13 @@ test("supports a custom comment format (composite)", () => {
     },
   });
 
-  const actual = replaceActions(input, { ...action }, " {ref}").toString();
+  const actual = replaceActions(input, { ...action }, " {ref}");
 
   expect(actual).toContain("uses: mheap/test-action@sha-here # master");
 });
 
 test("replaces a single action with a sha (reusable)", () => {
-  const input = convertToAst({
+  const input = convertToYaml({
     name: "Sample Reusable",
     jobs: {
       test: {
@@ -108,7 +108,7 @@ test("replaces a single action with a sha (reusable)", () => {
 });
 
 test("supports a custom comment format (reusable)", () => {
-  const input = convertToAst({
+  const input = convertToYaml({
     name: "Sample Reusable",
     jobs: {
       test: {
@@ -117,13 +117,13 @@ test("supports a custom comment format (reusable)", () => {
     },
   });
 
-  const actual = replaceActions(input, { ...action }, " {ref}").toString();
+  const actual = replaceActions(input, { ...action }, " {ref}");
 
   expect(actual).toContain("uses: mheap/test-action@sha-here # master");
 });
 
 test("replaces an existing sha with a different sha, not changing the pinned branch", () => {
-  const input = convertToAst({
+  const input = convertToYaml({
     name: "PR",
     on: ["pull_request"],
     jobs: {
@@ -144,11 +144,191 @@ test("replaces an existing sha with a different sha, not changing the pinned bra
     pinnedVersion: "v1",
     currentVersion: "sha-one",
     newVersion: "sha-two",
-  }).toString();
+  });
+
+  expect(actual).toContain("uses: mheap/test-action@sha-two # pin@v1");
+});
+
+test("maintains formatting when adding a pin", () => {
+  const input = `name: PR
+on:
+  - pull_request
+jobs:
+  test-job:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: mheap/test-action@master
+        name: test where uses comes after the dash
+
+      - name: test with extra leading spaces
+        uses:   mheap/test-action@master
+
+      - name: test with space after 'uses'
+        uses : mheap/test-action@master
+
+      - name: no final newline
+        uses: mheap/test-action@master`;
+
+  const actual = replaceActions(input, { ...action });
+
+  expect(actual).toContain("- uses: mheap/test-action@sha-here # pin@master");
+  expect(actual).toContain("uses:   mheap/test-action@sha-here # pin@master");
+  expect(actual).toContain("uses : mheap/test-action@sha-here # pin@master");
+  expect(actual).toMatch(/uses: mheap\/test-action@sha-here # pin@master$/);
+});
+
+test("maintains formatting when updating a pin", () => {
+  const input = `name: PR
+on:
+  - pull_request
+jobs:
+  test-job:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: mheap/test-action@sha-one # pin@v1
+        name: test where uses comes after the dash
+
+      - name: test with extra leading spaces
+        uses:   mheap/test-action@sha-one # pin@v1
+
+      - name: test with space after 'uses'
+        uses : mheap/test-action@sha-one # pin@v1
+
+      - name: test with multiple spaces before the comment
+        uses: mheap/test-action@sha-one  # pin@v1
+
+      - name: test without space after the comment starts
+        uses: mheap/test-action@sha-one #pin@v1
+
+      - name: test without space before the comment starts
+        uses: mheap/test-action@sha-one# pin@v1
+
+      - name: test without space before or after the comment start
+        uses:  mheap/test-action@sha-one#pin@v1
+
+      - name: test with random comment
+        uses:  mheap/test-action@sha-one # foobar
+
+      - name: no final newline
+        uses: mheap/test-action@sha-one # pin@v1`;
+
+  const actual = replaceActions(input, {
+    ...action,
+    pinnedVersion: "v1",
+    currentVersion: "sha-one",
+    newVersion: "sha-two",
+  });
 
+  expect(actual).toContain("- uses: mheap/test-action@sha-two # pin@v1");
+  expect(actual).toContain("uses:   mheap/test-action@sha-two # pin@v1");
+  expect(actual).toContain("uses : mheap/test-action@sha-two # pin@v1");
+  expect(actual).toContain("uses: mheap/test-action@sha-two  # pin@v1");
   expect(actual).toContain("uses: mheap/test-action@sha-two # pin@v1");
+  expect(actual).toContain("uses: mheap/test-action@sha-two# pin@v1");
+  expect(actual).toContain("uses:  mheap/test-action@sha-two# pin@v1");
+  expect(actual).toContain("uses:  mheap/test-action@sha-two # pin@v1");
+  expect(actual).toMatch(/uses: mheap\/test-action@sha-two # pin@v1$/);
+});
+
+test("maintains indentation when adding a pin", () => {
+  let input = `name: PR
+on:
+  - pull_request
+jobs:
+  test-job:
+    runs-on: ubuntu-latest
+    steps:
+      - name: test indentation #1
+        uses: mheap/test-action@master
+`;
+
+  let actual = replaceActions(input, { ...action });
+  expect(actual).toContain(
+    "        uses: mheap/test-action@sha-here # pin@master"
+  );
+
+  input = `name: PR
+on:
+- pull_request
+jobs:
+  test-job:
+    runs-on: ubuntu-latest
+    steps:
+    - name: test indentation #2
+      uses: mheap/test-action@master
+`;
+
+  actual = replaceActions(input, { ...action });
+  expect(actual).toContain(
+    "      uses: mheap/test-action@sha-here # pin@master"
+  );
+});
+
+test("maintains indentation when updating a pin", () => {
+  let input = `name: PR
+on:
+  - pull_request
+jobs:
+  test-job:
+    runs-on: ubuntu-latest
+    steps:
+      - name: test indentation #1
+        uses: mheap/test-action@sha-one # pin@v1
+`;
+
+  let actual = replaceActions(input, {
+    ...action,
+    pinnedVersion: "v1",
+    currentVersion: "sha-one",
+    newVersion: "sha-two",
+  });
+  expect(actual).toContain("        uses: mheap/test-action@sha-two # pin@v1");
+
+  input = `name: PR
+on:
+- pull_request
+jobs:
+  test-job:
+    runs-on: ubuntu-latest
+    steps:
+    - name: test indentation #2
+      uses: mheap/test-action@sha-one # pin@v1
+`;
+
+  actual = replaceActions(input, {
+    ...action,
+    pinnedVersion: "v1",
+    currentVersion: "sha-one",
+    newVersion: "sha-two",
+  });
+  expect(actual).toContain("      uses: mheap/test-action@sha-two # pin@v1");
+});
+
+test("handles RegExp meta-characters", () => {
+  let input = `name: PR
+on:
+  - pull_request
+jobs:
+  test-job:
+    runs-on: ubuntu-latest
+    steps:
+      - name: test indentation #1
+        uses: mheap/test-action@sha-one # pin@v1
+`;
+
+  expect(() => {
+    replaceActions(input, {
+      ...action,
+      owner: "a[b",
+      repo: "c[d",
+      path: "e[f",
+      pinnedVersion: "v1",
+      currentVersion: "sha[one",
+      newVersion: "sha[two",
+    });
+  }).not.toThrow();
 });
 
-function convertToAst(input) {
-  return YAML.parseDocument(YAML.stringify(input));
+function convertToYaml(input) {
+  return YAML.stringify(input);
 }