feat: validate type comments and generate `.d.ts` #204

voxpelli · 2024-03-08T22:09:00Z

This is an alternative to #60 which also fixes #150

As this PR doesn't rewrite to TS its easier to keep up to date as other changes happen.

No files has been renamed, the build script remains largely the same, the code as well mostly.

The generated type file

import * as estree from 'estree';
import * as eslint from 'eslint';

type Node = estree.Node | estree.Expression;

declare const READ: unique symbol;
declare const CALL: unique symbol;
declare const CONSTRUCT: unique symbol;
declare const ESM: unique symbol;
declare class ReferenceTracker {
    constructor(globalScope: eslint.Scope.Scope, { mode, globalObjectNames, }?: {
        mode?: "legacy" | "strict" | undefined;
        globalObjectNames?: string[] | undefined;
    } | undefined);
    variableStack: eslint.Scope.Variable[];
    globalScope: eslint.Scope.Scope;
    mode: "legacy" | "strict";
    globalObjectNames: string[];
    iterateGlobalReferences(traceMap: TraceMap): IterableIterator<Reference>;
    iterateCjsReferences(traceMap: TraceMap): IterableIterator<Reference>;
    iterateEsmReferences(traceMap: TraceMap): IterableIterator<Reference>;
    _iterateVariableReferences(variable: eslint.Scope.Variable, path: string[], traceMap: TraceMap, shouldReport: boolean): IterableIterator<Reference>;
    _iteratePropertyReferences(rootNode: RichNode, path: string[], traceMap: TraceMap): IterableIterator<Reference>;
    _iterateLhsReferences(patternNode: RichNode, path: string[], traceMap: TraceMap): IterableIterator<Reference>;
    _iterateImportReferences(specifierNode: RichNode, path: string[], traceMap: TraceMap): IterableIterator<Reference>;
}
declare namespace ReferenceTracker {
    export { READ };
    export { CALL };
    export { CONSTRUCT };
    export { ESM };
}
type ReferenceType = typeof READ | typeof CALL | typeof CONSTRUCT;
type TraceMap = {
    [key: string]: TraceMap;
} & Partial<Record<ReferenceType, boolean>>;
type RichNode = eslint.Rule.Node | Node;
type Reference = {
    node: RichNode;
    path: string[];
    type: ReferenceType;
    info: unknown;
};

declare function findVariable(initialScope: eslint.Scope.Scope, nameOrNode: string | Node): eslint.Scope.Variable | null;

declare function getFunctionHeadLocation(node: Extract<eslint.Rule.Node, {
    type: 'FunctionDeclaration' | 'FunctionExpression' | 'ArrowFunctionExpression';
}>, sourceCode: eslint.SourceCode): eslint.AST.SourceLocation | null;

declare function getFunctionNameWithKind(node: Extract<eslint.Rule.Node, {
    type: 'FunctionDeclaration' | 'FunctionExpression' | 'ArrowFunctionExpression';
}>, sourceCode?: eslint.SourceCode | undefined): string;

declare function getInnermostScope(initialScope: eslint.Scope.Scope, node: Node): eslint.Scope.Scope;

declare function getPropertyName(node: Extract<Node, {
    type: 'MemberExpression' | 'Property' | 'MethodDefinition' | 'PropertyDefinition';
}>, initialScope?: eslint.Scope.Scope | undefined): string | null;

declare function getStaticValue(node: Node, initialScope?: eslint.Scope.Scope | null | undefined): {
    value: unknown;
    optional?: never;
} | {
    value: undefined;
    optional?: true;
} | null;

declare function getStringIfConstant(node: Node, initialScope?: eslint.Scope.Scope | null | undefined): string | null;

declare function hasSideEffect(node: eslint.Rule.Node, sourceCode: eslint.SourceCode, { considerGetters, considerImplicitTypeConversion }?: VisitOptions | undefined): boolean;
type VisitOptions = {
    considerGetters?: boolean | undefined;
    considerImplicitTypeConversion?: boolean | undefined;
};

declare function isArrowToken(token: eslint.AST.Token): boolean;
declare function isCommaToken(token: eslint.AST.Token): boolean;
declare function isSemicolonToken(token: eslint.AST.Token): boolean;
declare function isColonToken(token: eslint.AST.Token): boolean;
declare function isOpeningParenToken(token: eslint.AST.Token): boolean;
declare function isClosingParenToken(token: eslint.AST.Token): boolean;
declare function isOpeningBracketToken(token: eslint.AST.Token): boolean;
declare function isClosingBracketToken(token: eslint.AST.Token): boolean;
declare function isOpeningBraceToken(token: eslint.AST.Token): boolean;
declare function isClosingBraceToken(token: eslint.AST.Token): boolean;
declare function isCommentToken(token: eslint.AST.Token): boolean;
declare function isNotArrowToken(token: eslint.AST.Token): boolean;
declare function isNotCommaToken(token: eslint.AST.Token): boolean;
declare function isNotSemicolonToken(token: eslint.AST.Token): boolean;
declare function isNotColonToken(token: eslint.AST.Token): boolean;
declare function isNotOpeningParenToken(token: eslint.AST.Token): boolean;
declare function isNotClosingParenToken(token: eslint.AST.Token): boolean;
declare function isNotOpeningBracketToken(token: eslint.AST.Token): boolean;
declare function isNotClosingBracketToken(token: eslint.AST.Token): boolean;
declare function isNotOpeningBraceToken(token: eslint.AST.Token): boolean;
declare function isNotClosingBraceToken(token: eslint.AST.Token): boolean;
declare function isNotCommentToken(token: eslint.AST.Token): boolean;

declare function isParenthesized(timesOrNode: number, nodeOrSourceCode: eslint.Rule.Node, optionalSourceCode: eslint.SourceCode): boolean;
declare function isParenthesized(timesOrNode: eslint.Rule.Node, nodeOrSourceCode: eslint.SourceCode): boolean;

declare class PatternMatcher {
    constructor(pattern: RegExp, { escaped }?: {
        escaped?: boolean | undefined;
    } | undefined);
    execAll(str: string): IterableIterator<RegExpExecArray>;
    test(str: string): boolean;
    [Symbol.replace](str: string, replacer: string | ((...strs: (string | number)[]) => string)): string;
}

declare namespace _default {
    export { CALL };
    export { CONSTRUCT };
    export { ESM };
    export { findVariable };
    export { getFunctionHeadLocation };
    export { getFunctionNameWithKind };
    export { getInnermostScope };
    export { getPropertyName };
    export { getStaticValue };
    export { getStringIfConstant };
    export { hasSideEffect };
    export { isArrowToken };
    export { isClosingBraceToken };
    export { isClosingBracketToken };
    export { isClosingParenToken };
    export { isColonToken };
    export { isCommaToken };
    export { isCommentToken };
    export { isNotArrowToken };
    export { isNotClosingBraceToken };
    export { isNotClosingBracketToken };
    export { isNotClosingParenToken };
    export { isNotColonToken };
    export { isNotCommaToken };
    export { isNotCommentToken };
    export { isNotOpeningBraceToken };
    export { isNotOpeningBracketToken };
    export { isNotOpeningParenToken };
    export { isNotSemicolonToken };
    export { isOpeningBraceToken };
    export { isOpeningBracketToken };
    export { isOpeningParenToken };
    export { isParenthesized };
    export { isSemicolonToken };
    export { PatternMatcher };
    export { READ };
    export { ReferenceTracker };
}

export { CALL, CONSTRUCT, ESM, PatternMatcher, READ, ReferenceTracker, _default as default, findVariable, getFunctionHeadLocation, getFunctionNameWithKind, getInnermostScope, getPropertyName, getStaticValue, getStringIfConstant, hasSideEffect, isArrowToken, isClosingBraceToken, isClosingBracketToken, isClosingParenToken, isColonToken, isCommaToken, isCommentToken, isNotArrowToken, isNotClosingBraceToken, isNotClosingBracketToken, isNotClosingParenToken, isNotColonToken, isNotCommaToken, isNotCommentToken, isNotOpeningBraceToken, isNotOpeningBracketToken, isNotOpeningParenToken, isNotSemicolonToken, isOpeningBraceToken, isOpeningBracketToken, isOpeningParenToken, isParenthesized, isSemicolonToken };

ota-meshi · 2024-03-09T03:49:28Z

That's a good idea!
Could you please fix the CI error?

scagood · 2024-04-03T20:48:43Z

Following up from this comment: eslint-community/eslint-plugin-n#169 (comment)

I made a couple of utility changes to the .d.ts file I stole from your description 😄
If this is not the right place for these comments please point me in the right direction!

`TraceMap`

I made a couple of changes to TraceMap:

I added a configurable parameter

type TraceMap<Info extends unknown> =
  & { [key: string]: TraceMap<Info>; }
  & Partial<Record<ReferenceType, Info>>;

I unwrapped the TraceMap to be one object with all the properties as optional.

type TraceMap<Info extends unknown> = {
  [READ]?: Info;
  [CALL]?: Info;
  [CONSTRUCT]?: Info;
  [key: string]: TraceMap<Info>;
}

This second one confuses me a little, as I know it works, but I dont know why! Before I would get a small army of these kinds of error:

88 errors to be exact 😓

Object literal may only specify known properties, and 'alloc' does not exist in type 'Partial<Record<ReferenceType, SupportInfo>>'

`Reference`

Reference also got its own argument:

type Reference<Info extends unknown> = {
  node: RichNode;
  path: string[];
  type: ReferenceType;
  // Previously:
  // info: unknown;
  info: Info;
};

`ReferenceTracker`

Because TraceMap and Reference now have inferable arguments, we make make the iterateXXXReferences functions a little more friendly by adding the Info argument. This allows for simple type inference

  iterateGlobalReferences<Info extends unknown>(traceMap: TraceMap<Info>): IterableIterator<Reference<Info>>;
  iterateCjsReferences<Info extends unknown>(traceMap: TraceMap<Info>): IterableIterator<Reference<Info>>;
  iterateEsmReferences<Info extends unknown>(traceMap: TraceMap<Info>): IterableIterator<Reference<Info>>;

This allows the following to just work out of the box:

/**
 * @typedef CustomInfo
 * @property {string[]} experimental
 * @property {string[]} supported
 * @property {string[]} deprecated
 */

/** @type {TraceMap<CustomInfo>} */
const traceMap = {}

for (const reference of tracker.iterateGlobalReferences(traceMap)) {
  console.info(reference.info.supported);
}

For the sake of simplicity I threw together an example of this on the TS playground:
https://www.typescriptlang.org/play?#code/ ...

I am happy to contibute where ever is needed as long as I'm not step on anyones toes 😁

voxpelli · 2024-04-07T12:40:33Z

I'll get around to update this PR real soon.

One challenge right now is that this module support quite old versions of Node.js.

I would prefer to align with eslint-plugin-n, eslint@9.0.0 etc the engine range they use: ^18.18.0 || ^20.9.0 || >=21.1.0

That's a separate issue though, but one I would want to deal with before spending work on backporting changes to those older versions.

cc @JoshuaKGoldberg, would be lovely with some typescript-eslint feedback on this PR eventually as well :)

.eslintrc.js

Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com>

JoshuaKGoldberg · 2024-04-08T21:03:54Z

.eslintrc.js

    rules: {
+        semi: ["error", "never"],
+        "semi-spacing": ["error", { before: false, after: true }],


There's something deeply funny to me about an eslint-community project using ESLint for formatting.

JoshuaKGoldberg · 2024-04-08T21:08:27Z

package.json

        "coverage": "opener ./coverage/lcov-report/index.html",
        "docs:build": "vitepress build docs",
        "docs:watch": "vitepress dev docs",
        "format": "npm run -s format:prettier -- --write",
        "format:prettier": "prettier .",
        "format:check": "npm run -s format:prettier -- --check",
-        "lint": "eslint .",
+        "lint:eslint": "eslint .",
+        "lint:tsc": "tsc",


Naming: I wouldn't call TypeScript a "lint" task. Even if it acts essentially as a linter in this setup, I've found it confusing for newer contributors to see it named as one.

JoshuaKGoldberg · 2024-04-08T21:21:21Z

src/get-static-value.mjs

 const operations = Object.freeze({
    ArrayExpression(node, initialScope) {
+        if (node.type !== "ArrayExpression") {
+            return null
+        }


🤔 are these ifs actually necessary, or just to appease a type checker that doesn't understand what's happening?

If it's the latter, the general right thing to do would be to fix TypeScript's understanding of the types so it doesn't yell at you.

For posterity: I tried adding /** @param {import("estree").ArrayExpression} node */ to the ArrayExpression one and got a type error on operations:

Type 'Readonly<{ ArrayExpression(node: ArrayExpression, initialScope: Scope | undefined): { value: any[]; } | null; AssignmentExpression(node: Node, initialScope: Scope | undefined): StaticValue | null; ... 14 more ...; UnaryExpression(node: Node, initialScope: Scope | undefined): { ...; } | ... 3 more ... | null; }>' is not assignable to type 'Readonly<Partial<Record<"CatchClause" | "ClassBody" | "Identifier" | "Literal" | "MethodDefinition" | "PrivateIdentifier" | "Program" | "Property" | "PropertyDefinition" | "SpreadElement" | ... 60 more ... | "VariableDeclaration", VisitorCallback>>>'. Types of property 'ArrayExpression' are incompatible. Type '(node: ArrayExpression, initialScope: Scope | undefined) => { value: any[]; } | null' is not assignable to type 'VisitorCallback'.ts(2322)

Tricky.

I got around it by:

Renaming types.mjs to types.d.ts, so I could use "real" TypeScript syntax and not the annoying JSDoc sludge

Making the VisitorCallback type generic:

export type VisitorCallback<InNode extends Node> = ( node: InNode, initialScope: eslint.Scope.Scope | undefined, ) => StaticValue | null

Using a type annotation on the ArrayExpression member:

/** @type {Readonly<Partial<Record<import('eslint').Rule.NodeTypes, import("./types.js").VisitorCallback<any>>>>} */ const operations = Object.freeze({ /** @type {import("./types.js").VisitorCallback<import("estree").ArrayExpression>} */ ArrayExpression(node, initialScope) {

YMMV.

voxpelli added 10 commits March 4, 2024 21:25

Initial type setup

c6a385f

Type it up

d24cd96

Have static value pass strict check

b430a97

Check and increase type-coverage

94d9faf

Improve has-side-effect types

5b7ea93

Improve the types some more

722f27e

Fix tests

5a7e9a2

Improve types

fe80e3c

Enforce no semi colons

82a1134

Fix incorrect check in getStringIfConstant

dbbf50d

voxpelli self-assigned this Mar 8, 2024

voxpelli requested a review from a team March 8, 2024 22:09

voxpelli added 3 commits March 8, 2024 23:10

Run prettier

a3abcf9

Mimic #60 and move build to lint step on CI

77cff12

Prettier broke an ignore

33adae4

voxpelli mentioned this pull request Mar 8, 2024

chore(deps-dev): bump rollup from 2.79.1 to 4.12.1 #203

Closed

voxpelli mentioned this pull request Mar 9, 2024

ci: add more linting checks #205

Merged

scagood mentioned this pull request Mar 21, 2024

feat: typescript (jsdoc) checking and definition generation eslint-community/eslint-plugin-n#169

Merged

JoshuaKGoldberg reviewed Apr 8, 2024

View reviewed changes

.eslintrc.js Outdated Show resolved Hide resolved

Update .eslintrc.js

fece1fa

Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com>

JoshuaKGoldberg reviewed Apr 8, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: validate type comments and generate `.d.ts` #204

feat: validate type comments and generate `.d.ts` #204

voxpelli commented Mar 8, 2024

ota-meshi commented Mar 9, 2024

scagood commented Apr 3, 2024

voxpelli commented Apr 7, 2024

JoshuaKGoldberg Apr 8, 2024

JoshuaKGoldberg Apr 8, 2024

JoshuaKGoldberg Apr 8, 2024

feat: validate type comments and generate .d.ts #204

Are you sure you want to change the base?

feat: validate type comments and generate .d.ts #204

Conversation

voxpelli commented Mar 8, 2024

ota-meshi commented Mar 9, 2024

scagood commented Apr 3, 2024

TraceMap

Reference

ReferenceTracker

voxpelli commented Apr 7, 2024

JoshuaKGoldberg Apr 8, 2024

Choose a reason for hiding this comment

JoshuaKGoldberg Apr 8, 2024

Choose a reason for hiding this comment

JoshuaKGoldberg Apr 8, 2024

Choose a reason for hiding this comment

feat: validate type comments and generate `.d.ts` #204

feat: validate type comments and generate `.d.ts` #204

`TraceMap`

`Reference`

`ReferenceTracker`