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

Update: Fix accessor-pairs to enforce pairs per property in literals #12062

Merged
merged 3 commits into from Aug 18, 2019
Merged

Update: Fix accessor-pairs to enforce pairs per property in literals #12062

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c61ca1e
Update: Fix accessor-pairs to enforce pairs per property in literals
mdjermanovic Aug 5, 2019
f12bd75
Use getFunctionNameWithKind and getFunctionHeadLoc, fix tests
mdjermanovic Aug 13, 2019
9246eae
Add Known Limitations section in documentation
mdjermanovic Aug 14, 2019
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
212 changes: 179 additions & 33 deletions lib/rules/accessor-pairs.js
View file
Open in desktop
Expand Up @@ -5,10 +5,87 @@

"use strict";

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

const astUtils = require("./utils/ast-utils");

//------------------------------------------------------------------------------
// Typedefs
//------------------------------------------------------------------------------

/**
* Property name if it can be computed statically, otherwise the list of the tokens of the key node.
* @typedef {string|Token[]} Key
*/

/**
* Accessor nodes with the same key.
* @typedef {Object} AccessorData
* @property {Key} key Accessor's key
* @property {ASTNode[]} getters List of getter nodes.
* @property {ASTNode[]} setters List of setter nodes.
*/

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

/**
* Checks whether or not the given lists represent the equal tokens in the same order.
* Tokens are compared by their properties, not by instance.
* @param {Token[]} left First list of tokens.
* @param {Token[]} right Second list of tokens.
* @returns {boolean} `true` if the lists have same tokens.
*/
function areEqualTokenLists(left, right) {
if (left.length !== right.length) {
return false;
}

for (let i = 0; i < left.length; i++) {
const leftToken = left[i],
rightToken = right[i];

if (leftToken.type !== rightToken.type || leftToken.value !== rightToken.value) {
return false;
}
}

return true;
}

/**
* Checks whether or not the given keys are equal.
* @param {Key} left First key.
* @param {Key} right Second key.
* @returns {boolean} `true` if the keys are equal.
*/
function areEqualKeys(left, right) {
if (typeof left === "string" && typeof right === "string") {

// Statically computed names.
return left === right;
}
if (Array.isArray(left) && Array.isArray(right)) {

// Token lists.
return areEqualTokenLists(left, right);
}

return false;
}

/**
* Checks whether or not a given node is of an accessor kind ('get' or 'set').
* @param {ASTNode} node - A node to check.
* @returns {boolean} `true` if the node is of an accessor kind.
*/
function isAccessorKind(node) {
return node.kind === "get" || node.kind === "set";
}

/**
* Checks whether or not a given node is an `Identifier` node which was named a given name.
* @param {ASTNode} node - A node to check.
Expand Down Expand Up @@ -105,61 +182,130 @@ module.exports = {
const config = context.options[0] || {};
const checkGetWithoutSet = config.getWithoutSet === true;
const checkSetWithoutGet = config.setWithoutGet !== false;
const sourceCode = context.getSourceCode();

/**
* Reports the given node.
* @param {ASTNode} node The node to report.
* @param {string} messageId Id of the report message.
* @returns {void}
* @private
*/
function report(node, messageId) {
context.report({ node, messageId });
}

/**
* Checks a object expression to see if it has setter and getter both present or none.
* @param {ASTNode} node The node to check.
* Reports each of the nodes in the given list using the same messageId.
* @param {ASTNode[]} nodes Nodes to report.
* @param {string} messageId Id of the report message.
* @returns {void}
* @private
*/
function checkLonelySetGet(node) {
let isSetPresent = false;
let isGetPresent = false;
const isDescriptor = isPropertyDescriptor(node);
function reportList(nodes, messageId) {
for (const node of nodes) {
report(node, messageId);
}
}

for (let i = 0, end = node.properties.length; i < end; i++) {
const property = node.properties[i];
/**
* Creates a new `AccessorData` object for the given getter or setter node.
* @param {ASTNode} node A getter or setter node.
* @returns {AccessorData} New `AccessorData` object that contains the given node.
* @private
*/
function createAccessorData(node) {
const name = astUtils.getStaticPropertyName(node);
const key = (name !== null) ? name : sourceCode.getTokens(node.key);

let propToCheck = "";
return {
key,
getters: node.kind === "get" ? [node] : [],
setters: node.kind === "set" ? [node] : []
};
}

if (property.kind === "init") {
if (isDescriptor && !property.computed) {
propToCheck = property.key.name;
}
} else {
propToCheck = property.kind;
}
/**
* Merges the given `AccessorData` object into the given accessors list.
* @param {AccessorData[]} accessors The list to merge into.
* @param {AccessorData} accessorData The object to merge.
* @returns {AccessorData[]} The same instance with the merged object.
* @private
*/
function mergeAccessorData(accessors, accessorData) {
const equalKeyElement = accessors.find(a => areEqualKeys(a.key, accessorData.key));

switch (propToCheck) {
case "set":
isSetPresent = true;
break;
if (equalKeyElement) {
equalKeyElement.getters.push(...accessorData.getters);
equalKeyElement.setters.push(...accessorData.setters);
} else {
accessors.push(accessorData);
}

case "get":
isGetPresent = true;
break;
return accessors;
}

default:
/**
* Checks accessor pairs in the given list of nodes.
* @param {ASTNode[]} nodes The list to check.
* @returns {void}
* @private
*/
function checkList(nodes) {
const accessors = nodes
.filter(isAccessorKind)
.map(createAccessorData)
.reduce(mergeAccessorData, []);

// Do nothing
for (const { getters, setters } of accessors) {
if (checkSetWithoutGet && setters.length && !getters.length) {
reportList(setters, "getter");
}

if (isSetPresent && isGetPresent) {
break;
if (checkGetWithoutSet && getters.length && !setters.length) {
reportList(getters, "setter");
}
}
}

if (checkSetWithoutGet && isSetPresent && !isGetPresent) {
context.report({ node, messageId: "getter" });
} else if (checkGetWithoutSet && isGetPresent && !isSetPresent) {
context.report({ node, messageId: "setter" });
/**
* Checks accessor pairs in an object literal.
* @param {ASTNode} node `ObjectExpression` node to check.
* @returns {void}
* @private
*/
function checkObjectLiteral(node) {
checkList(node.properties.filter(p => p.type === "Property"));
}

/**
* Checks accessor pairs in a property descriptor.
* @param {ASTNode} node Property descriptor `ObjectExpression` node to check.
* @returns {void}
* @private
*/
function checkPropertyDescriptor(node) {
const namesToCheck = node.properties
.filter(p => p.type === "Property" && p.kind === "init" && !p.computed)
.map(({ key }) => key.name);

const hasGetter = namesToCheck.includes("get");
const hasSetter = namesToCheck.includes("set");

if (checkSetWithoutGet && hasSetter && !hasGetter) {
report(node, "getter");
}
if (checkGetWithoutSet && hasGetter && !hasSetter) {
report(node, "setter");
}
}

return {
ObjectExpression(node) {
if (checkSetWithoutGet || checkGetWithoutSet) {
checkLonelySetGet(node);
checkObjectLiteral(node);
if (isPropertyDescriptor(node)) {
checkPropertyDescriptor(node);
}
}
}
};
Expand Down