Wrong detection of unused operator

[Arnaud73]

I use this configuration for tslint :

"rxjs-add": {
    "options": [{
        "allowElsewhere": false,
        "allowUnused": false,
        "file": "./src/rxjs-imports.ts"
    }],
    "severity": "error"
},
"rxjs-no-wholesale": {
    "severity": "error"
}

It detects an unused operator in a file:

ERROR: /Users/.../app/src/rxjs-imports.ts[8, 1]: Unused patched observable in ./src/rxjs-imports.ts: from

If I comment this operator in my global file, I then have the following error :

ERROR: /Users/.../app/src/app/shared/store/error.actions.ts[60, 33]: RxJS add import is missing from ./src/rxjs-imports.ts: from

Which is correct since the from operator is used in this file.

[cartant]

A couple of things:

• If you are using the Angular CLI, please read this and (if necessary) refer to the changes I made in the included example - see 73872cc.
• If you are using TSLint's no-unused-variable rule, you should disable it, as it's buggy and causes problems in seemingly unrelated rules. See issue Beware of TSLint's no-unused-variable rule #4 in this repo.

[Arnaud73]

Thanks for the update.

I do indeed use Angular CLI.
My rxjs-imports.ts file is included in main.ts and test.ts.
I've just added the configuration you are referring to in e2e/tslint.json, but it does not improve anything for this error (but it did remove a previous warning for the rxjs-imports.ts not being included in the output file).
I do not use the rule no-unused-variable of TSLint.

A few weeks ago, it was working without any problem for this rule, but then it changed. It's hard to tell if this is related to having upgraded your package, or refactored a big part of the application.

[cartant]

If you use npm (or yarn) with a lockfile, you could temporarily revert to an older commit and re-install. If that solves the problem, it might offer some clues.

There is also this bug that I found, today, but I think that's unlikely to be your problem. (It seems to be upstream; either in TSLint or TypeScript.)

Also, you should be aware of this gotcha.

If all else fails, you could try replicating the problem by adding imports - similar to the problematic imports you are using in your app - to the Angular CLI example that's in this repo. If you're able to use the example to create a repro, I can likely be of more help.

Good luck.

[Arnaud73]

I'll give a try to reverting to a previous build that did not show the problem. My concern is that the fix was probably some old code containing the from operator that masked this issue.

Right now, the from operator is used in :

export function errorHandlerBuilder(...jsonApiErrorHandlers): (err: any) => Observable<Action> {
    return (err) => (Observable.from(errorHandlerBuilderInstant(...jsonApiErrorHandlers)(err)));
}

In case it gives you a clue about the potential issue.

[cartant]

Try adding some redundant usage of from in the file that contains the code in your previous question. Something like:

const foo = Observable.from([]);

If that solves the problem then you'll know that it is the usage in errorHandlerBuilder that's not recognized. And I can attempt to build a repro as a fixture.

[Arnaud73]

Curiously, replacing the return call to const foo = Observable.from([]); did not improve anything. tslint still returns the same error.

[cartant]

Put the redundant usage outside of the function - i.e. at the start or end of the file.

[Arnaud73]

I put it as the first element of the file :

import { Observable } from 'rxjs/Observable';

import { Action } from '@ngrx/store';

import { ActionWithPayload } from '../model/action-with-payload.model';
import { JsonApiErrors } from '../model/json-api-errors.model';

export const foo = Observable.from([]);

export class ErrorAction<P> extends ActionWithPayload<P> {
}

export class ErrorDefaultAction extends ErrorAction<{ code: string | number; redirect?: boolean }> {
}

/**
 * Builds a function to handle errors, using the provided error handlers for specific cases
 *
 * This version directly returns the actions (no Observable), it is meant to be used in direct service
 * calls (as opposed to calls from effects).
 *
 * @param jsonApiErrorHandlers the specific handlers
 * @returns Action[] the actions to play has error handling
 */
export function errorHandlerBuilderInstant(...jsonApiErrorHandlers): (err: any) => Action[] {
    return (err) => {
        const actions: Action[] = [];
        try {
            (<JsonApiErrors>err.error).errors.forEach(error => {
                jsonApiErrorHandlers.forEach(handler => {
                    // try the handler, if it doesn't return an action, it wasn't
                    // designed to handle this specific error
                    const action = handler(error);
                    if (action) {
                        // if it was designed for this error, add the resulting actions
                        actions.push(action);
                    }
                });
                if (actions.length === 1) {
                    // if no dedicated handler was able to handle the error, default to a snack message based on the http status
                    actions.push(new ErrorDefaultAction({ code: err.status }));
                }
            });
        } catch (e) {
            // if the error wasn't in the jsonApi format, default to a snack message based on the http status
            actions.push(new ErrorDefaultAction({ code: err.status }));
        }
        // and return the actions
        return actions;
    };
}

/**
 * Builds a function to handle errors, using the provided error handlers for specific cases
 *
 * This version is the default one to use in catch() blocks in effects
 *
 * @param jsonApiErrorHandlers the specific handlers
 * @returns Observable<Action> the actions to play has error handling
 */
export function errorHandlerBuilder(...jsonApiErrorHandlers): (err: any) => Observable<Action> {
    return (err) => (Observable.from(errorHandlerBuilderInstant(...jsonApiErrorHandlers)(err)));
}

Still, no improvement :(

[Arnaud73]

I also put this redundant code in the main.ts and some other files... Still no improvement.

[cartant]

Enable one of the rules that will let you force an error in that file - e g. rxjs-no-do - then add some temporary code that will fail the rule. If you don't get the expected error, it's likely that TSLint is somehow misconfigured and is not linting that file.

[Arnaud73]

I just added import 'rxjs'; at the top of the file using the from operator, and directly get greeted with two new errors :

ERROR: /Users/.../app/shared/store/error.actions.ts[1, 1]: Wholesale RxJS imports are forbidden
ERROR: /Users/.../app/shared/store/error.actions.ts[1, 9]: This import is blacklisted, import a submodule instead

[cartant]

Try to reproduce the problem with the example in this repo. It doesn't use from, so add it:

https://github.com/cartant/rxjs-tslint-rules/blob/master/examples/ng-cli-example/src/rxjs.imports.ts

If you cannot reproduce it, I don't think I can solve it.

[Arnaud73]

I just made a copy of my app... Problem was still reproducible as expected...
Then I removed all modules and made some changes in app.module.ts, and the main app component...
The problem disappeared...
So it seems that something in some file is causing the issue...
I will have to remove directories one by one... Any clue about how to select what to remove?

[cartant]

I've added a fixture for this issue to ensure that Obserable.from behaves as it should with the rxjs-add rule. It does.

In doing so, I've recalled how the allowUsed works. TSLint walks the files one-by-one, so to determine whether or not there are patching observables or operators in the central file that are unused, the rule walks the entire program when the central file is being linted.

Do you have a program that compiles without error? With TSLint, an AST can be produced even if there are some errors. However, the AST might be missing type information and if there are unknowns in it, the rules will be unable to determine what's observable and what isn't.

I would set allowUnused to true and would get everything working before switching it back on. Make sure that it builds without error.

I've just seen your last comment. No, I don't have any suggestions. I think there is a general TypeScript/typing-related problem somewhere that's not specific to rxjs-tslint-rules.

[cartant]

Closing this in the absence of a repro.