Skip to content

fix(eslint-plugin): correct rules.d.ts types to not rely on non-existent imports #9339

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Jun 29, 2024
Merged

fix(eslint-plugin): correct rules.d.ts types to not rely on non-existent imports #9339

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2e97705
fix(eslint-plugin): use ./dist/util path in rules.d.ts
JoshuaKGoldberg Jun 12, 2024
f08e1e4
Merge branch 'v8' into eslint-plugin-rules-util-path
JoshuaKGoldberg Jun 12, 2024
ea7cb79
Fix introduced type complaint
JoshuaKGoldberg Jun 22, 2024
8878dd4
test: one more fix for default
JoshuaKGoldberg Jun 25, 2024
c76814f
Move to copying the types, as we don't publish .d.ts
JoshuaKGoldberg Jun 25, 2024
b3d2b25
import from rules, heck yes
JoshuaKGoldberg Jun 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 32 additions & 3 deletions packages/eslint-plugin/rules.d.ts
View file
Open in desktop
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 don't love this copy & pasting of types between files. But we don't publish .d.ts files under dist/ for this package and don't publish src/.

My preference would be to start publishing .d.ts files to move towards a future where folks can strongly type individual rule options in their eslint.config.ts / // @ts-check'd eslint.config.js. But that seems like a bigger change, with potential for serious package size increases, out of scope for this PR.

Copy link
Member

Choose a reason for hiding this comment

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

I wonder what if we remove ESLintPluginDocs and ESLintPluginRuleModule from packages/eslint-plugin/src/util/createRule.ts and import them from packages/eslint-plugin/rules.d.ts instead? I don't see any reason why this could end badly 🙂

 import { ESLintUtils } from '@typescript-eslint/utils';
-import type {
-  RuleModuleWithMetaDocs,
-  RuleRecommendation,
-  RuleRecommendationAcrossConfigs,
-} from '@typescript-eslint/utils/ts-eslint';
-
-export interface ESLintPluginDocs {
-  /**
-   * Does the rule extend (or is it based off of) an ESLint code rule?
-   * Alternately accepts the name of the base rule, in case the rule has been renamed.
-   * This is only used for documentation purposes.
-   */
-  extendsBaseRule?: boolean | string;
-
-  /**
-   * If a string config name, which starting config this rule is enabled in.
-   * If an object, which settings it has enabled in each of those configs.
-   */
-  recommended?: RuleRecommendation | RuleRecommendationAcrossConfigs<unknown[]>;
-
-  /**
-   * Does the rule require us to create a full TypeScript Program in order for it
-   * to type-check code. This is only used for documentation purposes.
-   */
-  requiresTypeChecking?: boolean;
-}
+import { ESLintPluginDocs } from '../../rules';

 export const createRule = ESLintUtils.RuleCreator<ESLintPluginDocs>(
   name => `https://typescript-eslint.io/rules/${name}`,
 );

-export type ESLintPluginRuleModule = RuleModuleWithMetaDocs<
-  string,
-  readonly unknown[],
-  ESLintPluginDocs
->;

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 am ... deeply amused at how much better of a solution this is. 😄

Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,45 @@ This is likely not portable. A type annotation is necessary. ts(2742)
```
*/

import type { RuleModuleWithMetaDocs } from '@typescript-eslint/utils/ts-eslint';
import type {
RuleModuleWithMetaDocs,
RuleModuleWithMetaDocs,
RuleRecommendation,
RuleRecommendationAcrossConfigs,
} from '@typescript-eslint/utils/ts-eslint';

import type { ESLintPluginDocs, ESLintPluginRuleModule } from './src/util';
export interface ESLintPluginDocs {
/**
* Does the rule extend (or is it based off of) an ESLint code rule?
* Alternately accepts the name of the base rule, in case the rule has been renamed.
* This is only used for documentation purposes.
*/
extendsBaseRule?: boolean | string;

export { ESLintPluginDocs, ESLintPluginRuleModule };
/**
* If a string config name, which starting config this rule is enabled in.
* If an object, which settings it has enabled in each of those configs.
*/
recommended?: RuleRecommendation | RuleRecommendationAcrossConfigs<unknown[]>;

/**
* Does the rule require us to create a full TypeScript Program in order for it
* to type-check code. This is only used for documentation purposes.
*/
requiresTypeChecking?: boolean;
}

export type ESLintPluginRuleModule = RuleModuleWithMetaDocs<
string,
readonly unknown[],
ESLintPluginDocs
>;

export type TypeScriptESLintRules = Record<
string,
RuleModuleWithMetaDocs<string, unknown[], ESLintPluginDocs>
>;

declare const rules: TypeScriptESLintRules;
// eslint-disable-next-line import/no-default-export
export default rules;
32 changes: 1 addition & 31 deletions packages/eslint-plugin/src/util/createRule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,37 +1,7 @@
import { ESLintUtils } from '@typescript-eslint/utils';
import type {
RuleModuleWithMetaDocs,
RuleRecommendation,
RuleRecommendationAcrossConfigs,
} from '@typescript-eslint/utils/ts-eslint';

export interface ESLintPluginDocs {
/**
* Does the rule extend (or is it based off of) an ESLint code rule?
* Alternately accepts the name of the base rule, in case the rule has been renamed.
* This is only used for documentation purposes.
*/
extendsBaseRule?: boolean | string;

/**
* If a string config name, which starting config this rule is enabled in.
* If an object, which settings it has enabled in each of those configs.
*/
recommended?: RuleRecommendation | RuleRecommendationAcrossConfigs<unknown[]>;

/**
* Does the rule require us to create a full TypeScript Program in order for it
* to type-check code. This is only used for documentation purposes.
*/
requiresTypeChecking?: boolean;
}
import type { ESLintPluginDocs } from '../../rules';

export const createRule = ESLintUtils.RuleCreator<ESLintPluginDocs>(
name => `https://typescript-eslint.io/rules/${name}`,
);

export type ESLintPluginRuleModule = RuleModuleWithMetaDocs<
string,
readonly unknown[],
ESLintPluginDocs
>;
4 changes: 2 additions & 2 deletions packages/eslint-plugin/tests/configs.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,14 @@ function filterAndMapRuleConfigs({
if (recommendations) {
result = result.filter(([, rule]) => {
switch (typeof rule.meta.docs?.recommended) {
case 'undefined':
return false;
case 'object':
return Object.keys(rule.meta.docs.recommended).some(recommended =>
recommendations.includes(recommended as RuleRecommendation),
);
case 'string':
return recommendations.includes(rule.meta.docs.recommended);
default:
return false;
}
});
}
Expand Down
4 changes: 2 additions & 2 deletions packages/typescript-eslint/tests/configs.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,14 @@ function filterAndMapRuleConfigs({
if (recommendations) {
result = result.filter(([, rule]) => {
switch (typeof rule.meta.docs.recommended) {
case 'undefined':
return false;
case 'object':
return Object.keys(rule.meta.docs.recommended).some(recommended =>
recommendations.includes(recommended as RuleRecommendation),
);
case 'string':
return recommendations.includes(rule.meta.docs.recommended);
default:
return false;
}
});
}
Expand Down
4 changes: 2 additions & 2 deletions packages/website/plugins/generated-rule-docs/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import pluginRules from '@typescript-eslint/eslint-plugin/use-at-your-own-risk/r
import type { Plugin } from 'unified';

import { nodeIsParent } from '../utils/nodes';
import { isAnyRuleModuleWithMetaDocs, isVFileWithStem } from '../utils/rules';
import { isESLintPluginRuleModule, isVFileWithStem } from '../utils/rules';
import { addESLintHashToCodeBlocksMeta } from './addESLintHashToCodeBlocksMeta';
import { createRuleDocsPage } from './createRuleDocsPage';
import { insertBaseRuleReferences } from './insertions/insertBaseRuleReferences';
Expand All @@ -21,7 +21,7 @@ export const generatedRuleDocs: Plugin = () => {
}

const rule = pluginRules[file.stem];
if (!isAnyRuleModuleWithMetaDocs(rule)) {
if (!isESLintPluginRuleModule(rule)) {
return;
}

Expand Down
12 changes: 5 additions & 7 deletions packages/website/plugins/utils/rules.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import type {
AnyRuleModuleWithMetaDocs,
RuleModule,
} from '@typescript-eslint/utils/ts-eslint';
import type { ESLintPluginRuleModule } from '@typescript-eslint/eslint-plugin/use-at-your-own-risk/rules';
import type { RuleModule } from '@typescript-eslint/utils/ts-eslint';
import * as fs from 'fs';
import * as lz from 'lz-string';
import * as path from 'path';
Expand Down Expand Up @@ -55,9 +53,9 @@ export function getUrlForRuleTest(ruleName: string): string {
throw new Error(`Could not find test file for ${ruleName}.`);
}

export function isAnyRuleModuleWithMetaDocs(
rule: RuleModule<string, unknown[]> | undefined,
): rule is AnyRuleModuleWithMetaDocs {
export function isESLintPluginRuleModule(
rule: RuleModule<string, readonly unknown[]> | undefined,
): rule is ESLintPluginRuleModule {
return !!rule?.meta.docs;
}

Expand Down
Loading