Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eslint: Enable consistent-type-imports #1832

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
1 change: 1 addition & 0 deletions .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
"assertionStyle": "as"
}
],
"@typescript-eslint/consistent-type-imports": "error",
"@typescript-eslint/naming-convention": "warn",
"@typescript-eslint/no-explicit-any": "off",
"@typescript-eslint/no-inferrable-types": "off",
Expand Down
3 changes: 2 additions & 1 deletion packages/cheatsheet-local/src/app/app.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { CheatsheetPage, CheatsheetInfo } from "@cursorless/cheatsheet";
import type { CheatsheetInfo } from "@cursorless/cheatsheet";
import { CheatsheetPage } from "@cursorless/cheatsheet";
Comment on lines +1 to +2
Copy link
Member

@pokey pokey Aug 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's possible to do this in one line

Suggested change
import type { CheatsheetInfo } from "@cursorless/cheatsheet";
import { CheatsheetPage } from "@cursorless/cheatsheet";
import { type CheatsheetInfo, CheatsheetPage } from "@cursorless/cheatsheet";

Is there a way to get it to do that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh looks like there are 4 different lint rules / options related to this question:

I'd be tempted to have one PR that gets us to the configuration we're after in one go, rather than multiple PRs that touch all of our imports, given that I'd imagine they could interact in interesting ways

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@auscompgeek maybe let's aim to discuss this inline type thing at a meet-up? to discuss Plan to discuss at meet-up

Copy link
Member

@pokey pokey Sep 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After discussion, we want to:

  1. enable "@typescript-eslint/consistent-type-imports": "error"
  2. enable --verbatimModuleSyntax TS flag
  3. enable "import/consistent-type-specifier-style": ["error", "prefer-top-level"]
  4. enable "import/no-duplicates": "error" (note we don't need prefer-inline because we don't use inline)

The reasoning is that we want import type to be like its own kind of statement that we know always gets dropped in the output Javascript, and in addition, if we were to have a rule where import {type foo} => import type {foo} only when there are no non-type imports in the statement, it would feel a bit inconsistent that it doesn't happen when there are non-type imports. The proposed rule is simpler: types always get imported in their own statement, and never have side effects

Each step should be its own PR, and they should be in the above order. Note that 1) must come before 2) because without 2), 1) should have no side effects, and then the effects of 2) should become more clear once 1) is in

Note that we don't need no-import-type-side-effects lint rule because 2 and 3 won't allow us to have inline type anywhere

Copy link
Member

@pokey pokey Sep 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to test the following cases:

  • you use something as a type but not runtime and it's currently imported as runtime
  • you use something as a type that is not yet imported
  • you use something as runtime that is currently imported as a type

For each of the above cases, we want to test:

  • Auto-fix
  • Organize imports
  • Auto-import

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still never got a chance to test that matrix, but up until TypeScript 5.3 whether auto-import used import type depends on the tsconfig. TypeScript 5.3 adds an IDE setting to prefer auto-importing with import type https://devblogs.microsoft.com/typescript/announcing-typescript-5-3/#settings-to-prefer-type-auto-imports

The presence of this lint rule will obviously not affect auto-import behaviour since eslint doesn't change the tsconfig. The auto-import behaviour probably isn't too big of an issue anyway since we all have eslint autofix on save, I believe.

import "../styles.css";

declare global {
Expand Down
2 changes: 1 addition & 1 deletion packages/cheatsheet/src/lib/cheatsheet.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";
import { faCircleQuestion } from "@fortawesome/free-solid-svg-icons/faCircleQuestion";
import CheatsheetNotesComponent from "./components/CheatsheetNotesComponent";
import SmartLink from "./components/SmartLink";
import { CheatsheetInfo } from "./CheatsheetInfo";
import type { CheatsheetInfo } from "./CheatsheetInfo";

type CheatsheetPageProps = {
cheatsheetInfo: CheatsheetInfo;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from "react";
import { CheatsheetLegend } from "../cheatsheetLegend";
import type { CheatsheetLegend } from "../cheatsheetLegend";
import useIsHighlighted from "../hooks/useIsHighlighted";
import { formatCaptures } from "./formatCaptures";
import SmartLink from "./SmartLink";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { CheatsheetSection, Variation } from "../CheatsheetInfo";
import type { CheatsheetSection, Variation } from "../CheatsheetInfo";
import useIsHighlighted from "../hooks/useIsHighlighted";
import { formatCaptures } from "./formatCaptures";

Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/getFakeCommandServerApi.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { CommandServerApi } from "./types/CommandServerApi";
import type { CommandServerApi } from "./types/CommandServerApi";

export function getFakeCommandServerApi(): CommandServerApi {
return {
Expand Down
31 changes: 18 additions & 13 deletions packages/common/src/ide/PassthroughIDEBase.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,24 @@
import { GeneralizedRange } from "../types/GeneralizedRange";
import { TextDocument } from "../types/TextDocument";
import { EditableTextEditor, TextEditor } from "../types/TextEditor";
import { Capabilities } from "./types/Capabilities";
import { Clipboard } from "./types/Clipboard";
import { Configuration } from "./types/Configuration";
import { TextDocumentChangeEvent } from "./types/Events";
import {
import type { GeneralizedRange } from "../types/GeneralizedRange";
import type { TextDocument } from "../types/TextDocument";
import type { EditableTextEditor, TextEditor } from "../types/TextEditor";
import type { Capabilities } from "./types/Capabilities";
import type { Clipboard } from "./types/Clipboard";
import type { Configuration } from "./types/Configuration";
import type { TextDocumentChangeEvent } from "./types/Events";
import type {
TextEditorSelectionChangeEvent,
TextEditorVisibleRangesChangeEvent,
} from "./types/events.types";
import { FlashDescriptor } from "./types/FlashDescriptor";
import { Disposable, IDE, RunMode, WorkspaceFolder } from "./types/ide.types";
import { Messages } from "./types/Messages";
import { QuickPickOptions } from "./types/QuickPickOptions";
import { State } from "./types/State";
import type { FlashDescriptor } from "./types/FlashDescriptor";
import type {
Disposable,
IDE,
RunMode,
WorkspaceFolder,
} from "./types/ide.types";
import type { Messages } from "./types/Messages";
import type { QuickPickOptions } from "./types/QuickPickOptions";
import type { State } from "./types/State";

export default class PassthroughIDEBase implements IDE {
configuration: Configuration;
Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/ide/fake/FakeCapabilities.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Capabilities } from "../types/Capabilities";
import type { Capabilities } from "../types/Capabilities";

export class FakeCapabilities implements Capabilities {
commands = {
Expand Down
6 changes: 3 additions & 3 deletions packages/common/src/ide/fake/FakeConfiguration.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { get } from "lodash";
import { Notifier } from "../../util/Notifier";
import {
import type {
Configuration,
ConfigurationScope,
CONFIGURATION_DEFAULTS,
CursorlessConfigKey,
CursorlessConfiguration,
} from "../types/Configuration";
import { GetFieldType, Paths } from "../types/Paths";
import { CONFIGURATION_DEFAULTS } from "../types/Configuration";
import type { GetFieldType, Paths } from "../types/Paths";

interface ConfigurationScopeValues {
scope: ConfigurationScope;
Expand Down
10 changes: 5 additions & 5 deletions packages/common/src/ide/fake/FakeIDE.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
import type { EditableTextEditor, TextEditor } from "../..";
import { pull } from "lodash";
import { GeneralizedRange } from "../../types/GeneralizedRange";
import { TextDocument } from "../../types/TextDocument";
import type { GeneralizedRange } from "../../types/GeneralizedRange";
import type { TextDocument } from "../../types/TextDocument";
import type { TextDocumentChangeEvent } from "../types/Events";
import {
import type {
Event,
TextEditorSelectionChangeEvent,
TextEditorVisibleRangesChangeEvent,
} from "../types/events.types";
import { FlashDescriptor } from "../types/FlashDescriptor";
import type { FlashDescriptor } from "../types/FlashDescriptor";
import type {
Disposable,
IDE,
RunMode,
WorkspaceFolder,
} from "../types/ide.types";
import { QuickPickOptions } from "../types/QuickPickOptions";
import type { QuickPickOptions } from "../types/QuickPickOptions";
import { FakeCapabilities } from "./FakeCapabilities";
import FakeClipboard from "./FakeClipboard";
import FakeConfiguration from "./FakeConfiguration";
Expand Down
16 changes: 8 additions & 8 deletions packages/common/src/ide/normalized/NormalizedIDE.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import { getFixturePath } from "../../index";
import { GeneralizedRange } from "../../types/GeneralizedRange";
import { TextEditor } from "../../types/TextEditor";
import FakeClipboard from "../fake/FakeClipboard";
import FakeConfiguration from "../fake/FakeConfiguration";
import FakeGlobalState from "../fake/FakeGlobalState";
import FakeIDE from "../fake/FakeIDE";
import type { GeneralizedRange } from "../../types/GeneralizedRange";
import type { TextEditor } from "../../types/TextEditor";
import type FakeClipboard from "../fake/FakeClipboard";
import type FakeConfiguration from "../fake/FakeConfiguration";
import type FakeGlobalState from "../fake/FakeGlobalState";
import type FakeIDE from "../fake/FakeIDE";
import PassthroughIDEBase from "../PassthroughIDEBase";
import { FlashDescriptor } from "../types/FlashDescriptor";
import type { FlashDescriptor } from "../types/FlashDescriptor";
import type { IDE } from "../types/ide.types";
import { QuickPickOptions } from "../types/QuickPickOptions";
import type { QuickPickOptions } from "../types/QuickPickOptions";

export class NormalizedIDE extends PassthroughIDEBase {
configuration: FakeConfiguration;
Expand Down
9 changes: 5 additions & 4 deletions packages/common/src/ide/spy/SpyIDE.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { pickBy, values } from "lodash";
import { GeneralizedRange } from "../../types/GeneralizedRange";
import { TextEditor } from "../../types/TextEditor";
import type { GeneralizedRange } from "../../types/GeneralizedRange";
import type { TextEditor } from "../../types/TextEditor";
import PassthroughIDEBase from "../PassthroughIDEBase";
import { FlashDescriptor } from "../types/FlashDescriptor";
import type { FlashDescriptor } from "../types/FlashDescriptor";
import type { HighlightId, IDE } from "../types/ide.types";
import SpyMessages, { Message } from "./SpyMessages";
import type { Message } from "./SpyMessages";
import SpyMessages from "./SpyMessages";

interface Highlight {
highlightId: HighlightId | undefined;
Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/ide/types/Capabilities.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { CommandId } from "./CommandId";
import type { CommandId } from "./CommandId";

export interface Capabilities {
readonly commands: CommandCapabilityMap;
Expand Down
6 changes: 3 additions & 3 deletions packages/common/src/ide/types/Configuration.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Listener } from "../..";
import type { Listener } from "../..";
import { HatStability } from "./HatStability";
import { Disposable } from "./ide.types";
import { GetFieldType, Paths } from "./Paths";
import type { Disposable } from "./ide.types";
import type { GetFieldType, Paths } from "./Paths";

export type CursorlessConfiguration = {
tokenHatSplittingMode: TokenHatSplittingMode;
Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/ide/types/FileSystem.types.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Disposable } from "./ide.types";
import type { Disposable } from "./ide.types";

export type PathChangeListener = () => void;

Expand Down
4 changes: 2 additions & 2 deletions packages/common/src/ide/types/FlashDescriptor.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { TextEditor } from "../..";
import { GeneralizedRange } from "../../types/GeneralizedRange";
import type { TextEditor } from "../..";
import type { GeneralizedRange } from "../../types/GeneralizedRange";

export enum FlashStyle {
pendingDelete = "pendingDelete",
Expand Down
8 changes: 4 additions & 4 deletions packages/common/src/ide/types/Hats.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { Range } from "../../types/Range";
import { TextEditor } from "../../types/TextEditor";
import { Listener } from "../../util/Notifier";
import type { Range } from "../../types/Range";
import type { TextEditor } from "../../types/TextEditor";
import type { Listener } from "../../util/Notifier";
import type { HatStyleName } from "./hatStyles.types";
import { Disposable } from "./ide.types";
import type { Disposable } from "./ide.types";

export interface HatRange {
styleName: HatStyleName;
Expand Down
22 changes: 11 additions & 11 deletions packages/common/src/ide/types/ide.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,21 @@ import type {
TextDocument,
TextEditor,
} from "../..";
import { URI } from "vscode-uri";
import { GeneralizedRange } from "../../types/GeneralizedRange";
import { Capabilities } from "./Capabilities";
import { Clipboard } from "./Clipboard";
import { Configuration } from "./Configuration";
import { TextDocumentChangeEvent } from "./Events";
import {
import type { URI } from "vscode-uri";
import type { GeneralizedRange } from "../../types/GeneralizedRange";
import type { Capabilities } from "./Capabilities";
import type { Clipboard } from "./Clipboard";
import type { Configuration } from "./Configuration";
import type { TextDocumentChangeEvent } from "./Events";
import type {
Event,
TextEditorSelectionChangeEvent,
TextEditorVisibleRangesChangeEvent,
} from "./events.types";
import { FlashDescriptor } from "./FlashDescriptor";
import { Messages } from "./Messages";
import { QuickPickOptions } from "./QuickPickOptions";
import { State } from "./State";
import type { FlashDescriptor } from "./FlashDescriptor";
import type { Messages } from "./Messages";
import type { QuickPickOptions } from "./QuickPickOptions";
import type { State } from "./State";

export type RunMode = "production" | "development" | "test";
export type HighlightId = string;
Expand Down
3 changes: 2 additions & 1 deletion packages/common/src/ide/util/messages.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { MessageId, Messages, MessageType } from "../types/Messages";
import type { MessageId, Messages } from "../types/Messages";
import { MessageType } from "../types/Messages";

/**
* Displays a warning message {@link message} to the user along with possible
Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/testUtil/TestCaseSnapshot.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {
import type {
RangePlainObject,
SelectionPlainObject,
SerializedMarks,
Expand Down
4 changes: 2 additions & 2 deletions packages/common/src/testUtil/extractTargetedMarks.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { splitKey } from "..";
import { ReadOnlyHatMap } from "../types/HatTokenMap";
import { Token } from "../types/Token";
import type { ReadOnlyHatMap } from "../types/HatTokenMap";
import type { Token } from "../types/Token";

export function extractTargetedMarks(
targetKeys: string[],
Expand Down
4 changes: 2 additions & 2 deletions packages/common/src/testUtil/serializeTestFixture.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { TestCaseFixtureLegacy } from "../types/TestCaseFixture";
import { EnforceUndefined } from "../util/typeUtils";
import type { TestCaseFixtureLegacy } from "../types/TestCaseFixture";
import type { EnforceUndefined } from "../util/typeUtils";
import { serialize } from "./serialize";

function reorderFields(
Expand Down
7 changes: 4 additions & 3 deletions packages/common/src/testUtil/toPlainObject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ import type {
LineRange,
Message,
SpyIDERecordedValues,
FlashStyle,
} from "..";
import { FlashStyle, isLineRange } from "..";
import { Token } from "../types/Token";
import { Selection } from "../types/Selection";
import { isLineRange } from "..";
import type { Token } from "../types/Token";
import type { Selection } from "../types/Selection";

export type PositionPlainObject = {
line: number;
Expand Down
6 changes: 3 additions & 3 deletions packages/common/src/types/GeneralizedRange.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Position } from "./Position";
import { Range } from "./Range";
import { TextEditor } from "./TextEditor";
import type { Position } from "./Position";
import type { Range } from "./Range";
import type { TextEditor } from "./TextEditor";

/**
* A range of lines in a document, unlike the standard {@link Range}, which is
Expand Down
6 changes: 3 additions & 3 deletions packages/common/src/types/HatTokenMap.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { HatStyleName } from "../ide/types/hatStyles.types";
import { Range } from "./Range";
import { Token } from "./Token";
import type { HatStyleName } from "../ide/types/hatStyles.types";
import type { Range } from "./Range";
import type { Token } from "./Token";

/**
* Maps from (hatStyle, character) pairs to tokens
Expand Down
6 changes: 3 additions & 3 deletions packages/common/src/types/TestCaseFixture.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Command, CommandLatest } from "..";
import { TestCaseSnapshot } from "../testUtil/TestCaseSnapshot";
import { PlainSpyIDERecordedValues } from "../testUtil/toPlainObject";
import type { Command, CommandLatest } from "..";
import type { TestCaseSnapshot } from "../testUtil/TestCaseSnapshot";
import type { PlainSpyIDERecordedValues } from "../testUtil/toPlainObject";

export type ThrownError = {
name: string;
Expand Down
6 changes: 3 additions & 3 deletions packages/common/src/types/Token.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
/**
* A token within a text editor
*/
import { Range } from "./Range";
import { RangeOffsets } from "./RangeOffsets";
import { TextEditor } from "./TextEditor";
import type { Range } from "./Range";
import type { RangeOffsets } from "./RangeOffsets";
import type { TextEditor } from "./TextEditor";

/**
* A token within a text editor
Expand Down
4 changes: 2 additions & 2 deletions packages/common/src/types/command/ActionDescriptor.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import {
import type {
PartialTargetDescriptor,
ScopeType,
} from "./PartialTargetDescriptor.types";
import { DestinationDescriptor } from "./DestinationDescriptor.types";
import type { DestinationDescriptor } from "./DestinationDescriptor.types";

/**
* A simple action takes only a single target and no other arguments.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {
import type {
PartialListTargetDescriptor,
PartialPrimitiveTargetDescriptor,
PartialRangeTargetDescriptor,
Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/types/command/command.types.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { CommandV6 } from "./CommandV6.types";
import type { CommandV6 } from "./CommandV6.types";
import type { CommandV0, CommandV1 } from "./legacy/CommandV0V1.types";
import type { CommandV2 } from "./legacy/CommandV2.types";
import type { CommandV3 } from "./legacy/CommandV3.types";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { PartialTargetDescriptorV3 } from "./PartialTargetDescriptorV3.types";
import type { PartialTargetDescriptorV3 } from "./PartialTargetDescriptorV3.types";

type ActionType =
| "callAsFunction"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { PartialTargetDescriptorV4 } from "./PartialTargetDescriptorV4.types";
import type { PartialTargetDescriptorV4 } from "./PartialTargetDescriptorV4.types";

type ActionType =
| "callAsFunction"
Expand Down
3 changes: 2 additions & 1 deletion packages/common/src/types/generalizedRangeTouches.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import assert = require("assert");
import { GeneralizedRange, generalizedRangeTouches, Position } from "..";
import type { GeneralizedRange } from "..";
import { generalizedRangeTouches, Position } from "..";

suite("generalizedRangeTouches", () => {
test("character", () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/common/src/types/location.types.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Range } from "./Range";
import { TextEditor } from "./TextEditor";
import type { Range } from "./Range";
import type { TextEditor } from "./TextEditor";

export interface TextEditorRange {
editor: TextEditor;
Expand Down
4 changes: 2 additions & 2 deletions packages/common/src/types/snippet.types.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { TextFormatterName } from "../util/textFormatters";
import { SimpleScopeTypeType } from "./command/PartialTargetDescriptor.types";
import type { TextFormatterName } from "../util/textFormatters";
import type { SimpleScopeTypeType } from "./command/PartialTargetDescriptor.types";

export interface SnippetScope {
/**
Expand Down
Loading