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

noUnusedLocals: Don't count self-call in recursive function #16078

Closed
ghost opened this issue May 25, 2017 · 5 comments · Fixed by #17495
Closed

noUnusedLocals: Don't count self-call in recursive function #16078

ghost opened this issue May 25, 2017 · 5 comments · Fixed by #17495
Labels
Suggestion An idea for TypeScript

Comments

@ghost
Copy link

ghost commented May 25, 2017

TypeScript Version: nightly (2.4.0-dev.20170525)

Code

function f(): void { f(); }

Expected behavior:

Function f is unused.

Actual behavior:

No error.

(Ref: palantir/tslint#1937)

@mhegazy mhegazy added Bug A bug in TypeScript Suggestion An idea for TypeScript and removed Bug A bug in TypeScript labels May 25, 2017
@mhegazy
Copy link
Contributor

mhegazy commented May 25, 2017

how far do we want to go with this? is a class that is only instantiated in an instance method the same? a var that is only referenced in its own intializer? etc..

@ghost
Copy link
Author

ghost commented May 25, 2017

It makes sense to disclude any use of a class inside itself, since the self-references won't be used unless the class is. But I don't think I would treat instantiation specially, since it's hard to tell when a reference to a class might instantiate it.

class C {
    static s() {
        new C().m();
    }

    m() {
        new C();
    }
}

maybeThisInstantiatesIt(C);

function maybeThisInstantiatesIt(klass: any) {
    klass.s();
}

Without the maybeThisInstantiatesIt(C) call, I would definitely mark the class as unused.

A variable initialized to itself is an error anyway if you use const or let, so I don't think that's an issue.

@aozgaa
Copy link
Contributor

aozgaa commented Jun 9, 2017

On the other hand, the mere presence of a static property may be side-affecting in its initialization, so the following code, which marks D unused today, actually uses the static initialization of s.

function sideAffecting(): number {
    console.log("called");
    return 10;
}

function foo() {
    class D {
        static s: number = sideAffecting();
    }
}
foo();

But perhaps this should be a separate issue.

@ghost
Copy link
Author

ghost commented Jun 9, 2017

D itself is unused though. If you write const x = sideAffecting(); we will also give you an unused-variable error because x is unused even if its initializer has side effects.

@falsandtru
Copy link
Contributor

I agree with @andy-ms . This is an issue on checking recursive functions. We don't need to consider classes.

@aozgaa In your case, D should be unused, and sideAffecting should be used.

This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants