Skip to content

Commit

Permalink
refactor: follow very nice code advice
Browse files Browse the repository at this point in the history
  • Loading branch information
shulaoda committed May 14, 2024
1 parent 308d8c9 commit dfe869e
Showing 1 changed file with 157 additions and 63 deletions.
220 changes: 157 additions & 63 deletions lib/rules/object-shorthand.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ module.exports = {
type: "suggestion",

docs: {
description: "Require or disallow method and property shorthand syntax for object literals",
description:
"Require or disallow method and property shorthand syntax for object literals",
recommended: false,
url: "https://eslint.org/docs/latest/rules/object-shorthand"
},
Expand All @@ -41,7 +42,14 @@ module.exports = {
type: "array",
items: [
{
enum: ["always", "methods", "properties", "never", "consistent", "consistent-as-needed"]
enum: [
"always",
"methods",
"properties",
"never",
"consistent",
"consistent-as-needed"
]
}
],
minItems: 0,
Expand Down Expand Up @@ -98,20 +106,25 @@ module.exports = {
},

messages: {
expectedAllPropertiesShorthanded: "Expected shorthand for all properties.",
expectedLiteralMethodLongform: "Expected longform method syntax for string literal keys.",
expectedAllPropertiesShorthanded:
"Expected shorthand for all properties.",
expectedLiteralMethodLongform:
"Expected longform method syntax for string literal keys.",
expectedPropertyShorthand: "Expected property shorthand.",
expectedPropertyLongform: "Expected longform property syntax.",
expectedMethodShorthand: "Expected method shorthand.",
expectedMethodLongform: "Expected longform method syntax.",
unexpectedMix: "Unexpected mix of shorthand and non-shorthand properties."
unexpectedMix:
"Unexpected mix of shorthand and non-shorthand properties."
}
},

create(context) {
const APPLY = context.options[0] || OPTIONS.always;
const APPLY_TO_METHODS = APPLY === OPTIONS.methods || APPLY === OPTIONS.always;
const APPLY_TO_PROPS = APPLY === OPTIONS.properties || APPLY === OPTIONS.always;
const APPLY_TO_METHODS =
APPLY === OPTIONS.methods || APPLY === OPTIONS.always;
const APPLY_TO_PROPS =
APPLY === OPTIONS.properties || APPLY === OPTIONS.always;
const APPLY_NEVER = APPLY === OPTIONS.never;
const APPLY_CONSISTENT = APPLY === OPTIONS.consistent;
const APPLY_CONSISTENT_AS_NEEDED = APPLY === OPTIONS.consistentAsNeeded;
Expand Down Expand Up @@ -157,7 +170,13 @@ module.exports = {
* @private
*/
function canHaveShorthand(property) {
return (property.kind !== "set" && property.kind !== "get" && property.type !== "SpreadElement" && property.type !== "SpreadProperty" && property.type !== "ExperimentalSpreadProperty");
return (
property.kind !== "set" &&
property.kind !== "get" &&
property.type !== "SpreadElement" &&
property.type !== "SpreadProperty" &&
property.type !== "ExperimentalSpreadProperty"
);
}

/**
Expand All @@ -178,7 +197,7 @@ module.exports = {
function isShorthand(property) {

// property.method is true when `{a(){}}`.
return (property.shorthand || property.method);
return property.shorthand || property.method;
}

/**
Expand Down Expand Up @@ -230,10 +249,14 @@ module.exports = {
* If all properties of the object contain a method or value with a name matching it's key,
* all the keys are redundant.
*/
const canAlwaysUseShorthand = properties.every(isRedundant);
const canAlwaysUseShorthand =
properties.every(isRedundant);

if (canAlwaysUseShorthand) {
context.report({ node, messageId: "expectedAllPropertiesShorthanded" });
context.report({
node,
messageId: "expectedAllPropertiesShorthanded"
});
}
}
}
Expand All @@ -251,9 +274,16 @@ module.exports = {
? sourceCode.getFirstToken(node, astUtils.isOpeningBracketToken)
: sourceCode.getFirstToken(node.key);
const lastKeyToken = node.computed
? sourceCode.getFirstTokenBetween(node.key, node.value, astUtils.isClosingBracketToken)
? sourceCode.getFirstTokenBetween(
node.key,
node.value,
astUtils.isClosingBracketToken
)
: sourceCode.getLastToken(node.key);
const keyText = sourceCode.text.slice(firstKeyToken.range[0], lastKeyToken.range[1]);
const keyText = sourceCode.text.slice(
firstKeyToken.range[0],
lastKeyToken.range[1]
);
let keyPrefix = "";

// key: /* */ () => {}
Expand All @@ -272,44 +302,47 @@ module.exports = {
const methodPrefix = keyPrefix + keyText;

if (node.value.type === "FunctionExpression") {
const functionToken = sourceCode.getTokens(node.value).find(token => token.type === "Keyword" && token.value === "function");
const tokenBeforeParams = node.value.generator ? sourceCode.getTokenAfter(functionToken) : functionToken;
const functionToken = sourceCode
.getTokens(node.value)
.find(
token =>
token.type === "Keyword" &&
token.value === "function"
);
const tokenBeforeParams = node.value.generator
? sourceCode.getTokenAfter(functionToken)
: functionToken;

return fixer.replaceTextRange(
fixRange,
methodPrefix + sourceCode.text.slice(tokenBeforeParams.range[1], node.value.range[1])
methodPrefix +
sourceCode.text.slice(
tokenBeforeParams.range[1],
node.value.range[1]
)
);
}

const arrowToken = sourceCode.getTokenBefore(node.value.body, astUtils.isArrowToken);
const fnBody = sourceCode.text.slice(arrowToken.range[1], node.value.range[1]);

let tokenBeforeParams;
let shouldAddParensAroundParameters = false;
const arrowToken = sourceCode.getTokenBefore(
node.value.body,
astUtils.isArrowToken
);
const fnBody = sourceCode.text.slice(
arrowToken.range[1],
node.value.range[1]
);

// First token should not be `async`
if (node.value.async) {
tokenBeforeParams = sourceCode.getFirstToken(node.value, token => !(token.type === "Identifier" && token.value === "async"));
} else {
tokenBeforeParams = sourceCode.getFirstToken(node.value);
}

if (node.value.params.length === 1) {
const token = sourceCode.getTokenBefore(node.value.params[0]);

const hasParen = astUtils.isOpeningParenToken(token);
const isTokenOutsideNode = token.range[0] < node.value.range[0];

shouldAddParensAroundParameters = !hasParen || isTokenOutsideNode;
}
const firstValueToken = sourceCode.getFirstToken(node.value, {
skip: node.value.async ? 1 : 0
});

const sliceStart = shouldAddParensAroundParameters
? node.value.params[0].range[0]
: tokenBeforeParams.range[0];
const sliceStart = firstValueToken.range[0];
const sliceEnd = sourceCode.getTokenBefore(arrowToken).range[1];
const shouldAddParens = node.value.params.length === 1 && node.value.params[0].range[0] === sliceStart;

const oldParamText = sourceCode.text.slice(sliceStart, sliceEnd);
const newParamText = shouldAddParensAroundParameters ? `(${oldParamText})` : oldParamText;
const newParamText = shouldAddParens ? `(${oldParamText})` : oldParamText;

return fixer.replaceTextRange(
fixRange,
Expand All @@ -324,9 +357,20 @@ module.exports = {
* @returns {Object} A fix for this node
*/
function makeFunctionLongform(fixer, node) {
const firstKeyToken = node.computed ? sourceCode.getTokens(node).find(token => token.value === "[") : sourceCode.getFirstToken(node.key);
const lastKeyToken = node.computed ? sourceCode.getTokensBetween(node.key, node.value).find(token => token.value === "]") : sourceCode.getLastToken(node.key);
const keyText = sourceCode.text.slice(firstKeyToken.range[0], lastKeyToken.range[1]);
const firstKeyToken = node.computed
? sourceCode
.getTokens(node)
.find(token => token.value === "[")
: sourceCode.getFirstToken(node.key);
const lastKeyToken = node.computed
? sourceCode
.getTokensBetween(node.key, node.value)
.find(token => token.value === "]")
: sourceCode.getLastToken(node.key);
const keyText = sourceCode.text.slice(
firstKeyToken.range[0],
lastKeyToken.range[1]
);
let functionHeader = "function";

if (node.value.async) {
Expand All @@ -336,7 +380,10 @@ module.exports = {
functionHeader = `${functionHeader}*`;
}

return fixer.replaceTextRange([node.range[0], lastKeyToken.range[1]], `${keyText}: ${functionHeader}`);
return fixer.replaceTextRange(
[node.range[0], lastKeyToken.range[1]],
`${keyText}: ${functionHeader}`
);
}

/*
Expand All @@ -361,9 +408,15 @@ module.exports = {
*/
function enterFunction(node) {
lexicalScopeStack.unshift(new Set());
sourceCode.getScope(node).variables.filter(variable => variable.name === "arguments").forEach(variable => {
variable.references.map(ref => ref.identifier).forEach(identifier => argumentsIdentifiers.add(identifier));
});
sourceCode
.getScope(node)
.variables.filter(variable => variable.name === "arguments")
.forEach(variable => {
variable.references
.map(ref => ref.identifier)
.forEach(identifier =>
argumentsIdentifiers.add(identifier));
});
}

/**
Expand All @@ -380,7 +433,8 @@ module.exports = {
* @returns {void}
*/
function reportLexicalIdentifier() {
lexicalScopeStack[0].forEach(arrowFunction => arrowsWithLexicalIdentifiers.add(arrowFunction));
lexicalScopeStack[0].forEach(arrowFunction =>
arrowsWithLexicalIdentifiers.add(arrowFunction));
}

//--------------------------------------------------------------------------
Expand All @@ -405,7 +459,10 @@ module.exports = {
ThisExpression: reportLexicalIdentifier,
Super: reportLexicalIdentifier,
MetaProperty(node) {
if (node.meta.name === "new" && node.property.name === "target") {
if (
node.meta.name === "new" &&
node.property.name === "target"
) {
reportLexicalIdentifier();
}
},
Expand Down Expand Up @@ -437,15 +494,25 @@ module.exports = {
}

// only computed methods can fail the following checks
if (node.computed && node.value.type !== "FunctionExpression" && node.value.type !== "ArrowFunctionExpression") {
if (
node.computed &&
node.value.type !== "FunctionExpression" &&
node.value.type !== "ArrowFunctionExpression"
) {
return;
}

//--------------------------------------------------------------
// Checks for property/method shorthand.
if (isConciseProperty) {
if (node.method && (APPLY_NEVER || AVOID_QUOTES && isStringLiteral(node.key))) {
const messageId = APPLY_NEVER ? "expectedMethodLongform" : "expectedLiteralMethodLongform";
if (
node.method &&
(APPLY_NEVER ||
(AVOID_QUOTES && isStringLiteral(node.key)))
) {
const messageId = APPLY_NEVER
? "expectedMethodLongform"
: "expectedLiteralMethodLongform";

// { x() {} } should be written as { x: function() {} }
context.report({
Expand All @@ -459,18 +526,35 @@ module.exports = {
context.report({
node,
messageId: "expectedPropertyLongform",
fix: fixer => fixer.insertTextAfter(node.key, `: ${node.key.name}`)
fix: fixer =>
fixer.insertTextAfter(
node.key,
`: ${node.key.name}`
)
});
}
} else if (APPLY_TO_METHODS && !node.value.id && (node.value.type === "FunctionExpression" || node.value.type === "ArrowFunctionExpression")) {
if (IGNORE_CONSTRUCTORS && node.key.type === "Identifier" && isConstructor(node.key.name)) {
} else if (
APPLY_TO_METHODS &&
!node.value.id &&
(node.value.type === "FunctionExpression" ||
node.value.type === "ArrowFunctionExpression")
) {
if (
IGNORE_CONSTRUCTORS &&
node.key.type === "Identifier" &&
isConstructor(node.key.name)
) {
return;
}

if (METHODS_IGNORE_PATTERN) {
const propertyName = astUtils.getStaticPropertyName(node);
const propertyName =
astUtils.getStaticPropertyName(node);

if (propertyName !== null && METHODS_IGNORE_PATTERN.test(propertyName)) {
if (
propertyName !== null &&
METHODS_IGNORE_PATTERN.test(propertyName)
) {
return;
}
}
Expand All @@ -480,19 +564,24 @@ module.exports = {
}

// {[x]: function(){}} should be written as {[x]() {}}
if (node.value.type === "FunctionExpression" ||
node.value.type === "ArrowFunctionExpression" &&
node.value.body.type === "BlockStatement" &&
AVOID_EXPLICIT_RETURN_ARROWS &&
!arrowsWithLexicalIdentifiers.has(node.value)
if (
node.value.type === "FunctionExpression" ||
(node.value.type === "ArrowFunctionExpression" &&
node.value.body.type === "BlockStatement" &&
AVOID_EXPLICIT_RETURN_ARROWS &&
!arrowsWithLexicalIdentifiers.has(node.value))
) {
context.report({
node,
messageId: "expectedMethodShorthand",
fix: fixer => makeFunctionShorthand(fixer, node)
});
}
} else if (node.value.type === "Identifier" && node.key.name === node.value.name && APPLY_TO_PROPS) {
} else if (
node.value.type === "Identifier" &&
node.key.name === node.value.name &&
APPLY_TO_PROPS
) {

// {x: x} should be written as {x}
context.report({
Expand All @@ -502,7 +591,12 @@ module.exports = {
return fixer.replaceText(node, node.value.name);
}
});
} else if (node.value.type === "Identifier" && node.key.type === "Literal" && node.key.value === node.value.name && APPLY_TO_PROPS) {
} else if (
node.value.type === "Identifier" &&
node.key.type === "Literal" &&
node.key.value === node.value.name &&
APPLY_TO_PROPS
) {
if (AVOID_QUOTES) {
return;
}
Expand Down

0 comments on commit dfe869e

Please sign in to comment.