aladdin-add / eslint-plugin

autofix some errors reported by eslint rules.
107 stars 10 forks source link

no-unused-vars messes up typescript imports #60

Closed BenniG82 closed 3 years ago

BenniG82 commented 4 years ago

Tell us about your environment

export class MockService { getSomeThing(val: string): Observable { return of('hello'); } }


**Expected behavior**
Should remove only all unused imports and variables:

import {Observable, of} from 'rxjs';

export class MockService { getSomeThing(): Observable { return of('hello'); } }


**Actual behavior**
Messes up the imports:

import {Observable of} from 'rxjs';

export class MockService { getSomeThing(): Observable { return of('hello'); } }

BenniG82 commented 4 years ago

Found the issue, take a look at "FIX". Trying to make a PR tomorrow:

 switch (parent.type) {
                case "FunctionExpression":
                case "FunctionDeclaration":
                case "ArrowFunctionExpression":
                    // Don't autofix unused functions, it's likely a mistake
                    if (parent.id === node) {
                        return null;
                    }

                    switch (config.args) {
                        case "all": {
                            const prefix = getIgnoredArgPrefix(config);
                            if (prefix === null) {
                                return null;
                            }

                            return fixer.insertTextBefore(node, prefix);
                        }
                        case "after-used": {
                            console.log(grand);
                            const comma = sourceCode.getTokenBefore(node, commaFilter);
                            // <<<<< FIX >>>>
                           if (comma && comma.range && grand.range && comma.range[0] >= grand.range[0]) {
                                return [fixer.remove(comma), fixer.remove(node)]
                            } else {
                                const commaAfter = sourceCode.getTokenAfter(node, commaFilter);
                                if (commaAfter && commaAfter.range && grand.range && commaAfter.range[1] <= grand.range[1]) {
                                    return [fixer.remove(node), fixer.remove(commaAfter)]
                                }
                            }
                            return [fixer.remove(node)];
                            // <<<<< //FIX >>>>
aladdin-add commented 4 years ago

hi, the plugin does not support typescript -- it has not been fully tested to run typescript.

does it also happen in javascript?

BenniG82 commented 4 years ago

Yes, it also happens in JS/ES. If you look into my fix, the problem is obvious. You are Looking for a comma before the current node. But if the parameter to remove is the first / only one, there is no comma before it:

class MockService {
    getSomeThing(val) {
        return of('hello');
    }
}

So a "random" comma before the current node gets picked. In my case from an import.

In our case we decided to go back to tslint even though it will be deprecated soon, because it has more rules that support angular development.

aladdin-add commented 4 years ago

yep, I see. are you willing to submit a PR to fix it? 😄

aladdin-add commented 3 years ago

released in https://www.npmjs.com/package/eslint-plugin-autofix/v/1.0.4 🎉