Investigating ESLint migration about dtslint

[43081j]

@JoshuaKGoldberg it is being tracked in #325 by @sandersn , although he's presumably been preoccupied with other stuff recently. But reviewing that pr would be a good start I think

[sandersn]

do you still want assistance in moving to ESLint internally

Yes, although it makes sense to move dtslint into DefinitelyTyped-tools first (in my opinion), and a completed eslint-plugin-expect-type might replace dtslint's expectRule.

If nobody else gets to it first, I'll probably work on the dtslint move during my next DT rotation, some time in November I think.

[JoshuaKGoldberg]

Yeah I'm planning on looking at this within the next week or so. My rough thoughts are that it would be cleanest long term to promote eslint-plugin-expect-type, so that folks don't need a separate linting tool other than ESLint.

From the perspective of DefinitelyTyped it would likely be very difficult to immediately switch from ESLint to TSLint given the sheer scale of the codebase and how many equivalent rules have subtle differences between the two linters. My first guess for a strategy would be something like:

1. Add ESLint + typescript-eslint for explicitly just the expect-type ruleset, removing the need for most or all of dtslint [edit: see below comment]
   • Use typescript-eslint's TSLint runner to run TSLint inside ESLint
2. Switch over rules one by one from TSLint to ESLint
3. Remove TSLint altogether

...but that's just a rough guess that should be validated 😄

[43081j]

keep in mind dtslint has plenty of useful rules beyond type assertions. that is actually only one rule, a small amount of the source of dtslint.

if there's enough complexity in that one rule to justify pulling in third party libs, fair enough.

the rest can mostly be converted fairly easily (but time consuming). so it would be super nice if we could have them running alongside each other which i think is what you're saying. so we can migrate a rule at a time, leaving the rest to continue using tslint.

i also already have a branch with some of those rules converted but none of the surrounding architecture updated to call them properly. so let me know if you want any of that in your efforts

[43081j]

that makes sense i think, especially maintaining this repo and just shuffling it around.

i already converted a fair few of the rules to eslint rules roughly (untested) so would be happy to contribute those if we did make some kind of eslint-plugin-dts

i think however we reorganise it, though, the big job is migrating dtslint to run eslint rather than tslint. the rules are already essentially lint rules, so pulling those out into a separate package isn't a blocker to migrating the core imo. may be worth starting to look at that before anything else

[sandersn]

• checkTslintJson/testDependencies: Maybe better moved to a new @definitelytyped/dtslint-utils package, and then called from dtslint? @definitelytyped/dtslint-runner is the package that calls dtslint, but it's not quite the right place for testDependencies since it's just supposed to determine which packages to call dtslint on. (There's surely a better name than dtslint-utils, of course.)
• updateConfig: Yes, move to DT under scripts/
• assertPathIsInDefinitelyTyped: Yes, should be a lint rule.

I like the monorepo idea, although it's a bit of work to set up. Unless it would be tremendously difficult, it'd be better to keep it here and replace the existing structure.

My main piece of advice, having recently upgraded all of DT at once to support exactOptionalPropertyTypes, is to make the transition incremental. Have dtslint run both tslint and eslint, and switch one rule over at a time. That'll make it easier to alternate between switching a lint rule and making sure all the DT packages still pass.

[sandersn]

Update: @andrewbranch and I talked about problems we've had updating dtslint with the rest of the @definitelytyped tools. We think it would make sense to move the bulk of dtslint into the @definitelytyped monorepo and either (1) publish a small wrapper for non-DT use from this repo or (2) keep publishing the package as dtslint from inside the monorepo.

Our motivation is the dependency @definitelytyped/utils -> dtslint -> @definitelytyped/dtslint-runner, which means that the @definitelytyped monorepo ends up with dependencies on itself in its yarn.lock.

Does that make sense? @JoshuaKGoldberg it's basically what you're doing, but even more so.

[JoshuaKGoldberg]

👍 I have been convinced, thanks for the detailed explanations! Keeping DT specific runtime to the DT repo makes sense.

Since there's a clear path forward for moving into DT my plan is to stop sending PRs to this repo & DT that split out functionality, and instead focus on eslint-plugin-expect-type. LMK if there's anything else I can do to help! (do you still want assistance in moving to ESLint internally given it's moving to the other repo too?)

[orta]

Hi! Sorry it took a while for us to respond.

We've also been thinking about this off and on, and are definitely open to getting it working and replacing tslint! I've pushed a new branch which we can work towards: https://github.com/microsoft/dtslint/tree/eslint_base

[43081j]

do you want me to open a draft PR against that? its been a while so ill catch it up from master too while im in there.

then we can decide if the stuff i already did is worth using as a starting point or not

[orta]

Yeah, that'd be great!

[JoshuaKGoldberg]

👋 is anything happening here? I'd be interested in pitching in!

[43081j]

#325 is stale as sanders doesn't currently have the time to push ahead with it, so any takers for opening a new PR would be great.

some feedback from me:

• we should probably see if there's a sensible way to migrate a rule at a time, one PR per rule
• similarly investigate how we can run both linters alongside each other when necessary
• IMO we should start with little or no eslint plugins to avoid pulling in bloat, ideally this repo can ultimately behave like an eslint plugin in the end

[JoshuaKGoldberg]

I've reached out to @ibezkrovnyi to talk about eslint-plugin-expect-type. Looking through https://github.com/microsoft/dtslint/tree/76886d9/docs, indeed, there are quite a few rules that don't make sense under an "expect type" umbrella. I wonder if changing to a name like eslint-plugin-types is a reasonable future?

[sandersn]

I don't know the eslint naming scheme, but the rules are really style rules for Definitely Typed. I'd say that (at least) a few of them don't make sense outside that context, like the lint rules about when to use declare vs export in a d.ts file. Does eslint-plugin-definitely-typed make sense? That's just a guess.

[JoshuaKGoldberg]

Maybe two packages then, eslint-plugin-types and eslint-plugin-definitelytyped?

My main goal here is extracting the parts of dtslint that are broadly applicable into a standard ESLint plugin. My secondary goal is making dtslint use a typical ESLint plugin architecture so it can be made portable.

[43081j]

Yeah I mean there's a good amount of purely dt-specific rules that should continue to live in this repo (even if this repo becomes an eslint plugin). But it's true a couple of them could be useful to other people, through even then someone could just use this plugin (eslint-plugin-dts for example, we could call it).

I've also converted a fair few of the rules already to eslint rules. if someone gets time to convert the architecture in general I could easily pr them in

[JoshuaKGoldberg]

FWIW I've taken on ownership of eslint-plugin-expect-type and would be happy to fold everything into a consolidated repo. I'll try to find time to look at converting architecture in general Soon™️!

Edit: note that eslint-plugin-expect-type is on the Apache-2.0 License. I'll make sure we do whatever we need to not violate that, such as attribution/permission.

[43081j]

so would the plan be to update dtslint to make use of the expect-type plugin? given that it has many more DT-specific rules that don't belong in such a plugin (but rather in a DT-specific plugin, like this repo right here)

[JoshuaKGoldberg]

Tentatively, my proposal is that we convert this repo into a monorepo containing at least three packages:

• eslint-plugin-types: essentially what is eslint-plugin-expect-type today, with the addition of more general-use type rules not specifically related to type expectations
• eslint-plugin-definitely-typed: rules specific to DefinitelyTyped and equivalent type definitions
  • Maybe this should be eslint-plugin-dts? If any rules exist exclusively for just the DT repo, maybe they should live in that repo?
• dtslint: wrapper around ESLint that behaves equivalently to dtslint today

In theory we could use the separate eslint-plugin-expect-type repo as a source for the type expect rules. A few drawbacks IMO:

• We'd now have >=2 repositories' worth of tooling to maintain, despite this one likely wanting to be a monorepo anyway (to split the dtslint CLI and its community-applicable rules)
• This repository has a lot more eyes on it & previous bugs + features filed
• It'd be nice to bring in type assertions a little closer to TypeScript organizationally, since we can't build them into the language 😉

[JoshuaKGoldberg]

Poking through #325's changes and some of the adjacent areas of code, there are a few logic paths in dtslint that are specific to DefinitelyTyped.

• checkTslintJson: asserting that a tslint.json exists if a package is inferred as a DT one
  • Proposal: move that into a standalone piece of logic in DefinitelyTyped itself?
  • Same with checkPackageJson & checkTsconfig
• assertPathIsInDefinitelyTyped: if a file contains a // Type Definition header, it should be inside DefinitelyTyped
  • Proposal: switch that into a general-use rule as a part of a recommended eslint-plugin-dts preset?
• updateConfig: standalone script to update TSLint configs for DefinitelyTyped
  • Proposal: move that into a standalone piece of logic in DefinitelyTyped itself?
  • It's basically https://github.com/amanda-mitchell/suppress-eslint-errors, but for dtslint
• testDependencies: test module resolution in for someone needing to npm install when they first edit a DT module
  • Proposal: move that into a standalone piece of logic in DefinitelyTyped itself?

cc @sandersn

[JoshuaKGoldberg]

Hi! Sorry I haven't had much time recently to respond (thanks for the ping andrewbranch). I think I understand the general direction of the proposal, but just clarifying...

@sandersn if you wouldn't mind humoring me, a few clarifying questions...

• Do you have any more specifics on what you're looking at moving into dtslint vs keeping separate?
• Do you think it worthwhile to extract the DT-specific portions out of dtslint?

Our motivation is the dependency @definitelytyped/utils -> dtslint -> @definitelytyped/dtslint-runner

Instead of moving dtslint into DT, an alternative could be to move the DT pieces dtslint uses into dtslint, and import them in DT. For the thread, right now dtslint takes in the following from @definitelytyped/utils:

• src/index.ts: cleanTypeScriptInstalls, installAllTypeScriptVersions, installTypeScriptNext
• src/lint.ts: typeScriptPath

Do you have an opinion moving those to dtslint so the dependency chain becomes something like @definitelytyped/dtslint-runner -> @definitelytyped/utils -> dtslint?

---

Speculating / posting for visibility: now that the core of dtslint is being moved to ESLint plugins, and most consumers likely have typescript-eslint setup anyway, maybe that community-first dtslint isn't a worthwhile goal? Is that the conclusion we should make here? Should the recommendation be to use a package like eslint-plugin-expect-type?

My big hope here was that we could make dtslint into a community-first tool akin to ts-prune or typescript-eslint (as opposed to DT-first), where the entirety of the core tool is in a general open source repo. The benefit here is that the community gets a first-class package for what would end up being a package that can install & run the necessary ESLint+typescript-eslint+plugin(s) to lint .d.ts files. In that world anything DT needs on top of it would exist in the DT repo alone -- which sounds like a very nice separation of concerns?

[43081j]

dtslint contains rules only useful to DT, though. There's only maybe one or two rules appropriate for a "community" plugin.

It still is a tool specific to DT and exists to help maintain that repo and nothing else. I feel like because of that, dtslint should continue to exist whether it's in DT repo or not.

Even if you split out the expect type rule, that's a really small part of a bigger tool.

[sandersn]

The way I think of dtslint today is this:

1. Primarily a tool for CI (and overnight tests) of Definitely Typed packages.
2. Secondarily, a tool for others who want to maintain an DT-like repo internal to their company.
3. Tertiarily [1], a tool for people who want specific rules for linting d.ts files.

Notably, (2) is the same as (1) except that you want to pass a repo path that's not ../DefinitelyTyped. Additionally, tslint vs eslint is primarily an implementation problem -- I want to upgrade mostly because nobody else uses tslint, so we're basically responsible for fixing tslint bugs ourselves now.

Considering (3), I think linting and testing d.ts files is a good idea, but I suspect that with // @ts-expect-error and eslint-plugin-expect-type, it's better for people to integrate d.ts files with the rest of their tests/lints rather than running the dtslint tool on a DT-structured section of their repo. I could be wrong, though: type-linting-as-complete-package might help people get started. It depends on (1) how complex eslint is (2) whether you feel strongly about promoting that aspect of dtslint.

If we started with (3) as a new package, DT might be able to use it as a basis when it was finished. My two concerns are (1) whether a general set of rules would be useful on DT (probably yes?) and (2) whether fixes we need would be easy enough (they aren't too common, so probably yes?).
On the other hand, I don't know how much of dtslint would be useful in building a standalone eslint-centric tool. Our custom rules for types reflect good practice as we understand it (mostly), so are maybe good for porting to eslint. (I don't know whether everybody else would agree, though.)

In summary: Writing all that clarified my thinking a bit: I'd recommend either

1. Create a new eslint runner for d.ts files. (Hopefully with support for existing eslint setups.) Some of dtslint's rules might inform the new eslint rules, and DT might be able to use the new package once it's done.
2. Move all of dtslint into @definitelytyped, leaving behind big signs saying "use eslint-plugin-expect-type!". Maybe a small sign promising an external frontend for people who seriously want to re-create Definitely Typed.

(1) followed by (2) also makes sense.

[1] Yes! I can't believe that's a word.

[andrewbranch]

I have to admit I’m skeptical that dtslint has a bright future as a community-first tool. It definitely has some rough edges, and if the community wanted to adopt the project, they would rightly want to make a lot of changes to smooth them out. At the same time, having it in a separate repo from the rest of DefinitelyTyped-tools, even one under full Microsoft control, is creating a maintenance burden now. Personally, I think these concerns are countervailing forces and both parties would be better served by making a clean break, encouraging new community projects to be built on ESLint and maintained outside of DT/Microsoft. People who are currently depending on dtslint can continue to use it, but its original microsoft/dtslint repo will encourage them not to, and it will be built out of microsoft/DefinitelyTyped-tools instead. That’s my recommendation—admittedly biased because it’s the most convenient path forward for me personally, but I feel like it makes sense. Thoughts?

[43081j]

I think i agree..

To me it makes most sense that we migrate the runner from tslint to eslint and migrate the rules (one at a time if possible, but all at once otherwise). The biggest piece of work there is migrating the runner, i would say and maybe some of the more complex rules.

Moving that to definitelytyped-tools or some other appropriate DT repo makes a lot of sense too , and doesn't change how the migration happens really.

the expect-type rule being an eslint rule others can use makes sense but dtslint contains a lot more rules than that, most of which are very specific to the DT repo. its very unlikely anyone else would ever want to use them unless they had a private DT-like repo. thats why i think even if we extract the expect-type rule, we should stick to keeping dtslint (or an equivalent) inside a DT repo and treated as an internal maintenance tool.

I feel a linter shouldn't be used for testing language features and source code logic.
IMHO, the expect-type feature is the one thing that should definitely become its own package.

[andrewbranch]

I know at one point we were suggesting people use https://www.npmjs.com/package/tsd, but I haven’t heard much about it lately.

[loynoir]

Hi, guys. Newbie question here.

I saw some PR with title eslint.
Is dtslint somehow usable within eslint now ?
Here is a newbie need some docs. 😂

PS: I used tsd, works great, but seems no editor plugin yet.

[loynoir]

typescript < 4.5

https://www.npmjs.com/package/eslint-plugin-dtslint

$ tail dtstest.ts dtslint.eslintrc.cjs tsconfig.json
==> dtstest.ts <==
const answer = 42; // $ExpectType 41

==> dtslint.eslintrc.cjs <==
const { join } = require("path");
module.exports = {
  parser: "@typescript-eslint/parser",
  parserOptions: {
    ecmaVersion: 2019,
    project: join(__dirname, "./tsconfig.json"),
    sourceType: "module"
  },
  extends: ["plugin:dtslint/recommended"],
};

==> tsconfig.json <==
{
  "extends": "@tsconfig/node14"
}
$ eslint -c dtslint.eslintrc.cjs dtstest.ts 

  1:1  error  Expected type to be: 41; got: 42  dtslint/expect-type

✖ 1 problem (1 error, 0 warnings)