Skip to content

Commit

Permalink
[Fix] jsx-tag-spacing: change multiline-always to proportional-always
Browse files Browse the repository at this point in the history
- If the node is multiline then require newline
- If the node is singleline then require space

Note: breaking wrt #3260, but it’s unreleased.

Signed-off-by: Sebastian Malton <sebastian@malton.name>
  • Loading branch information
Nokel81 authored and ljharb committed Apr 5, 2022
1 parent 9356230 commit 316bc40
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 28 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Expand Up @@ -8,7 +8,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
### Added
* [`destructuring-assignment`]: add option `destructureInSignature` ([#3235][] @golopot)
* [`no-unknown-property`]: Allow crossOrigin on image tag (SVG) ([#3251][] @zpao)
* [`jsx-tag-spacing`]: Add `multiline-always` option ([#3260][] @Nokel81)
* [`jsx-tag-spacing`]: Add `multiline-always` option ([#3260][], [#3264][] @Nokel81)
* [`function-component-definition`]: replace `var` by `const` in certain situations ([#3248][] @JohnBerd @SimeonC)
* add [`jsx-no-leaked-render`] ([#3203][] @Belco90)
* [`require-default-props`]: add option `functions` ([#3249][] @nix6839)
Expand Down Expand Up @@ -45,6 +45,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
[#3267]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3267
[#3266]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3266
[#3265]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3265
[#3264]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3264
[#3261]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3261
[#3260]: https://github.jsx-eslintckcr/eslint-plugin-react/pull/3260
[#3259]: https://githubjsx-eslintickcr/eslint-plugin-react/pull/3259
Expand Down
34 changes: 25 additions & 9 deletions lib/rules/jsx-tag-spacing.js
Expand Up @@ -101,7 +101,7 @@ function validateBeforeSelfClosing(context, node, option) {
const leftToken = getTokenBeforeClosingBracket(node);
const closingSlash = sourceCode.getTokenAfter(leftToken);

if (node.loc.start.line !== node.loc.end.line && option === 'multiline-always') {
if (node.loc.start.line !== node.loc.end.line && option === 'proportional-always') {
if (leftToken.loc.end.line === closingSlash.loc.start.line) {
report(context, messages.beforeSelfCloseNeedNewline, 'beforeSelfCloseNeedNewline', {
node,
Expand All @@ -110,22 +110,25 @@ function validateBeforeSelfClosing(context, node, option) {
return fixer.insertTextBefore(closingSlash, '\n');
},
});
return;
}
}

if (leftToken.loc.end.line !== closingSlash.loc.start.line) {
return;
}

if (option === 'always' && !sourceCode.isSpaceBetweenTokens(leftToken, closingSlash)) {
const adjacent = !sourceCode.isSpaceBetweenTokens(leftToken, closingSlash);

if ((option === 'always' || option === 'proportional-always') && adjacent) {
report(context, messages.beforeSelfCloseNeedSpace, 'beforeSelfCloseNeedSpace', {
node,
loc: closingSlash.loc.start,
fix(fixer) {
return fixer.insertTextBefore(closingSlash, ' ');
},
});
} else if (option === 'never' && sourceCode.isSpaceBetweenTokens(leftToken, closingSlash)) {
} else if (option === 'never' && !adjacent) {
report(context, messages.beforeSelfCloseNoSpace, 'beforeSelfCloseNoSpace', {
node,
loc: closingSlash.loc.start,
Expand Down Expand Up @@ -180,11 +183,12 @@ function validateBeforeClosing(context, node, option) {
// Don't enforce this rule for self closing tags
if (!node.selfClosing) {
const sourceCode = context.getSourceCode();
const lastTokens = sourceCode.getLastTokens(node, 2);
const closingToken = lastTokens[1];
const leftToken = lastTokens[0];
const leftToken = option === 'proportional-always'
? getTokenBeforeClosingBracket(node)
: sourceCode.getLastTokens(node, 2)[0];
const closingToken = sourceCode.getTokenAfter(leftToken);

if (node.loc.start.line !== node.loc.end.line && option === 'multiline-always') {
if (node.loc.start.line !== node.loc.end.line && option === 'proportional-always') {
if (leftToken.loc.end.line === closingToken.loc.start.line) {
report(context, messages.beforeCloseNeedNewline, 'beforeCloseNeedNewline', {
node,
Expand All @@ -193,6 +197,7 @@ function validateBeforeClosing(context, node, option) {
return fixer.insertTextBefore(closingToken, '\n');
},
});
return;
}
}

Expand Down Expand Up @@ -224,6 +229,17 @@ function validateBeforeClosing(context, node, option) {
return fixer.insertTextBefore(closingToken, ' ');
},
});
} else if (option === 'proportional-always' && node.type === 'JSXOpeningElement' && adjacent !== (node.loc.start.line === node.loc.end.line)) {
report(context, messages.beforeCloseNeedSpace, 'beforeCloseNeedSpace', {
node,
loc: {
start: leftToken.loc.end,
end: closingToken.loc.start,
},
fix(fixer) {
return fixer.insertTextBefore(closingToken, ' ');
},
});
}
}
}
Expand Down Expand Up @@ -259,13 +275,13 @@ module.exports = {
enum: ['always', 'never', 'allow'],
},
beforeSelfClosing: {
enum: ['always', 'multiline-always', 'never', 'allow'],
enum: ['always', 'proportional-always', 'never', 'allow'],
},
afterOpening: {
enum: ['always', 'allow-multiline', 'never', 'allow'],
},
beforeClosing: {
enum: ['always', 'multiline-always', 'never', 'allow'],
enum: ['always', 'proportional-always', 'never', 'allow'],
},
},
default: optionDefaults,
Expand Down
2 changes: 1 addition & 1 deletion lib/util/getTokenBeforeClosingBracket.js
Expand Up @@ -7,7 +7,7 @@
*/
function getTokenBeforeClosingBracket(node) {
const attributes = node.attributes;
if (attributes.length === 0) {
if (!attributes || attributes.length === 0) {
return node.name;
}
return attributes[attributes.length - 1];
Expand Down
38 changes: 21 additions & 17 deletions tests/lib/rules/jsx-tag-spacing.js
Expand Up @@ -114,21 +114,13 @@ ruleTester.run('jsx-tag-spacing', rule, {
code: '<App/>',
options: beforeSelfClosingOptions('never'),
},
{
code: '<App/>',
options: beforeSelfClosingOptions('multiline-always'),
},
{
code: '<App />',
options: beforeSelfClosingOptions('multiline-always'),
},
{
code: '<App foo/>',
options: beforeSelfClosingOptions('multiline-always'),
options: beforeSelfClosingOptions('proportional-always'),
},
{
code: '<App foo />',
options: beforeSelfClosingOptions('multiline-always'),
options: beforeSelfClosingOptions('proportional-always'),
},
{
code: `
Expand All @@ -139,23 +131,23 @@ ruleTester.run('jsx-tag-spacing', rule, {
hello
</App>
`,
options: beforeClosingOptions('multiline-always'),
options: beforeClosingOptions('proportional-always'),
},
{
code: `
<App foo={bar}>
hello
</App>
`,
options: beforeClosingOptions('multiline-always'),
options: beforeClosingOptions('proportional-always'),
},
{
code: `
<App
foo={bar}
/>
`,
options: beforeSelfClosingOptions('multiline-always'),
options: beforeSelfClosingOptions('proportional-always'),
},
{
code: '<App foo/>',
Expand Down Expand Up @@ -345,6 +337,18 @@ ruleTester.run('jsx-tag-spacing', rule, {
options: beforeSelfClosingOptions('never'),
errors: [{ messageId: 'beforeSelfCloseNoSpace' }],
},
{
code: '<App/>',
output: '<App />',
options: beforeSelfClosingOptions('proportional-always'),
errors: [{ messageId: 'beforeSelfCloseNeedSpace' }],
},
{
code: '<App foo/>',
output: '<App foo />',
options: beforeSelfClosingOptions('proportional-always'),
errors: [{ messageId: 'beforeSelfCloseNeedSpace' }],
},
{
code: `
<App
Expand All @@ -353,7 +357,7 @@ ruleTester.run('jsx-tag-spacing', rule, {
<App
foo={bar}
/>`,
options: beforeSelfClosingOptions('multiline-always'),
options: beforeSelfClosingOptions('proportional-always'),
errors: [{ messageId: 'beforeSelfCloseNeedNewline' }],
},
{
Expand All @@ -364,7 +368,7 @@ ruleTester.run('jsx-tag-spacing', rule, {
<App
foo={bar}${' '}
/>`,
options: beforeSelfClosingOptions('multiline-always'),
options: beforeSelfClosingOptions('proportional-always'),
errors: [{ messageId: 'beforeSelfCloseNeedNewline' }],
},
{
Expand All @@ -383,7 +387,7 @@ ruleTester.run('jsx-tag-spacing', rule, {
hello
</App>
`,
options: beforeClosingOptions('multiline-always'),
options: beforeClosingOptions('proportional-always'),
errors: [{ messageId: 'beforeCloseNeedNewline' }],
},
{
Expand All @@ -400,7 +404,7 @@ ruleTester.run('jsx-tag-spacing', rule, {
hello
</App>
`,
options: beforeClosingOptions('multiline-always'),
options: beforeClosingOptions('proportional-always'),
errors: [{ messageId: 'beforeCloseNeedNewline' }],
},
{
Expand Down

0 comments on commit 316bc40

Please sign in to comment.