diff --git a/apps/oxlint/src-js/plugins/source_code.ts b/apps/oxlint/src-js/plugins/source_code.ts index 83139a95027bb..0289ac018b66e 100644 --- a/apps/oxlint/src-js/plugins/source_code.ts +++ b/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; diff --git a/apps/oxlint/src-js/plugins/tokens.ts b/apps/oxlint/src-js/plugins/tokens.ts index 09e8e79349f1d..3eec8dc4f56cc 100644 --- a/apps/oxlint/src-js/plugins/tokens.ts +++ b/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)); +} diff --git a/apps/oxlint/test/e2e.test.ts b/apps/oxlint/test/e2e.test.ts index 41946eb47a9bf..8f2166f2baf22 100644 --- a/apps/oxlint/test/e2e.test.ts +++ b/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'); + }); }); diff --git a/apps/oxlint/test/fixtures/isSpaceBetween/.oxlintrc.json b/apps/oxlint/test/fixtures/isSpaceBetween/.oxlintrc.json new file mode 100644 index 0000000000000..f027cdd032566 --- /dev/null +++ b/apps/oxlint/test/fixtures/isSpaceBetween/.oxlintrc.json @@ -0,0 +1,7 @@ +{ + "jsPlugins": ["./plugin.ts"], + "categories": { "correctness": "off" }, + "rules": { + "test-plugin/is-space-between": "error" + } +} diff --git a/apps/oxlint/test/fixtures/isSpaceBetween/files/index.js b/apps/oxlint/test/fixtures/isSpaceBetween/files/index.js new file mode 100644 index 0000000000000..8f4c0ad6359ec --- /dev/null +++ b/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; diff --git a/apps/oxlint/test/fixtures/isSpaceBetween/output.snap.md b/apps/oxlint/test/fixtures/isSpaceBetween/output.snap.md new file mode 100644 index 0000000000000..b5b1b41d8bc1e --- /dev/null +++ b/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. +``` diff --git a/apps/oxlint/test/fixtures/isSpaceBetween/plugin.ts b/apps/oxlint/test/fixtures/isSpaceBetween/plugin.ts new file mode 100644 index 0000000000000..9e211c718c27f --- /dev/null +++ b/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;