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
Chore: add internal rule to enforce meta.docs conventions (fixes #6954) #7155
Changes from 3 commits
342e3b7
8b998a2
f835537
f847c52
69f71e7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
rules: | ||
internal-no-invalid-meta: "error" | ||
internal-consistent-docs-description: "error" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,119 @@ | ||
/** | ||
* @fileoverview Internal rule to enforce meta.docs.description conventions. | ||
* @author Vitor Balocco | ||
*/ | ||
|
||
"use strict"; | ||
|
||
const ALLOWED_FIRST_WORDS = [ | ||
"enforce", | ||
"require", | ||
"disallow" | ||
]; | ||
|
||
const FIRST_WORD_REGEX = /^([\s]*[\S]+).*$/; | ||
|
||
//------------------------------------------------------------------------------ | ||
// Helpers | ||
//------------------------------------------------------------------------------ | ||
|
||
/** | ||
* Gets the property of the Object node passed in that has the name specified. | ||
* | ||
* @param {string} property Name of the property to return. | ||
* @param {ASTNode} node The ObjectExpression node. | ||
* @returns {ASTNode} The Property node or null if not found. | ||
*/ | ||
function getPropertyFromObject(property, node) { | ||
const properties = node.properties; | ||
|
||
for (let i = 0; i < properties.length; i++) { | ||
if (properties[i].key.name === property) { | ||
return properties[i]; | ||
} | ||
} | ||
|
||
return null; | ||
} | ||
|
||
/** | ||
* Whether this node is the correct format for a rule definition or not. | ||
* | ||
* @param {ASTNode} node node that the rule exports. | ||
* @returns {boolean} `true` if the exported node is the correct format for a rule definition | ||
*/ | ||
function isCorrectExportsFormat(node) { | ||
return node.type === "ObjectExpression"; | ||
} | ||
|
||
/** | ||
* Verifies that the meta.docs.description property follows our internal conventions. | ||
* | ||
* @param {RuleContext} context The ESLint rule context. | ||
* @param {ASTNode} exportsNode ObjectExpression node that the rule exports. | ||
* @returns {void} | ||
*/ | ||
function checkMetaDocsDescription(context, exportsNode) { | ||
if (!isCorrectExportsFormat(exportsNode)) { | ||
|
||
// if the exported node is not the correct format, "internal-no-invalid-meta" will already report this. | ||
return; | ||
} | ||
|
||
const metaProperty = getPropertyFromObject("meta", exportsNode); | ||
const metaDocs = metaProperty && getPropertyFromObject("docs", metaProperty.value); | ||
const metaDocsDescription = metaDocs && getPropertyFromObject("description", metaDocs.value); | ||
|
||
if (!metaDocsDescription) { | ||
|
||
// if there is no `meta.docs.description` property, "internal-no-invalid-meta" will already report this. | ||
return; | ||
} | ||
|
||
const description = metaDocsDescription.value.value; | ||
const matchFirstWord = FIRST_WORD_REGEX.exec(description); | ||
const firstWord = (matchFirstWord && matchFirstWord[1]) ? matchFirstWord[1] : description; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if it might be a bit clearer if this line was There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch, the check for |
||
|
||
if (ALLOWED_FIRST_WORDS.indexOf(firstWord) === -1) { | ||
context.report({ | ||
node: metaDocsDescription.value, | ||
message: "`meta.docs.description` should start with one of the following words: {{ allowedWords }}. Started with \"{{ firstWord }}\" instead.", | ||
data: { | ||
allowedWords: ALLOWED_FIRST_WORDS.join(", "), | ||
firstWord | ||
} | ||
}); | ||
return; | ||
} | ||
} | ||
|
||
//------------------------------------------------------------------------------ | ||
// Rule Definition | ||
//------------------------------------------------------------------------------ | ||
|
||
module.exports = { | ||
meta: { | ||
docs: { | ||
description: "enforce correct conventions of `meta.docs.description` property in core rules", | ||
category: "Internal", | ||
recommended: false | ||
}, | ||
|
||
schema: [] | ||
}, | ||
|
||
create(context) { | ||
return { | ||
AssignmentExpression(node) { | ||
if (node.left && | ||
node.right && | ||
node.left.type === "MemberExpression" && | ||
node.left.object.name === "module" && | ||
node.left.property.name === "exports") { | ||
|
||
checkMetaDocsDescription(context, node.right); | ||
} | ||
} | ||
}; | ||
} | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,3 @@ | ||
rules: | ||
internal-no-invalid-meta: "error" | ||
internal-consistent-docs-description: "error" |
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.
The function name as it stands now makes it sound like it's actually checking the structure of the exported object. Maybe something like
isObjectExpression()
would be better? In fact, any reason why we can't just inlineexportsNode.type === "ObjectExpression"
since it's only called once and only does that check?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 think inlining it makes sense here. Thanks for the suggestion!