no-unused-variable configuration request

[scriby]

As the no-unused-variable is implemented now, I find it has too many false positives to really be useful. However, if I could just enable a portion of it that would check only local variables, that would be really useful.

[ashwinr]

i see. can you please give a couple of examples of these false positives?

[csnover]

The big problem I think is “private class members”. Imports, variables, and functions can all never be accessed dynamically without using eval, but private class members can easily be accessed dynamically using the array operator. So, private class members should be excluded from no-unused-variable or at least configurable.

[ashwinr]

I don't buy that. private members are used to prevent access; sure javascript has no semantics for this, but typescript does. if you're accessing a private member only using the array operator, then i'd suggest making it public and use it directly.

[csnover]

private members are used to prevent access

What does the access control model for class properties in TypeScript have to do with preventing the accidental creation of unused variables? They’re two different things.

Unused private class methods can be discovered easily during unit testing of the public APIs using code coverage analysis, by identifying methods that are never called, so it’s not critical to lint them; this is not the case for unused variables whose declarations will be covered in testing even when the variables are never actually accessed.

Additionally, in the design of the language itself, variables are guaranteed to be unused if they are never accessed within the scope where they are defined; as we have already discussed, the same is not true about object properties, which both JavaScript and TypeScript allow to be accessed using array operators. This leads to false positives:

class A {
  private _handles:Handle[];

  constructor(emitter:EventEmitter) {
    this._handles = [
      emitter.on('response', lang.hitch(this, '_handleResponse'));
    ];
  }

  destroy():void {
    var handle:Handle;
    while ((handle = this._handles.pop()) {
      handle.remove();
    }
  }

  // this method is used, but tslint says it isn’t
  private _handleResponse(event:Event) {}
}

sure javascript has no semantics for this, but typescript does

As mentioned above, TypeScript does not prescribe that you must access a property of a class using dot operators. a.foo and a['foo'] are both accepted.

if you're accessing a private member only using the array operator, then i'd suggest making it public

1. Public properties introduce new interface properties where private properties do not, which makes such a change inappropriate in most/all cases;
2. Projects that need to support ES3 environments without modifying global objects don’t have Function.prototype.bind available, so use convenience functions like lang.hitch(this, '_foo') in order to generate a new function with the correctly bound this context when passing to event listeners instead of hand-rolling closures every time. This is a very common pattern which relies on the normal practice of using an array operator instead of dot operator for property access. Because this convenience function is generic it also supports late binding of other objects that can have their properties mutated at runtime.

Thanks for your consideration,

[JoshuaKGoldberg]

Sorry if bumping this is against the rules - I've come across the same problem.

class Test {
    private done: boolean;

    constructor() {
        (function (): void {
            this.doPrivateThing();
            (<Test>this).doPrivateThing();
        }).bind(this)();
    }

    private doPrivateThing(): void {
        this.done = false;
    }
}

...gives the following error:

>> test.ts[11, 13]: unused variable: 'doPrivateThing'

It looks like tslint isn't checking inside the bound function. It's understandable that function.bind erases some information, but should <Test>this.doPrivateThing let it know that the variable's being used?

[senhongo]

I have a similar problem maybe?
A variable used for type declaration will trigger the no-unused-variable rule.

import Foo = this.that.Foo;

module some.module.blah {
 export  class bar {
    private foo: Foo;
  }
}

returns the error unused variable: 'Foo

[nikklassen]

The use of an import alias is also not detected when it's used with extends

import DateTimeOpts = Intl.DateTimeFormatOptions;

interface MyDateTimeOpts extends DateTimeOpts {
    timezoneOffset: number;
}

let opts: MyDateTimeOpts;
console.log(opts.timezoneOffset - 1);

[adidahiya]

@SenHeng, @nikklassen: those look like related issues; I've filed a new ticket for those false positives: #613.

as for the other issues in this thread, I think we could address them by adding a new option that makes the no-unused-variable rule skip private class members: #614. I don't want to do anything much more complex / involved than that; I really don't want to encourage any coding style that references private members with dynamic access as suggested in some of the examples above.

[adidahiya]

closing this for now. I can reopen if someone wants to discuss further... or feel free to file a new, more specific issue

[JoshuaKGoldberg]

I think we could address them by adding a new option that makes the no-unused-variable rule skip private class members

Forgive me if I'm misunderstanding, but does that not solve a different problem? Caring whether private variables are unused is different from correctly reporting whether they're unused, yes?

For further clarification, in the below example, waste should be flagged as unused, but done should not be.

class Test {
    private waste: boolean;
    private done: boolean;

    constructor() {
        (function (): void {
            this.doPrivateThing();
            (<Test>this).doPrivateThing();
        }).bind(this)();
    }

    private doPrivateThing(): void {
        this.done = false;
    }
}

[adidahiya]

ok, I think I see what you mean with the difference between waste and done there... but I don't see a straightforward approach that would correctly mark waste as unused while not also marking doPrivateThing() as unused. any ideas here?

basically the option I've suggested in #614 is a less strict version of the no-unused-variable rule for users who want to use dynamic access of private members/methods at all in their codebase.