feat(eslint-plugin): [prefer-optional-chain] support suggesting !foo || !foo.bar as a valid match for the rule

packages/eslint-plugin/docs/rules/prefer-optional-chain.md

@@ -1,6 +1,6 @@
 # `prefer-optional-chain`
 
-Enforces using concise optional chain expressions instead of chained logical ands.
+Enforces using concise optional chain expressions instead of chained logical ands, negated logical ors, or empty objects.
 
 TypeScript 3.7 added support for the optional chain operator.
 This operator allows you to safely access properties and methods on objects when they are potentially `null` or `undefined`.
@@ -61,9 +61,15 @@ foo && foo.a && foo.a.b && foo.a.b.c;
 foo && foo['a'] && foo['a'].b && foo['a'].b.c;
 foo && foo.a && foo.a.b && foo.a.b.method && foo.a.b.method();
 
+// With empty objects
 (((foo || {}).a || {}).b || {}).c;
 (((foo || {})['a'] || {}).b || {}).c;
 
+// With negated `or`s
+!foo || !foo.bar;
+!foo || !foo[bar];
+!foo || !foo.bar || !foo.bar.baz || !foo.bar.baz();
+
 // this rule also supports converting chained strict nullish checks:
 foo &&
   foo.a != null &&
@@ -81,6 +87,10 @@ foo?.['a']?.b?.c;
 foo?.a?.b?.method?.();
 
 foo?.a?.b?.c?.d?.e;
+
+!foo?.bar;
+!foo?.[bar];
+!foo?.bar?.baz?.();
 ```
 
 **Note:** there are a few edge cases where this rule will false positive. Use your best judgement when evaluating reported errors.

packages/eslint-plugin/src/rules/explicit-function-return-type.ts

@@ -78,7 +78,7 @@ export default util.createRule<Options, MessageIds>({
         | TSESTree.FunctionExpression
         | TSESTree.FunctionDeclaration,
     ): boolean {
-      if (!options.allowedNames || !options.allowedNames.length) {
+      if (!options.allowedNames?.length) {
         return false;
       }
 

packages/eslint-plugin/src/rules/no-useless-constructor.ts

@@ -34,11 +34,8 @@ function checkAccessibility(node: TSESTree.MethodDefinition): boolean {
  * Check if method is not unless due to typescript parameter properties
  */
 function checkParams(node: TSESTree.MethodDefinition): boolean {
-  return (
-    !node.value.params ||
-    !node.value.params.some(
-      param => param.type === AST_NODE_TYPES.TSParameterProperty,
-    )
+  return !node.value.params?.some(
+    param => param.type === AST_NODE_TYPES.TSParameterProperty,
   );
 }
 

packages/eslint-plugin/src/rules/prefer-optional-chain.ts

@@ -34,7 +34,7 @@ export default util.createRule({
     type: 'suggestion',
     docs: {
       description:
-        'Enforce using concise optional chain expressions instead of chained logical ands',
+        'Enforce using concise optional chain expressions instead of chained logical ands, negated logical ors, or empty objects',
       recommended: 'strict',
       suggestion: true,
     },
@@ -111,6 +111,96 @@ export default util.createRule({
           ],
         });
       },
+      [[
+        'LogicalExpression[operator="||"] > UnaryExpression[operator="!"] > Identifier',
+        'LogicalExpression[operator="||"] > UnaryExpression[operator="!"] > MemberExpression',
+        'LogicalExpression[operator="||"] > UnaryExpression[operator="!"] > ChainExpression > MemberExpression',
+      ].join(',')](
+        initialIdentifierOrNotEqualsExpr:
+          | TSESTree.BinaryExpression
+          | TSESTree.Identifier
+          | TSESTree.MemberExpression,
+      ): void {
+        // selector guarantees this cast
+        const initialExpression = (
+          initialIdentifierOrNotEqualsExpr.parent?.type ===
+          AST_NODE_TYPES.ChainExpression
+            ? initialIdentifierOrNotEqualsExpr.parent.parent
+            : initialIdentifierOrNotEqualsExpr.parent
+        )?.parent as TSESTree.LogicalExpression;
+
+        if (
+          initialExpression.left.type !== AST_NODE_TYPES.UnaryExpression ||
+          initialExpression.left.argument !== initialIdentifierOrNotEqualsExpr
+        ) {
+          // the node(identifier or member expression) is not the deepest left node
+          return;
+        }
+        if (!isValidChainTarget(initialIdentifierOrNotEqualsExpr, true)) {
+          return;
+        }
+
+        // walk up the tree to figure out how many logical expressions we can include
+        let previous: TSESTree.LogicalExpression = initialExpression;
+        let current: TSESTree.Node = initialExpression;
+        let previousLeftText = getText(initialIdentifierOrNotEqualsExpr);
+        let optionallyChainedCode = previousLeftText;
+        let expressionCount = 1;
+        while (current.type === AST_NODE_TYPES.LogicalExpression) {
+          if (
+            current.right.type !== AST_NODE_TYPES.UnaryExpression ||
+            !isValidChainTarget(
+              current.right.argument,
+              // only allow identifiers for the first chain - foo && foo()
+              expressionCount === 1,
+            )
+          ) {
+            break;
+          }
+
+          const leftText = previousLeftText;
+          const rightText = getText(current.right.argument);
+          // can't just use startsWith because of cases like foo && fooBar.baz;
+          const matchRegex = new RegExp(
+            `^${
+              // escape regex characters
+              leftText.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
+            }[^a-zA-Z0-9_$]`,
+          );
+          if (
+            !matchRegex.test(rightText) &&
+            // handle redundant cases like foo.bar && foo.bar
+            leftText !== rightText
+          ) {
+            break;
+          }
+
+          ({
+            expressionCount,
+            previousLeftText,
+            optionallyChainedCode,
+            previous,
+            current,
+          } = normalizeRepeatingPatterns(
+            rightText,
+            leftText,
+            expressionCount,
+            previousLeftText,
+            optionallyChainedCode,
+            previous,
+            current,
+          ));
+        }
+
+        reportIfMoreThanOne({
+          expressionCount,
+          previous,
+          optionallyChainedCode,
+          sourceCode,
+          context,
+          shouldHandleChainedAnds: false,
+        });
+      },
       [[
         'LogicalExpression[operator="&&"] > Identifier',
         'LogicalExpression[operator="&&"] > MemberExpression',
@@ -173,78 +263,31 @@ export default util.createRule({
             break;
           }
 
-          // omit weird doubled up expression that make no sense like foo.bar && foo.bar
-          if (rightText !== leftText) {
-            expressionCount += 1;
-            previousLeftText = rightText;
-
-            /*
-            Diff the left and right text to construct the fix string
-            There are the following cases:
-
-            1)
-            rightText === 'foo.bar.baz.buzz'
-            leftText === 'foo.bar.baz'
-            diff === '.buzz'
-
-            2)
-            rightText === 'foo.bar.baz.buzz()'
-            leftText === 'foo.bar.baz'
-            diff === '.buzz()'
-
-            3)
-            rightText === 'foo.bar.baz.buzz()'
-            leftText === 'foo.bar.baz.buzz'
-            diff === '()'
-
-            4)
-            rightText === 'foo.bar.baz[buzz]'
-            leftText === 'foo.bar.baz'
-            diff === '[buzz]'
-
-            5)
-            rightText === 'foo.bar.baz?.buzz'
-            leftText === 'foo.bar.baz'
-            diff === '?.buzz'
-            */
-            const diff = rightText.replace(leftText, '');
-            if (diff.startsWith('?')) {
-              // item was "pre optional chained"
-              optionallyChainedCode += diff;
-            } else {
-              const needsDot = diff.startsWith('(') || diff.startsWith('[');
-              optionallyChainedCode += `?${needsDot ? '.' : ''}${diff}`;
-            }
-          }
-
-          previous = current;
-          current = util.nullThrows(
-            current.parent,
-            util.NullThrowsReasons.MissingParent,
-          );
+          ({
+            expressionCount,
+            previousLeftText,
+            optionallyChainedCode,
+            previous,
+            current,
+          } = normalizeRepeatingPatterns(
+            rightText,
+            leftText,
+            expressionCount,
+            previousLeftText,
+            optionallyChainedCode,
+            previous,
+            current,
+          ));
         }
 
-        if (expressionCount > 1) {
-          if (previous.right.type === AST_NODE_TYPES.BinaryExpression) {
-            // case like foo && foo.bar !== someValue
-            optionallyChainedCode += ` ${
-              previous.right.operator
-            } ${sourceCode.getText(previous.right.right)}`;
-          }
-
-          context.report({
-            node: previous,
-            messageId: 'preferOptionalChain',
-            suggest: [
-              {
-                messageId: 'optionalChainSuggest',
-                fix: (fixer): TSESLint.RuleFix[] => [
-                  fixer.replaceText(previous, optionallyChainedCode),
-                ],
-              },
-            ],
-          });
-        }
+        reportIfMoreThanOne({
+          expressionCount,
+          previous,
+          optionallyChainedCode,
+          sourceCode,
+          context,
+          shouldHandleChainedAnds: true,
+        });
       },
     };
 
@@ -394,6 +437,132 @@ const ALLOWED_NON_COMPUTED_PROP_TYPES: ReadonlySet<AST_NODE_TYPES> = new Set([
   AST_NODE_TYPES.Identifier,
 ]);
 
+interface ReportIfMoreThanOneOptions {
+  expressionCount: number;
+  previous: TSESTree.LogicalExpression;
+  optionallyChainedCode: string;
+  sourceCode: Readonly<TSESLint.SourceCode>;
+  context: Readonly<
+    TSESLint.RuleContext<
+      'preferOptionalChain' | 'optionalChainSuggest',
+      never[]
+    >
+  >;
+  shouldHandleChainedAnds: boolean;
+}
+
+function reportIfMoreThanOne({
+  expressionCount,
+  previous,
+  optionallyChainedCode,
+  sourceCode,
+  context,
+  shouldHandleChainedAnds,
+}: ReportIfMoreThanOneOptions): void {
+  if (expressionCount > 1) {
+    if (
+      shouldHandleChainedAnds &&
+      previous.right.type === AST_NODE_TYPES.BinaryExpression
+    ) {
+      // case like foo && foo.bar !== someValue
+      optionallyChainedCode += ` ${
+        previous.right.operator
+      } ${sourceCode.getText(previous.right.right)}`;
+    }
+
+    context.report({
+      node: previous,
+      messageId: 'preferOptionalChain',
+      suggest: [
+        {
+          messageId: 'optionalChainSuggest',
+          fix: (fixer): TSESLint.RuleFix[] => [
+            fixer.replaceText(
+              previous,
+              `${shouldHandleChainedAnds ? '' : '!'}${optionallyChainedCode}`,
+            ),
+          ],
+        },
+      ],
+    });
+  }
+}
+
+interface NormalizedPattern {
+  expressionCount: number;
+  previousLeftText: string;
+  optionallyChainedCode: string;
+  previous: TSESTree.LogicalExpression;
+  current: TSESTree.Node;
+}
+
+function normalizeRepeatingPatterns(
+  rightText: string,
+  leftText: string,
+  expressionCount: number,
+  previousLeftText: string,
+  optionallyChainedCode: string,
+  previous: TSESTree.Node,
+  current: TSESTree.Node,
+): NormalizedPattern {
+  // omit weird doubled up expression that make no sense like foo.bar && foo.bar
+  if (rightText !== leftText) {
+    expressionCount += 1;
+    previousLeftText = rightText;
+
+    /*
+    Diff the left and right text to construct the fix string
+    There are the following cases:
+
+    1)
+    rightText === 'foo.bar.baz.buzz'
+    leftText === 'foo.bar.baz'
+    diff === '.buzz'
+
+    2)
+    rightText === 'foo.bar.baz.buzz()'
+    leftText === 'foo.bar.baz'
+    diff === '.buzz()'
+
+    3)
+    rightText === 'foo.bar.baz.buzz()'
+    leftText === 'foo.bar.baz.buzz'
+    diff === '()'
+
+    4)
+    rightText === 'foo.bar.baz[buzz]'
+    leftText === 'foo.bar.baz'
+    diff === '[buzz]'
+
+    5)
+    rightText === 'foo.bar.baz?.buzz'
+    leftText === 'foo.bar.baz'
+    diff === '?.buzz'
+    */
+    const diff = rightText.replace(leftText, '');
+    if (diff.startsWith('?')) {
+      // item was "pre optional chained"
+      optionallyChainedCode += diff;
+    } else {
+      const needsDot = diff.startsWith('(') || diff.startsWith('[');
+      optionallyChainedCode += `?${needsDot ? '.' : ''}${diff}`;
+    }
+  }
+
+  previous = current as TSESTree.LogicalExpression;
+  current = util.nullThrows(
+    current.parent,
+    util.NullThrowsReasons.MissingParent,
+  );
+  return {
+    expressionCount,
+    previousLeftText,
+    optionallyChainedCode,
+    previous,
+    current,
+  };
+}
+
 function isValidChainTarget(
   node: TSESTree.Node,
   allowIdentifier: boolean,