refactor(linter/plugins): reimplement isSpaceBetween() isSpaceBetweenTokens() using tokens (#16055)

lilnasy · overlookmotel · web-flow · commit f3e9913f534e · 2025-11-27T13:20:04.000Z
- Follow up to #15498 and #15861 - Note: JSX special case (as part of the `isSpaceBetweenTokens()` method) is being removed in ESLint v10 (see [eslint/eslint#20137](https://github.com/eslint/eslint/pull/20137/files#diff-e85eaebdcf6af45c06572019cbd900cff91518c789049c3bd0e8724c18645ab0)) ### Tasks: - [x] Token-based reimplementation of `isSpaceBetween()` - [x] Token-based reimplementation of `isSpaceBetweenTokens()` - [x] Pass existing tests from #15498 - [x] New tests for JS syntax - [x] New tests for JSX syntax --------- Co-authored-by: overlookmotel <theoverlookmotel@gmail.com>
diff --git a/apps/oxlint/src-js/plugins/tokens.ts b/apps/oxlint/src-js/plugins/tokens.ts
@@ -3,7 +3,7 @@
  */
 
 import { createRequire } from "node:module";
-import { sourceText, initSourceText } from "./source_code.js";
+import { sourceText } from "./source_code.js";
 import { debugAssert, debugAssertIsNonNull } from "../utils/asserts.js";
 
 import type { Comment, Node, NodeOrToken } from "./types.ts";
@@ -1699,9 +1699,7 @@ export function getTokenByRangeStart(
   return null;
 }
 
-// Regex that tests for whitespace.
-// TODO: Is this too liberal? Should it be a more constrained set of whitespace characters?
-const WHITESPACE_REGEXP = /\s/;
+const JSX_WHITESPACE_REGEXP = /\s/u;
 
 /**
  * Determine if two nodes or tokens have at least one whitespace character between them.
@@ -1712,23 +1710,25 @@ const WHITESPACE_REGEXP = /\s/;
  * Checks for whitespace *between tokens*, not including whitespace *inside tokens*.
  * e.g. Returns `false` for `isSpaceBetween(x, y)` in `x+" "+y`.
  *
- * TODO: Implementation is not quite right at present.
- * We don't use tokens, so return `true` for `isSpaceBetween(x, y)` in `x+" "+y`, but should return `false`.
- * Note: `checkInsideOfJSXText === false` in ESLint's implementation of `sourceCode.isSpaceBetween`.
- * https://github.com/eslint/eslint/blob/523c076866400670fb2192a3f55dbf7ad3469247/lib/languages/js/source-code/source-code.js#L182-L230
- *
- * @param nodeOrToken1 - The first node or token to check between.
- * @param nodeOrToken2 - The second node or token to check between.
+ * @param first - The first node or token to check between.
+ * @param second - The second node or token to check between.
  * @returns `true` if there is a whitespace character between
  *   any of the tokens found between the two given nodes or tokens.
  */
-export function isSpaceBetween(nodeOrToken1: NodeOrToken, nodeOrToken2: NodeOrToken): boolean {
-  const range1 = nodeOrToken1.range,
-    range2 = nodeOrToken2.range,
-    start1 = range1[0],
-    start2 = range2[0];
+export function isSpaceBetween(first: NodeOrToken, second: NodeOrToken): boolean {
+  if (tokensWithComments === null) {
+    if (tokens === null) initTokens();
+    initTokensWithComments();
+  }
+  debugAssertIsNonNull(tokensWithComments);
 
-  // Find the gap between the two nodes/tokens.
+  const range1 = first.range,
+    range2 = second.range;
+
+  // Find the range between the two nodes/tokens.
+  //
+  // Unlike other methods which require the user to pass the nodes in order of appearance,
+  // `isSpaceBetween()` is invariant over the sequence of the two nodes.
   //
   // 1 node/token can completely enclose another, but they can't *partially* overlap.
   // ```
@@ -1741,23 +1741,43 @@ export function isSpaceBetween(nodeOrToken1: NodeOrToken, nodeOrToken2: NodeOrTo
   //       |------------|
   // ```
   // We use that invariant to reduce this to a single branch.
-  let gapStart, gapEnd;
-  if (start1 < start2) {
-    gapStart = range1[1]; // end1
-    gapEnd = start2;
+  let rangeStart: number = range1[0],
+    rangeEnd: number = range2[0];
+  if (rangeStart < rangeEnd) {
+    rangeStart = range1[1];
   } else {
-    gapStart = range2[1]; // end2;
-    gapEnd = start1;
+    rangeEnd = rangeStart;
+    rangeStart = range2[1];
+  }
+
+  // Binary search for the first token past `rangeStart`.
+  // Unless `first` and `second` are adjacent or overlapping,
+  // the token will be the first token between the two nodes.
+  const tokensWithCommentsLength = tokensWithComments.length;
+  let tokenBetweenIndex = tokensWithCommentsLength;
+  for (let lo = 0; lo < tokenBetweenIndex; ) {
+    const mid = (lo + tokenBetweenIndex) >> 1;
+    if (tokensWithComments[mid].range[0] < rangeStart) {
+      lo = mid + 1;
+    } else {
+      tokenBetweenIndex = mid;
+    }
   }
 
-  // If `gapStart >= gapEnd`, one node encloses the other, or the two are directly adjacent
-  if (gapStart >= gapEnd) return false;
-
-  // Check if there's any whitespace in the gap
-  if (sourceText === null) initSourceText();
-  debugAssertIsNonNull(sourceText);
+  for (
+    let lastTokenEnd = rangeStart;
+    tokenBetweenIndex < tokensWithCommentsLength;
+    tokenBetweenIndex++
+  ) {
+    const { range } = tokensWithComments[tokenBetweenIndex];
+    const tokenStart = range[0];
+    // The first token of the later node should undergo the check in the second branch
+    if (tokenStart > rangeEnd) break;
+    if (tokenStart !== lastTokenEnd) return true;
+    lastTokenEnd = range[1];
+  }
 
-  return WHITESPACE_REGEXP.test(sourceText.slice(gapStart, gapEnd));
+  return false;
 }
 
 /**
@@ -1775,13 +1795,67 @@ export function isSpaceBetween(nodeOrToken1: NodeOrToken, nodeOrToken2: NodeOrTo
  *
  * @deprecated Use `sourceCode.isSpaceBetween` instead.
  *
- * TODO: Implementation is not quite right at present, for same reasons as `SourceCode#isSpaceBetween`.
- *
- * @param nodeOrToken1 - The first node or token to check between.
- * @param nodeOrToken2 - The second node or token to check between.
+ * @param first - The first node or token to check between.
+ * @param second - The second node or token to check between.
  * @returns `true` if there is a whitespace character between
  *   any of the tokens found between the two given nodes or tokens.
  */
-export function isSpaceBetweenTokens(token1: NodeOrToken, token2: NodeOrToken): boolean {
-  return isSpaceBetween(token1, token2);
+export function isSpaceBetweenTokens(first: NodeOrToken, second: NodeOrToken): boolean {
+  if (tokensWithComments === null) {
+    if (tokens === null) initTokens();
+    initTokensWithComments();
+  }
+  debugAssertIsNonNull(tokensWithComments);
+
+  const range1 = first.range,
+    range2 = second.range;
+
+  // Find the range between the two nodes/tokens.
+  // Unlike other methods which require the user to pass the nodes in order of appearance,
+  // `isSpaceBetweenTokens()` is invariant over the sequence of the two nodes.
+  // See comment in `isSpaceBetween` about why this is a single branch.
+  let rangeStart: number = range1[0],
+    rangeEnd: number = range2[0];
+  if (rangeStart < rangeEnd) {
+    rangeStart = range1[1];
+  } else {
+    rangeEnd = rangeStart;
+    rangeStart = range2[1];
+  }
+
+  // Binary search for the first token past `rangeStart`.
+  // Unless `first` and `second` are adjacent or overlapping,
+  // the token will be the first token between the two nodes.
+  const tokensWithCommentsLength = tokensWithComments.length;
+  let tokenBetweenIndex = tokensWithCommentsLength;
+  for (let lo = 0; lo < tokenBetweenIndex; ) {
+    const mid = (lo + tokenBetweenIndex) >> 1;
+    if (tokensWithComments[mid].range[0] < rangeStart) {
+      lo = mid + 1;
+    } else {
+      tokenBetweenIndex = mid;
+    }
+  }
+
+  for (
+    let lastTokenEnd = rangeStart;
+    tokenBetweenIndex < tokensWithCommentsLength;
+    tokenBetweenIndex++
+  ) {
+    const token = tokensWithComments[tokenBetweenIndex],
+      { range } = token,
+      tokenStart = range[0];
+
+    // The first token of the later node should undergo the check in the second branch
+    if (tokenStart > rangeEnd) break;
+    if (
+      tokenStart !== lastTokenEnd ||
+      (token.type === "JSXText" && JSX_WHITESPACE_REGEXP.test(token.value))
+    ) {
+      return true;
+    }
+    lastTokenEnd = range[1];
+  }
+
+  return false;
 }
diff --git a/apps/oxlint/test/fixtures/isSpaceBetween/files/index.js b/apps/oxlint/test/fixtures/isSpaceBetween/files/index.js
@@ -21,6 +21,5 @@ newlineAfter
 // prettier-ignore
 nested = 7 + 8;
 
-// We should return `false` for `isSpaceBetween(beforeString, afterString)`, but we currently return `true`
 // prettier-ignore
 beforeString," ",afterString;
diff --git a/apps/oxlint/test/fixtures/isSpaceBetween/files/index.jsx b/apps/oxlint/test/fixtures/isSpaceBetween/files/index.jsx
@@ -1,9 +1,7 @@
 <Foo>aaa</Foo>;
 
-// We should return `false` for `isSpaceBetween(openingElement, closingElement)`, but we currently return `true`
 <Bar>b c</Bar>;
 
-// We should return `false` for `isSpaceBetween(openingElement, closingElement)`, but we currently return `true`
 // prettier-ignore
 <Qux>d
 e</Qux>;
diff --git a/apps/oxlint/test/fixtures/isSpaceBetween/output.snap.md b/apps/oxlint/test/fixtures/isSpaceBetween/output.snap.md
@@ -168,13 +168,13 @@
     `----
 
   x test-plugin(is-space-between):
-  | isSpaceBetween(beforeString, afterString): true
-  | isSpaceBetweenTokens(beforeString, afterString): true
-  | isSpaceBetween(afterString, beforeString): true
-  | isSpaceBetweenTokens(afterString, beforeString): true
-    ,-[files/index.js:26:1]
- 25 | // prettier-ignore
- 26 | beforeString," ",afterString;
+  | isSpaceBetween(beforeString, afterString): false
+  | isSpaceBetweenTokens(beforeString, afterString): false
+  | isSpaceBetween(afterString, beforeString): false
+  | isSpaceBetweenTokens(afterString, beforeString): false
+    ,-[files/index.js:25:1]
+ 24 | // prettier-ignore
+ 25 | beforeString," ",afterString;
     : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     `----
 
@@ -190,26 +190,26 @@
    `----
 
   x test-plugin(is-space-between):
-  | isSpaceBetween(openingElement, closingElement): true
+  | isSpaceBetween(openingElement, closingElement): false
   | isSpaceBetweenTokens(openingElement, closingElement): true
-  | isSpaceBetween(closingElement, openingElement): true
+  | isSpaceBetween(closingElement, openingElement): false
   | isSpaceBetweenTokens(closingElement, openingElement): true
-   ,-[files/index.jsx:4:1]
- 3 | // We should return `false` for `isSpaceBetween(openingElement, closingElement)`, but we currently return `true`
- 4 | <Bar>b c</Bar>;
+   ,-[files/index.jsx:3:1]
+ 2 | 
+ 3 | <Bar>b c</Bar>;
    : ^^^^^^^^^^^^^^
- 5 | 
+ 4 | 
    `----
 
   x test-plugin(is-space-between):
-  | isSpaceBetween(openingElement, closingElement): true
+  | isSpaceBetween(openingElement, closingElement): false
   | isSpaceBetweenTokens(openingElement, closingElement): true
-  | isSpaceBetween(closingElement, openingElement): true
+  | isSpaceBetween(closingElement, openingElement): false
   | isSpaceBetweenTokens(closingElement, openingElement): true
-   ,-[files/index.jsx:8:1]
- 7 |     // prettier-ignore
- 8 | ,-> <Qux>d
- 9 | `-> e</Qux>;
+   ,-[files/index.jsx:6:1]
+ 5 |     // prettier-ignore
+ 6 | ,-> <Qux>d
+ 7 | `-> e</Qux>;
    `----
 
 Found 0 warnings and 13 errors.
diff --git a/apps/oxlint/test/isSpaceBetween.test.ts b/apps/oxlint/test/isSpaceBetween.test.ts
