From 6a5efe7fa8810395e944601ac2103361ca7c9df2 Mon Sep 17 00:00:00 2001 From: Oskari Noppa Date: Mon, 30 Aug 2021 15:19:05 +0300 Subject: [PATCH] Print semi before Flow variance sigil w/ no-semi (#11398) * Print semi before Flow variance sigil w/ no-semi Fixes #11397. Omitting semicolon before class property with a Flow variance sigil `+` or `-` may lead to output that is syntatically incorrect or has incorrect runtime behaviour. * Add changelog for PR 11398 * Put semi to the end of previous line Put the semicolon for Flow class property variance sigil to the end of the previous line, not at the beginning of the the line where the variance sigil is. This approach was suggested in the comments of #11398. * Add babel-flow to parser to Flow no-semi specs As per discussion in PR #11398, babel-flow parser should also be tested. * Print semi before private fields w/ variance * Add private fields to changelog * Use existing isClassProperty helper Use existing helper function `isClassProperty` to simplify the conditional a bit. Note that this also adds new case, `type === "PropertyDefinition"`, that would be a match. However, nextNode.type can't be "PropertyDefinition" in practice, as it's part of estree parser output which doesn't support Flow. There's no real downside in checking that extra case, though, and this does simplify the code a bit. * Add new property tests to the snapshot spec * Change one of the variance sigils to `-` (contravariant) * Add one computed property * Make one of the property values be a function These cases also require semi in the previous line. * Add new testcases from previous patch to changelog * Add variance sigil to computed property in spec --- changelog_unreleased/flow/11398.md | 26 +++++++ src/language-js/print/statement.js | 11 +++ .../no-semi/__snapshots__/jsfmt.spec.js.snap | 69 +++++++++++++++++-- .../flow/no-semi/flow-class-properties.js | 8 +++ tests/format/flow/no-semi/jsfmt.spec.js | 4 +- 5 files changed, 110 insertions(+), 8 deletions(-) create mode 100644 changelog_unreleased/flow/11398.md create mode 100644 tests/format/flow/no-semi/flow-class-properties.js diff --git a/changelog_unreleased/flow/11398.md b/changelog_unreleased/flow/11398.md new file mode 100644 index 000000000..02b564d6f --- /dev/null +++ b/changelog_unreleased/flow/11398.md @@ -0,0 +1,26 @@ +#### Print leading semi before Flow variance sigils (#11398 by @noppa) + + +```jsx +// Input +class A { + +one = function() {}; + -two = val(); + +#privOne = val(); + static +#privTwo = val(); +} +// Prettier stable +class A { + +one = function() {} + -two = val() + +#privOne = val() + static +#privTwo = val() +} +// Prettier main +class A { + +one = function() {}; + -two = val(); + +#privOne = val() + static +#privTwo = val() +} +``` diff --git a/src/language-js/print/statement.js b/src/language-js/print/statement.js index 9dc33a742..c7573faf7 100644 --- a/src/language-js/print/statement.js +++ b/src/language-js/print/statement.js @@ -201,6 +201,17 @@ function shouldPrintSemicolonAfterClassProperty(node, nextNode) { } } + // Flow variance sigil +/- requires semi if there's no + // "declare" or "static" keyword before it. + if ( + isClassProperty(nextNode) && + nextNode.variance && + !nextNode.static && + !nextNode.declare + ) { + return true; + } + switch (nextNode.type) { case "ClassProperty": case "PropertyDefinition": diff --git a/tests/format/flow/no-semi/__snapshots__/jsfmt.spec.js.snap b/tests/format/flow/no-semi/__snapshots__/jsfmt.spec.js.snap index 2a25e9c70..36cf0be3c 100644 --- a/tests/format/flow/no-semi/__snapshots__/jsfmt.spec.js.snap +++ b/tests/format/flow/no-semi/__snapshots__/jsfmt.spec.js.snap @@ -2,7 +2,7 @@ exports[`comments.js - {"semi":false} format 1`] = ` ====================================options===================================== -parsers: ["flow", "babel"] +parsers: ["flow", "babel", "babel-flow"] printWidth: 80 semi: false | printWidth @@ -37,7 +37,7 @@ x exports[`comments.js format 1`] = ` ====================================options===================================== -parsers: ["flow", "babel"] +parsers: ["flow", "babel", "babel-flow"] printWidth: 80 | printWidth =====================================input====================================== @@ -69,9 +69,66 @@ x; ================================================================================ `; +exports[`flow-class-properties.js - {"semi":false} format 1`] = ` +====================================options===================================== +parsers: ["flow", "babel", "babel-flow"] +printWidth: 80 +semi: false + | printWidth +=====================================input====================================== +class A { + +one = function() {}; + -two = val(); + static +three = val(); + +#privOne = val(); + static +#privTwo = val(); + +[computed] = val(); +} + +=====================================output===================================== +class A { + +one = function () {}; + -two = val() + static +three = val(); + +#privOne = val() + static +#privTwo = val(); + +[computed] = val() +} + +================================================================================ +`; + +exports[`flow-class-properties.js format 1`] = ` +====================================options===================================== +parsers: ["flow", "babel", "babel-flow"] +printWidth: 80 + | printWidth +=====================================input====================================== +class A { + +one = function() {}; + -two = val(); + static +three = val(); + +#privOne = val(); + static +#privTwo = val(); + +[computed] = val(); +} + +=====================================output===================================== +class A { + +one = function () {}; + -two = val(); + static +three = val(); + +#privOne = val(); + static +#privTwo = val(); + +[computed] = val(); +} + +================================================================================ +`; + exports[`flow-interfaces.js - {"semi":false} format 1`] = ` ====================================options===================================== -parsers: ["flow", "babel"] +parsers: ["flow", "babel", "babel-flow"] printWidth: 80 semi: false | printWidth @@ -157,7 +214,7 @@ interface E { exports[`flow-interfaces.js format 1`] = ` ====================================options===================================== -parsers: ["flow", "babel"] +parsers: ["flow", "babel", "babel-flow"] printWidth: 80 | printWidth =====================================input====================================== @@ -242,7 +299,7 @@ interface E { exports[`no-semi.js - {"semi":false} format 1`] = ` ====================================options===================================== -parsers: ["flow", "babel"] +parsers: ["flow", "babel", "babel-flow"] printWidth: 80 semi: false | printWidth @@ -263,7 +320,7 @@ semi: false exports[`no-semi.js format 1`] = ` ====================================options===================================== -parsers: ["flow", "babel"] +parsers: ["flow", "babel", "babel-flow"] printWidth: 80 | printWidth =====================================input====================================== diff --git a/tests/format/flow/no-semi/flow-class-properties.js b/tests/format/flow/no-semi/flow-class-properties.js new file mode 100644 index 000000000..e6cd0014e --- /dev/null +++ b/tests/format/flow/no-semi/flow-class-properties.js @@ -0,0 +1,8 @@ +class A { + +one = function() {}; + -two = val(); + static +three = val(); + +#privOne = val(); + static +#privTwo = val(); + +[computed] = val(); +} diff --git a/tests/format/flow/no-semi/jsfmt.spec.js b/tests/format/flow/no-semi/jsfmt.spec.js index 2f93d9870..b01ec0778 100644 --- a/tests/format/flow/no-semi/jsfmt.spec.js +++ b/tests/format/flow/no-semi/jsfmt.spec.js @@ -1,2 +1,2 @@ -run_spec(__dirname, ["flow", "babel"]); -run_spec(__dirname, ["flow", "babel"], { semi: false }); +run_spec(__dirname, ["flow", "babel", "babel-flow"]); +run_spec(__dirname, ["flow", "babel", "babel-flow"], { semi: false });