[consistent-type-imports] restrict report to the specific offending specifier

[Josh-Cena]

• [x] I have tried restarting my IDE and the issue persists.
• [x] I have updated to the latest version of the packages.
• [x] I have read the FAQ and my problem is not listed.

Repro

{
  "rules": {
    "@typescript-eslint/consistent-type-imports": "error"
  }
}

import { a, type b } from "c";

type c = a;
type d = b;

playground

Expected Result

The rule only reports the first ImportSpecifier

Actual Result

The rule reports the entire ImportDeclaration, which makes debugging a little harder when the declaration is long.

I'm not using autofix, because #4338 is not there yet.

Additional Info

Versions

package	version
@typescript-eslint/eslint-plugin	5.22.0
@typescript-eslint/parser	5.22.0
TypeScript	4,6.2
ESLint	8.14.0
node	18.0.0

[Josh-Cena]

The fifteen levels of indentation in that rule's source makes me want to instantly quit coding every time I look at it

[bradzacher]

Yeah the fixer is dog ugly logic to implement unfortunately because imports are so damn complex.

Most of the rule logic isn't THAT bad though. It probably needs a pass over it to rewrite it now that it's written and finalised.

[kirkwaiblinger]

I think that this kind of implies having a separate loc for each reported import, too, for example

import { a, type b, c } from "outside-module";
         ^          ^

type A = a;
type B = b;
type C = c;

This is kinda of problematic, until https://github.com/typescript-eslint/typescript-eslint/issues/8554 is closed (which looks to be very soon!), due to the amount of fixer logic in the rule. With multipass fixes, I think we can just have each symbol report and fix "individually" but still assert that the actual result is correct

[bradzacher]

TBH idk if we should do individual reporting if there are multiple per import. The fixer logic is complicated AF and refactoring it to be able to do each specifier in isolation will be 🤮

[Josh-Cena]

I think we definitely should!

[bradzacher]

@Josh-Cena the issue with multiple report locations is that it creates multiple fixers and those fixers cannot talk to one another - so either they will conflict or they'll do the same thing

For example imagine this

import { a, b, c } from "outside-module";
         ^     ^

If we want to fix this to an inline style it's easy - for sure:

// fix 1
-import { a, b, c } from "outside-module";
+import { type a, b, c } from "outside-module";

// fix 2
-import { a, b, c } from "outside-module";
+import { a, b, type c } from "outside-module";

These fixes are non-overlapping so easy as. But if you want to fix to a top-level style:

// fix 1
-import { a, b, c } from "outside-module";
+import { b, c } from "outside-module";
+import type { a } from "outside-module";

// fix 2
-import { a, b, c } from "outside-module";
+import { a, b } from "outside-module";
+import type { c } from "outside-module";

Now you have this issue either

• eslint will see that the fixers add the same line and so it'll reject the second fixer - which will guarantee an extra lint pass to recalculate and apply the second fix, or
• eslint will see two non-overlapping insertions - causing two import statements to be inserted for the same module

Either way - that's not a great result! A slower lint run or duplicate code. IMO that's not a great trade-off for the end user for what is a relatively minor improvement.

[kirkwaiblinger]

Has the idea of a non-contiguous loc for a single report been thought about before? That would allow us to get semantically sensible underlines but one fix... Would also make sense for method-signature-style where multiple method declarations, currently flagged separately, get collapsed to 1 by the fix

[bradzacher]

I don't think it has - it's a pretty fundamental change to how eslint does things.

I think there's huge value in it eg rust's clippy does multi range reports (but only for "related spans", not primary ones) and it's a great way to expose additional info to users.

[Josh-Cena]

What about this: we still uphold the principle of "report what will be changed".

• If we need to split out a separate statement, report the statement as a whole.
  • However, if we are only splitting out one identifier, only report that identifier.
• If we are changing a few identifiers (inline style), report those identifiers.