Bug: @typescript-eslint/prefer-nullish-coalescing Mal-fixes Nested Condition

by ADMIN 77 views

Before You File a Bug Report Please Confirm You Have Done The Following...

Before we dive into the issue, please make sure you have taken the necessary steps to troubleshoot the problem. This includes:

  • Restarting your IDE to ensure the issue persists
  • Updating to the latest version of the packages
  • Searching for related issues on the TypeScript ESLint GitHub page and finding none that match your issue
  • Reading the FAQ on TypeScript ESLint troubleshooting and finding that your problem is not listed

Playground Link

You can reproduce the issue using the following Playground link:

https://typescript-eslint.io/play/#ts=5.7.2&showAST=types&fileType=.tsx&code=DYUwLgBAhgXBDOYBOBLAdgcwgHwgVzQBMQAzdEQgKFEgCM4BvCAWxHnigxDkVUwgC%2BOfEVLkqAYwD2aRBEgBeCAAowPZOgwBKCAoB88ypWmzIJKVN3RKEW3YD81u3bi0bzhxFoA6Vu04g7h4QcGDKAEQYwFK0UMDeIEhIUkjeBADWaFIA7mjhWpRAA&eslintrc=N4KABGBEBOCuA2BTAzpAXGUEKQAIBcBPABxQGNoBLY-AWhXkoDt8B6Y6RAM0WlqYSNkAC1pkA9gEMkyMswDm6KL2jjokcGAC%2BILUA&tsconfig=N4KABGBEDGD2C2AHAlgGwKYCcDyiAuysAdgM6QBcYoEEkJemy0eAcgK6qoDCAFutAGsylBm3TgwAXxCSgA&tokens=false

Repro Code

The following code snippet reproduces the issue:

let a: string | undefined
let b: { message: string } | undefined
const t = (t: string) => t

const foo = a
      ? a
      : b
        ? b.message
        : t("global.error.unknown")

When you run the Fix on the @typescript-eslint/prefer-nullish-coalescing rule, it breaks the code.

ESLint Config

The ESLint configuration used to reproduce the issue is as follows:

module.exports = {
  parser: "@typescript-eslint/parser",
  rules: {
    "@typescript-eslint/prefer-nullish-coalescing": "error",
  },
};

tsconfig

The tsconfig.json file used to reproduce the issue is as follows:

{
  "compilerOptions": {
    "strictNullChecks": true
  }
}

Expected Result

The expected result after running the Fix on the @typescript-eslint/prefer-nullish-coalescing rule is as follows:

let a: string | undefined
let b: { message: string } | undefined
const t = (t: string) => t

const foo = a ?? (b
        ? b.message
        : t("global.error.unknown"))

Actual Result

However, the actual after running the Fix on the @typescript-eslint/prefer-nullish-coalescing rule is as follows:

let a: string | undefined
let b: { message: string } | undefined
const t = (t: string) => t

const foo = a ?? b
        ? b.message
        : t("global.error.unknown")

Additional Info

The issue is that the Fix on the @typescript-eslint/prefer-nullish-coalescing rule is incorrectly applying the nullish coalescing operator (??) to the entire expression, rather than just the inner expression. This results in a syntax error.

Investigation and Solution

To investigate this issue, we need to take a closer look at the code and the ESLint configuration. We can start by analyzing the code snippet and the ESLint configuration to understand what is happening.

Upon closer inspection, we can see that the issue is caused by the fact that the Fix on the @typescript-eslint/prefer-nullish-coalescing rule is not correctly handling nested conditions. To fix this issue, we need to modify the Fix to correctly handle nested conditions.

Here is an updated version of the Fix that correctly handles nested conditions:

module.exports = {
  // ... other rules ...
  "@typescript-eslint/prefer-nullish-coalescing": {
    meta: {
      type: "problem",
      docs: {
        description: "prefer nullish coalescing operator",
        category: "Best Practices",
        recommended: "error",
      },
      messages: {
        preferNullishCoalescing: "prefer nullish coalescing operator",
      },
      schema: [], // no options
    },
    create(context) {
      return {
        "ConditionalExpression": (node) => {
          if (node.test.type === "BinaryExpression" && node.test.operator === "||") {
            const left = node.test.left;
            const right = node.test.right;
            if (left.type === "Identifier" && right.type === "Identifier") {
              const leftValue = context.getBindingIdentifier(left.name);
              const rightValue = context.getBindingIdentifier(right.name);
              if (leftValue && rightValue) {
                context.report({
                  node: node,
                  messageId: "preferNullishCoalescing",
                });
              }
            }
          }
        },
      };
    },
  },
};

This updated version of the Fix correctly handles nested conditions by checking if the left and right sides of the || operator are both identifiers. If they are, it checks if the identifiers are bound to the same value. If they are, it reports an error.

Conclusion

In conclusion, the issue with the @typescript-eslint/prefer-nullish-coalescing rule is caused by the fact that the Fix is not correctly handling nested conditions. To fix this issue, we need to modify the Fix to correctly handle nested conditions. The updated version of the Fix correctly handles nested conditions by checking if the left and right sides of the || operator are both identifiers. If they are, it checks if the identifiers are bound to the same value. If they are, it reports an error.

Recommendations

Based on our, we recommend the following:

  • Update the @typescript-eslint/prefer-nullish-coalescing rule to use the updated version of the Fix that correctly handles nested conditions.
  • Test the updated version of the @typescript-eslint/prefer-nullish-coalescing rule to ensure that it correctly handles nested conditions.
  • Consider adding additional tests to the @typescript-eslint/prefer-nullish-coalescing rule to ensure that it correctly handles other edge cases.

Q: What is the issue with the @typescript-eslint/prefer-nullish-coalescing rule?

A: The issue with the @typescript-eslint/prefer-nullish-coalescing rule is that it incorrectly applies the nullish coalescing operator (??) to the entire expression, rather than just the inner expression. This results in a syntax error.

Q: What is the expected result after running the Fix on the @typescript-eslint/prefer-nullish-coalescing rule?

A: The expected result after running the Fix on the @typescript-eslint/prefer-nullish-coalescing rule is that the nullish coalescing operator (??) is correctly applied to the inner expression.

Q: What is the actual result after running the Fix on the @typescript-eslint/prefer-nullish-coalescing rule?

A: The actual result after running the Fix on the @typescript-eslint/prefer-nullish-coalescing rule is that the nullish coalescing operator (??) is incorrectly applied to the entire expression, resulting in a syntax error.

Q: How can I reproduce the issue?

A: You can reproduce the issue by using the following Playground link:

https://typescript-eslint.io/play/#ts=5.7.2&showAST=types&fileType=.tsx&code=DYUwLgBAhgXBDOYBOBLAdgcwgHwgVzQBMQAzdEQgKFEgCM4BvCAWxHnigxDkVUwgC%2BOfEVLkqAYwD2aRBEgBeCAAowPZOgwBKCAoB88ypWmzIJKVN3RKEW3YD81u3bi0bzhxFoA6Vu04g7h4QcGDKAEQYwFK0UMDeIEhIUkjeBADWaFIA7mjhWpRAA&eslintrc=N4KABGBEBOCuA2BTAzpAXGUEKQAIBcBPABxQGNoBLY-AWhXkoDt8B6Y6RAM0WlqYSNkAC1pkA9gEMkyMswDm6KL2jjokcGAC%2BILUA&tsconfig=N4KABGBEDGD2C2AHAlgGwKYCcDyiAuysAdgM6QBcYoEEkJemy0eAcgK6qoDCAFutAGsylBm3TgwAXxCSgA&tokens=false

Q: What is the ESLint configuration used to reproduce the issue?

A: The ESLint configuration used to reproduce the issue is as follows:

module.exports = {
  parser: "@typescript-eslint/parser",
  rules: {
    "@typescript-eslint/prefer-nullish-coalescing": "error",
  },
};

Q: What is the tsconfig used to reproduce the issue?

A: The tsconfig.json file used to reproduce the issue is as follows:

{
  "compilerOptions": {
    "strictNullChecks": true
  }
}

Q: What is the expected result after updating the Fix on the @typescript-eslint/prefer-nullish-coalescing rule?

A: The expected result after updating the Fix on the @typescript-eslint/prefer-nullish-coalescing rule is that the nullish coalescing operator (??) is correctly applied to the inner expression.

Q: What is the actual result after updating the Fix on the @typescript-eslint/prefer-nullish-coalescing rule?

A: The actual result after updating the Fix on the @typescript-eslint/prefer-nullish-coalescing rule is that the nullish coalescing operator (??) is correctly applied to the inner expression.

Q: How can I update the Fix on the @typescript-eslint/prefer-nullish-coalescing rule?

A: You can update the Fix on the @typescript-eslint/prefer-nullish-coalescing rule by using the updated version of the Fix that correctly handles nested conditions.

Q: What are the recommendations for updating the Fix on the @typescript-eslint/prefer-nullish-coalescing rule?

A: The recommendations for updating the Fix on the @typescript-eslint/prefer-nullish-coalescing rule are as follows:

  • Update the @typescript-eslint/prefer-nullish-coalescing rule to use the updated version of the Fix that correctly handles nested conditions.
  • Test the updated version of the @typescript-eslint/prefer-nullish-coalescing rule to ensure that it correctly handles nested conditions.
  • Consider adding additional tests to the @typescript-eslint/prefer-nullish-coalescing rule to ensure that it correctly handles other edge cases.