From 53b59d2d39fd38079a587c7ce7e7e62cc1cbb591 Mon Sep 17 00:00:00 2001 From: Georgii Dolzhykov Date: Thu, 28 Jan 2021 00:29:34 +0200 Subject: [PATCH] Fix lineSuffixBoundary (#10122) --- changelog_unreleased/api/10122.md | 34 +++++++++++++++ src/document/doc-printer.js | 25 +++++++++-- .../__tests__/line-suffix-boundary.js | 41 +++++++++++++++++++ tests_integration/printDoc.js | 30 ++++++++++++++ 4 files changed, 126 insertions(+), 4 deletions(-) create mode 100644 changelog_unreleased/api/10122.md create mode 100644 tests_integration/__tests__/line-suffix-boundary.js create mode 100644 tests_integration/printDoc.js diff --git a/changelog_unreleased/api/10122.md b/changelog_unreleased/api/10122.md new file mode 100644 index 000000000..5cc6e6219 --- /dev/null +++ b/changelog_unreleased/api/10122.md @@ -0,0 +1,34 @@ +#### Fix `lineSuffixBoundary` (#10122 by @thorn0) + +There was a bug in the implementation of the [`lineSuffixBoundary`][lsb] command that significantly limited its usefulness: the printer algorithm didn't correctly consider it a potential line break. Now that the bug has been fixed, we urge plugin authors to give this command another try and see if it can help them simplify printing of trailing comments. + +[lsb]: https://github.com/prettier/prettier/blob/main/commands.md#linesuffixboundary + + +```jsx +// Input +group([ + "let foo = [", + indent([ + softline, + [lineSuffixBoundary, "item1,"], + line, + [lineSuffixBoundary, "item2,", lineSuffix(" // comment")], + line, + [lineSuffixBoundary, "item3"], + ]), + softline, + "];", +]) + +// Prettier stable +let foo = [item1, item2, // comment + item3]; + +// Prettier main +let foo = [ + item1, + item2, // comment + item3 +]; +``` diff --git a/src/document/doc-printer.js b/src/document/doc-printer.js index 39d8efdea..5e2558f1a 100644 --- a/src/document/doc-printer.js +++ b/src/document/doc-printer.js @@ -144,7 +144,7 @@ function trim(out) { return trimCount; } -function fits(next, restCommands, width, options, mustBeFlat) { +function fits(next, restCommands, width, options, hasLineSuffix, mustBeFlat) { let restIdx = restCommands.length; const cmds = [next]; // `out` is only used for width counting because `trim` requires to look @@ -237,6 +237,14 @@ function fits(next, restCommands, width, options, mustBeFlat) { return true; } break; + case "line-suffix": + hasLineSuffix = true; + break; + case "line-suffix-boundary": + if (hasLineSuffix) { + return false; + } + break; } } } @@ -306,8 +314,9 @@ function printDocToString(doc, options) { const next = [ind, MODE_FLAT, doc.contents]; const rem = width - pos; + const hasLineSuffix = lineSuffix.length > 0; - if (!doc.break && fits(next, cmds, rem, options)) { + if (!doc.break && fits(next, cmds, rem, options, hasLineSuffix)) { cmds.push(next); } else { // Expanded states are a rare case where a document @@ -335,7 +344,7 @@ function printDocToString(doc, options) { const state = doc.expandedStates[i]; const cmd = [ind, MODE_FLAT, state]; - if (fits(cmd, cmds, rem, options)) { + if (fits(cmd, cmds, rem, options, hasLineSuffix)) { cmds.push(cmd); break; @@ -387,7 +396,14 @@ function printDocToString(doc, options) { const [content, whitespace] = parts; const contentFlatCmd = [ind, MODE_FLAT, content]; const contentBreakCmd = [ind, MODE_BREAK, content]; - const contentFits = fits(contentFlatCmd, [], rem, options, true); + const contentFits = fits( + contentFlatCmd, + [], + rem, + options, + lineSuffix.length > 0, + true + ); if (parts.length === 1) { if (contentFits) { @@ -430,6 +446,7 @@ function printDocToString(doc, options) { [], rem, options, + lineSuffix.length > 0, true ); diff --git a/tests_integration/__tests__/line-suffix-boundary.js b/tests_integration/__tests__/line-suffix-boundary.js new file mode 100644 index 000000000..a420c561f --- /dev/null +++ b/tests_integration/__tests__/line-suffix-boundary.js @@ -0,0 +1,41 @@ +"use strict"; + +/** @type {import('prettier')} */ +const prettier = require("prettier-local"); + +const { + group, + indent, + line, + lineSuffix, + lineSuffixBoundary, + softline, +} = prettier.doc.builders; + +const printDoc = require("../printDoc"); + +describe("lineSuffixBoundary", () => { + test("should be correctly treated as a potential line break in `fits`", () => { + const doc = group([ + "let foo = [", + indent([ + softline, + [lineSuffixBoundary, "item1,"], + line, + [lineSuffixBoundary, "item2,", lineSuffix(" // comment")], + line, + [lineSuffixBoundary, "item3"], + ]), + softline, + "];", + ]); + + const expected = `let foo = [ + item1, + item2, // comment + item3 +];`; + + expect(printDoc(doc)).toBe(expected); + }); +}); diff --git a/tests_integration/printDoc.js b/tests_integration/printDoc.js new file mode 100644 index 000000000..08e58b2f4 --- /dev/null +++ b/tests_integration/printDoc.js @@ -0,0 +1,30 @@ +"use strict"; + +/** @type {import('prettier')} */ +const prettier = require("prettier-local"); + +function printDoc(doc) { + // This dummy plugin ignores the input and simply returns the given doc. + // This is to make sure that the doc will go through all the stages a real doc + // returned by a real plugin would go (e.g. `propagateBreaks`). + const dummyPlugin = { + parsers: { + dummy: { + parse: () => ({}), + astFormat: "dummy", + }, + }, + printers: { + dummy: { + print: () => doc, + }, + }, + }; + + return prettier.format("_", { + plugins: [dummyPlugin], + parser: "dummy", + }); +} + +module.exports = printDoc;