feat(linter/plugins): implement SourceCode#isSpaceBetween

apps/oxlint/src-js/plugins/source_code.ts

@@ -20,7 +20,7 @@ import * as scopeMethods from './scope.js';
 import * as tokenMethods from './tokens.js';
 
 import type { Program } from '../generated/types.d.ts';
-import type { BufferWithArrays, Node, NodeOrToken, Ranged } from './types.ts';
+import type { BufferWithArrays, Node, Ranged } from './types.ts';
 import type { ScopeManager } from './scope.ts';
 
 const { max } = Math;
@@ -191,19 +191,6 @@ export const SOURCE_CODE = Object.freeze({
     return ancestors.reverse();
   },
 
-  /**
-   * Determine if two nodes or tokens have at least one whitespace character between them.
-   * Order does not matter. Returns `false` if the given nodes or tokens overlap.
-   * @param nodeOrToken1 - The first node or token to check between.
-   * @param nodeOrToken2 - 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.
-   */
-  // oxlint-disable-next-line no-unused-vars
-  isSpaceBetween(nodeOrToken1: NodeOrToken, nodeOrToken2: NodeOrToken): boolean {
-    throw new Error('`sourceCode.isSpaceBetween` not implemented yet'); // TODO
-  },
-
   /**
    * Get the deepest node containing a range index.
    * @param index Range index of the desired node.
@@ -246,6 +233,7 @@ export const SOURCE_CODE = Object.freeze({
   getLastTokenBetween: tokenMethods.getLastTokenBetween,
   getLastTokensBetween: tokenMethods.getLastTokensBetween,
   getTokenByRangeStart: tokenMethods.getTokenByRangeStart,
+  isSpaceBetween: tokenMethods.isSpaceBetween,
 });
 
 export type SourceCode = typeof SOURCE_CODE;

apps/oxlint/src-js/plugins/tokens.ts

@@ -2,6 +2,8 @@
  * `SourceCode` methods related to tokens.
  */
 
+import { sourceText, initSourceText } from './source_code.js';
+
 import type { Comment, Node, NodeOrToken, Token } from './types.ts';
 
 // Options for various `SourceCode` methods e.g. `getFirstToken`.
@@ -276,3 +278,63 @@ export function getLastTokensBetween(
 export function getTokenByRangeStart(index: number, rangeOptions?: RangeOptions | null | undefined): Token | null {
   throw new Error('`sourceCode.getTokenByRangeStart` not implemented yet'); // TODO
 }
+
+// Regex that tests for whitespace.
+// TODO: Is this too liberal? Should it be a more constrained set of whitespace characters?
+const WHITESPACE_REGEXP = /\s/;
+
+/**
+ * Determine if two nodes or tokens have at least one whitespace character between them.
+ * Order does not matter.
+ *
+ * Returns `false` if the given nodes or tokens overlap.
+ *
+ * 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.
+ * @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];
+
+  // Find the gap between the two nodes/tokens.
+  //
+  // 1 node/token can completely enclose another, but they can't *partially* overlap.
+  // ```
+  // Possible:
+  // |------------|
+  //    |------|
+  //
+  // Impossible:
+  // |------------|
+  //       |------------|
+  // ```
+  // We use that invariant to reduce this to a single branch.
+  let gapStart, gapEnd;
+  if (start1 < start2) {
+    gapStart = range1[1]; // end1
+    gapEnd = start2;
+  } else {
+    gapStart = range2[1]; // end2;
+    gapEnd = start1;
+  }
+
+  // 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();
+
+  return WHITESPACE_REGEXP.test(sourceText.slice(gapStart, gapEnd));
+}

apps/oxlint/test/e2e.test.ts

@@ -267,4 +267,8 @@ describe('oxlint CLI', () => {
   it('wrapping context should work', async () => {
     await testFixture('context_wrapping');
   });
+
+  it('should support `isSpaceBetween` in `context.sourceCode`', async () => {
+    await testFixture('isSpaceBetween');
+  });
 });

apps/oxlint/test/fixtures/isSpaceBetween/.oxlintrc.json

@@ -0,0 +1,7 @@
+{
+  "jsPlugins": ["./plugin.ts"],
+  "categories": { "correctness": "off" },
+  "rules": {
+    "test-plugin/is-space-between": "error"
+  }
+}

apps/oxlint/test/fixtures/isSpaceBetween/files/index.js

@@ -0,0 +1,18 @@
+noSpace=1;
+
+singleSpaceBefore =2;
+
+singleSpaceAfter= 3;
+
+multipleSpaces   =   4;
+
+newlineBefore=
+5;
+
+newlineAfter
+=6;
+
+nested = 7 + 8;
+
+// We should return `false` for `isSpaceBetween(beforeString, afterString)`, but we currently return `true`
+beforeString," ",afterString;

apps/oxlint/test/fixtures/isSpaceBetween/output.snap.md

@@ -0,0 +1,139 @@
+# Exit code
+1
+
+# stdout
+```
+  x test-plugin(is-space-between):
+  | isSpaceBetween(left, right): false
+  | isSpaceBetween(right, left): false
+  | isSpaceBetween(left, node): false
+  | isSpaceBetween(node, left): false
+  | isSpaceBetween(right, node): false
+  | isSpaceBetween(node, right): false
+   ,-[files/index.js:1:1]
+ 1 | noSpace=1;
+   : ^^^^^^^^^
+ 2 | 
+   `----
+
+  x test-plugin(is-space-between):
+  | isSpaceBetween(leftExtended, right): false
+  | isSpaceBetween(right, leftExtended): false
+   ,-[files/index.js:1:1]
+ 1 | noSpace=1;
+   : ^^^^^^^^^
+ 2 | 
+   `----
+
+  x test-plugin(is-space-between):
+  | isSpaceBetween(left, right): true
+  | isSpaceBetween(right, left): true
+  | isSpaceBetween(left, node): false
+  | isSpaceBetween(node, left): false
+  | isSpaceBetween(right, node): false
+  | isSpaceBetween(node, right): false
+   ,-[files/index.js:3:1]
+ 2 | 
+ 3 | singleSpaceBefore =2;
+   : ^^^^^^^^^^^^^^^^^^^^
+ 4 | 
+   `----
+
+  x test-plugin(is-space-between):
+  | isSpaceBetween(left, right): true
+  | isSpaceBetween(right, left): true
+  | isSpaceBetween(left, node): false
+  | isSpaceBetween(node, left): false
+  | isSpaceBetween(right, node): false
+  | isSpaceBetween(node, right): false
+   ,-[files/index.js:5:1]
+ 4 | 
+ 5 | singleSpaceAfter= 3;
+   : ^^^^^^^^^^^^^^^^^^^
+ 6 | 
+   `----
+
+  x test-plugin(is-space-between):
+  | isSpaceBetween(left, right): true
+  | isSpaceBetween(right, left): true
+  | isSpaceBetween(left, node): false
+  | isSpaceBetween(node, left): false
+  | isSpaceBetween(right, node): false
+  | isSpaceBetween(node, right): false
+   ,-[files/index.js:7:1]
+ 6 | 
+ 7 | multipleSpaces   =   4;
+   : ^^^^^^^^^^^^^^^^^^^^^^
+ 8 | 
+   `----
+
+  x test-plugin(is-space-between):
+  | isSpaceBetween(left, right): true
+  | isSpaceBetween(right, left): true
+  | isSpaceBetween(left, node): false
+  | isSpaceBetween(node, left): false
+  | isSpaceBetween(right, node): false
+  | isSpaceBetween(node, right): false
+    ,-[files/index.js:9:1]
+  8 |     
+  9 | ,-> newlineBefore=
+ 10 | `-> 5;
+ 11 |     
+    `----
+
+  x test-plugin(is-space-between):
+  | isSpaceBetween(left, right): true
+  | isSpaceBetween(right, left): true
+  | isSpaceBetween(left, node): false
+  | isSpaceBetween(node, left): false
+  | isSpaceBetween(right, node): false
+  | isSpaceBetween(node, right): false
+    ,-[files/index.js:12:1]
+ 11 |     
+ 12 | ,-> newlineAfter
+ 13 | `-> =6;
+ 14 |     
+    `----
+
+  x test-plugin(is-space-between):
+  | isSpaceBetween(node, binaryLeft): false
+  | isSpaceBetween(binaryLeft, node): false
+    ,-[files/index.js:15:1]
+ 14 | 
+ 15 | nested = 7 + 8;
+    : ^^^^^^^^^^^^^^
+ 16 | 
+    `----
+
+  x test-plugin(is-space-between):
+  | isSpaceBetween(left, right): true
+  | isSpaceBetween(right, left): true
+  | isSpaceBetween(left, node): false
+  | isSpaceBetween(node, left): false
+  | isSpaceBetween(right, node): false
+  | isSpaceBetween(node, right): false
+    ,-[files/index.js:15:1]
+ 14 | 
+ 15 | nested = 7 + 8;
+    : ^^^^^^^^^^^^^^
+ 16 | 
+    `----
+
+  x test-plugin(is-space-between):
+  | isSpaceBetween(beforeString, afterString): true
+  | isSpaceBetween(afterString, beforeString): true
+    ,-[files/index.js:18:1]
+ 17 | // We should return `false` for `isSpaceBetween(beforeString, afterString)`, but we currently return `true`
+ 18 | beforeString," ",afterString;
+    : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+    `----
+
+Found 0 warnings and 10 errors.
+Finished in Xms on 1 file using X threads.
+```
+
+# stderr
+```
+WARNING: JS plugins are experimental and not subject to semver.
+Breaking changes are possible while JS plugins support is under development.
+```

apps/oxlint/test/fixtures/isSpaceBetween/plugin.ts

@@ -0,0 +1,82 @@
+import assert from 'node:assert';
+
+import type { Plugin, Rule, Node } from '../../../dist/index.js';
+
+const testRule: Rule = {
+  create(context) {
+    return {
+      AssignmentExpression(node) {
+        const { isSpaceBetween } = context.sourceCode,
+          { left, right } = node;
+
+        context.report({
+          message:
+            '\n' +
+            // Test where 2 nodes are separated, maybe with whitespace in between
+            `isSpaceBetween(left, right): ${isSpaceBetween(left, right)}\n` +
+            `isSpaceBetween(right, left): ${isSpaceBetween(right, left)}\n` +
+            // Test where 1 node is inside another, sharing same `start` or `end`
+            `isSpaceBetween(left, node): ${isSpaceBetween(left, node)}\n` +
+            `isSpaceBetween(node, left): ${isSpaceBetween(node, left)}\n` +
+            `isSpaceBetween(right, node): ${isSpaceBetween(right, node)}\n` +
+            `isSpaceBetween(node, right): ${isSpaceBetween(node, right)}`,
+          node,
+        });
+
+        // Test where 1 node is inside another, not sharing same `start` or `end`
+        if (right.type === 'BinaryExpression') {
+          const binaryLeft = right.left;
+          context.report({
+            message:
+              '\n' +
+              `isSpaceBetween(node, binaryLeft): ${isSpaceBetween(node, binaryLeft)}\n` +
+              `isSpaceBetween(binaryLeft, node): ${isSpaceBetween(binaryLeft, node)}`,
+            node,
+          });
+        }
+
+        // Test where 2 nodes are completely adjacent to each other.
+        // We don't have tokens yet, so adjust ranges of 1 node so they touch.
+        assert(left.type === 'Identifier');
+        if (left.name === 'noSpace') {
+          const leftExtended: Node = { ...left, end: left.end + 1, range: [left.range[0], left.range[1] + 1] };
+          assert(leftExtended.end === right.start);
+          assert(leftExtended.range[1] === right.range[0]);
+
+          context.report({
+            message:
+              '\n' +
+              `isSpaceBetween(leftExtended, right): ${isSpaceBetween(leftExtended, right)}\n` +
+              `isSpaceBetween(right, leftExtended): ${isSpaceBetween(right, leftExtended)}`,
+            node,
+          });
+        }
+      },
+
+      SequenceExpression(node) {
+        const { isSpaceBetween } = context.sourceCode,
+          [beforeString, , afterString] = node.expressions;
+
+        // We get this wrong. Should be `false`, but we get `true`.
+        context.report({
+            message:
+              '\n' +
+              `isSpaceBetween(beforeString, afterString): ${isSpaceBetween(beforeString, afterString)}\n` +
+              `isSpaceBetween(afterString, beforeString): ${isSpaceBetween(afterString, beforeString)}`,
+            node,
+          });
+      },
+    };
+  },
+};
+
+const plugin: Plugin = {
+  meta: {
+    name: 'test-plugin',
+  },
+  rules: {
+    'is-space-between': testRule,
+  },
+};
+
+export default plugin;