Tslint: New rule: no-duplicate-imports

[adidahiya]

Just like http://eslint.org/docs/rules/no-duplicate-imports

bad:

import { merge } from 'module';
import something from 'another-module';
import { find } from 'module';

good:

import { merge, find } from 'module';
import something from 'another-module';

[chances]

In some cases you cannot simply merge imports.

Why _should_ this cause a linting error?

If they're impossible to merge then that's not an error.

[IllusionMH]

@chances can you provide examples when they cannot be merged?
If it is common case - might be added as rule option or may not be included into recommended config (not it is only in all and latest).

In most cases it may be causes by copy/paste or bugs in auto-imports.
Rule helps to avoid such error cases. exceptional cases can be handled with inline disable comments or rule can be disabled in your configuration.

[ajafff]

That's a quote from my PR. Here the example:

import foo from 'foo';
import foo2 from 'foo'; // cannot be combined with preceding import

import * as bar from 'bar';
import {baz} from 'bar'; // cannot be combined with preceding import

If you find something like that in your code, it should be refactored anyway.
"cannot simply be merged" means that the autofixer would need to rename all references to the merged import in the file.

[buu700]

@ajafff, how is it recommended to refactor those cases? Right now I have a case where I need both bar and baz, and the only workaround I can see is to either disable this rule or use bar.baz everywhere (which would work, but just be a little verbose in this case). Specifically, a library I'm using exports bar as a class and baz as an interface.

It seems like it'd make more sense for non-fixable instances to be treated by the rule as valid.

[IllusionMH]

Cases mentioned above looks like lazy copy/paste. Both cases can be normalized with hands and I don't see reason to leave them as separate imports.

@buu700 if bar.baz is too verbose, that what reason for * as barin first place? Only to export it?
Otherwise it is better to use several named imports ({anything, baz}) from this module or stick to bar.anything for consistency.

[buu700]

As mentioned, I need to use both baz and bar (as a class), which is because the imported module is a JS library that does things that would be silly/unexpected in idiomatic TypeScript.

bar.baz is basically okay, and is the solution I ended up going with, but there are two reasons I don't really like it:

1. Other classes reference baz directly, so the fact that this class is using bar.baz all over the place is a little inconsistent.

2. The slight extra verbosity has turned some lines that would've each been one line into groups of three lines due to our configured max line length.

Not the end of the world, but in this case I'm not sure it's obvious that the rule helped more than harmed.

[ajafff]

You could use a local alias:

import * as bar from 'bar';
import baz = bar.baz;

[buu700]

Yeah, I considered that as well, but I think the version of this that triggers the error still isn't obviously worse than any of the alternatives. Usually when a lint rule fails there's a direct or recommended fix that's a clear improvement, but in this case it seems like all the possible workarounds are just to avoid triggering it on a technicality.

Maybe this could be allowed via an option similar to quotemark's avoid-escape option?

[GabeAtWork]

Are there any plans to implement autofix for this rule?

[JoshuaKGoldberg]

@GabeAtWork no, but only because nobody has filed a issue asking for it. Good idea! We should discuss in a separate issue in case their are off edge cases to hammer out.