Add consistent-negative-index-access rule (prettier#10569)

.eslintrc.yml

@@ -118,6 +118,7 @@ overrides:
       prettier-internal-rules/better-parent-property-check-in-needs-parens: error
   - files: src/**/*.js
     rules:
+      prettier-internal-rules/consistent-negative-index-access: error
       prettier-internal-rules/flat-ast-path-call: error
       prettier-internal-rules/no-doc-builder-concat: error
       prettier-internal-rules/no-empty-flat-contents-for-if-break: error

scripts/tools/eslint-plugin-prettier-internal-rules/consistent-negative-index-access.js

@@ -0,0 +1,72 @@
+"use strict";
+
+const selector = [
+  "MemberExpression",
+  "[computed=true]",
+  "[optional=false]",
+  '[property.type="BinaryExpression"]',
+  '[property.operator="-"]',
+  '[property.left.type="MemberExpression"]',
+  "[property.left.optional=false]",
+  "[property.left.computed=false]",
+  '[property.left.property.type="Identifier"]',
+  '[property.left.property.name="length"]',
+  '[property.right.type="Literal"]',
+  `:not(${[
+    "AssignmentExpression > .left",
+    "UpdateExpression > .argument",
+    // Ignore `getPenultimate` and `getLast` function self
+    'VariableDeclarator[id.name="getPenultimate"] > ArrowFunctionExpression.init *',
+    'VariableDeclarator[id.name="getLast"] > ArrowFunctionExpression.init *',
+  ].join(", ")})`,
+].join("");
+
+const messageId = "consistent-negative-index-access";
+
+module.exports = {
+  meta: {
+    type: "suggestion",
+    docs: {
+      url:
+        "https://github.com/prettier/prettier/blob/main/scripts/tools/eslint-plugin-prettier-internal-rules/consistent-negative-index-access.js",
+    },
+    messages: {
+      [messageId]: "Prefer `{{method}}(…)` over `…[….length - {{index}}]`.",
+    },
+    fixable: "code",
+  },
+  create(context) {
+    const sourceCode = context.getSourceCode();
+
+    return {
+      [selector](node) {
+        const { value: index } = node.property.right;
+
+        if (index !== 1 && index !== 2) {
+          return;
+        }
+
+        const { object } = node;
+        const lengthObject = node.property.left.object;
+
+        const objectText = sourceCode.getText(object);
+        // Simply use text to compare object
+        if (sourceCode.getText(lengthObject) !== objectText) {
+          return;
+        }
+
+        const method = ["getLast", "getPenultimate"][index - 1];
+
+        context.report({
+          node,
+          messageId,
+          data: {
+            index,
+            method,
+          },
+          fix: (fixer) => fixer.replaceText(node, `${method}(${objectText})`),
+        });
+      },
+    };
+  },
+};

scripts/tools/eslint-plugin-prettier-internal-rules/index.js

@@ -3,6 +3,7 @@
 module.exports = {
   rules: {
     "better-parent-property-check-in-needs-parens": require("./better-parent-property-check-in-needs-parens"),
+    "consistent-negative-index-access": require("./consistent-negative-index-access"),
     "directly-loc-start-end": require("./directly-loc-start-end"),
     "flat-ast-path-call": require("./flat-ast-path-call"),
     "jsx-identifier-case": require("./jsx-identifier-case"),

scripts/tools/eslint-plugin-prettier-internal-rules/test.js

@@ -53,6 +53,50 @@ test("better-parent-property-check-in-needs-parens", {
   })),
 });
 
+test("consistent-negative-index-access", {
+  valid: [
+    "getLast(foo)",
+    "getPenultimate(foo)",
+    "foo[foo.length]",
+    "foo[foo.length - 3]",
+    "foo[foo.length + 1]",
+    "foo[foo.length + -1]",
+    "foo[foo.length * -1]",
+    "foo.length - 1",
+    "foo?.[foo.length - 1]",
+    "foo[foo?.length - 1]",
+    "foo[foo['length'] - 1]",
+    "foo[bar.length - 1]",
+    "foo.bar[foo.      bar.length - 1]",
+    "foo[foo.length - 1]++",
+    "--foo[foo.length - 1]",
+    "foo[foo.length - 1] += 1",
+    "foo[foo.length - 1] = 1",
+  ],
+  invalid: [
+    {
+      code: "foo[foo.length - 1]",
+      output: "getLast(foo)",
+      errors: 1,
+    },
+    {
+      code: "foo[foo.length - 2]",
+      output: "getPenultimate(foo)",
+      errors: 1,
+    },
+    {
+      code: "foo[foo.length - 0b10]",
+      output: "getPenultimate(foo)",
+      errors: 1,
+    },
+    {
+      code: "foo()[foo().length - 1]",
+      output: "getLast(foo())",
+      errors: 1,
+    },
+  ],
+});
+
 test("directly-loc-start-end", {
   valid: [],
   invalid: [

src/document/doc-printer.js

@@ -130,15 +130,15 @@ function trim(out) {
   // Trim whitespace at the end of line
   while (
     out.length > 0 &&
-    typeof out[out.length - 1] === "string" &&
-    /^[\t ]*$/.test(out[out.length - 1])
+    typeof getLast(out) === "string" &&
+    /^[\t ]*$/.test(getLast(out))
   ) {
     trimCount += out.pop().length;
   }
 
-  if (out.length > 0 && typeof out[out.length - 1] === "string") {
-    const trimmed = out[out.length - 1].replace(/[\t ]*$/, "");
-    trimCount += out[out.length - 1].length - trimmed.length;
+  if (out.length > 0 && typeof getLast(out) === "string") {
+    const trimmed = getLast(out).replace(/[\t ]*$/, "");
+    trimCount += getLast(out).length - trimmed.length;
     out[out.length - 1] = trimmed;
   }
 
@@ -353,8 +353,7 @@ function printDocToString(doc, options) {
                 // group has these, we need to manually go through
                 // these states and find the first one that fits.
                 if (doc.expandedStates) {
-                  const mostExpanded =
-                    doc.expandedStates[doc.expandedStates.length - 1];
+                  const mostExpanded = getLast(doc.expandedStates);
 
                   if (doc.break) {
                     cmds.push([ind, MODE_BREAK, mostExpanded]);
@@ -388,7 +387,7 @@ function printDocToString(doc, options) {
           }
 
           if (doc.id) {
-            groupModeMap[doc.id] = cmds[cmds.length - 1][1];
+            groupModeMap[doc.id] = getLast(cmds)[1];
           }
           break;
         // Fills each line with as much code as possible before moving to a new

src/document/doc-utils.js

@@ -1,5 +1,5 @@
 "use strict";
-
+const getLast = require("../utils/get-last");
 const { literalline } = require("./doc-builders");
 
 const isConcat = (doc) => Array.isArray(doc) || (doc && doc.type === "concat");
@@ -128,7 +128,7 @@ function willBreak(doc) {
 
 function breakParentGroup(groupStack) {
   if (groupStack.length > 0) {
-    const parentGroup = groupStack[groupStack.length - 1];
+    const parentGroup = getLast(groupStack);
     // Breaks are not propagated through conditional groups because
     // the user is expected to manually handle what breaks.
     if (!parentGroup.expandedStates && !parentGroup.break) {
@@ -210,7 +210,7 @@ function stripDocTrailingHardlineFromDoc(doc) {
     }
 
     if (parts.length > 0) {
-      const lastPart = stripDocTrailingHardlineFromDoc(parts[parts.length - 1]);
+      const lastPart = stripDocTrailingHardlineFromDoc(getLast(parts));
       parts[parts.length - 1] = lastPart;
     }
     return Array.isArray(doc) ? parts : { ...doc, parts };
@@ -289,10 +289,7 @@ function cleanDocFn(doc) {
     const [currentPart, ...restParts] = isConcat(part)
       ? getDocParts(part)
       : [part];
-    if (
-      typeof currentPart === "string" &&
-      typeof parts[parts.length - 1] === "string"
-    ) {
+    if (typeof currentPart === "string" && typeof getLast(parts) === "string") {
       parts[parts.length - 1] += currentPart;
     } else {
       parts.push(currentPart);
@@ -335,7 +332,7 @@ function normalizeParts(parts) {
 
     if (
       newParts.length > 0 &&
-      typeof newParts[newParts.length - 1] === "string" &&
+      typeof getLast(newParts) === "string" &&
       typeof part === "string"
     ) {
       newParts[newParts.length - 1] += part;

src/language-css/parser-postcss.js

@@ -1,6 +1,7 @@
 "use strict";
 
 const createError = require("../common/parser-create-error");
+const getLast = require("../utils/get-last");
 const parseFrontMatter = require("../utils/front-matter/parse");
 const { hasPragma } = require("./pragma");
 const {
@@ -45,7 +46,7 @@ function parseValueNode(valueNode, options) {
       isSCSS(options.parser, node.value) &&
       node.type === "number" &&
       node.unit === ".." &&
-      node.value[node.value.length - 1] === "."
+      getLast(node.value) === "."
     ) {
       // Work around postcss bug parsing `50...` as `50.` with unit `..`
       // Set the unit to `...` to "accidentally" have arbitrary arguments work in the same way that cases where the node already had a unit work.
@@ -116,11 +117,11 @@ function parseValueNode(valueNode, options) {
       }
 
       commaGroupStack.pop();
-      commaGroup = commaGroupStack[commaGroupStack.length - 1];
+      commaGroup = getLast(commaGroupStack);
       commaGroup.groups.push(parenGroup);
 
       parenGroupStack.pop();
-      parenGroup = parenGroupStack[parenGroupStack.length - 1];
+      parenGroup = getLast(parenGroupStack);
     } else if (node.type === "comma") {
       parenGroup.groups.push(commaGroup);
       commaGroup = {

src/language-css/printer-postcss.js

@@ -1,5 +1,6 @@
 "use strict";
 
+const getLast = require("../utils/get-last");
 const {
   printNumber,
   printString,
@@ -855,7 +856,7 @@ function genericPrint(path, options, print) {
 
       const isSCSSMapItem = isSCSSMapItemNode(path);
 
-      const lastItem = node.groups[node.groups.length - 1];
+      const lastItem = getLast(node.groups);
       const isLastItemComment = lastItem && lastItem.type === "value-comment";
       const isKey = isKeyInValuePairNode(node, parentNode);
 
@@ -941,7 +942,7 @@ function genericPrint(path, options, print) {
         // Don't add spaces on escaped colon `:`, e.g: grid-template-rows: [row-1-00\:00] auto;
         (prevNode &&
           typeof prevNode.value === "string" &&
-          prevNode.value[prevNode.value.length - 1] === "\\") ||
+          getLast(prevNode.value) === "\\") ||
         // Don't add spaces on `:` in `url` function (i.e. `url(fbglyph: cross-outline, fig-white)`)
         insideValueFunctionNode(path, "url")
           ? ""

src/language-handlebars/utils.js

@@ -1,21 +1,22 @@
 "use strict";
 
 const htmlVoidElements = require("html-void-elements");
+const getLast = require("../utils/get-last");
 
 function isLastNodeOfSiblings(path) {
   const node = path.getValue();
   const parentNode = path.getParentNode(0);
 
   if (
     isParentOfSomeType(path, ["ElementNode"]) &&
-    parentNode.children[parentNode.children.length - 1] === node
+    getLast(parentNode.children) === node
   ) {
     return true;
   }
 
   if (
     isParentOfSomeType(path, ["Block"]) &&
-    parentNode.body[parentNode.body.length - 1] === node
+    getLast(parentNode.body) === node
   ) {
     return true;
   }

src/language-html/ast.js

@@ -2,6 +2,7 @@
 
 const fromPairs = require("lodash/fromPairs");
 const { isNonEmptyArray } = require("../common/util");
+const getLast = require("../utils/get-last");
 const NODES_KEYS = {
   attrs: true,
   children: true,
@@ -75,9 +76,7 @@ class Node {
   }
 
   get lastChild() {
-    return isNonEmptyArray(this.children)
-      ? this.children[this.children.length - 1]
-      : null;
+    return isNonEmptyArray(this.children) ? getLast(this.children) : null;
   }
 
   // for element and attribute

src/language-html/parser-html.js

@@ -6,6 +6,7 @@ const {
   ParseSourceFile,
 } = require("angular-html-parser/lib/compiler/src/parse_util");
 const parseFrontMatter = require("../utils/front-matter/parse");
+const getLast = require("../utils/get-last");
 const createError = require("../common/parser-create-error");
 const { inferParserByLanguage } = require("../common/util");
 const {
@@ -314,7 +315,7 @@ function _parse(text, options, parserOptions, shouldParseFrontMatter = true) {
     );
     subAst.sourceSpan = new ParseSourceSpan(
       startSpan,
-      subAst.children[subAst.children.length - 1].sourceSpan.end
+      getLast(subAst.children).sourceSpan.end
     );
     const firstText = subAst.children[0];
     if (firstText.length === offset) {

src/language-html/print-preprocess.js

@@ -3,6 +3,7 @@
 const {
   ParseSourceSpan,
 } = require("angular-html-parser/lib/compiler/src/parse_util");
+const getLast = require("../utils/get-last");
 const {
   htmlTrim,
   getLeadingAndTrailingHtmlWhitespace,
@@ -141,7 +142,7 @@ function mergeNodeIntoText(ast, shouldMerge, getValue) {
 
           if (
             newChildren.length === 0 ||
-            newChildren[newChildren.length - 1].type !== "text"
+            getLast(newChildren).type !== "text"
           ) {
             newChildren.push(newChild);
             continue;

src/language-html/syntax-attribute.js

@@ -1,6 +1,7 @@
 "use strict";
 
 const parseSrcset = require("parse-srcset");
+const getLast = require("../utils/get-last");
 const {
   builders: { group, ifBreak, indent, join, line, softline },
 } = require("../document");
@@ -89,7 +90,7 @@ function printClassNames(value) {
     ) {
       groupedByPrefix.push([]);
     }
-    groupedByPrefix[groupedByPrefix.length - 1].push(classNames[i]);
+    getLast(groupedByPrefix).push(classNames[i]);
     previousPrefix = prefix;
   }
 

src/language-js/comments.js

@@ -392,10 +392,7 @@ function handleClassComments({
       isNonEmptyArray(enclosingNode.decorators) &&
       !(followingNode && followingNode.type === "Decorator")
     ) {
-      addTrailingComment(
-        enclosingNode.decorators[enclosingNode.decorators.length - 1],
-        comment
-      );
+      addTrailingComment(getLast(enclosingNode.decorators), comment);
       return true;
     }