Skip to content

[@typescript-eslint/no-unnecessary-condition] False-negative with ??= #3553

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
3 tasks done
boris-petrov opened this issue Jun 18, 2021 · 4 comments · Fixed by #6594
Closed
3 tasks done

[@typescript-eslint/no-unnecessary-condition] False-negative with ??= #3553

boris-petrov opened this issue Jun 18, 2021 · 4 comments · Fixed by #6594
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement: plugin rule option New rule option for an existing eslint-plugin rule package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@boris-petrov
Copy link

  • I have tried restarting my IDE and the issue persists.
  • I have updated to the latest version of the packages.
  • I have read the FAQ and my problem is not listed.

Repro

const result: { [key: number]: number } = {};
result[1] ??= 1;
{
  "rules": {
    '@typescript-eslint/no-unnecessary-condition': ['error', {
      allowConstantLoopConditions: true,
      allowRuleToRunWithoutStrictNullChecksIKnowWhatIAmDoing: false,
    }],
  }
}

Expected Result

An error of that rule to be reported.

Actual Result

No error.

Additional Info

if (result[1] == null) {
  result[1] = 1;
}

This does result in an error so the other code should too.

Versions

package version
@typescript-eslint/eslint-plugin 4.27.0
@typescript-eslint/parser 4.27.0
TypeScript 4.3.4
ESLint 7.28.0
node 16.3.0
@boris-petrov boris-petrov added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Jun 18, 2021
@bradzacher
Copy link
Member

The rule just doesn't support these operators yet.

@bradzacher bradzacher added enhancement: plugin rule option New rule option for an existing eslint-plugin rule and removed triage Waiting for team members to take a look labels Jun 18, 2021
@JoshuaKGoldberg JoshuaKGoldberg added the accepting prs Go ahead, send a pull request that resolves this issue label Oct 25, 2021
@bradzacher
Copy link
Member

Just to be clear for posterity's sake - the assignment operators are equivalent to the expression variable operator (variable = value) eg:

const result: { [key: number]: number } = {};
result[1] ??= 1;
result[1] ?? (result[1] = 1);

declare let foo: null;
foo ||= null;
foo || (foo = null);

declare let bar: {};
bar &&= 1;
bar && (bar = 1);

But all of those cases are supported - so it's a no brainer to add this
playground

@chharvey
Copy link

chharvey commented Mar 14, 2023

I think I found a bug when tsconfig’s exactOptionalPropertyTypes is on. With this setting, optional properties are treated as nullish when accessed but as non-nullish when assigned.

// tsconfig:compilerOptions.exactOptionalPropertyTypes = true

const obj: { prop?: [number] } = {};

obj.prop;
//  ^? (property) prop?: [number] | undefined

obj.prop = [42];
//  ^? (property) prop?: [number]

I have a function that sets the property once and returns it on the first call, but subsequent calls only return it without setting it:

function setOnce(value: number): [number] {
    return obj.prop ??= [value]; // ! @typescript-eslint/no-unnecessary-condition: "Unnecessary conditional, expected left-hand side of `??` operator to be possibly null or undefined."
}

setOnce(1); //=> [1]
setOnce(2); //=> [1]
obj.prop.push(3);
setOnce(1);  //=> [1, 3]

The problem here is that typescript-eslint thinks obj.prop is non-nullish and reports an error, even though it is nullish before being assigned.

I’m not sure what the right solution is. We want to keep --exactOptionalPropertyTypes on to avoid statements like obj.prop = undefined;, but we still want to allow the ??= assignment operator.

@Josh-Cena
Copy link
Member

@chharvey Thanks for the report. This seems related to #5344 (an optional property is different from a property with undefined). Could you send a new issue?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement: plugin rule option New rule option for an existing eslint-plugin rule package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants