Skip to content
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

Wrong unused variable with logical assigment #44802

Open
leonovalex opened this issue Jun 29, 2021 · 8 comments
Open

Wrong unused variable with logical assigment #44802

leonovalex opened this issue Jun 29, 2021 · 8 comments
Labels
Bug A bug in TypeScript
Milestone

Comments

@leonovalex
Copy link

leonovalex commented Jun 29, 2021

Bug Report

πŸ”Ž Search Terms

TS6133

πŸ•— Version & Regression Information

Version: 4.3.4 (nightly is also affected)

⏯ Playground Link

https://www.typescriptlang.org/play?noUnusedLocals=true&ts=4.3.4&strict=true#code/MYewdgzgLgBAZjAvDAFASiQPhgbwFAyEwA2AprAA4BOIAtgJYSkBcMACjQ0wDwCuYAazAgA7mGwAfGPwAmpOPTCkZSaWDkKlMgNwEiVcryphUGRNnxErManUakYAfkfIOdpgDoDEEMQBupOgeUAAWpGAo6Fg2nPaqsvKKymh6hAC+eGlAA

πŸ’» Code

const f = () => {
    let promise: Promise<unknown> | undefined = undefined; // error: TS6133: 'promise' is declared but its value is never read.
    return () => {
        promise ??= Promise.resolve().then(() => promise = undefined)
    }
}

πŸ™ Actual behavior

error: TS6133: 'promise' is declared but its value is never read.

πŸ™‚ Expected behavior

Should be ok. Operator ??= reads variable (as well as ||= and other logical assignments)

@MartinJohns
Copy link
Contributor

The variable is not actually read (aka used) anywhere. That the operators evaluate the content of the variable does not count as it being used.

@andrewbranch andrewbranch added the Working as Intended The behavior described is the intended behavior; this is not a bug label Jun 29, 2021
@typescript-bot
Copy link
Collaborator

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@leonovalex
Copy link
Author

leonovalex commented Jul 29, 2021

The variable is not actually read (aka used) anywhere. That the operators evaluate the content of the variable does not count as it being used.

It might be true if variable was local.
But it isn't.
If inner function called several times then init value should be read to avoid second init
I insist that it is a bug
These operators are conditional assignments. To evaluate condition reading is occurs

@RyanCavanaugh RyanCavanaugh added Bug A bug in TypeScript and removed Working as Intended The behavior described is the intended behavior; this is not a bug labels Jul 29, 2021
@RyanCavanaugh RyanCavanaugh reopened this Jul 29, 2021
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Jul 29, 2021
@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jul 29, 2021

The other compound operators += &= etc are considered to not be a "read" of their LHS, which is correct because x += e has the same "read" effect on x as e;, but this logic shouldn't apply to ??= since there are control flow effects derived from it.

You could imagine writing

let done = undefined;
done ??= doSomething();
done ??= doSomethingElse();
done ??= doAnotherThing();

as some kind of weird control flow scheme. The final statement will always be suspect, but not in an "unused variable" kind of way

@leonovalex
Copy link
Author

leonovalex commented Jul 30, 2021

I think ||= and &&= is also involved for same reason.
Demo: (() => { let a = true; a ||= console.log('Executed')})() will not output 'Executed'

@RyanCavanaugh
Copy link
Member

I think ||= and &&= is also involved for same reason.

πŸ‘

@bennypowers
Copy link

bennypowers commented Dec 14, 2022

consider the case of ecmascript private fields:

export class ReadsPrivateInTruth {
    #priv = false;
    async readsActually() {
        this.#priv ||= await window.expensiveOp();
    }
}

declare global {
    interface Window {
        expensiveOp(): Promise<boolean>;
    }
}

#priv exists to gate the expensiveOp. ||= actually does read #priv. the error is misleading because #priv exists for this purpose.

@odraencoded
Copy link

I was going to post this as a separate issue, but then I found this.

Using ||= (logical OR assignment) or ??= (null-coalescing assignment) with a variable doesn't mark it as read, so you can get the error "variable is declared but its value is never read" with them.

This issue is similar to #51371 and #42422 which were about +=. The only difference is that these assignment operators can be used for flow control since the right side of the expression isn't executed the second time the variable is assigned.

⏯ Playground Link

Playground link

πŸ’» Code

class Foo {
    private foo: string | undefined;

    constructor(startingFoo?: string) {
        this.foo = startingFoo;
    }

    fish(bar: string): void {
        this.foo ??= this.createFoo(bar);
    }

    createFoo(bar: string): string {
        console.log(bar);
        return 'foo: ' + bar;
    }
}

const newFoo = new Foo();
newFoo.fish("This is logged.");
newFoo.fish("This isn't logged.");

πŸ™ Actual behavior

The error 'foo' is declared but its value is never read. is given, despite this.foo deciding whether this.createFoo(bar) is executed or not. The same logic would apply to ||=.

πŸ™‚ Expected behavior

For whomever is the maniac using assignment operators for flow control to not do that and program like a normal person.

I mean. No error. It's a valid strat!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript
Projects
None yet
Development

No branches or pull requests

7 participants