New typescript flow rule

[azyzz228]

Draft version of the new rule proposed at #2390.

This new rule allows for changing between inline type imports (type modifiers introduced in TS 4.5) and separate type import statements (introduced in TS 3.8). The rule also fixes imports to the preferred option. The rule assumes that user specifies himself which imports are type imports. For example, if there is enum that is used as a type, user should import the enum as type. This rule does not check the usage of imports, but rather helps to enforce consistent style for importing types.

The rule has 2 strict options (inline and separate) and one preferred flow option. In preferred flow option, user passes array of inline and separate in the order of preference. Rule checks if inline is supported, if not, falls back to "separate" as an option. If separate comes first in the array, then resume as if there is strict "separate" option.

Strict option enforces either inline type imports with type modifiers, or separate type imports.

Rule schema for strict options: we pass single string to the rule

"import/typescript-flow": [ 0 | 1 | 2, "inline" | "strict" ]

separate option

Definition: All type imports must be imported in a separate import type statement.

The following patterns are not considered warnings:

// good1.js

// Separate import type statement
import type { MyType } from "./typescript.ts"

// good2.js

// Separate import type statement
import type { MyType as Persona, Bar as Foo } from "./typescript.ts"

The following patterns are considered warnings:

// bad1.js

// type imports must be imported in separate import type statement
import {type MyType,Bar} from "./typescript.ts"

// gets fixed to the following:
import {Bar} from "./typescript.ts"
import type { MyType } from "./typescript.ts"

// bad2.js

// type imports must be imported in separate import type statement
import {type MyType,type Foo} from "./typescript.ts"

// gets fixed to the following:
import type { MyType, Foo} from "./typescript.ts"

// bad3.js

// type imports must be imported in separate import type statement
import {type MyType as Persona,Bar,type Foo as Baz} from "./typescript.ts"

// gets fixed to the following:
import {Bar} from "./typescript.ts"\nimport type { MyType as Persona, Foo as Baz } from "./typescript.ts"

inline option

Definition: All type imports must be inline imported with type modifiers.

Patterns that do not raise the warning:

// good1.js

// no separate import type statement. inline type import exists
import { type MyType } from "./typescript.ts"

// good2.js

// no separate import type statement. inline type import exists
import { type MyType, Bar, type Foo as Baz } from "./typescript.ts"

Patterns are considered warnings and fixed:

// bad1.js

// type imports must be imported inline
import type {MyType} from "./typescript.ts"

// gets fixed to the following:
import  {type MyType} from "./typescript.ts"

// bad1.js

// type imports must be imported inline
import type {MyType, Bar} from "./typescript.ts"

// gets fixed to the following:
import  {type MyType, type Bar} from "./typescript.ts"

// bad3.js

// type imports must be imported inline
import type {MyType, Bar} from "./typescript.ts"
import {Value} from "./typescript.ts"

// Rule can check if non-type import exists. If yes, then adds inline type imports there. gets fixed to the following:
import {Value, type MyType, type Bar} from "./typescript.ts"

Questions:

1. How to handle the cases where there are 2 spaces left when "type" is removed from separate import type. I could not figure out the token for the spaces. Seems like spaces are completely ignored from estree?

// option === 'inline'

// input
import type {MyType, Bar} from "./typescript.ts"

//output
import  {type MyType, type Bar} from "./typescript.ts" // there are 2 spaces after import

P.S. Rule name can be changed. Changes to ExportMap to be removed

[azyzz228]

How do I make package.js include my new rule? It looks like a lot of Github Actions tests are failing because of that

[ljharb]

The new rule needs to be added to the shared configs in src/index.js, I think.

[azyzz228]

Not sure what is causing errors :/. I tried to replicate them on my local machine (node v10, eslint v6), but had many many test cases failing, and completely different error (mine was ELIFECYCLE error). Was wondering maybe you might have any thoughts?

[ljharb]

It seems like the tests that are failing are eslint 6 or 7, combined with node < 12. The new TS parser will only be installed on eslint >= 6, and will be v3 on node 10 and 11, and probably v2 on node < 10. On eslint 6, the old TS parser will be present on v20. On node < 8, eslint-import-resolver-typescript will be v1.0.2.

The easiest way to replicate this locally is to run the tests/dep-time-travel.sh script yourself, with the same arguments CI uses.

[latin-1]

I believe import { type MyType } from "./typescript.ts" is not something we really want to have. It will fail if the option importsNotUsedAsValues in the tsconfig.json is set to error. They are not interchangeable because they have different meanings. See also https://github.com/microsoft/TypeScript/issues/47118

import type { T } from "./module"; // ok
import { type T } from "./module"; // error ts(1371)

[ljharb]

It'd be great to rebase this, so any remaining failing tests can be resolved.

[azyzz228]

apologies this PR got abandoned... Would you say there is still a need for this rule? Suggestion for the rule came from here #2390.

[ljharb]

There's already a rule for whether you use import type or import { type, but i don't think there's a rule yet that forces you to use a type signifier for types or forces you not to, so it still seems valuable to me.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 97.77778% with 3 lines in your changes missing coverage. Please review.

Project coverage is 95.48%. Comparing base (55fa203) to head (05c62b8).

Files with missing lines	Patch %	Lines
src/rules/typescript-flow.js	97.77%	3 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2614      +/-   ##
==========================================
+ Coverage   95.20%   95.48%   +0.28%     
==========================================
  Files          82       83       +1     
  Lines        3565     3700     +135     
  Branches     1245     1269      +24     
==========================================
+ Hits         3394     3533     +139     
+ Misses        171      167       -4     

Flag	Coverage Δ
	95.48% <97.77%> (+0.28%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.