Permalink
Browse files

Add some protection to metavars

This adds lots of error when attempting to match a metavar that would lead to
ambiguity.
  • Loading branch information...
Ryan Patterson
Ryan Patterson committed Jan 10, 2012
1 parent 3b611fd commit 8bb3d606067583a4087679ac33472ed266f49b09
Showing with 94 additions and 46 deletions.
  1. +2 −0 README.mdown
  2. +92 −46 lib/jsgrep.js
View
@@ -46,8 +46,10 @@ the `-p` flag to print only a particular matched variable.
## TODO
+* Consider a metavar that matches but doesn't save, to prevent ambiguities.
* Consider support for --where/--eval and/or not-patterns
* Support for most statements in patterns
+* Testing
## Contributors
View
@@ -43,10 +43,16 @@ var jsgrep = exports.jsgrep = function(options) {
});
}
-function astMatchEllipsis(nodes, patterns, config) {
- // XXX(rpatterson): this needs testing!
+/**
+ * XXX(rpatterson): this needs testing!
+ */
+function astMatchEllipsis(nodes, patterns, config, matcher) {
const tokens = Narcissus.definitions.tokenIds;
- var permitVar = true, clonedConfig = null;
+
+ if (!matcher) {
+ matcher = astIsEqual;
+ }
+
function go(i, j) {
if (i == nodes.length && j == patterns.length) {
return true;
@@ -65,28 +71,20 @@ function astMatchEllipsis(nodes, patterns, config) {
}
if (patterns[j].type == tokens.ELLIPSIS) {
- permitVar = false;
- clonedConfig = _.clone(config);
- return go(i, j + 1) || go(i + 1, j);
+ // ..., A, ... is ambiguous, and if A was matched later in the pattern, we
+ // would have to backtrack to the ellipsis to try a different match.
+ // That's annoying.
+ // XXX(rpatterson): This incorrectly bails for (..., A)
+ config.failOnMetavar = true;
+ var ret = go(i, j + 1) || go(i + 1, j);
+ config.failOnMetavar = false;
+ return ret;
}
- return astIsEqual(nodes[i], patterns[j],
- permitVar ? config : clonedConfig) &&
+ return matcher(nodes[i], patterns[j], config) &&
go(i + 1, j + 1);
}
var result = go(0, 0);
- if (!permitVar &&
- _.keys(clonedConfig).length != _.keys(config.variables).length) {
- // ..., A, ... is ambiguous, and if A was matched later in the pattern, we
- // would have to backtrack to the ellipsis to try a different match. That's
- // annoying.
- //
- // XXX(rpatterson): This incorrectly bails for (..., A)
- throw {
- message: "Matching metavariables inside partially-matched lists is " +
- "unsupported. Sorry!"
- };
- }
return result;
}
@@ -99,6 +97,12 @@ var astIsEqual = exports.astIsEqual = function(node, pattern, config) {
// Variable already matched, compare this node to that value
return astIsEqual(node, config.variables[pattern.value]);
} else {
+ if (config.failOnMetavar) {
+ throw {
+ message: "Matching metavariables inside partially-matched lists is " +
+ "unsupported. Sorry!"
+ };
+ }
// Bind variable to this value
config.variables[pattern.value] = node;
return true;
@@ -118,7 +122,10 @@ var astIsEqual = exports.astIsEqual = function(node, pattern, config) {
var keys = _.clone(pattern.children);
for (var i = 0; i < node.children.length; i++) {
for (var j = 0; j < keys.length;) {
- if (astIsEqual(node.children[i], keys[j], config)) {
+ config.failOnMetavar = true;
+ var match = astIsEqual(node.children[i], keys[j], config);
+ config.failOnMetavar = false;
+ if (match) {
keys.splice(j, 1);
break;
} else {
@@ -201,7 +208,6 @@ var astIsEqual = exports.astIsEqual = function(node, pattern, config) {
case tokens.INDEX:
// Special
case tokens.OBJECT_INIT:
- case tokens.PROPERTY_INIT:
//case tokens.SCRIPT:
if (node.children.length == pattern.children.length) {
for (var i = 0; i < node.children.length; i++) {
@@ -213,6 +219,22 @@ var astIsEqual = exports.astIsEqual = function(node, pattern, config) {
}
break;
+ case tokens.PROPERTY_INIT:
+ if (node.children.length != 2 || pattern.children.length != 2) {
+ throw {
+ message: "astIsEqual: Unsupported PROPERTY_INIT"
+ };
+ }
+ // In strict mode, the ordering will prevent an ambiguous match.
+ config.failOnMetavar = !config.strictMatches;
+ var match = astIsEqual(node.children[0], pattern.children[0], config);
+ config.failOnMetavar = false;
+ if (!match) {
+ return false;
+ }
+ return astIsEqual(node.children[1], pattern.children[1], config);
+ break;
+
case tokens.ARRAY_INIT:
case tokens.LIST:
return astMatchEllipsis(node.children, pattern.children, config);
@@ -222,42 +244,66 @@ var astIsEqual = exports.astIsEqual = function(node, pattern, config) {
case tokens.VAR:
// All of var's children are IDENTIFIERs with name/initializer values
// TODO: this does not support destructuring assignments
- if (pattern.children.length > node.children.length) {
- return false;
- }
+ // XXX: Note that "A = B" as a pattern will *not* match "var A = B"
+ if (config.strictMatches) {
+ // TODO: this is using astMatchEllipsis even though the parser won't
+ // accept an ellipsis in a var.
+ return astMatchEllipsis(node.children, pattern.children, config,
+ function(node, pattern, config) {
+ if (!astIsEqual(node, pattern, config)) {
+ return false;
+ }
- var keys = _.clone(pattern.children);
- for (var i = 0; i < node.children.length; i++) {
- for (var j = 0; j < keys.length;) {
- if (astIsEqual(node.children[i], keys[j], config)) {
- // If the pattern has an initializer, it must be equal to the one
- // in the source.
- if (keys[j].initializer &&
- (!node.children[i].initializer ||
- !astIsEqual(node.children[i].initializer,
- keys[j].initializer, config))) {
+ // If the pattern has an initializer, it must be equal to the one in
+ // the source.
+ if (pattern.initializer && (!node.initializer ||
+ !astIsEqual(node.initializer, pattern.initializer, config))) {
return false;
}
// If in strict mode and the pattern has no initializer, neither can
// the source.
- if (!keys[j].initializer && config.strictMatches &&
- node.children[i].initializer) {
+ if (!pattern.initializer && config.strictMatches &&
+ node.initializer) {
return false;
}
- keys.splice(j, 1);
+
+ return true;
+ });
+ } else {
+ if (pattern.children.length > node.children.length) {
+ return false;
+ }
+
+ var keys = _.clone(pattern.children);
+ for (var i = 0; i < node.children.length; i++) {
+ for (var j = 0; j < keys.length;) {
+ config.failOnMetavar = true;
+ var match = astIsEqual(node.children[i], keys[j], config);
+ config.failOnMetavar = false;
+ if (match) {
+ // If the pattern has an initializer, it must be equal to the one
+ // in the source.
+ if (keys[j].initializer &&
+ (!node.children[i].initializer ||
+ !astIsEqual(node.children[i].initializer,
+ keys[j].initializer, config))) {
+ return false;
+ }
+ keys.splice(j, 1);
+ break;
+ } else {
+ j++;
+ }
+ }
+
+ if (keys.length == 0) {
break;
- } else {
- j++;
}
}
- if (keys.length == 0) {
- break;
- }
+ // No keys left over -> match.
+ return keys.length == 0;
}
-
- // No keys left over -> match.
- return keys.length == 0;
break;
case tokens.SEMICOLON:

0 comments on commit 8bb3d60

Please sign in to comment.