no-unnecessary-generics shouldn't error on return types

[cyrilgandon]

• TypeScript version: 2.5.2
• Running TSLint via: Visual Studio Code

TypeScript code being linted

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/69acc07b23b9ef174732a352fc732c363149db15/types/angular/index.d.ts#L1040

interface IDeferred<T> {
    resolve(value?: T | IPromise<T>): void;
    reject(reason?: any): void;
    notify(state?: any): void;
    promise: IPromise<T>;
}

interface IQService {
    defer<T>(): IDeferred<T>; // error on this line
}

with tslint.json configuration:

"no-unnecessary-generics": true

Actual behavior

Error:

Type parameter is only used once

Expected behavior

No error, the return type is an important information.

Given the docs:

Bad: function parse(): T; const x = parse();

Good: function parse(): {}; const x = parse() as number;

Means that instead of doing

const deferred = $q.defer<string>();

We should doing

const deferred = $q.defer() as IDeferred<string>;
// or
const deferred = <IDeferred<string>>$q.defer();

I find the first syntax better than the 2 others. Is this still the official position? Note that if we change from defer<T>(): IDeferred<T>; to defer(): IDeferred<{}>;, a lot of typescript builds are going to fail. This is the same linting error we found for HttpPromise.

Should we disable the rule for angular, or is the rule make sense on return types?

[ghost]

Is this still the official position?

I believe so. For new packages on DefinitelyTyped we recommend against doing casts using the type argument syntax. I'll leave this issue open in case anyone else wants to provide feedback.

[danvk]

I ran into this today and was quite surprised. What exactly is the proposed alternative to:

const sampler = Reservoir<MyType>();
sampler.pushSome(x);  // x should be MyType
const el = sampler[0];  // el is MyType

Is it this?

const sampler = Reservoir() as Reservoir<MyType>;

that seems needlessly repetitive.

/**
 * Create a new reservoir sampler.
 *
 * @param reservoirSize is the maximum size of the reservoir. This is the number of elements
 *   to be randomly chosen from the input provided to it using pushSome. Default is 1.
 * @param randomNumberGenerator is an optional random number generating function to use in
 *   place of the default Math.random.
 */
declare function Reservoir<T>(
    reservoirSize?: number,
    randomNumberGenerator?: () => number
): Reservoir.ReservoirArray<T>;  // tslint:disable-line:no-unnecessary-generics

/*~ If you want to expose types from your module as well, you can
 *~ place them in this block. Often you will want to describe the
 *~ shape of the return type of the function; that type should
 *~ be declared in here, as this example shows.
 */
interface ReservoirArray<T> extends Array<T> {
    /**
     * datum: one or more elements to consider for inclusion into the reservoir.
     * Returns the current length of the reservoir.
     */
    pushSome(...datum: T[]): number;
}

[ghost]

I think the thing to look for here is whether the cast would be unsafe or not. createArray<number>() is safe. unsafeCollection.get<number>() is not. For the case of creating an empty collection and specifying the element type, you can use tslint:disable.