148563 2015-08-27 22:50:25 -0700 Web Inspector: Improve ScriptSyntaxTree a bit 2015-08-28 12:31:57 -0700 1 1 1 Unclassified WebKit Web Inspector WebKit Nightly Build All All RESOLVED FIXED InRadar P2 Normal --- 1 joepeck joepeck bburg commit-queue graouts joepeck mattbaker nvasilyev saam timothy webkit-bug-importer oldest_to_newest 1121599 0 joepeck 2015-08-27 22:50:25 -0700 * SUMMARY Improve ScriptSyntaxTree a bit. - Share and fix ClassDeclaration and ClassExpression recursion to visit the identifier (node.id) - Include the kind with a VariableDeclaration ("var", "let", "const") 1121600 1 webkit-bug-importer 2015-08-27 22:50:44 -0700 <rdar://problem/22470522> 1121601 2 260126 joepeck 2015-08-27 22:51:40 -0700 Created attachment 260126 [PATCH] Proposed Fix 1121602 3 joepeck 2015-08-27 22:52:33 -0700 = 1121664 4 260126 timothy 2015-08-28 05:14:32 -0700 Comment on attachment 260126 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=260126&action=review > Source/WebInspectorUI/UserInterface/Models/ScriptSyntaxTree.js:881 > + kind: node.kind Shoulder map this to an enum? 1121733 5 260126 timothy 2015-08-28 10:30:52 -0700 Comment on attachment 260126 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=260126&action=review >> Source/WebInspectorUI/UserInterface/Models/ScriptSyntaxTree.js:881 >> + kind: node.kind > > Shoulder map this to an enum? Should we* 1121771 6 joepeck 2015-08-28 11:37:41 -0700 (In reply to comment #5) > Comment on attachment 260126 [details] > [PATCH] Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=260126&action=review > > >> Source/WebInspectorUI/UserInterface/Models/ScriptSyntaxTree.js:881 > >> + kind: node.kind > > > > Shoulder map this to an enum? > > Should we* Currently we just carry over all the other basic properties from esprima right now. Ultimately we would want our own enum types here (and in all other nodes) if we really want ScriptSyntaxTree to abstract any parser. FWIW, there has been discussion of standardizing JS ASTs and their types in ESTree: https://github.com/estree/estree https://github.com/estree/estree/blob/master/es6.md Esprima _very_ closely matches that. I did notice a few minor differences that can be sorted out later (what is a MetaProperty?). But ultimately if JS AST generators produce ESTree ASTs swapping out any parser would be trivial. 1121775 7 joepeck 2015-08-28 11:43:47 -0700 Also interesting JavaScript "CSTs": https://github.com/mdevils/cst > CST means Concrete Syntax Tree. Unlike an AST (Abstract Syntax Tree), > a CST contains all the information from the JavaScript source file: > whitespace, punctuators, comments. This information is extremely > useful for code style checkers and other code linters. CST is also > useful for cases when you need to apply modifications to existing > JavaScript files while preserving the initial file formatting. > > This CST implementation is designed to be 100% compatible with JS AST > (https://github.com/estree/estree). I think an issue I ran into just yesterday could have benefitted from original source information instead of just token information. Likewise, our PrettyPrinter could be written to use this. Using ASTs we lacked whitespace information that was useful to keep "as authored". 1121785 8 saam 2015-08-28 12:05:39 -0700 (In reply to comment #5) > Comment on attachment 260126 [details] > [PATCH] Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=260126&action=review > > >> Source/WebInspectorUI/UserInterface/Models/ScriptSyntaxTree.js:881 > >> + kind: node.kind > > > > Shoulder map this to an enum? > > Should we* I think we should. We could follow what we do for node types here and just make everything a Symbol. 1121790 9 260126 commit-queue 2015-08-28 12:31:54 -0700 Comment on attachment 260126 [PATCH] Proposed Fix Clearing flags on attachment: 260126 Committed r189118: <http://trac.webkit.org/changeset/189118> 1121791 10 commit-queue 2015-08-28 12:31:57 -0700 All reviewed patches have been landed. Closing bug. 260126 2015-08-27 22:51:40 -0700 2015-08-28 12:31:54 -0700 [PATCH] Proposed Fix improves.patch text/plain 3295 joepeck ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cgYi9Tb3VyY2UvV2Vi SW5zcGVjdG9yVUkvQ2hhbmdlTG9nCmluZGV4IGRlN2JiYzguLjRiNmVmNDMgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkluc3BlY3Rv clVJL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDE5IEBACiAyMDE1LTA4LTI3ICBKb3NlcGggUGVjb3Jh cm8gIDxwZWNvcmFyb0BhcHBsZS5jb20+CiAKKyAgICAgICAgV2ViIEluc3BlY3RvcjogSW1wcm92 ZSBTY3JpcHRTeW50YXhUcmVlIGEgYml0CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD0xNDg1NjMKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9P UFMhKS4KKworICAgICAgICAqIFVzZXJJbnRlcmZhY2UvTW9kZWxzL1NjcmlwdFN5bnRheFRyZWUu anM6CisgICAgICAgIChXZWJJbnNwZWN0b3IuU2NyaXB0U3ludGF4VHJlZS5wcm90b3R5cGUuX3Jl Y3Vyc2UpOgorICAgICAgICAoV2ViSW5zcGVjdG9yLlNjcmlwdFN5bnRheFRyZWUucHJvdG90eXBl Ll9jcmVhdGVJbnRlcm5hbFN5bnRheFRyZWUpOgorICAgICAgICBTaGFyZSBhbmQgZml4IENsYXNz RGVjbGFyYXRpb24gYW5kIENsYXNzRXhwcmVzc2lvbiByZWN1cnNpb24gdG8gdmlzaXQKKyAgICAg ICAgdGhlIGlkZW50aWZpZXIgKG5vZGUuaWQpLiBJbmNsdWRlIHRoZSBraW5kIHdpdGggYSBWYXJp YWJsZURlY2xhcmF0aW9uCisgICAgICAgICgidmFyIiwgImxldCIsICJjb25zdCIpLgorCisyMDE1 LTA4LTI3ICBKb3NlcGggUGVjb3Jhcm8gIDxwZWNvcmFyb0BhcHBsZS5jb20+CisKICAgICAgICAg V2ViIEluc3BlY3RvcjogVHlwZSBQcm9maWxlciBkb2VzIG5vdCB1bmRlcnN0YW5kIEZ1bmN0aW9u cyB3aXRoaW4gRGVmYXVsdCBBcmd1bWVudCBFeHByZXNzaW9ucwogICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTQ4NTU3CiAKZGlmZiAtLWdpdCBhL1NvdXJj ZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL01vZGVscy9TY3JpcHRTeW50YXhUcmVlLmpz IGIvU291cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvTW9kZWxzL1NjcmlwdFN5bnRh eFRyZWUuanMKaW5kZXggNjFkM2E4Zi4uOWI4ZGRkMyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYklu c3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvTW9kZWxzL1NjcmlwdFN5bnRheFRyZWUuanMKKysrIGIv U291cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvTW9kZWxzL1NjcmlwdFN5bnRheFRy ZWUuanMKQEAgLTMyNiwxMiArMzI2LDkgQEAgV2ViSW5zcGVjdG9yLlNjcmlwdFN5bnRheFRyZWUg PSBjbGFzcyBTY3JpcHRTeW50YXhUcmVlIGV4dGVuZHMgV2ViSW5zcGVjdG9yLk9iamUKICAgICAg ICAgICAgIHRoaXMuX3JlY3Vyc2VBcnJheShub2RlLmJvZHksIGNhbGxiYWNrLCBzdGF0ZSk7CiAg ICAgICAgICAgICBicmVhazsKICAgICAgICAgY2FzZSBXZWJJbnNwZWN0b3IuU2NyaXB0U3ludGF4 VHJlZS5Ob2RlVHlwZS5DbGFzc0RlY2xhcmF0aW9uOgotICAgICAgICAgICAgY2FsbGJhY2sobm9k ZSwgc3RhdGUpOwotICAgICAgICAgICAgdGhpcy5fcmVjdXJzZShub2RlLnN1cGVyQ2xhc3MsIGNh bGxiYWNrLCBzdGF0ZSk7Ci0gICAgICAgICAgICB0aGlzLl9yZWN1cnNlKG5vZGUuYm9keSwgY2Fs bGJhY2ssIHN0YXRlKTsKLSAgICAgICAgICAgIGJyZWFrOwogICAgICAgICBjYXNlIFdlYkluc3Bl Y3Rvci5TY3JpcHRTeW50YXhUcmVlLk5vZGVUeXBlLkNsYXNzRXhwcmVzc2lvbjoKICAgICAgICAg ICAgIGNhbGxiYWNrKG5vZGUsIHN0YXRlKTsKKyAgICAgICAgICAgIHRoaXMuX3JlY3Vyc2Uobm9k ZS5pZCwgY2FsbGJhY2ssIHN0YXRlKTsKICAgICAgICAgICAgIHRoaXMuX3JlY3Vyc2Uobm9kZS5z dXBlckNsYXNzLCBjYWxsYmFjaywgc3RhdGUpOwogICAgICAgICAgICAgdGhpcy5fcmVjdXJzZShu b2RlLmJvZHksIGNhbGxiYWNrLCBzdGF0ZSk7CiAgICAgICAgICAgICBicmVhazsKQEAgLTM2OSwx MiArMzY2LDYgQEAgV2ViSW5zcGVjdG9yLlNjcmlwdFN5bnRheFRyZWUgPSBjbGFzcyBTY3JpcHRT eW50YXhUcmVlIGV4dGVuZHMgV2ViSW5zcGVjdG9yLk9iamUKICAgICAgICAgICAgIHRoaXMuX3Jl Y3Vyc2Uobm9kZS5ib2R5LCBjYWxsYmFjaywgc3RhdGUpOwogICAgICAgICAgICAgYnJlYWs7CiAg ICAgICAgIGNhc2UgV2ViSW5zcGVjdG9yLlNjcmlwdFN5bnRheFRyZWUuTm9kZVR5cGUuRnVuY3Rp b25EZWNsYXJhdGlvbjoKLSAgICAgICAgICAgIGNhbGxiYWNrKG5vZGUsIHN0YXRlKTsKLSAgICAg ICAgICAgIHRoaXMuX3JlY3Vyc2Uobm9kZS5pZCwgY2FsbGJhY2ssIHN0YXRlKTsKLSAgICAgICAg ICAgIHRoaXMuX3JlY3Vyc2VBcnJheShub2RlLnBhcmFtcywgY2FsbGJhY2ssIHN0YXRlKTsKLSAg ICAgICAgICAgIHRoaXMuX3JlY3Vyc2VBcnJheShub2RlLmRlZmF1bHRzLCBjYWxsYmFjaywgc3Rh dGUpOwotICAgICAgICAgICAgdGhpcy5fcmVjdXJzZShub2RlLmJvZHksIGNhbGxiYWNrLCBzdGF0 ZSk7Ci0gICAgICAgICAgICBicmVhazsKICAgICAgICAgY2FzZSBXZWJJbnNwZWN0b3IuU2NyaXB0 U3ludGF4VHJlZS5Ob2RlVHlwZS5GdW5jdGlvbkV4cHJlc3Npb246CiAgICAgICAgICAgICBjYWxs YmFjayhub2RlLCBzdGF0ZSk7CiAgICAgICAgICAgICB0aGlzLl9yZWN1cnNlKG5vZGUuaWQsIGNh bGxiYWNrLCBzdGF0ZSk7CkBAIC04ODYsNyArODc3LDggQEAgV2ViSW5zcGVjdG9yLlNjcmlwdFN5 bnRheFRyZWUgPSBjbGFzcyBTY3JpcHRTeW50YXhUcmVlIGV4dGVuZHMgV2ViSW5zcGVjdG9yLk9i amUKICAgICAgICAgY2FzZSAiVmFyaWFibGVEZWNsYXJhdGlvbiI6CiAgICAgICAgICAgICByZXN1 bHQgPSB7CiAgICAgICAgICAgICAgICAgdHlwZTogV2ViSW5zcGVjdG9yLlNjcmlwdFN5bnRheFRy ZWUuTm9kZVR5cGUuVmFyaWFibGVEZWNsYXJhdGlvbiwKLSAgICAgICAgICAgICAgICBkZWNsYXJh dGlvbnM6IG5vZGUuZGVjbGFyYXRpb25zLm1hcCh0aGlzLl9jcmVhdGVJbnRlcm5hbFN5bnRheFRy ZWUuYmluZCh0aGlzKSkKKyAgICAgICAgICAgICAgICBkZWNsYXJhdGlvbnM6IG5vZGUuZGVjbGFy YXRpb25zLm1hcCh0aGlzLl9jcmVhdGVJbnRlcm5hbFN5bnRheFRyZWUuYmluZCh0aGlzKSksCisg ICAgICAgICAgICAgICAga2luZDogbm9kZS5raW5kCiAgICAgICAgICAgICB9OwogICAgICAgICAg ICAgYnJlYWs7CiAgICAgICAgIGNhc2UgIlZhcmlhYmxlRGVjbGFyYXRvciI6Cg==