Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix: isSpaceBetweenTokens() recognizes spaces in JSXText (fixes #12614) #12616

Merged
merged 5 commits into from Nov 30, 2019
Merged

Fix: isSpaceBetweenTokens() recognizes spaces in JSXText (fixes #12614) #12616

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8e73ee3
Fix: isSpaceBetween() recognizes spaces in JSXText (fixes #12614)
mysticatea Nov 28, 2019
45c21b7
apply this fix only for isSpaceBetweenTokens()
mysticatea Nov 30, 2019
6e3c683
move tests to the section for nodes
mysticatea Nov 30, 2019
b79395e
add tests for isSpaceBetween()
mysticatea Nov 30, 2019
8580121
add tests for reversed order
mysticatea Nov 30, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
82 changes: 57 additions & 25 deletions lib/source-code/source-code.js
View file
Open in desktop
Expand Up @@ -90,6 +90,56 @@ function nodesOrTokensOverlap(first, second) {
(second.range[0] <= first.range[0] && second.range[1] >= first.range[0]);
}

/**
* Determines 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 {SourceCode} sourceCode The source code object.
* @param {ASTNode|Token} first The first node or token to check between.
* @param {ASTNode|Token} second The second node or token to check between.
* @param {boolean} checkInsideOfJSXText If `true` is present, check inside of JSXText tokens for backward compatibility.
* @returns {boolean} True if there is a whitespace character between
* any of the tokens found between the two given nodes or tokens.
* @public
*/
function isSpaceBetween(sourceCode, first, second, checkInsideOfJSXText) {
if (nodesOrTokensOverlap(first, second)) {
return false;
}

const [startingNodeOrToken, endingNodeOrToken] = first.range[1] <= second.range[0]
? [first, second]
: [second, first];
const firstToken = sourceCode.getLastToken(startingNodeOrToken) || startingNodeOrToken;
const finalToken = sourceCode.getFirstToken(endingNodeOrToken) || endingNodeOrToken;
let currentToken = firstToken;

while (currentToken !== finalToken) {
const nextToken = sourceCode.getTokenAfter(currentToken, { includeComments: true });

if (
currentToken.range[1] !== nextToken.range[0] ||

/*
* For backward compatibility, check speces in JSXText.
* https://github.com/eslint/eslint/issues/12614
*/
(
checkInsideOfJSXText &&
nextToken !== finalToken &&
nextToken.type === "JSXText" &&
/\s/u.test(nextToken.value)
)
) {
return true;
}

currentToken = nextToken;
}

return false;
}

//------------------------------------------------------------------------------
// Public Interface
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -433,42 +483,24 @@ class SourceCode extends TokenStore {
* @public
*/
isSpaceBetween(first, second) {
if (nodesOrTokensOverlap(first, second)) {
return false;
}

const [startingNodeOrToken, endingNodeOrToken] = first.range[1] <= second.range[0]
? [first, second]
: [second, first];
const firstToken = this.getLastToken(startingNodeOrToken) || startingNodeOrToken;
const finalToken = this.getFirstToken(endingNodeOrToken) || endingNodeOrToken;
let currentToken = firstToken;

while (currentToken !== finalToken) {
const nextToken = this.getTokenAfter(currentToken, { includeComments: true });

if (currentToken.range[1] !== nextToken.range[0]) {
return true;
}

currentToken = nextToken;
}

return false;
return isSpaceBetween(this, first, second, false);
}

/**
* Determines 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 {...ASTNode|Token} args The nodes or tokens to check between.
* For backward compatibility, this method returns true if there are
* `JSXText` tokens that contain whitespaces between the two.
* @param {ASTNode|Token} first The first node or token to check between.
* @param {ASTNode|Token} second The second node or token to check between.
* @returns {boolean} True if there is a whitespace character between
* any of the tokens found between the two given nodes or tokens.
* @deprecated in favor of isSpaceBetween().
* @public
*/
isSpaceBetweenTokens(...args) {
return this.isSpaceBetween(...args);
isSpaceBetweenTokens(first, second) {
return isSpaceBetween(this, first, second, true);
}

/**
Expand Down
126 changes: 126 additions & 0 deletions tests/lib/source-code/source-code.js
View file
Open in desktop
Expand Up @@ -2076,6 +2076,69 @@ describe("SourceCode", () => {
});
});
});

it("JSXText tokens that contain only whitespaces should NOT be handled as space", () => {
const code = "let jsx = <div>\n {content}\n</div>";
const ast = espree.parse(code, { ...DEFAULT_CONFIG, ecmaFeatures: { jsx: true } });
const sourceCode = new SourceCode(code, ast);
const jsx = ast.body[0].declarations[0].init;
const interpolation = jsx.children[1];

assert.strictEqual(
sourceCode.isSpaceBetween(jsx.openingElement, interpolation),
false
);
assert.strictEqual(
sourceCode.isSpaceBetween(interpolation, jsx.closingElement),
false
);

// Reversed order
assert.strictEqual(
sourceCode.isSpaceBetween(interpolation, jsx.openingElement),
false
);
assert.strictEqual(
sourceCode.isSpaceBetween(jsx.closingElement, interpolation),
false
);
});

it("JSXText tokens that contain both letters and whitespaces should NOT be handled as space", () => {
const code = "let jsx = <div>\n Hello\n</div>";
const ast = espree.parse(code, { ...DEFAULT_CONFIG, ecmaFeatures: { jsx: true } });
const sourceCode = new SourceCode(code, ast);
const jsx = ast.body[0].declarations[0].init;

assert.strictEqual(
sourceCode.isSpaceBetween(jsx.openingElement, jsx.closingElement),
false
);

// Reversed order
assert.strictEqual(
sourceCode.isSpaceBetween(jsx.closingElement, jsx.openingElement),
false
);
});

it("JSXText tokens that contain only letters should NOT be handled as space", () => {
const code = "let jsx = <div>Hello</div>";
const ast = espree.parse(code, { ...DEFAULT_CONFIG, ecmaFeatures: { jsx: true } });
const sourceCode = new SourceCode(code, ast);
const jsx = ast.body[0].declarations[0].init;

assert.strictEqual(
sourceCode.isSpaceBetween(jsx.openingElement, jsx.closingElement),
false
);

// Reversed order
assert.strictEqual(
sourceCode.isSpaceBetween(jsx.closingElement, jsx.openingElement),
false
);
});
});

describe("should return false either of the arguments' location is inside the other one", () => {
Expand Down Expand Up @@ -2409,6 +2472,69 @@ describe("SourceCode", () => {
});
});
});

it("JSXText tokens that contain only whitespaces should be handled as space", () => {
const code = "let jsx = <div>\n {content}\n</div>";
const ast = espree.parse(code, { ...DEFAULT_CONFIG, ecmaFeatures: { jsx: true } });
const sourceCode = new SourceCode(code, ast);
const jsx = ast.body[0].declarations[0].init;
const interpolation = jsx.children[1];

assert.strictEqual(
sourceCode.isSpaceBetweenTokens(jsx.openingElement, interpolation),
true
);
assert.strictEqual(
sourceCode.isSpaceBetweenTokens(interpolation, jsx.closingElement),
true
);

// Reversed order
assert.strictEqual(
sourceCode.isSpaceBetweenTokens(interpolation, jsx.openingElement),
true
);
assert.strictEqual(
sourceCode.isSpaceBetweenTokens(jsx.closingElement, interpolation),
true
);
});

it("JSXText tokens that contain both letters and whitespaces should be handled as space", () => {
const code = "let jsx = <div>\n Hello\n</div>";
const ast = espree.parse(code, { ...DEFAULT_CONFIG, ecmaFeatures: { jsx: true } });
const sourceCode = new SourceCode(code, ast);
const jsx = ast.body[0].declarations[0].init;

assert.strictEqual(
sourceCode.isSpaceBetweenTokens(jsx.openingElement, jsx.closingElement),
true
);

// Reversed order
assert.strictEqual(
sourceCode.isSpaceBetweenTokens(jsx.closingElement, jsx.openingElement),
true
);
});

it("JSXText tokens that contain only letters should NOT be handled as space", () => {
const code = "let jsx = <div>Hello</div>";
const ast = espree.parse(code, { ...DEFAULT_CONFIG, ecmaFeatures: { jsx: true } });
const sourceCode = new SourceCode(code, ast);
const jsx = ast.body[0].declarations[0].init;

assert.strictEqual(
sourceCode.isSpaceBetweenTokens(jsx.openingElement, jsx.closingElement),
false
);

// Reversed order
assert.strictEqual(
sourceCode.isSpaceBetweenTokens(jsx.closingElement, jsx.openingElement),
false
);
});
});

describe("should return false either of the arguments' location is inside the other one", () => {
Expand Down