Make moduleVisitor extendable by plugins

[KinIcy]

Hello,

This is a feature request (kind of a continuation of #906),

Some modules require module paths for some calls (like the jest.mock() in Jest). As a developer, I would like eslint-plugin-import to apply its rules to paths to modules in other places different than require() or import calls so that I can quickly catch possible issues with such calls (i.e. the path in some call is pointing to an unresolved module).

Given this requirement is out of the scope of eslint-plugin-import (it shouldn't/can't keep track of paths used in calls other different modules), it can instead provide an interface for other modules to extend the scope of the default moduleVisitor (like the one for custom resolvers).

For instance, As a module author, I should be able to write an eslint-import-visitor-jest module that exports a function such as:

function checkJestMocks(visitors, checkSourceValue) {
    const currentCallExpression = visitors['CallExpression'];
    visitors['CallExpression'] = function (call) {
        if (currentCallExpression) currentCallExpression(call)
        if (call.callee.type !== 'MemberExpression') return
        if (call.callee.object.name !== 'jest') return
        if (call.callee.property.name !== 'mock') return
        if (call.arguments.length !== 1 || call.arguments.length !== 2) return
        const modulePath = call.arguments[0]
        if (modulePath.type !== 'Literal') return
        if (typeof modulePath.value !== 'string') return

        checkSourceValue(modulePath, call)
    }
}
module.exports.visitor = checkJestMocks;

Then a developer can include my module and point eslint-plugin-import to use the exported visitor via a setting.

I would like to write this new feature if you consider and accept a PR for this.

[ljharb]

I'm trying to understand the ask here.

eslint rules do not typically expose AST node information (except for no-unrestricted syntax). Additionally, "what the rule does with a node" depends highly on what kind of node it is. There's no generic way to "handle a node", if only you had a way to point to it.

Is your goal here so that jest.mock calls are treated the same as require calls, for the purpose of a specific rule? (seems like no-unresolved?)

[KinIcy]

Yes, one application of this feature is to get jest.mock treated as a require for rules like no-unresolved, no-restricted-paths, no-absolute-path, no-useless-path-segments, etc, in general rules that target the path being imported. All those rules depend on moduleVisitor if I'm not wrong.

Since all those rules can be applied to a jest.mock call, As a module author, I don't want to reinvent the wheel. If you provide an interface for other module authors to extended what is treated as module then I will only have to care about my plugin is compliant with the interface. And, in exchange, you will only need to care about getting the interface working.

Review your codebase I see you do something similar with custom resolvers, a developer can include a module that adds a new resolver and then link it to eslint-plugin-import via the eslint settings. In the same way, a developer should be able to include a module that adds a new "visitor" and then link it to eslint-plugin-import via the settings.

[ljharb]

A new resolver, yes, but not a new resolution API.

What you want seems fine for jest.mock, but I don't think it would be at all a good idea to basically open the floodgates and allow anyone to treat any arbitrary AST node as if it had require or import semantics.

I do see how it's technically feasible, if by "visitor" you mean something that would provide an AST node name, and a function that returns some kind of generic container that can indicate, explicitly, the specifier to look at.

What other use case could this have? Jest is, thankfully, the only thing I know of that's invented completely non-standard and impossible-to-standardize semantics for module loading. Are there others?

[KinIcy]

For now, the most popular use case would be jest.mock but I know there are other libraries that provide alternative ways of importing modules such as rewire. Both of them are used to make testing easier.

As for the interface, what I'm thinking is something like the example I provided in the first post. The interface would be function extendVisitor(visitors, checkSourceValue), so eslint-plugin-import exposes the list of visitors and the function that triggers the visit.

You can alternatively provide something more restricted like function extendVisitor(node): string, so you expose the node itself and expect as return the string literal that needs to be visited, but I believe such version is a bit more complicated to implement, My first proposal is based on already existing logic.

[ljharb]

Hmm.

jest.mock, rewire, and proxyquire are indeed three such use cases.

I would definitely want something maximally restrictive - so that these three cases, or something exactly like them, are all that's permitted.

[KinIcy]

With maximally restrictive, you mean like the second approach, right? Would you accept a PR implementing this feature? I already have a draft with the implementation.

[ljharb]

I'd be more than happy to review a draft PR implementing it :-) it's hard to commit to accepting it before seeing the implementation.

Unless you already have some, don't worry about tests until we can be sure it's not going to be a waste of effort.