diff --git a/changelog_unreleased/javascript/9704.md b/changelog_unreleased/javascript/9704.md new file mode 100644 index 000000000..014c7204b --- /dev/null +++ b/changelog_unreleased/javascript/9704.md @@ -0,0 +1,19 @@ +#### Fix node ends at `rangeStart` was formatted (#9704 by @fisker) + +Previously a node ends at `rangeStart` in considered in range, now it's excluded. + + +```js +// Prettier stable +foo = 1.0000;bar = 1.0000;baz=1.0000; + ^^^^^^^^^^^^^ Range + +// Prettier stable +foo = 1.0; +bar = 1.0;baz=1.0000; + +// Prettier master +foo = 1.0000;bar = 1.0;baz=1.0000; +``` + +_This effect other languages support range format too._ diff --git a/src/main/core.js b/src/main/core.js index 8ad34c4ec..e4e8cf99f 100644 --- a/src/main/core.js +++ b/src/main/core.js @@ -278,6 +278,13 @@ function format(originalText, originalOptions) { normalizeOptions(originalOptions) ); + if (options.rangeStart >= options.rangeEnd && text !== "") { + return { + formatted: originalText, + cursorOffset: originalOptions.cursorOffset, + }; + } + const selectedParser = parser.resolveParser(options); const hasPragma = !selectedParser.hasPragma || selectedParser.hasPragma(text); if (options.requirePragma && !hasPragma) { diff --git a/src/main/range-util.js b/src/main/range-util.js index 344926dd0..20860a0a1 100644 --- a/src/main/range-util.js +++ b/src/main/range-util.js @@ -1,5 +1,6 @@ "use strict"; +const assert = require("assert"); const comments = require("./comments"); function findSiblingAncestors(startNodeAndParents, endNodeAndParents, opts) { @@ -43,8 +44,24 @@ function findSiblingAncestors(startNodeAndParents, endNodeAndParents, opts) { }; } -function findNodeAtOffset(node, offset, options, predicate, parentNodes = []) { - if (offset < options.locStart(node) || offset > options.locEnd(node)) { +function findNodeAtOffset( + node, + offset, + options, + predicate, + parentNodes = [], + type +) { + const { locStart, locEnd } = options; + const start = locStart(node); + const end = locEnd(node); + + if ( + offset > end || + offset < start || + (type === "rangeEnd" && offset === start) || + (type === "rangeStart" && offset === end) + ) { return; } @@ -54,7 +71,8 @@ function findNodeAtOffset(node, offset, options, predicate, parentNodes = []) { offset, options, predicate, - [node, ...parentNodes] + [node, ...parentNodes], + type ); if (childResult) { return childResult; @@ -133,37 +151,41 @@ function isSourceElement(opts, node) { } function calculateRange(text, opts, ast) { + let { rangeStart: start, rangeEnd: end, locStart, locEnd } = opts; + assert.ok(end > start); // Contract the range so that it has non-whitespace characters at its endpoints. // This ensures we can format a range that doesn't end on a node. - const rangeStringOrig = text.slice(opts.rangeStart, opts.rangeEnd); - const startNonWhitespace = Math.max( - opts.rangeStart + rangeStringOrig.search(/\S/), - opts.rangeStart - ); - let endNonWhitespace; - for ( - endNonWhitespace = opts.rangeEnd; - endNonWhitespace > opts.rangeStart; - --endNonWhitespace - ) { - if (/\S/.test(text[endNonWhitespace - 1])) { - break; + const firstNonWhitespaceCharacterIndex = text.slice(start, end).search(/\S/); + const isAllWhitespace = firstNonWhitespaceCharacterIndex === -1; + if (!isAllWhitespace) { + start += firstNonWhitespaceCharacterIndex; + for (; end > start; --end) { + if (/\S/.test(text[end - 1])) { + break; + } } } const startNodeAndParents = findNodeAtOffset( ast, - startNonWhitespace, + start, opts, - (node) => isSourceElement(opts, node) + (node) => isSourceElement(opts, node), + [], + "rangeStart" ); - const endNodeAndParents = findNodeAtOffset( - ast, - endNonWhitespace, - opts, - (node) => isSourceElement(opts, node) - ); - + const endNodeAndParents = + // No need find Node at `end`, it will be the same as `startNodeAndParents` + isAllWhitespace + ? startNodeAndParents + : findNodeAtOffset( + ast, + end, + opts, + (node) => isSourceElement(opts, node), + [], + "rangeEnd" + ); if (!startNodeAndParents || !endNodeAndParents) { return { rangeStart: 0, @@ -178,8 +200,8 @@ function calculateRange(text, opts, ast) { ); return { - rangeStart: Math.min(opts.locStart(startNode), opts.locStart(endNode)), - rangeEnd: Math.max(opts.locEnd(startNode), opts.locEnd(endNode)), + rangeStart: Math.min(locStart(startNode), locStart(endNode)), + rangeEnd: Math.max(locEnd(startNode), locEnd(endNode)), }; } diff --git a/tests/js/range/__snapshots__/jsfmt.spec.js.snap b/tests/js/range/__snapshots__/jsfmt.spec.js.snap index 339cca526..4378d8da2 100644 --- a/tests/js/range/__snapshots__/jsfmt.spec.js.snap +++ b/tests/js/range/__snapshots__/jsfmt.spec.js.snap @@ -26,6 +26,88 @@ a = [, , , , , , , a]; ================================================================================ `; +exports[`boundary.js format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 +rangeEnd: 26 +rangeStart: 13 + | | printWidth +=====================================input====================================== +> 1 | foo = 1.0000;bar = 1.0000;baz=1.0000; + | ^^^^^^^^^^^^^ + 2 | // The range will be 13~26 + 3 | // \`foo\` ends at 13, should not format + 4 | // \`bar\` ends at 26, should format + 5 | +=====================================output===================================== +foo = 1.0000;bar = 1.0;baz=1.0000; +// The range will be 13~26 +// \`foo\` ends at 13, should not format +// \`bar\` ends at 26, should format + +================================================================================ +`; + +exports[`boundary-2.js format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 +rangeEnd: 45 +rangeStart: 28 + | | printWidth +=====================================input====================================== + 1 | function a( + 2 | ){ + 3 | a ( + 4 | ); + 5 | b ( +> 6 | ); c ( + | ^^^^^^^^^^^^^^^^^ + 7 | ); d( + 8 | ); + 9 | + 10 | } + 11 | +=====================================output===================================== +function a( +){ + a(); + b(); + c(); + d(); +} + +================================================================================ +`; + +exports[`boundary-3.js format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 +rangeEnd: 30 +rangeStart: 13 + | | printWidth +=====================================input====================================== + 1 | a ( + 2 | ); + 3 | b ( +> 4 | ); c ( + | ^^^^^^^^^^^^^^^^^ + 5 | ); d( + 6 | ); + 7 | +=====================================output===================================== +a ( +); +b ( +); c ( +); d( +); + +================================================================================ +`; + exports[`class-declaration.js format 1`] = ` ====================================options===================================== parsers: ["babel", "flow", "typescript"] @@ -851,6 +933,44 @@ call( ================================================================================ `; +exports[`reversed-range.js format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 +rangeEnd: 13 +rangeStart: 26 + | | printWidth +=====================================input====================================== +> 1 | foo = 1.0000;bar = 1.0000;baz=1.0000; + | ^^^^^^^^^^^^^ [Reversed range] + 2 | // The range will be 26~13, should not format anything + 3 | +=====================================output===================================== +foo = 1.0000;bar = 1.0000;baz=1.0000; +// The range will be 26~13, should not format anything + +================================================================================ +`; + +exports[`start-equals-end.js format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 +rangeEnd: 13 +rangeStart: 13 + | | printWidth +=====================================input====================================== +> 1 | foo = 1.0000;bar = 1.0000;baz=1.0000; + | ^ + 2 | // The range will be 13~13, should not format anything + 3 | +=====================================output===================================== +foo = 1.0000;bar = 1.0000;baz=1.0000; +// The range will be 13~13, should not format anything + +================================================================================ +`; + exports[`try-catch.js format 1`] = ` ====================================options===================================== parsers: ["babel", "flow", "typescript"] diff --git a/tests/js/range/boundary-2.js b/tests/js/range/boundary-2.js new file mode 100644 index 000000000..6b94539a8 --- /dev/null +++ b/tests/js/range/boundary-2.js @@ -0,0 +1,10 @@ +function a( +){ +a ( +); +b ( +);<<>> <<>>c ( +); d( +); + +} diff --git a/tests/js/range/boundary-3.js b/tests/js/range/boundary-3.js new file mode 100644 index 000000000..32c8512f8 --- /dev/null +++ b/tests/js/range/boundary-3.js @@ -0,0 +1,6 @@ +a ( +); +b ( +);<<>> <<>>c ( +); d( +); diff --git a/tests/js/range/boundary.js b/tests/js/range/boundary.js new file mode 100644 index 000000000..ce9ff23f6 --- /dev/null +++ b/tests/js/range/boundary.js @@ -0,0 +1,4 @@ +foo = 1.0000;<<>>bar = 1.0000;<<>>baz=1.0000; +// The range will be 13~26 +// `foo` ends at 13, should not format +// `bar` ends at 26, should format diff --git a/tests/js/range/reversed-range.js b/tests/js/range/reversed-range.js new file mode 100644 index 000000000..0fe95737a --- /dev/null +++ b/tests/js/range/reversed-range.js @@ -0,0 +1,2 @@ +foo = 1.0000;<<>>bar = 1.0000;<<>>baz=1.0000; +// The range will be 26~13, should not format anything diff --git a/tests/js/range/start-equals-end.js b/tests/js/range/start-equals-end.js new file mode 100644 index 000000000..65d3ca905 --- /dev/null +++ b/tests/js/range/start-equals-end.js @@ -0,0 +1,2 @@ +foo = 1.0000;<<>><<>>bar = 1.0000;baz=1.0000; +// The range will be 13~13, should not format anything diff --git a/tests_config/run_spec.js b/tests_config/run_spec.js index d2e2a1f04..6887e6162 100644 --- a/tests_config/run_spec.js +++ b/tests_config/run_spec.js @@ -334,7 +334,7 @@ function runTest({ }); } - if (!code.includes("\r") && !formatOptions.requirePragma) { + if (!shouldSkipEolTest(code, formatResult.options)) { for (const eol of ["\r\n", "\r"]) { test(`[${parser}] EOL ${JSON.stringify(eol)}`, () => { const output = format(code.replace(/\n/g, eol), formatOptions) @@ -361,6 +361,25 @@ function runTest({ } } +function shouldSkipEolTest(code, options) { + if (code.includes("\r")) { + return true; + } + const { requirePragma, rangeStart, rangeEnd } = options; + if (requirePragma) { + return true; + } + + if ( + typeof rangeStart === "number" && + typeof rangeEnd === "number" && + rangeStart >= rangeEnd + ) { + return true; + } + return false; +} + function parse(source, options) { return prettier.__debug.parse(source, options, /* massage */ true).ast; } diff --git a/tests_config/utils/visualize-range.js b/tests_config/utils/visualize-range.js index d0e362814..d8125a06a 100644 --- a/tests_config/utils/visualize-range.js +++ b/tests_config/utils/visualize-range.js @@ -5,6 +5,9 @@ const { codeFrameColumns } = require("@babel/code-frame"); const codeFrameColumnsOptions = { linesAbove: Infinity, linesBelow: Infinity }; const locationForRange = (text, rangeStart, rangeEnd) => { + if (rangeStart > rangeEnd) { + [rangeStart, rangeEnd] = [rangeEnd, rangeStart]; + } const lines = new LinesAndColumns(text); const start = lines.locationForIndex(rangeStart); const end = lines.locationForIndex(rangeEnd); @@ -26,7 +29,9 @@ const visualizeRange = (text, { rangeStart = 0, rangeEnd = text.length }) => codeFrameColumns( text, locationForRange(text, rangeStart, rangeEnd), - codeFrameColumnsOptions + rangeStart > rangeEnd + ? { ...codeFrameColumnsOptions, message: "[Reversed range]" } + : codeFrameColumnsOptions ); module.exports = visualizeRange;