search

goldibex / targaryen

Test Firebase security rules without connecting to Firebase.
ISC License
242 stars 36 forks source link

Properly handle computed member expressions #114

Closed pthrasher closed 7 years ago

pthrasher commented 7 years ago

Without this change, computed member expressions throw on parse.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 94.269% when pulling 479c4aa4a48cf09ff8d707948f116007c98bf6f0 on pthrasher:bug/v3-member-expressions into f13b3716cd542fdc3b390f2baad2182c6a3bb294 on goldibex:master.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 94.269% when pulling 479c4aa4a48cf09ff8d707948f116007c98bf6f0 on pthrasher:bug/v3-member-expressions into f13b3716cd542fdc3b390f2baad2182c6a3bb294 on goldibex:master.

dinoboff commented 7 years ago

@pthrasher I tried those tests on master and they don't trigger the bug. I don't think you can write a test triggering the bug with an access to auth. root["val"]() == null might trigger that bug, but there's other bug in front of it (Targaryen assume computed property on snapshot is not valid).

dinoboff commented 7 years ago

root["val"]() == null would catch this bug; it's not related to the scope.

Once #115 is fix, we could write a regression test by checking the error message from parser.parse('root[$foo]() == null', ['$foo']). It shouldn't complain about property access not about the scope.

dinoboff commented 7 years ago

I made some adjustment but It didn't let me push to your PR. I create a new PR instead: #119 fixes the tests, flatten the fix and rebased with master.

dinoboff commented 7 years ago

Thanks @pthrasher I will release the fix today.

dinoboff commented 7 years ago

@pthrasher released