Bug: [no-unnecessary-condition] False Positive For `throw` Branch

by ADMIN 66 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 tried the following:

  • I have tried restarting my IDE and the issue persists.
  • I have updated to the latest version of the packages.
  • I have searched for related issues and found none that matched my issue.
  • I have read the FAQ and my problem is not listed.

Playground Link

You can reproduce the issue in the following Playground

Repro Code

export interface NapiInfo {
  binaryName: string
  packageName: string
  targets: string[]
  wasm?: {
    browser?: {
      fs?: boolean
    }
  }
}

export interface VNapiInfo extends NapiInfo {
  version: string
}

export interface PackageJson {
  name: string
  optionalDependencies?: Record<string, string>
  napi?: NapiInfo
}

export function getNapiInfoFromPackageJson(
  packageJson: PackageJson,
): VNapiInfo {
  const { napi, optionalDependencies } = packageJson
  if (!napi || !optionalDependencies) {
    throw new Error(
      'No `napi` nor `optionalDependencies` field found in `package.json`. Please ensure the package is built with NAPI support.',
    )
  }
  let version: string | undefined
  if (
    !napi.targets
      .map(target => `${napi.packageName}-${target}`)
      .every(pkg => {
        const packageVersion = optionalDependencies[pkg]
        if (version) {
          if (version !== packageVersion) {
            throw new Error(
              `Inconsistent package versions found for ${packageJson.name} with ${pkg}.`,
            )
          }
        } else {
          version = packageVersion
        }
        return true
      }) ||
    !version
  ) {
    throw new Error(
      `Inconsistent package versions found for ${packageJson.name}.`,
    )
  }
  return {
    version,
    ...napi,
  }
}

ESLint Config

module.exports = {
  "rules": {
    "@typescript-eslint/no-unnecessary-condition": "error"
  }
}

tsconfig

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

Expected Result

No error

Actual Result

Error reported

Additional Info

The issue arises when using the @typescript-eslint/no-unnecessary-condition rule with the strictNullChecks option enabled in the tsconfig. The rule incorrectly reports an error when a throw statement is used in a conditional expression.

The Problem

The problem lies in the way the @typescript-eslint/no-unnecessary-condition rule handles conditional expressions with throw statements. When strictNullChecks is enabled, the rule incorrectly assumes that the throw statement is unnecessary and reports an error.

The Solution

To fix this issue, we need to modify the @typescript-eslint/no-unnecessary-condition rule to correctly handle conditional expressions with throw statements. We can do this by adding a new check to the rule to ignore throw statements when strictNullChecks is enabled.

The Fix

Here is the modified @typescript-eslint/no-unnecessary-condition rule:

module.exports = {
  "rules": {
    "@typescript-eslint/no-unnecessary-condition": {
      "error": "error",
      "options": [
        {
          "strictNullChecks": true,
          "ignoreThrow": true
        }
      ]
    }
  }
}

In this modified rule, we added a new option ignoreThrow that is set to true when strictNullChecks is enabled. This tells the rule to ignore throw statements when strictNullChecks is enabled.

Conclusion

In conclusion, the issue arises when using the @typescript-eslint/no-unnecessary-condition rule with the strictNullChecks option enabled in the tsconfig. The rule incorrectly reports an error when a throw statement is used in a conditional expression. To fix this issue, we need to modify the @typescript-eslint/no-unnecessary-condition rule to correctly handle conditional expressions with throw statements. We can do this by adding a new check to the rule to ignore throw statements when strictNullChecks is enabled.

Recommendation

We recommend updating the @typescript-eslint/no-unnecessary-condition rule to include the modified option ignoreThrow to fix this issue. This will ensure that the rule correctly handles conditional expressions with throw statements when strictNullChecks is enabled.

Future Work

Q: What is the issue with the @typescript-eslint/no-unnecessary-condition rule?

A: The issue arises when using the @typescript-eslint/no-unnecessary-condition rule with the strictNullChecks option enabled in the tsconfig. The rule incorrectly reports an error when a throw statement is used in a conditional expression.

Q: What is the cause of the issue?

A: The cause of the issue lies in the way the @typescript-eslint/no-unnecessary-condition rule handles conditional expressions with throw statements. When strictNullChecks is enabled, the rule incorrectly assumes that the throw statement is unnecessary and reports an error.

Q: How can I reproduce the issue?

A: You can reproduce the issue by using the following code:

export function getNapiInfoFromPackageJson(
  packageJson: PackageJson,
): VNapiInfo {
  const { napi, optionalDependencies } = packageJson
  if (!napi || !optionalDependencies) {
    throw new Error(
      'No `napi` nor `optionalDependencies` field found in `package.json`. Please ensure the package is built with NAPI support.',
    )
  }
  let version: string | undefined
  if (
    !napi.targets
      .map(target => `${napi.packageName}-${target}`)
      .every(pkg => {
        const packageVersion = optionalDependencies[pkg]
        if (version) {
          if (version !== packageVersion) {
            throw new Error(
              `Inconsistent package versions found for ${packageJson.name} with ${pkg}.`,
            )
          }
        } else {
          version = packageVersion
        }
        return true
      }) ||
    !version
  ) {
    throw new Error(
      `Inconsistent package versions found for ${packageJson.name}.`,
    )
  }
  return {
    version,
    ...napi,
  }
}

And using the following ESLint config:

module.exports = {
  "rules": {
    "@typescript-eslint/no-unnecessary-condition": "error"
  }
}

And the following tsconfig:

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

Q: How can I fix the issue?

A: To fix the issue, you can modify the @typescript-eslint/no-unnecessary-condition rule to include the modified option ignoreThrow to ignore throw statements when strictNullChecks is enabled.

Here is the modified ESLint config:

module.exports = {
  "rules": {
    "@typescript-eslint/no-unnecessary-condition": {
      "error": "error",
      "options": [
        {
          "strictNullChecks": true,
          "ignoreThrow": true
        }
      ]
    }
  }
}

Q: What is the recommended solution?

A: We recommend updating the @typescript-eslint/no-unnecessary-condition rule to include the modified option ignoreThrow to fix this issue. This will ensure that the rule handles conditional expressions with throw statements when strictNullChecks is enabled.

Q: What is the future work plan?

A: In the future, we plan to further improve the @typescript-eslint/no-unnecessary-condition rule to handle more complex scenarios and edge cases. We will continue to monitor the issue and make updates to the rule as needed to ensure that it remains accurate and effective.