-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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.
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
New: template-tag-spacing
rule (fixes #7631)
#7913
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
ec14157
New: `template-tag-spacing` rule (fixes #7631)
jwilsson f83481c
Check template tags wrapped in parens
jwilsson cd8fd2c
Change rule category to "Stylistic Issues"
jwilsson 8c1bb50
Rephrase rule documentation intro
jwilsson d001c13
Update docs to better explain the rule concept
jwilsson f24deb3
Add tests for tags with comments
jwilsson 72fb1ab
Documentation nits
jwilsson 4699b52
Adjust template tag autofixing behaviour when there's comments present
jwilsson File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
# Require or disallow spacing between template tags and their literals (template-tag-spacing) | ||
|
||
(fixable) The `--fix` option on the [command line](../user-guide/command-line-interface#fix) automatically fixes problems reported by this rule. | ||
|
||
With ES6, it's possible to create functions called [tagged template literals](#further-reading) where the function parameters consist of a template literal's strings and expressions. | ||
|
||
When using tagged template literals, it's possible to insert whitespace between the tag function and the template literal. Since this whitespace is optional, the following lines are equivalent: | ||
|
||
```js | ||
let hello = func`Hello world`; | ||
let hello = func `Hello world`; | ||
``` | ||
|
||
## Rule Details | ||
|
||
This rule aims to maintain consistency around the spacing between template tag functions and their template literals. | ||
|
||
## Options | ||
|
||
```json | ||
{ | ||
"template-tag-spacing": ["error", "never"] | ||
} | ||
``` | ||
|
||
This rule has one option whose value can be set to "never" or "always" | ||
|
||
* `"never"` (default) - Disallows spaces between a tag function and its template literal. | ||
* `"always"` - Requires one or more spaces between a tag function and its template literal. | ||
|
||
## Examples | ||
|
||
### never | ||
|
||
Examples of **incorrect** code for this rule with the default `"never"` option: | ||
|
||
```js | ||
/*eslint template-tag-spacing: "error"*/ | ||
|
||
func `Hello world`; | ||
``` | ||
|
||
Examples of **correct** code for this rule with the default `"never"` option: | ||
|
||
```js | ||
/*eslint template-tag-spacing: "error"*/ | ||
|
||
func`Hello world`; | ||
``` | ||
|
||
### always | ||
|
||
Examples of **incorrect** code for this rule with the `"always"` option: | ||
|
||
```js | ||
/*eslint template-tag-spacing: ["error", "always"]*/ | ||
|
||
func`Hello world`; | ||
``` | ||
|
||
Examples of **correct** code for this rule with the `"always"` option: | ||
|
||
```js | ||
/*eslint template-tag-spacing: ["error", "always"]*/ | ||
|
||
func `Hello world`; | ||
``` | ||
|
||
## When Not To Use It | ||
|
||
If you don't want to be notified about usage of spacing between tag functions and their template literals, then it's safe to disable this rule. | ||
|
||
## Further Reading | ||
|
||
If you want to learn more about tagged template literals, check out the links below: | ||
|
||
* [Template literals (MDN)](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals#Tagged_template_literals) | ||
* [Examples of using tagged template literals (Exploring ES6)](http://exploringjs.com/es6/ch_template-literals.html#_examples-of-using-tagged-template-literals) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,77 @@ | ||
/** | ||
* @fileoverview Rule to check spacing between template tags and their literals | ||
* @author Jonathan Wilsson | ||
*/ | ||
|
||
"use strict"; | ||
|
||
//------------------------------------------------------------------------------ | ||
// Rule Definition | ||
//------------------------------------------------------------------------------ | ||
|
||
module.exports = { | ||
meta: { | ||
docs: { | ||
description: "require or disallow spacing between template tags and their literals", | ||
category: "Stylistic Issues", | ||
recommended: false | ||
}, | ||
|
||
fixable: "whitespace", | ||
|
||
schema: [ | ||
{ enum: ["always", "never"] } | ||
] | ||
}, | ||
|
||
create(context) { | ||
const never = context.options[0] !== "always"; | ||
const sourceCode = context.getSourceCode(); | ||
|
||
/** | ||
* Check if a space is present between a template tag and its literal | ||
* @param {ASTNode} node node to evaluate | ||
* @returns {void} | ||
* @private | ||
*/ | ||
function checkSpacing(node) { | ||
const tagToken = sourceCode.getTokenBefore(node.quasi); | ||
const literalToken = sourceCode.getFirstToken(node.quasi); | ||
const hasWhitespace = sourceCode.isSpaceBetweenTokens(tagToken, literalToken); | ||
|
||
if (never && hasWhitespace) { | ||
context.report({ | ||
node, | ||
loc: tagToken.loc.start, | ||
message: "Unexpected space between template tag and template literal.", | ||
fix(fixer) { | ||
const comments = sourceCode.getComments(node.quasi).leading; | ||
|
||
// Don't fix anything if there's a single line comment after the template tag | ||
if (comments.some(comment => comment.type === "Line")) { | ||
return null; | ||
} | ||
|
||
return fixer.replaceTextRange( | ||
[tagToken.range[1], literalToken.range[0]], | ||
comments.reduce((text, comment) => text + sourceCode.getText(comment), "") | ||
); | ||
} | ||
}); | ||
} else if (!never && !hasWhitespace) { | ||
context.report({ | ||
node, | ||
loc: tagToken.loc.start, | ||
message: "Missing space between template tag and template literal.", | ||
fix(fixer) { | ||
return fixer.insertTextAfter(tagToken, " "); | ||
} | ||
}); | ||
} | ||
} | ||
|
||
return { | ||
TaggedTemplateExpression: checkSpacing | ||
}; | ||
} | ||
}; |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind also adding this to
packages/eslint-config-eslint/default.yml
and turning it on? I think it would be useful to dogfood the rule in the ESLint codebase.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@not-an-aardvark Should that be in a separate (chore) PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would be fine with either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea on the dog fooding! But agreed with @platinumazure that it should probably be a separate PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd be happy to submit a chore PR once this one is finished.