natefaubion / matches.js

Powerful pattern matching for Javascript
MIT License
772 stars 37 forks source link

fails on overriden prototype functions #4

Closed slavah closed 12 years ago

slavah commented 12 years ago

Hey Nate - amazing lib, was looking for something like this for long time. While playing with it found some issues. Cant really call it a bug, i understand its more of a PEG issue. What do you think?

Test Code:

 var matcher = matches.pattern({
                '{record, ...}': function (val) {     //first line always would match
                    return record = 'first';
                },
                "record@{className:'testClass', ...}": function (val, obj) {
                    return record = 'second';
                },
  });
  var testObj = function() {
                this.a = 5;
                this.toString = function() { return 'This would cause issues';  };
            };
   testObj.prototype.className = 'testClass';
   var f = new testObj();

   matcher (f);
   expect(record == 'second').toBeTruthy();    //fails

The issue is in this function in PatternFn:

function(args,runt) { //.... for (var arg_0_i in arg_0) { if (arg_0_i in arg_0_keys) arg_0_count += 1; //its going to match on every toString in prototype chain. } if (arg_0_count !== 1) return false; //..... };

Thanks.

natefaubion commented 12 years ago

Not a PEG issue. I had a naive lookup in the compiled function. It creates a lookup object for keys to watch out for when iterating over the object. I was just using in to check for a key's presence in the lookup object. That was causing your explicit toString to match the prototype toString of the lookup object. Fixed by using hasOwnProperty instead.

slavah commented 12 years ago

Oh i see, i was under impression that peg was generating those functions. Yes, now i see that you are defining pattern in matches and compiling in compiler.js.

Thanks a lot for quickly fixing it.