Adapting the DefinitelyTyped types should be fine in terms of licensing, although I think it doesn't quite align with this project's guidelines:

This can also be JSXIdentifier.

It's true, but we need to update the base interface Scope.Reference in ESLint first, because we can't broaden the type of the identifier member in the implementing class.

Question: there are some static properties like the following:

Should these types also be applied to the Variable class?

Good question. These are the possible values for the type property of aDefinition. These constants aren't documented, but neither are the constructor signatures for Scope, Variable, Definition, etc, which all have their type definitions. I'd say it's fine to add types in this case, but I don't feel strongly.

I also don't feel strongly about it either, but I think it's helpful to have one, since the typing below is just string rather than a more specific type:

But, I'm also open to other suggestions.

Could you clarify your suggestion with an example?

Sorry for not being clearer.

I intended to ask that the following (or similar) properties be added to the Variable class type:

/**
 * Represents a variable in a scope.
 */
export class Variable<TReference extends Reference = Reference> {
    // Existing implementation...

+   static readonly CatchClause: "CatchClause";
+   static readonly Parameter: "Parameter";
+   static readonly FunctionName: "FunctionName";
+   static readonly ClassName: "ClassName";
+   static readonly Variable: "Variable";
+   static readonly ImportBinding: "ImportBinding";
+   static readonly ImplicitGlobalVariable: "ImplicitGlobalVariable";
}

Similarly, in the Reference class:

export class Reference {
    // Existing implementation...

+   static readonly READ: 1;
+   static readonly WRITE: 2;
+   static readonly RW: 3;
}

As a result,

Before: TypeScript shows a warning:

After: TypeScript doesn't show a warning:

Question: there are also some static properties like the following:

Should these types also be applied to the Reference class?

As with the previous question, these values are undocumented, so I don't feel strongly whether they should be included in the types or not.

I also don't feel strongly about it either, but I think it's helpful to have one, since the typing below is just string rather than a more specific type:

But, I'm also open to other suggestions.

Non-blocking, but a newer version just has been released :)

I'm not 100% sure, but for the same reason mentioned in #709 (comment), I think eslint-visitor-keys may also need to be listed in dependencies instead of dev-dependencies.

Assuming we've installed the new version of eslint-scope that includes type support for this change.

There are now four sub-dependencies: @types/esrecurse, @types/estree, esrecurse, and estraverse:

@types/esrecurse: https://npmgraph.js.org/?q=%40types%2Fesrecurse

@types/estree: https://npmgraph.js.org/?q=%40types%2Festree

esrecurse: https://npmgraph.js.org/?q=esrecurse

estraverse: https://npmgraph.js.org/?q=estraverse

However, none of the packages above include eslint-visitor-keys under the hood, even though we're importing types from it:

So, I think this explains why the previous @types/eslint-scope also included eslint-visitor-keys as a sub-dependency:

https://npmgraph.js.org/?q=%40types%2Feslint-scope#select=%40types%2Festree%401.0.8

Would it make sense to mark writeExpr, partial, and init as optional properties?

In the actual implementation, these properties may or may not be set depending on the result of the isWrite() method, so I think they can sometimes be omitted. What do you think?

Sorry for not being clearer.

I intended to ask that the following (or similar) properties be added to the Variable class type:

/**
 * Represents a variable in a scope.
 */
export class Variable<TReference extends Reference = Reference> {
    // Existing implementation...

+   static readonly CatchClause: "CatchClause";
+   static readonly Parameter: "Parameter";
+   static readonly FunctionName: "FunctionName";
+   static readonly ClassName: "ClassName";
+   static readonly Variable: "Variable";
+   static readonly ImportBinding: "ImportBinding";
+   static readonly ImplicitGlobalVariable: "ImplicitGlobalVariable";
}

Similarly, in the Reference class:

export class Reference {
    // Existing implementation...

+   static readonly READ: 1;
+   static readonly WRITE: 2;
+   static readonly RW: 3;
}

As a result,

Before: TypeScript shows a warning:

After: TypeScript doesn't show a warning:

Currently, we also export version and the Referencer type from the main entry point, index.js:

However, it seems the two types version and Referencer are not defined in index.d.cts. Would it make sense to include them in the index.d.cts file?

Just out of curiosity, it seems like there are two no-op methods, attach and detach, in ScopeManager:

I checked the archived eslint-scope repository and these methods appear to have been present since the project’s early days:

https://github.com/eslint/eslint-scope/blame/9b58d9d6943c6b74185bed2a27dad210fd9c02f0/lib/scope-manager.js

So, I am wonder, would it make sense to add these methods to the ScopeManager class type? They appear to be public methods, since they don’t use the __ prefix.

Looking at the built packages/eslint-scope/dist/eslint-scope.cjs file, it appears there are some additional named exports:

The Referencer and version types aren't defined as noted earlier (Ref: #709 (comment)), so I think it would be helpful to include them as well if we add their type definitions.

FYI: Currently, the type definitions for Referencer and version are missing:

It looks like these two methods always return true because they've been deprecated, so I think using true instead of boolean may be more appropriate here.

Also, since only FunctionScope returns a boolean instead of true, it might be nice to override these two methods only in FunctionScope. What do you think?

For example:

export class FunctionScope extends Scope {
    /**
     * Creates a new FunctionScope instance.
     * @param scopeManager The scope manager this scope belongs to.
     * @param upperScope The parent scope.
     * @param block The AST node that created this scope.
     * @param isMethodDefinition Whether this scope is for a method definition.
     */
    constructor(
        scopeManager: ScopeManager,
        upperScope: Scope,
        block: ESTree.Node,
        isMethodDefinition: boolean,
    );

    type: "function";

    functionExpressionScope: false;

+   isArgumentsMaterialized(): boolean;

+   isThisMaterialized(): boolean;
}

Minor typo!

Same here.