feat: add new package for strictly typing configuration files

[bradzacher]

Following on from eslint/rfcs#50, as it doesn't look like that idea is going to get in by itself, and the proposed alternative (new config file) would probably take quite a while.

cc interested parties - @G-Rath @mysticatea @kaicataldo

---

Playing with providing a framework to create a strictly typed config file.
The function is just an identity function, but via // @ts-check, typescript will consume the declared types for it, and strictly type the config object.

Plugin authors will be able to use declaration merging in order to augment our types, and provide strict types for their rules (see .eslintrc-example-test-temp.ts for a working example).

I.e. an eslint plugin can provide types themselves, within their package, or users can provide types for them via DefinitelyTyped.

If people like this idea, we can also add in tooling to this package to help plugin maintainers automatically generate types based on their json schemas (see https://github.com/bradzacher/eslint-config-brad/tree/master/src/types which is generated by https://github.com/bradzacher/eslint-config-brad/blob/master/scripts/generateTypes.ts)

[G-Rath]

This looks very cool!

While I've still got my hopes on the RFC being accepted, this is definitely useful, and it's also cool to see an actual real-world implementation of the RFC.

When I'm off work & having some spare time, I'll look to type eslint-plugin-jest to work w/ this.

[manuth]

Also add a rule-type for known options so that rule-authors can add their own definition:

export type RuleLevelAndKnownOptions<TRuleName extends keyof RuleStore> = RuleStore[TRuleName];
interface RuleStore
{
    "space": [
        RuleLevel,
        ...Array<(
            "tab" |
            number |
            {
                SwitchCase: number
            })>
    ];
}

[manuth]

Make sure Rules can also handle known options:

        type Rules = {
            [name: string]: RuleLevel | RuleLevelAndUnknownOptions;
        } & {
            [RuleName in keyof RuleStore]?: RuleLevel | RuleLevelAndKnownOptions<RuleName>
        };

[manuth]

This PR looks great to me but it would be nice to also have rule-types implemented.
Please refer to my comments.

[bradzacher]

Hey @manuth - have a look at the example config I've got here
https://github.com/typescript-eslint/typescript-eslint/pull/1296/files#diff-22d0b70cf9dccedb457cd2d933a88b36R22-R61

The premise behind this PR is that a plugin author can declare types for their rules via interface declaration merging.

I.e. within the eslint-plugin-eslint-comments plugin, they could declare the following to provide types for their rules by adding the following to their package. This will augment the already defined Rules interface, and teach typescript about the expected shape of the options for known rules.

declare global {
  namespace ESLintConfig {
    interface Rules {
      'eslint-comments/disable-enable-pair'?:
        | ESLintConfigBase.RuleLevelAndNoOptions
        | [
            ESLintConfigBase.RuleLevel,
            {
              allowWholeFile?: boolean;
            },
          ];
      'eslint-comments/no-aggregating-enable'?: ESLintConfigBase.RuleLevelAndNoOptions;
      'eslint-comments/no-duplicate-disable'?: ESLintConfigBase.RuleLevelAndNoOptions;
      'eslint-comments/no-restricted-disable'?:
        | ESLintConfigBase.RuleLevelAndNoOptions
        | [ESLintConfigBase.RuleLevel, ...string[]];
      'eslint-comments/no-unlimited-disable'?: ESLintConfigBase.RuleLevelAndNoOptions;
      'eslint-comments/no-unused-disable'?: ESLintConfigBase.RuleLevelAndNoOptions;
      'eslint-comments/no-unused-enable'?: ESLintConfigBase.RuleLevelAndNoOptions;
      'eslint-comments/no-use'?:
        | ESLintConfigBase.RuleLevelAndNoOptions
        | [
            ESLintConfigBase.RuleLevel,
            {
              allow?: (
                | 'eslint'
                | 'eslint-disable'
                | 'eslint-disable-line'
                | 'eslint-disable-next-line'
                | 'eslint-enable'
                | 'eslint-env'
                | 'exported'
                | 'global'
                | 'globals'
              )[];
            },
          ];
    }
  }
}

There are two problems with using type declarations:

1. They would require the user to import the types.
2. They do not support declaration merging.

Both of these are an instant non-starter for using types, because you cannot declare an .eslintrc.ts file; you have to use a .eslintrc.js. So we can't import type declarations, nor can you annotate things for users.

With the current solution, a user can just do the following in their .eslintrc.js, and it will cause typescript to strictly check the config for type errors.

+// @ts-check
+const typedConfig = require('@typescript-eslint/typed-config');

-module.exports = {
+module.exports = typedConfig({
   // existing config...
-}
+})

[manuth]

Though in my suggestion Rules is a type, people would be able to provide their own rules using RulesStore which is an interface which means it can be declaration-merged.

This leaves me with one question: will changing Rules from interface to type break the type hinting for Javascript files?

[bradzacher]

I guess my question is - what's the difference between your suggested RuleStore and what Rules currently is?

[manuth]

The only difference is that you don't have to add RuleLevel | RuleLevelWithNoOptions | or RuleLevel | aka. you can ensure it is possible for each rule to be executed without options.

This might be a good idea because if I understand correctly this must be possible by design of eslint.

[bradzacher]

Ah sorry, I misunderstood your intention. I understand now.

This might be a good idea because if I understand correctly this must be possible by design of eslint.

In the general case, yes, this holds true; generally rules are set up with optional options because authors provide default config for a rule so it's easier to configure, but it is not actually a requirement.

eslint uses JSON schemas to validate the options, and it's possible to define a JSON schema which requires the user provides an option.

For example, I very recently did this accidentally for a rule: #1472

Do I know of any rules that actually do this intentionally? Nope.
Should this tooling be built without support for it initially, with the intention of adding it if required? Potentially.

[tadhgmister]

@bradzacher I would love to champion this, I have many ideas and come from a background of spoon feeding typescript to javascript developers so I think I could do well. A few initial thoughts:

1. typedConfig should insert plugin:@typescript-eslint/base into the extends list so even typedConfig({}) is valid.
2. typedConfig should accept only off for built in rules we have extensions for.
3. can have a second helper like typedConfigEasySetup which enables a few rules that apply only to the lint config file like:
   • error on usage of base rules with extensions. auto fix renames it.
   • error on usage of a rule with a prefix (like import/first) that isn't in the plugins list. auto fix adds plugin.
   • rule that checks config of rules that are missing type info against the runtime schema so instead of eslint just failing with message we can have it give error directly on the rule that is invalid. (quite possible this rule already exists somewhere, will be sure to look before implementing it.)

would the best way to handle this to open a draft PR from my forked repo? do I make it to master or to typed-config branch?

[bradzacher]

1. typedConfig should insert plugin:@typescript-eslint/base into the extends list so even typedConfig({}) is valid.
2. typedConfig should accept only off for built in rules we have extensions for.

These make the assumption that a user has a typescript-only codebase.
It's difficult to provide defaults of these kinds because of this.
You'd be surprised how many codebases use our tooling and are in some form of a mixed state.

My original thinking for the first cut of this was to just keep it simple to start with and see how it grows. This will require adoption from the ecosystem for it to flourish, so we don't want to go all-in and put mountains of effort, only to find out that no plugins want to add types for their rule configs.

That's probably the most difficult part of this particular project - figuring out how we can make it dead simple for plugin owners to provide types for their plugin (or similarly for the community to add types to DefinitelyTyped for the plugin).

[tadhgmister]

You'd be surprised how many codebases use our tooling and are in some form of a mixed state.

I realized when writing the docs that explaining the parts of the config they need to setup and saying the function is a no-op is far simpler than explaining what it adds and why, so definitely will stay a no-op. Very interesting to hear that these cases exist though.

The way I see it, plugins already provide JSON schema for the rules and libraries exist to convert this to typescript definitions.
So to make this work we can provide users a script that reads the plugin info and generates type definitions automatically. this way it's dead simple for plugins to auto-generate the info, and users can generate it themselves for the plugins that don't provide it.

It's still far from production ready and some updates to json-schema-to-typescript to fit this use case would be needed, but I do like this direction.

[bradzacher]

Yeah that package is exactly what I've used to do this.

https://github.com/bradzacher/eslint-config-brad/tree/master/src/types

My work in the above package was the inspiration for this work and validation that it can work!

If we can make it all work ambiently then that'd be ace. But we could always fall back to a generic which takes a union of the plugin defs.

We'd also need some assistance from plugin authors to define correct schemas - some of them define schemas that produce bad types eg

jsx-eslint/eslint-plugin-react#2340
jsx-eslint/eslint-plugin-react#2341

[tadhgmister]

We'd also need some assistance from plugin authors to define correct schemas - some of them define schemas that produce bad types eg

Well I mean allowing an empty object to be specified isn't exactly a bad type, the auto generated type does match the actual allowed schema in those cases. The only "bad schema" I've seen is our own lines-between-class-members, it uses a object style merge with an array so it ends up with an object with keys '0' and '1' which is apparently enough for eslint to understand but messes up the type definition (should be ['always' | 'never', {...}])

If we can make it all work ambiently then that'd be ace.

As far as I can tell the config file will need at minimum a /// <reference/> for each plugin. But a linter rule that has auto-fix to add the relevant reference doesn't seem terribly complicated, as long as the convention for naming the declaration file is predictable.

[G-Rath]

As far as I can tell the config file will need at minimum a /// for each plugin

If I understand it correctly, the new config simplification ESLint are going with has you explicitly require plugins, meaning in future this should become even simpler :)

[glen-84]

For ESLint, I can use:

module.exports = /** @type {import("eslint").Linter.BaseConfig} */ ({
    // ...
});

... to get some basic code completion, etc.

However, this doesn't include some of the configuration extensions from this project – is there a similar type that I could use for TypeScript-ESLint?

[bradzacher]

@glen-84 we have our own package (which is used by all our tooling) that includes complete types for ESLint, extended to include our stuff.

typescript-eslint/packages/experimental-utils/src/ts-eslint/Linter.ts

Lines 117 to 184 in e4d737b

	// https://github.com/eslint/eslint/blob/v6.8.0/conf/config-schema.js
	interface BaseConfig {
	$schema?: string;
	/**
	* The environment settings.
	*/
	env?: { [name: string]: boolean };
	/**
	* The path to other config files or the package name of shareable configs.
	*/
	extends?: string | string[];
	/**
	* The global variable settings.
	*/
	globals?: { [name: string]: boolean };
	/**
	* The flag that disables directive comments.
	*/
	noInlineConfig?: boolean;
	/**
	* The override settings per kind of files.
	*/
	overrides?: ConfigOverride[];
	/**
	* The path to a parser or the package name of a parser.
	*/
	parser?: string;
	/**
	* The parser options.
	*/
	parserOptions?: ParserOptions;
	/**
	* The plugin specifiers.
	*/
	plugins?: string[];
	/**
	* The processor specifier.
	*/
	processor?: string;
	/**
	* The flag to report unused `eslint-disable` comments.
	*/
	reportUnusedDisableDirectives?: boolean;
	/**
	* The rule settings.
	*/
	rules?: RulesRecord;
	/**
	* The shared settings.
	*/
	settings?: { [name: string]: unknown };
	}
	export interface ConfigOverride extends BaseConfig {
	excludedFiles?: string | string[];
	files: string | string[];
	}
	export interface Config extends BaseConfig {
	/**
	* The glob patterns that ignore to lint.
	*/
	ignorePatterns?: string | string[];
	/**
	* The root flag.
	*/
	root?: boolean;
	}

I believe the import path would be

import('@typescript-eslint/experimental-utils').TSESLint.Linter.Config

[glen-84]

Cool, thanks. Will it ever be renamed from "experimental"? 😛

[bradzacher]

At this point I almost see it as an inside joke and I kind of don't want to change it 🙃

a7a03ce#diff-75b5ce2fecf42d2221d1f324712f902a177a59a84040d8bbad7390c32cb37a12R13

3eb5d45#diff-75b5ce2fecf42d2221d1f324712f902a177a59a84040d8bbad7390c32cb37a12R13

f5c271f#diff-75b5ce2fecf42d2221d1f324712f902a177a59a84040d8bbad7390c32cb37a12R19

[JoshuaKGoldberg]

@glen-84 just following up, it's been renamed now 😄. #4139