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

Improved to not report that a value is required when parsing error #1184

Merged
merged 1 commit into from Jun 5, 2020
Merged

Improved to not report that a value is required when parsing error #1184

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
45 changes: 22 additions & 23 deletions lib/rules/valid-v-bind-sync.js
View file
Open in desktop
Expand Up @@ -37,10 +37,7 @@ function isValidElement(node) {
* @returns {boolean} `true` if the node can be LHS.
*/
function isLhs(node) {
return (
Boolean(node) &&
(node.type === 'Identifier' || node.type === 'MemberExpression')
)
return node.type === 'Identifier' || node.type === 'MemberExpression'
}

// ------------------------------------------------------------------------------
Expand Down Expand Up @@ -85,30 +82,32 @@ module.exports = {
})
}

if (node.value) {
if (!isLhs(node.value.expression)) {
if (!node.value || !node.value.expression) {
return
}

if (!isLhs(node.value.expression)) {
context.report({
node,
loc: node.loc,
messageId: 'unexpectedNonLhsExpression'
})
}

for (const reference of node.value.references) {
const id = reference.id
if (id.parent.type !== 'VExpressionContainer') {
continue
}
const variable = reference.variable
if (variable) {
context.report({
node,
loc: node.loc,
messageId: 'unexpectedNonLhsExpression'
messageId: 'unexpectedUpdateIterationVariable',
data: { varName: id.name }
})
}

for (const reference of node.value.references) {
const id = reference.id
if (id.parent.type !== 'VExpressionContainer') {
continue
}
const variable = reference.variable
if (variable) {
context.report({
node,
loc: node.loc,
messageId: 'unexpectedUpdateIterationVariable',
data: { varName: id.name }
})
}
}
}
}
})
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/valid-v-bind.js
View file
Open in desktop
Expand Up @@ -48,7 +48,7 @@ module.exports = {
}
}

if (!utils.hasAttributeValue(node)) {
if (!node.value || utils.isEmptyValueDirective(node, context)) {
context.report({
node,
loc: node.loc,
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/valid-v-else-if.js
View file
Open in desktop
Expand Up @@ -70,7 +70,7 @@ module.exports = {
message: "'v-else-if' directives require no modifier."
})
}
if (!utils.hasAttributeValue(node)) {
if (!node.value || utils.isEmptyValueDirective(node, context)) {
context.report({
node,
loc: node.loc,
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/valid-v-else.js
View file
Open in desktop
Expand Up @@ -70,7 +70,7 @@ module.exports = {
message: "'v-else' directives require no modifier."
})
}
if (utils.hasAttributeValue(node)) {
if (node.value) {
context.report({
node,
loc: node.loc,
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/valid-v-for.js
View file
Open in desktop
Expand Up @@ -137,7 +137,7 @@ module.exports = {
message: "'v-for' directives require no modifier."
})
}
if (!utils.hasAttributeValue(node)) {
if (!node.value || utils.isEmptyValueDirective(node, context)) {
context.report({
node,
loc: node.loc,
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/valid-v-html.js
View file
Open in desktop
Expand Up @@ -44,7 +44,7 @@ module.exports = {
message: "'v-html' directives require no modifier."
})
}
if (!utils.hasAttributeValue(node)) {
if (!node.value || utils.isEmptyValueDirective(node, context)) {
context.report({
node,
loc: node.loc,
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/valid-v-if.js
View file
Open in desktop
Expand Up @@ -62,7 +62,7 @@ module.exports = {
message: "'v-if' directives require no modifier."
})
}
if (!utils.hasAttributeValue(node)) {
if (!node.value || utils.isEmptyValueDirective(node, context)) {
context.report({
node,
loc: node.loc,
Expand Down
52 changes: 26 additions & 26 deletions lib/rules/valid-v-model.js
View file
Open in desktop
Expand Up @@ -42,10 +42,7 @@ function isValidElement(node) {
* @returns {boolean} `true` if the node can be LHS.
*/
function isLhs(node) {
return (
node != null &&
(node.type === 'Identifier' || node.type === 'MemberExpression')
)
return node.type === 'Identifier' || node.type === 'MemberExpression'
}

/**
Expand Down Expand Up @@ -133,40 +130,43 @@ module.exports = {
}
}

if (!utils.hasAttributeValue(node)) {
if (!node.value || utils.isEmptyValueDirective(node, context)) {
context.report({
node,
loc: node.loc,
message: "'v-model' directives require that attribute value."
})
return
}
if (!node.value.expression) {
// Parsing error
return
}
if (node.value) {
if (!isLhs(node.value.expression)) {
if (!isLhs(node.value.expression)) {
context.report({
node,
loc: node.loc,
message:
"'v-model' directives require the attribute value which is valid as LHS."
})
}

for (const reference of node.value.references) {
const id = reference.id
if (id.parent.type !== 'VExpressionContainer') {
continue
}

const variable = getVariable(id.name, element)
if (variable != null) {
context.report({
node,
loc: node.loc,
message:
"'v-model' directives require the attribute value which is valid as LHS."
"'v-model' directives cannot update the iteration variable '{{varName}}' itself.",
data: { varName: id.name }
})
}

for (const reference of node.value.references) {
const id = reference.id
if (id.parent.type !== 'VExpressionContainer') {
continue
}

const variable = getVariable(id.name, element)
if (variable != null) {
context.report({
node,
loc: node.loc,
message:
"'v-model' directives cannot update the iteration variable '{{varName}}' itself.",
data: { varName: id.name }
})
}
}
}
}
})
Expand Down
30 changes: 20 additions & 10 deletions lib/rules/valid-v-on.js
View file
Open in desktop
Expand Up @@ -108,20 +108,30 @@ module.exports = {
}

if (
!utils.hasAttributeValue(node) &&
(!node.value || !node.value.expression) &&
!node.key.modifiers.some((modifier) =>
VERB_MODIFIERS.has(modifier.name)
)
) {
if (node.value && sourceCode.getText(node.value.expression)) {
const value = sourceCode.getText(node.value)
context.report({
node,
loc: node.loc,
message:
'Avoid using JavaScript keyword as "v-on" value: {{value}}.',
data: { value }
})
if (node.value && !utils.isEmptyValueDirective(node, context)) {
const valueText = sourceCode.getText(node.value)
let innerText = valueText
if (
(valueText[0] === '"' || valueText[0] === "'") &&
valueText[0] === valueText[valueText.length - 1]
) {
// quoted
innerText = valueText.slice(1, -1)
}
if (/^\w+$/.test(innerText)) {
context.report({
node,
loc: node.loc,
message:
'Avoid using JavaScript keyword as "v-on" value: {{value}}.',
data: { value: valueText }
})
}
} else {
context.report({
node,
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/valid-v-show.js
View file
Open in desktop
Expand Up @@ -44,7 +44,7 @@ module.exports = {
message: "'v-show' directives require no modifier."
})
}
if (!utils.hasAttributeValue(node)) {
if (!node.value || utils.isEmptyValueDirective(node, context)) {
context.report({
node,
loc: node.loc,
Expand Down
4 changes: 3 additions & 1 deletion lib/rules/valid-v-slot.js
View file
Open in desktop
Expand Up @@ -264,7 +264,9 @@ module.exports = {
if (
ownerElement === element &&
isDefaultSlot &&
!utils.hasAttributeValue(node)
(!node.value ||
utils.isEmptyValueDirective(node, context) ||
utils.isEmptyExpressionValueDirective(node, context))
) {
context.report({
node,
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/valid-v-text.js
View file
Open in desktop
Expand Up @@ -44,7 +44,7 @@ module.exports = {
message: "'v-text' directives require no modifier."
})
}
if (!utils.hasAttributeValue(node)) {
if (!node.value || utils.isEmptyValueDirective(node, context)) {
context.report({
node,
loc: node.loc,
Expand Down
73 changes: 65 additions & 8 deletions lib/utils/index.js
View file
Open in desktop
Expand Up @@ -290,16 +290,73 @@ module.exports = {
},

/**
* Check whether the given attribute has their attribute value.
* @param {ASTNode} node The attribute node to check.
* @returns {boolean} `true` if the attribute has their value.
* Check whether the given directive attribute has their empty value (`=""`).
* @param {ASTNode} node The directive attribute node to check.
* @param {RuleContext} context The rule context to use parser services.
* @returns {boolean} `true` if the directive attribute has their empty value (`=""`).
*/
hasAttributeValue (node) {
isEmptyValueDirective (node, context) {
assert(node && node.type === 'VAttribute')
return (
node.value != null &&
(node.value.expression != null || node.value.syntaxError != null)
)
if (node.value == null) {
return false
}
if (node.value.expression != null) {
return false
}

let valueText = context.getSourceCode().getText(node.value)
if ((valueText[0] === '"' || valueText[0] === "'") && valueText[0] === valueText[valueText.length - 1]) {
// quoted
valueText = valueText.slice(1, -1)
}
if (!valueText) {
// empty
return true
}
return false
},

/**
* Check whether the given directive attribute has their empty expression value (e.g. `=" "`, `="/* */"`).
* @param {ASTNode} node The directive attribute node to check.
* @param {RuleContext} context The rule context to use parser services.
* @returns {boolean} `true` if the directive attribute has their empty expression value.
*/
isEmptyExpressionValueDirective (node, context) {
assert(node && node.type === 'VAttribute')
if (node.value == null) {
return false
}
if (node.value.expression != null) {
return false
}

const valueNode = node.value
const tokenStore = context.parserServices.getTemplateBodyTokenStore()
let quote1 = null
let quote2 = null
// `node.value` may be only comments, so cannot get the correct tokens with `tokenStore.getTokens(node.value)`.
for (const token of tokenStore.getTokens(node)) {
if (token.range[1] <= valueNode.range[0]) {
continue
}
if (valueNode.range[1] <= token.range[0]) {
// empty
return true
}
if (!quote1 && token.type === 'Punctuator' && (token.value === '"' || token.value === "'")) {
quote1 = token
continue
}
if (!quote2 && quote1 && token.type === 'Punctuator' && (token.value === quote1.value)) {
quote2 = token
continue
}
// not empty
return false
}
// empty
return true
},

/**
Expand Down
15 changes: 15 additions & 0 deletions tests/lib/rules/valid-v-bind-sync.js
View file
Open in desktop
Expand Up @@ -130,6 +130,21 @@ tester.run('valid-v-bind-sync', rule, {
{
filename: 'test.vue',
code: '<template><MyComponent :foo.sync.unknown="foo" /></template>'
},
// parsing error
{
filename: 'parsing-error.vue',
code: '<template><MyComponent :foo.sync="." /></template>'
},
// comment value (parsing error)
{
filename: 'comment-value.vue',
code: '<template><MyComponent :foo.sync="/**/" /></template>'
},
// empty value (valid-v-bind)
{
filename: 'empty-value.vue',
code: '<template><MyComponent :foo.sync="" /></template>'
}
],
invalid: [
Expand Down
16 changes: 16 additions & 0 deletions tests/lib/rules/valid-v-bind.js
View file
Open in desktop
Expand Up @@ -74,6 +74,16 @@ tester.run('valid-v-bind', rule, {
{
filename: 'test.vue',
code: "<template><input v-bind='$attrs' /></template>"
},
// parsing error
{
filename: 'parsing-error.vue',
code: '<template><MyComponent :foo="." /></template>'
},
// comment value (parsing error)
{
filename: 'comment-value.vue',
code: '<template><MyComponent :foo="/**/" /></template>'
}
],
invalid: [
Expand All @@ -91,6 +101,12 @@ tester.run('valid-v-bind', rule, {
filename: 'test.vue',
code: "<template><div :aaa.unknown='bbb'></div></template>",
errors: ["'v-bind' directives don't support the modifier 'unknown'."]
},
// empty value
{
filename: 'empty-value.vue',
code: '<template><MyComponent :foo="" /></template>',
errors: ["'v-bind' directives require an attribute value."]
}
]
})