Exclude node at rangeStart (#9704)

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.
+
+<!-- prettier-ignore -->
+```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._

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) {

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)),
   };
 }
 

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"]

tests/js/range/boundary-2.js

@@ -0,0 +1,10 @@
+function a(
+){
+a (
+);
+b (
+);<<<PRETTIER_RANGE_START>>>                 <<<PRETTIER_RANGE_END>>>c (
+); d(
+);
+
+}

tests/js/range/boundary-3.js

@@ -0,0 +1,6 @@
+a (
+);
+b (
+);<<<PRETTIER_RANGE_START>>>                 <<<PRETTIER_RANGE_END>>>c (
+); d(
+);

tests/js/range/boundary.js

@@ -0,0 +1,4 @@
+foo = 1.0000;<<<PRETTIER_RANGE_START>>>bar = 1.0000;<<<PRETTIER_RANGE_END>>>baz=1.0000;
+// The range will be 13~26
+// `foo` ends at 13, should not format
+// `bar` ends at 26, should format

tests/js/range/reversed-range.js

@@ -0,0 +1,2 @@
+foo = 1.0000;<<<PRETTIER_RANGE_END>>>bar = 1.0000;<<<PRETTIER_RANGE_START>>>baz=1.0000;
+// The range will be 26~13, should not format anything

tests/js/range/start-equals-end.js

@@ -0,0 +1,2 @@
+foo = 1.0000;<<<PRETTIER_RANGE_START>>><<<PRETTIER_RANGE_END>>>bar = 1.0000;baz=1.0000;
+// The range will be 13~13, should not format anything

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;
 }

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;