Add scope annotator - Githubissues

bakkot commented 7 years ago

I.e., a helper which will insert comments in your original source with scope information.

This is mainly useful in tests, but export it anyway.

Also adds a test helper allowing you to write tests which look like

checkScopeAnnotation(`
  function outer(){
    function f/* declares f#0 */() {}
    {
      function f/* declares f#1 */() {}
      print(f/* reads f#1 */);
    }
    return f/* reads f#0 */;
  }
`);

which are substantially more readable than any of our existing tests.

michaelficarra commented 7 years ago

Will you be updating our current tests to use this new strategy as part of this PR?

bakkot commented 7 years ago

Will you be updating our current tests to use this new strategy as part of this PR?

I hadn't really planned on it, but I can if you think it's worth it.

michaelficarra commented 7 years ago

There's no tests with annotated scopes, just annotated identifiers. We'll need to at least exercise the scope annotation.

Also, yes, I would like to see the current tests converted to use annotations to prove that annotated tests are sufficient to replace the current tests.

bakkot commented 7 years ago

The intention was not that this would be the only form of test, just that there would be some tests for which it would be sufficient. It is necessarily less complete than the full serialization tests, simply because including all of the information from the serialization would make the annotated script unreadable.

bakkot commented 7 years ago

OK, converted some of the existing tests, including some which stress Scope annotations (not just Reference).

Also un-flattened the scope serialization tests. It's a lot more space, but also a lot easier to read.

bakkot commented 7 years ago

Yes, the position information is off. See https://github.com/shapesecurity/shift-parser-js/issues/363.

On Fri, Sep 15, 2017 at 8:12 PM, Michael Ficarra notifications@github.com wrote:

@michaelficarra commented on this pull request.

In test/unit.js https://github.com/shapesecurity/shift-scope-js/pull/43#discussion_r139276886 :

});

test('destructuring', () => {

checkScopeSerialization(

'var {x, a:{b:y = z}} = null; var [z] = y;',

'{"node": "Script_0", "type": "Global", "isDynamic": true, "through": [], "variables": [{"name": "x", "references": [{"node": "BindingIdentifier(x)_6", "accessibility": "Write"}], "declarations": [{"node": "BindingIdentifier(x)_6", "kind": "Var"}]}, {"name": "y", "references": [{"node": "BindingIdentifier(y)_13", "accessibility": "Write"}, {"node": "IdentifierExpression(y)_21", "accessibility": "Read"}], "declarations": [{"node": "BindingIdentifier(y)_13", "kind": "Var"}]}, {"name": "z", "references": [{"node": "IdentifierExpression(z)_14", "accessibility": "Read"}, {"node": "BindingIdentifier(z)_20", "accessibility": "Write"}], "declarations": [{"node": "BindingIdentifier(z)_20", "kind": "Var"}]}], "children": [{"node": "Script_0", "type": "Script", "isDynamic": false, "through": [{"node": "IdentifierExpression(z)_14", "accessibility": "Read"}, {"node": "IdentifierExpression(y)_21", "accessibility": "Read"}, {"node": "BindingIdentifier(x)_6", "accessibility": "Write"}, {"node": "BindingIdentifier(y)_13", "accessibility": "Write"}, {"node": "BindingIdentifier(z)_20", "accessibility": "Write"}], "variables": [], "children": []}]}'

);

checkScopeAnnotation(`

var {x/ declares x#0; writes x#0 /, a:{b:y / declares y#0; writes y#0 /= z/ reads z#0 /}} = null;

Never mind, I see the =.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/shapesecurity/shift-scope-js/pull/43#discussion_r139276886, or mute the thread https://github.com/notifications/unsubscribe-auth/ABk7Xv9dfMpWvzhWrShiL83FvbTiWKjMks5sizyEgaJpZM4PYHgg .

shapesecurity / shift-scope-js

Add scope annotator #43

@michaelficarra commented on this pull request.