Skip to content
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

Add tests for :scope and :root selectors #69

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Add tests for :scope and :root selectors #69

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c889544
Add test for nested descendant subject
jupenur Aug 22, 2017
3141c00
Add :root selector; see #2
jupenur Aug 22, 2017
597708c
Add :root to readme
jupenur Aug 22, 2017
bcff5ad
Implement relative selectors and :scope
jupenur Aug 22, 2017
57daae1
Add :scope to readme
jupenur Aug 22, 2017
23cf373
Improve :scope support
jupenur Aug 22, 2017
7e94456
Fixes #60: reimplement subject indicators using :has selectors
jupenur Aug 22, 2017
4d77947
Add tests for root and scope selectors
run1t Nov 12, 2018
949acdd
Add tests to scope and root selectors
run1t Nov 12, 2018
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
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,6 @@ The following selectors are supported:
* [matches-any](http://dev.w3.org/csswg/selectors4/#matches): `:matches([attr] > :first-child, :last-child)`
* [subject indicator](http://dev.w3.org/csswg/selectors4/#subject): `!IfStatement > [name="foo"]`
* class of AST node: `:statement`, `:expression`, `:declaration`, `:function`, or `:pattern`
* [root](https://drafts.csswg.org/selectors-4/#root-pseudo) and [scope](https://drafts.csswg.org/selectors-4/#scope-pseudo): `:root`, `:scope`

[![Build Status](https://travis-ci.org/estools/esquery.png?branch=master)](https://travis-ci.org/estools/esquery)
144 changes: 113 additions & 31 deletions esquery.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
/**
* Given a `node` and its ancestors, determine if `node` is matched by `selector`.
*/
function matches(node, selector, ancestry) {
function matches(node, selector, ancestry, scope) {
var path, ancestor, i, l, p;
if (!selector) { return true; }
if (!node) { return false; }
Expand All @@ -67,31 +67,32 @@

case 'matches':
for (i = 0, l = selector.selectors.length; i < l; ++i) {
if (matches(node, selector.selectors[i], ancestry)) { return true; }
if (matches(node, selector.selectors[i], ancestry, scope)) { return true; }
}
return false;

case 'compound':
for (i = 0, l = selector.selectors.length; i < l; ++i) {
if (!matches(node, selector.selectors[i], ancestry)) { return false; }
if (!matches(node, selector.selectors[i], ancestry, scope)) { return false; }
}
return true;

case 'not':
for (i = 0, l = selector.selectors.length; i < l; ++i) {
if (matches(node, selector.selectors[i], ancestry)) { return false; }
if (matches(node, selector.selectors[i], ancestry, scope)) { return false; }
}
return true;

case 'has':
var a, collector = [];
var a, collector = [], parent = ancestry[0];
Copy link

Choose a reason for hiding this comment

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

The naming here is a bit unclear as it's clobbered by the parent in the traverse method. Are they the same parent?

Copy link
Author

Choose a reason for hiding this comment

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

It's not, the parent in the traverse method is the parent of the node being traversed.
I agree with you it's a bit confusing. But I have no idea with which name we can change that 😅

for (i = 0, l = selector.selectors.length; i < l; ++i) {
a = [];
estraverse.traverse(node, {
enter: function (node, parent) {
if (parent != null) { a.unshift(parent); }
if (matches(node, selector.selectors[i], a)) {
collector.push(node);
a = ancestry.slice(parent ? 1 : 0);
estraverse.traverse(parent || node, {
enter: function (child, parent) {
if (parent == null) { return; }
a.unshift(parent);
if (matches(child, selector.selectors[i], a, node)) {
collector.push(child);
}
},
leave: function () { a.shift(); }
Expand All @@ -100,15 +101,15 @@
return collector.length !== 0;

case 'child':
if (matches(node, selector.right, ancestry)) {
return matches(ancestry[0], selector.left, ancestry.slice(1));
if (matches(node, selector.right, ancestry, scope)) {
return matches(ancestry[0], selector.left, ancestry.slice(1), scope);
}
return false;

case 'descendant':
if (matches(node, selector.right, ancestry)) {
if (matches(node, selector.right, ancestry, scope)) {
for (i = 0, l = ancestry.length; i < l; ++i) {
if (matches(ancestry[i], selector.left, ancestry.slice(i + 1))) {
if (matches(ancestry[i], selector.left, ancestry.slice(i + 1), scope)) {
return true;
}
}
Expand Down Expand Up @@ -140,27 +141,27 @@
}

case 'sibling':
return matches(node, selector.right, ancestry) &&
sibling(node, selector.left, ancestry, LEFT_SIDE) ||
return matches(node, selector.right, ancestry, scope) &&
sibling(node, selector.left, ancestry, LEFT_SIDE, scope) ||
selector.left.subject &&
matches(node, selector.left, ancestry) &&
sibling(node, selector.right, ancestry, RIGHT_SIDE);
matches(node, selector.left, ancestry, scope) &&
sibling(node, selector.right, ancestry, RIGHT_SIDE, scope);

case 'adjacent':
return matches(node, selector.right, ancestry) &&
adjacent(node, selector.left, ancestry, LEFT_SIDE) ||
return matches(node, selector.right, ancestry, scope) &&
adjacent(node, selector.left, ancestry, LEFT_SIDE, scope) ||
selector.right.subject &&
matches(node, selector.left, ancestry) &&
adjacent(node, selector.right, ancestry, RIGHT_SIDE);
matches(node, selector.left, ancestry, scope) &&
adjacent(node, selector.right, ancestry, RIGHT_SIDE, scope);

case 'nth-child':
return matches(node, selector.right, ancestry) &&
return matches(node, selector.right, ancestry, scope) &&
nthChild(node, ancestry, function (length) {
return selector.index.value - 1;
});

case 'nth-last-child':
return matches(node, selector.right, ancestry) &&
return matches(node, selector.right, ancestry, scope) &&
nthChild(node, ancestry, function (length) {
return length - selector.index.value;
});
Expand All @@ -185,6 +186,12 @@
node.type === 'ArrowFunctionExpression';
}
throw new Error('Unknown class name: ' + selector.name);

case 'scope':
return scope ? node === scope : ancestry.length === 0;

case 'root':
return ancestry.length === 0;
}

throw new Error('Unknown selector type: ' + selector.type);
Expand All @@ -193,7 +200,7 @@
/*
* Determines if the given node has a sibling that matches the given selector.
*/
function sibling(node, selector, ancestry, side) {
function sibling(node, selector, ancestry, side, scope) {
var parent = ancestry[0], listProp, startIndex, keys, i, l, k, lowerBound, upperBound;
if (!parent) { return false; }
keys = estraverse.VisitorKeys[parent.type];
Expand All @@ -210,7 +217,7 @@
upperBound = listProp.length;
}
for (k = lowerBound; k < upperBound; ++k) {
if (matches(listProp[k], selector, ancestry)) {
if (matches(listProp[k], selector, ancestry, scope)) {
return true;
}
}
Expand All @@ -222,7 +229,7 @@
/*
* Determines if the given node has an asjacent sibling that matches the given selector.
*/
function adjacent(node, selector, ancestry, side) {
function adjacent(node, selector, ancestry, side, scope) {
var parent = ancestry[0], listProp, keys, i, l, idx;
if (!parent) { return false; }
keys = estraverse.VisitorKeys[parent.type];
Expand All @@ -231,10 +238,10 @@
if (isArray(listProp)) {
idx = listProp.indexOf(node);
if (idx < 0) { continue; }
if (side === LEFT_SIDE && idx > 0 && matches(listProp[idx - 1], selector, ancestry)) {
if (side === LEFT_SIDE && idx > 0 && matches(listProp[idx - 1], selector, ancestry, scope)) {
return true;
}
if (side === RIGHT_SIDE && idx < listProp.length - 1 && matches(listProp[idx + 1], selector, ancestry)) {
if (side === RIGHT_SIDE && idx < listProp.length - 1 && matches(listProp[idx + 1], selector, ancestry, scope)) {
return true;
}
}
Expand Down Expand Up @@ -274,6 +281,81 @@
return results;
}

/*
* Clones a selector AST
*/
function clone(selector) {
if (typeof selector !== 'object' || selector instanceof RegExp) {
return selector;
}
var clonedSelector = selector instanceof Array ? [] : {};
for (var key in selector) {
if (!selector.hasOwnProperty(key)) { continue; }
clonedSelector[key] = clone(selector[key]);
}
return clonedSelector;
}

/*
* Transforms this query so that subject indicators are replaced with :has selectors
*/
function transform(selector) {
var root = { },
current = selector,
previous = root,
subjects = [ ];

if (!selector) { return selector; }

while ('left' in current && 'right' in current) {
if (current.right.subject) {
previous.type = 'scope';
subjects.push([ clone(current), clone(root) ]);
}
previous.type = current.type;
previous.right = current.right;
previous = previous.left = { }
current = current.left;
}
if (current.subject) {
previous.type = 'scope';
subjects.push([ clone(current), clone(root) ]);
}
if (subjects.length === 0) {
return selector;
} else {
var matches = {
type: 'matches',
selectors: subjects.map(function (subject) {
var result = subject[0], has = {
type: 'has',
selectors: [ subject[1] ]
};
if ('right' in subject[0]) {
delete result.right.subject;
result.right = {
type: 'compound',
selectors: result.right.type === 'compound'
? result.right.selectors.concat(has)
: [ result.right, has ]
};
} else {
delete result.subject;
result = {
type: 'compound',
selectors: result.type === 'compound'
? result.selectors.concat(has)
: [ result, has ]
};
}
return result;
})
};
return matches.selectors.length === 1
? matches.selectors[0] : matches;
}
}

/**
* From a JS AST and a selector AST, collect all JS AST nodes that match the selector.
*/
Expand Down Expand Up @@ -308,7 +390,7 @@
* Parse a selector string and return its AST.
*/
function parse(selector) {
return parser.parse(selector);
return transform(parser.parse(selector));
Copy link

Choose a reason for hiding this comment

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

This is my main concern about the changes. Is the transform necessary? Or is it just preventing this from being a breaking change? Could we just stop supporting the subject indicator?

Copy link
Author

Choose a reason for hiding this comment

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

It's just to prevent this from being a breaking change.
The subject indicators is deprecated for a long time now, It maybe a good thing to remove it.
I can make the change, if needed.

Copy link

Choose a reason for hiding this comment

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

I think it would be okay to make this a breaking change, I can imagine there being issues with the transform which might be annoying. Perhaps we can just document how a user would need to change their query?

Copy link
Author

Choose a reason for hiding this comment

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

It's sounds like a good idea, but I we drop the support for subject indicator we probably want to change the parser too and remove the related tests no ?

Copy link

Choose a reason for hiding this comment

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

Probably, I guess we could do that as a separate PR though?

}

/**
Expand Down
20 changes: 19 additions & 1 deletion grammar.pegjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,27 @@ selectors = s:selector ss:(_ "," _ selector)* {
return [s].concat(ss.map(function (s) { return s[3]; }));
}

relativeSelectors = s:relativeSelector ss:(_ "," _ relativeSelector)* {
return [s].concat(ss.map(function (s) { return s[3]; }));
}

selector
= a:sequence ops:(binaryOp sequence)* {
return ops.reduce(function (memo, rhs) {
return { type: rhs[0], left: memo, right: rhs[1] };
}, a);
}

relativeSelector
= a:sequence? ops:(binaryOp sequence)* {
return ops.reduce(function (memo, rhs) {
return { type: rhs[0], left: memo, right: rhs[1] };
},
!a || a.type === 'scope'
? { type: 'scope' }
: { type: 'descendant', left: { type: 'scope' }, right: a });
}

sequence
= subject:"!"? as:atom+ {
var b = as.length === 1 ? as[0] : { type: 'compound', selectors: as };
Expand All @@ -50,6 +64,7 @@ sequence
atom
= wildcard / identifier / attr / field / negation / matches
/ has / firstChild / lastChild / nthChild / nthLastChild / class
/ root / scope

wildcard = a:"*" { return { type: 'wildcard', value: a }; }
identifier = "#"? i:identifierName { return { type: 'identifier', value: i }; }
Expand Down Expand Up @@ -88,7 +103,7 @@ field = "." i:identifierName is:("." identifierName)* {

negation = ":not(" _ ss:selectors _ ")" { return { type: 'not', selectors: ss }; }
matches = ":matches(" _ ss:selectors _ ")" { return { type: 'matches', selectors: ss }; }
has = ":has(" _ ss:selectors _ ")" { return { type: 'has', selectors: ss }; }
has = ":has(" _ ss:relativeSelectors _ ")" { return { type: 'has', selectors: ss }; }

firstChild = ":first-child" { return nth(1); }
lastChild = ":last-child" { return nthLast(1); }
Expand All @@ -99,3 +114,6 @@ nthLastChild = ":nth-last-child(" _ n:[0-9]+ _ ")" { return nthLast(parseInt(n.j
class = ":" c:("statement"i / "expression"i / "declaration"i / "function"i / "pattern"i) {
return { type: 'class', name: c };
}

root = ":root" { return { type: 'root' }; }
scope = ":scope" { return { type: 'scope' }; }
Loading