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.

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

Merged
merged 5 commits into from Sep 27, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion docs/rules/sort-keys.md
@@ -1,4 +1,4 @@
# requires object keys to be sorted (sort-keys)
# require object keys to be sorted (sort-keys)

When declaring multiple properties, some developers prefer to sort property names alphabetically to be able to find necessary property easier at the later time. Others feel that it adds complexity and becomes burden to maintain.

Expand Down
3 changes: 3 additions & 0 deletions lib/internal-rules/.eslintrc.yml
@@ -0,0 +1,3 @@
rules:
internal-no-invalid-meta: "error"
internal-consistent-docs-description: "error"
131 changes: 131 additions & 0 deletions lib/internal-rules/internal-consistent-docs-description.js
@@ -0,0 +1,131 @@
/**
* @fileoverview Internal rule to enforce meta.docs.description conventions.
* @author Vitor Balocco
*/

"use strict";

const ALLOWED_FIRST_WORDS = [
"enforce",
"require",
"disallow"
];

//------------------------------------------------------------------------------
// 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;
}

/**
* 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 (exportsNode.type !== "ObjectExpression") {

// 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;

if (typeof description !== "string") {
context.report({
node: metaDocsDescription.value,
message: "`meta.docs.description` should be a string."
});
return;
}

if (description === "") {
context.report({
node: metaDocsDescription.value,
message: "`meta.docs.description` should not be empty.",
});
return;
}

if (description.indexOf(" ") === 0) {
context.report({
node: metaDocsDescription.value,
message: "`meta.docs.description` should not start with whitespace."
});
return;
}

const firstWord = description.split(" ")[0];
Copy link
Member

Choose a reason for hiding this comment

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

What if the description is
" requires spaces"

Copy link
Member Author

Choose a reason for hiding this comment

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

@gyandeeps That's a good point, thanks! I pushed a new commit that uses a regular expression to try to get the first word instead of relying on .split(). I also added some extra tests for that. Let me know what you think!


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);
}
}
};
}
};
1 change: 1 addition & 0 deletions lib/rules/.eslintrc.yml
@@ -1,2 +1,3 @@
rules:
internal-no-invalid-meta: "error"
internal-consistent-docs-description: "error"
4 changes: 2 additions & 2 deletions lib/rules/sort-keys.js
@@ -1,5 +1,5 @@
/**
* @fileoverview Rule to requires object keys to be sorted
* @fileoverview Rule to require object keys to be sorted
* @author Toru Nagashima
*/

Expand Down Expand Up @@ -74,7 +74,7 @@ const isValidOrders = {
module.exports = {
meta: {
docs: {
description: "requires object keys to be sorted",
description: "require object keys to be sorted",
category: "Stylistic Issues",
recommended: false
},
Expand Down