esquery icon indicating copy to clipboard operation
esquery copied to clipboard

Feature Request: get target node type from selector

Open nzakas opened this issue 11 years ago • 5 comments

I'm looking at using esquery in ESLint, and it looks very close to what I need for the implementation I have in mind. The one thing that is missing is a way to figure out the target node type of a given selector. Basically, something that can take the selector AST and returns:

"*" => "*"
"FunctionDeclaration > VariableDeclarator" => "VariableDeclarator"
":not(FunctionDeclaration)" => "*"

Basically, my implementation will look like:

var queryAST = esquery.parse(key);
var targetType = esquery.getTargetType(queryAST);

eslint.on(targetType, function(node) {
    If (esquery.matches(node, queryAST, parents)) {
        // apply rule
    }
}

nzakas avatar May 16 '14 19:05 nzakas

I'm not sure I understand your use case. Can you explain what eslint.on is doing with the first parameter? Have you looked at esdispatch yet? I designed its interface with tools like eslint in mind. It seems pretty similar to your proposal here.

michaelficarra avatar May 19 '14 15:05 michaelficarra

Yes, I've looked at esdispatch, and I don't like the approach of checking every node against every selector. I'd also rather have a direct dependency on esquery rather than having it be one step away in the dependency tree.

What I want is a way to narrow down which selectors should be checked against which nodes rather than always checking every one.

eslint emits events for each node type, so "FunctionDeclaration", for example. Currently rules, listen for those events and then do their processing. With esquery, that would change so that the event the rule listens for is the target type of the selector. Then, it checks to see if the whole selector matches before applying the rule. This allows us to keep the core of eslint working the same way (emitting events for node types) and just slightly change how that affects rules.

nzakas avatar May 19 '14 17:05 nzakas

Is this going to happen? I can make this functionality myself, it just seems like a nice fit with this library to bundle it.

And is esquery still under development? It looks like there hasn't been much movement recently.

nzakas avatar Jun 04 '14 22:06 nzakas

So I think you're confused about the matching process. Determining the "target type" is as expensive as doing a full match. Matching A > B fails fast if the tested node's type is not B. If it is, it continues to check if B's parent is A. Are you concerned that won't be performant?

edit: By the way, since esdispatch knows all of the selectors, we can build an optimised automaton to avoid performing the same test more than once.

I'm still happy to work on esquery, but I have no incentive right now. I don't run into any of the currently open issues myself, they're pretty much all minor, and there's no big widely-used project relying on it yet.

michaelficarra avatar Jun 05 '14 16:06 michaelficarra

Yes, I don't want to have to run every query on every node - that seems like wasted work when we know most of the time, the query won't match.

I may be missing something, but it seems like determining the target node type would be pretty fast since you're just walking the parsed query structure to get to the end node without actually doing a match or walking the Esprima AST. Am I wrong?

You have a bit of a chicken-and-egg problem. I'm afraid of relying on esquery because there's no activity on it, and you're not updating it because no one is relying on it. So, if you were to be committed to maintenance than I would commit to using it. :) ​

nzakas avatar Jun 05 '14 18:06 nzakas