diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 42126c5..40180ee 100644 --- a/.github/workflows/build.yml +++ b/.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: diff --git a/README.md b/README.md index 7a9666f..c9c042f 100644 --- a/README.md +++ b/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) +``` diff --git a/package.json b/package.json index b8f946a..af76b03 100644 --- a/package.json +++ b/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" }, diff --git a/src/GitHubClient.js b/src/GitHubClient.js index 0859e57..022432e 100644 --- a/src/GitHubClient.js +++ b/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); }, []); diff --git a/src/Helpers.js b/src/Helpers.js new file mode 100644 index 0000000..c1dee53 --- /dev/null +++ b/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; + }, +}; diff --git a/tests/makeComment.test.js b/tests/makeComment.test.js index d5a7e30..15cee57 100644 --- a/tests/makeComment.test.js +++ b/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,8 +121,8 @@ describe("makeComment", () => { `-#include "Common/Algorithm.h"`, " ", " using namespace std::literals::string_view_literals;", - " ", - ].join("\n") + " " + ) ); expect(makeComment(to, chunk)).toEqual({ path: "src/Config.cpp", @@ -131,16 +130,15 @@ describe("makeComment", () => { 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"), + "```" + ), }); }); }); diff --git a/tests/suggest.test.js b/tests/suggest.test.js index 0562e4c..d1e12d0 100644 --- a/tests/suggest.test.js +++ b/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(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,13 +104,12 @@ 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", @@ -83,12 +117,11 @@ const FIXTURE_UNIDIFF_PAYLOAD = { side: "RIGHT", start_line: 53, start_side: "RIGHT", - body: [ + body: concatStrings( "```suggestion", " explicit operator bool() const { return static_cast(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(); diff --git a/tests/trimQuotes.test.js b/tests/trimQuotes.test.js new file mode 100644 index 0000000..7469e13 --- /dev/null +++ b/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"); + }); +});