Skip to content

New: html-self-closing rule (fixes #31) #129

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

Merged
merged 5 commits into from
Aug 8, 2017
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 67 additions & 0 deletions docs/rules/html-self-closing-style.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
# Enforce self-closing style (html-self-closing-style)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can shorten the name to just html-self-closing?


In Vue.js template, we can use either two styles for elements which don't have their content.

1. `<your-component></your-component>`
2. `<your-component />` (self-closing)

Self-closing is simple and shorter, but it's not supported in raw HTML.
This rule helps you to unify the self-closing style.

## Rule Details

This rule has options which specify self-closing style for each context.

```json
{
"html-self-closing-style": ["error", {
"html": {
"normal": "never",
"void": "never",
"component": "always"
},
"svg": "always",
"math": "always"
}]
}
```

- `html.normal` (`"never"` by default) ... The style of well-known HTML elements except void elements.
- `html.void` (`"never"` by default) ... The style of well-known HTML void elements.
- `html.component` (`"always"` by default) ... The style of Vue.js custom components.
- `svg`(`"always"` by default) .... The style of well-known SVG elements.
- `math`(`"always"` by default) .... The style of well-known MathML elements.

Every option can be set to one of the following values:

- `"always"` ... Require self-closing at elements which don't have their content.
- `"never"` ... Disallow self-closing.
- `"any"` ... Don't enforce self-closing style.

----

:-1: Examples of **incorrect** code for this rule:

```html
/*eslint html-self-closing-style: "error"*/

<template>
<div />
<img />
<your-component></your-component>
<svg><path d=""></path></svg>
</template>
```

:+1: Examples of **correct** code for this rule:

```html
/*eslint html-self-closing-style: "error"*/

<template>
<div></div>
<img>
<your-component />
<svg><path d="" /></svg>
</template>
```
2 changes: 1 addition & 1 deletion lib/rules/html-end-tags.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ function create (context) {
utils.registerTemplateBodyVisitor(context, {
VElement (node) {
const name = node.name
const isVoid = utils.isVoidElementName(name)
const isVoid = utils.isHtmlVoidElementName(name)
const hasEndTag = node.endTag != null

if (isVoid && hasEndTag) {
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/html-no-self-closing.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ module.exports = {
description: 'disallow self-closing elements.',
category: 'Best Practices',
recommended: false,
replacedBy: []
replacedBy: ['html-self-closing-style']
},
deprecated: true,
fixable: 'code',
Expand Down
179 changes: 179 additions & 0 deletions lib/rules/html-self-closing-style.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
/**
* @author Toru Nagashima
* @copyright 2016 Toru Nagashima. All rights reserved.
* See LICENSE file in root directory for full license.
*/
'use strict'

// ------------------------------------------------------------------------------
// Requirements
// ------------------------------------------------------------------------------

const utils = require('../utils')

// ------------------------------------------------------------------------------
// Helpers
// ------------------------------------------------------------------------------

/**
* Kind strings.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove this comment, it sounds like Nice strings which is funny :D Kinds of strings would sound better, but it's redundant anyway.

* This strings wil be displayed in error messages.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These

*/
const KIND = Object.freeze({
NORMAL: 'HTML elements',
VOID: 'HTML void elements',
COMPONENT: 'Vue.js custom components',
SVG: 'SVG elements',
MATH: 'MathML elements'
})

/**
* Normalize the given options.
* @param {Object|undefined} options The raw options object.
* @returns {Object} Normalized options.
*/
function parseOptions (options) {
return {
[KIND.NORMAL]: (options && options.html && options.html.normal) || 'never',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like we could use https://github.com/letsgetrandy/brototype 😂

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or sindresorhus' dot-prop. However, I'm not sure if this is enough worth adding a new dependency. This may be not stylish, but I think enough readable.

[KIND.VOID]: (options && options.html && options.html.void) || 'never',
[KIND.COMPONENT]: (options && options.html && options.html.component) || 'always',
[KIND.SVG]: (options && options.svg) || 'always',
[KIND.MATH]: (options && options.math) || 'always'
}
}

/**
* Get the kind of the given element.
* @param {VElement} node The element node to get.
* @returns {string} The kind of the element.
*/
function getKind (node) {
if (utils.isCustomComponent(node)) {
return KIND.COMPONENT
}
if (utils.isHtmlElementNode(node)) {
if (utils.isHtmlVoidElementName(node.name)) {
return KIND.VOID
}
return KIND.NORMAL
}
if (utils.isSvgElementNode(node)) {
return KIND.SVG
}
if (utils.isMathMLElementNode(node)) {
return KIND.MATH
}
return 'unknown elements'
}

/**
* Check whether the given element is empty or not.
* This ignores whitespaces, doesn't ignore comments.
* @param {VElement} node The element node to check.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You missed the second parameter here

* @returns {boolean} `true` if the element is empty.
*/
function isEmpty (node, sourceCode) {
const start = node.startTag.range[1]
const end = (node.endTag != null) ? node.endTag.range[0] : node.range[1]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!==

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love == null, this is equivalent to (x === null || typeof x === "undefined") and readable.


return sourceCode.text.slice(start, end).trim() === ''
}

/**
* Creates AST event handlers for html-self-closing-style.
*
* @param {RuleContext} context - The rule context.
* @returns {object} AST event handlers.
*/
function create (context) {
const sourceCode = context.getSourceCode()
const options = parseOptions(context.options[0])

utils.registerTemplateBodyVisitor(context, {
'VElement' (node) {
const kind = getKind(node)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd keep kind simple as: normal, void, component and so on, and only reference the longer (more descriptive) name in message using e.g. getKindName(kind) {} function.

Btw. I think we should rename kind and call it for example htmlElementType or nodeType, what do you think? kind itself doesn't mean much, kind of what?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd keep kind simple as: normal, void, component and so on, and only reference the longer (more descriptive) name in message using e.g. getKindName(kind) {} function.

Hmm, I'm not sure if it increases maintainability/readability.

Btw. I think we should rename kind and call it for example htmlElementType or nodeType, what do you think? kind itself doesn't mean much, kind of what?

It's nice. My English skill is poor.

const mode = options[kind]

if (mode === 'always' && !node.startTag.selfClosing && isEmpty(node, sourceCode)) {
context.report({
node,
loc: node.loc,
message: 'Require self-closing on {{kind}}.',
data: { kind },
fix: (fixer) => {
const tokens = context.parserServices.getTemplateBodyTokenStore()
const close = tokens.getLastToken(node.startTag)
if (close.type !== 'HTMLTagClose') {
return null
}
return fixer.replaceTextRange([close.range[0], node.range[1]], '/>')
}
})
}

if (mode === 'never' && node.startTag.selfClosing) {
context.report({
node,
loc: node.loc,
message: 'Disallow self-closing on {{kind}}.',
data: { kind },
fix: (fixer) => {
const tokens = context.parserServices.getTemplateBodyTokenStore()
const close = tokens.getLastToken(node.startTag)
if (close.type !== 'HTMLSelfClosingTagClose') {
return null
}
if (kind === KIND.VOID) {
return fixer.replaceText(close, '>')
}
return fixer.replaceText(close, `></${node.rawName}>`)
}
})
}
}
})

return {}
}

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------

module.exports = {
create,
meta: {
docs: {
description: 'enforce self-closing style.',
category: 'Stylistic Issues',
recommended: false
},
fixable: 'code',
schema: {
definitions: {
optionValue: {
enum: ['always', 'never', 'any']
}
},
type: 'array',
items: [{
type: 'object',
properties: {
html: {
type: 'object',
properties: {
normal: { $ref: '#/definitions/optionValue' },
void: { $ref: '#/definitions/optionValue' },
component: { $ref: '#/definitions/optionValue' }
},
additionalProperties: false
},
svg: { $ref: '#/definitions/optionValue' },
math: { $ref: '#/definitions/optionValue' }
},
additionalProperties: false
}],
maxItems: 1
}
}
}
19 changes: 15 additions & 4 deletions lib/utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ module.exports = {
assert(node && node.type === 'VElement')

return (
!(this.isKnownHtmlElementNode(node) || this.isSvgElementNode(node) || this.isMathMLElementNode(node)) ||
(this.isHtmlElementNode(node) && !this.isHtmlWellKnownElementName(node.name)) ||
this.hasAttribute(node, 'is') ||
this.hasDirective(node, 'bind', 'is')
)
Expand All @@ -194,10 +194,10 @@ module.exports = {
* @param {ASTNode} node The node to check.
* @returns {boolean} `true` if the node is a HTML element.
*/
isKnownHtmlElementNode (node) {
isHtmlElementNode (node) {
assert(node && node.type === 'VElement')

return node.namespace === vueEslintParser.AST.NS.HTML && HTML_ELEMENT_NAMES.has(node.name.toLowerCase())
return node.namespace === vueEslintParser.AST.NS.HTML
},

/**
Expand All @@ -222,12 +222,23 @@ module.exports = {
return node.namespace === vueEslintParser.AST.NS.MathML
},

/**
* Check whether the given name is an well-known element or not.
* @param {string} name The name to check.
* @returns {boolean} `true` if the name is an well-known element name.
*/
isHtmlWellKnownElementName (name) {
assert(typeof name === 'string')

return HTML_ELEMENT_NAMES.has(name.toLowerCase())
},

/**
* Check whether the given name is a void element name or not.
* @param {string} name The name to check.
* @returns {boolean} `true` if the name is a void element name.
*/
isVoidElementName (name) {
isHtmlVoidElementName (name) {
assert(typeof name === 'string')

return VOID_ELEMENT_NAMES.has(name.toLowerCase())
Expand Down
Loading