fix: Handle diffs that were piped to diff (#8)

.github/workflows/build.yml

@@ -8,12 +8,12 @@ jobs:
   build:
     runs-on: ubuntu-latest
     steps:
-      - name: Checkout
-        uses: actions/checkout@v2
       - name: Set up Node.js
         uses: actions/setup-node@v1
         with:
           node-version: 12
+      - name: Checkout
+        uses: actions/checkout@v2
       - name: Cache /node_modules
         uses: actions/cache@v2
         with:

README.md

@@ -62,3 +62,22 @@ to format only changed files:
 curl --silent --show-error --remote-name https://raw.githubusercontent.com/llvm/llvm-project/release/10.x/clang/tools/clang-format/clang-format-diff.py
 yarn suggestion-bot "$(git diff --unified=0 --no-color @^ | python clang-format-diff.py -p1 -sort-includes)"
 ```
+
+## Using `suggestion-bot` with Prettier
+
+We must first write a script that pipes [Prettier](https://prettier.io/)'s
+output to `diff` so we can feed it to `suggestion-bot` later.
+
+```sh
+#!/bin/sh
+for f in $(yarn --silent prettier --list-different $(git ls-files '*.js')); do
+  yarn --silent prettier "$f" | diff -u "$f" -
+done
+```
+
+Save the script somewhere, e.g. `scripts/prettier-diff.sh`, then use it as
+follows:
+
+```sh
+yarn suggestion-bot $(scripts/prettier-diff.sh)
+```

package.json

@@ -23,7 +23,7 @@
   ],
   "homepage": "https://github.com/tido64/suggestion-bot",
   "scripts": {
-    "format": "prettier --write $(git ls-files '*.js' '*.yml')",
+    "format": "prettier --end-of-line auto --write $(git ls-files '*.js' '*.yml')",
     "lint": "eslint $(git ls-files '*.js')",
     "test": "jest"
   },

src/GitHubClient.js

@@ -5,6 +5,8 @@
 // LICENSE file in the root directory of this source tree.
 //
 
+const { trimQuotes } = require("./Helpers");
+
 /** @typedef {{ auth: string; }} Options */
 
 /**
@@ -87,12 +89,12 @@ function makeReview(diff, options) {
   }
 
   const comments = files.reduce((comments, file) => {
-    const { chunks, to } = file;
+    const { chunks, from, to } = file;
     if (chunks.length === 0) {
       return comments;
     }
     return chunks.reduce((comments, chunk) => {
-      comments.push(makeComment(to, chunk));
+      comments.push(makeComment(to === "-" ? trimQuotes(from) : to, chunk));
       return comments;
     }, comments);
   }, []);

src/Helpers.js

@@ -0,0 +1,25 @@
+module.exports = {
+  /**
+   * Concatenates specified strings.
+   * @param {string[]} strings
+   * @returns {string}
+   */
+  concatStrings: (...strings) => {
+    return strings.concat("").join("\n");
+  },
+
+  /**
+   * Trims specified string for wrapping quotation marks.
+   * @param {string} p
+   * @returns {string}
+   */
+  trimQuotes: (p) => {
+    if (
+      (p.startsWith('"') && p.endsWith('"')) ||
+      (p.startsWith("'") && p.endsWith("'"))
+    ) {
+      return p.slice(1, p.length - 1);
+    }
+    return p;
+  },
+};

tests/makeComment.test.js

@@ -8,6 +8,7 @@
 
 const parse = require("parse-diff");
 const { makeComment } = require("../src/GitHubClient");
+const { concatStrings } = require("../src/Helpers");
 
 function extractChunk(diff) {
   const { chunks, to } = parse(diff)[0];
@@ -49,7 +50,7 @@ describe("makeComment", () => {
 
   test("single line change with context", () => {
     const [to, chunk] = extractChunk(
-      [
+      concatStrings(
         "diff --git a/src/Common/Algorithm.h b/src/Common/Algorithm.h",
         "index d30d6e11..d138ef04 100644",
         "--- a/src/Common/Algorithm.h",
@@ -62,50 +63,48 @@ describe("makeComment", () => {
         "+    void quick_erase(T& container, typename T::iterator pos)",
         "     {",
         "         std::swap(*pos, container.back());",
-        "         container.pop_back();",
-      ].join("\n")
+        "         container.pop_back();"
+      )
     );
     expect(makeComment(to, chunk)).toEqual({
       path: "src/Common/Algorithm.h",
       line: 136,
       side: "RIGHT",
-      body: [
+      body: concatStrings(
         "```suggestion",
         "    void quick_erase(T& container, typename T::iterator pos)",
-        "```",
-        "",
-      ].join("\n"),
+        "```"
+      ),
     });
   });
 
   test("single line change without context", () => {
     const [to, chunk] = extractChunk(
-      [
+      concatStrings(
         "diff --git a/src/Common/Algorithm.h b/src/Common/Algorithm.h",
         "index d30d6e11..d138ef04 100644",
         "--- a/src/Common/Algorithm.h",
         "+++ b/src/Common/Algorithm.h",
         "@@ -136 +137 @@ namespace rainbow",
         "-    void quick_erase(T &container, typename T::iterator pos)",
-        "+    void quick_erase(T& container, typename T::iterator pos)",
-      ].join("\n")
+        "+    void quick_erase(T& container, typename T::iterator pos)"
+      )
     );
     expect(makeComment(to, chunk)).toEqual({
       path: "src/Common/Algorithm.h",
       line: 136,
       side: "RIGHT",
-      body: [
+      body: concatStrings(
         "```suggestion",
         "    void quick_erase(T& container, typename T::iterator pos)",
-        "```",
-        "",
-      ].join("\n"),
+        "```"
+      ),
     });
   });
 
   test("moved lines", () => {
     const [to, chunk] = extractChunk(
-      [
+      concatStrings(
         "diff --git a/src/Config.cpp b/src/Config.cpp",
         "index d0a84e17..c73dd760 100644",
         "--- a/src/Config.cpp",
@@ -122,25 +121,24 @@ describe("makeComment", () => {
         `-#include "Common/Algorithm.h"`,
         " ",
         " using namespace std::literals::string_view_literals;",
-        " ",
-      ].join("\n")
+        " "
+      )
     );
     expect(makeComment(to, chunk)).toEqual({
       path: "src/Config.cpp",
       line: 19,
       side: "RIGHT",
       start_line: 15,
       start_side: "RIGHT",
-      body: [
+      body: concatStrings(
         "```suggestion",
         `#include "Common/Algorithm.h"`,
         `#include "Common/Data.h"`,
         `#include "Common/Logging.h"`,
         `#include "FileSystem/File.h"`,
         `#include "FileSystem/FileSystem.h"`,
-        "```",
-        "",
-      ].join("\n"),
+        "```"
+      ),
     });
   });
 });

tests/suggest.test.js

@@ -9,6 +9,7 @@
 jest.mock("fs");
 
 const { makeReview } = require("../src/GitHubClient");
+const { concatStrings } = require("../src/Helpers");
 
 /**
  * @typedef {{ auth: string; }} Options
@@ -22,7 +23,41 @@ const { makeReview } = require("../src/GitHubClient");
    }} Review
  */
 
-const FIXTURE_UNIDIFF = [
+const FIXTURE_PIPED = concatStrings(
+  `--- "src/GitHubClient.js"      2020-07-26 20:24:35.497737700 +0200`,
+  "+++ -   2020-07-26 20:25:43.893994400 +0200",
+  "@@ -92,7 +92,7 @@",
+  "       return comments;",
+  "     }",
+  "     return chunks.reduce((comments, chunk) => {",
+  "-      comments.push(makeComment(to === '-' ? from : to, chunk));",
+  `+      comments.push(makeComment(to === "-" ? from : to, chunk));`,
+  "       return comments;",
+  "     }, comments);",
+  "   }, []);"
+);
+
+const FIXTURE_PIPED_PAYLOAD = {
+  accept: "application/vnd.github.comfort-fade-preview+json",
+  owner: "tido64",
+  repo: "suggestion-bot",
+  pull_number: 0,
+  event: "COMMENT",
+  comments: [
+    {
+      path: "src/GitHubClient.js",
+      line: 95,
+      side: "RIGHT",
+      body: concatStrings(
+        "```suggestion",
+        `      comments.push(makeComment(to === "-" ? from : to, chunk));`,
+        "```"
+      ),
+    },
+  ],
+};
+
+const FIXTURE_UNIDIFF = concatStrings(
   "diff --git a/src/Graphics/TextureAllocator.gl.h b/src/Graphics/TextureAllocator.gl.h",
   "index 366b30f7..f17e3c88 100644",
   "--- a/src/Graphics/TextureAllocator.gl.h",
@@ -53,8 +88,8 @@ const FIXTURE_UNIDIFF = [
   "+        explicit operator bool() const { return static_cast<bool>(array_); }",
   " ",
   "     private:",
-  " #ifdef USE_VERTEX_ARRAY_OBJECT",
-].join("\n");
+  " #ifdef USE_VERTEX_ARRAY_OBJECT"
+);
 
 const FIXTURE_UNIDIFF_PAYLOAD = {
   accept: "application/vnd.github.comfort-fade-preview+json",
@@ -69,26 +104,24 @@ const FIXTURE_UNIDIFF_PAYLOAD = {
       side: "RIGHT",
       start_line: 21,
       start_side: "RIGHT",
-      body: [
+      body: concatStrings(
         "```suggestion",
         "        [[maybe_unused, nodiscard]] auto max_size() const noexcept",
         "            -> size_t override;",
-        "```",
-        "",
-      ].join("\n"),
+        "```"
+      ),
     },
     {
       path: "src/Graphics/VertexArray.h",
       line: 56,
       side: "RIGHT",
       start_line: 53,
       start_side: "RIGHT",
-      body: [
+      body: concatStrings(
         "```suggestion",
         "        explicit operator bool() const { return static_cast<bool>(array_); }",
-        "```",
-        "",
-      ].join("\n"),
+        "```"
+      ),
     },
   ],
 };
@@ -139,6 +172,17 @@ describe("suggest", () => {
     expect(payload).toEqual(FIXTURE_UNIDIFF_PAYLOAD);
   });
 
+  test("supports piped diffs", async () => {
+    let payload = undefined;
+    await makeReview(FIXTURE_PIPED, {
+      createReview: (review) => {
+        payload = review;
+        return Promise.resolve();
+      },
+    });
+    expect(payload).toEqual(FIXTURE_PIPED_PAYLOAD);
+  });
+
   test("dumps the payload on failure", async () => {
     const dirSpy = jest.spyOn(global.console, "dir").mockImplementation();
     const errorSpy = jest.spyOn(global.console, "error").mockImplementation();

tests/trimQuotes.test.js

@@ -0,0 +1,17 @@
+const { trimQuotes } = require("../src/Helpers");
+
+describe("trimQuotes", () => {
+  test("trims only leading and trailing quotes", () => {
+    expect(trimQuotes('""')).toBe("");
+    expect(trimQuotes('"""')).toBe('"');
+    expect(trimQuotes('""""')).toBe('""');
+    expect(trimQuotes('"str"ing"')).toBe('str"ing');
+    expect(trimQuotes('str"ing')).toBe('str"ing');
+
+    expect(trimQuotes("''")).toBe("");
+    expect(trimQuotes("'''")).toBe("'");
+    expect(trimQuotes("''''")).toBe("''");
+    expect(trimQuotes("'str'ing'")).toBe("str'ing");
+    expect(trimQuotes("str'ing")).toBe("str'ing");
+  });
+});