RESOLVED FIXED 148563
Web Inspector: Improve ScriptSyntaxTree a bit
https://bugs.webkit.org/show_bug.cgi?id=148563
Summary Web Inspector: Improve ScriptSyntaxTree a bit
Joseph Pecoraro
Reported 2015-08-27 22:50:25 PDT
* 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")
Attachments
[PATCH] Proposed Fix (3.22 KB, patch)
2015-08-27 22:51 PDT, Joseph Pecoraro
no flags
Radar WebKit Bug Importer
Comment 1 2015-08-27 22:50:44 PDT
Joseph Pecoraro
Comment 2 2015-08-27 22:51:40 PDT
Created attachment 260126 [details] [PATCH] Proposed Fix
Joseph Pecoraro
Comment 3 2015-08-27 22:52:33 PDT
=
Timothy Hatcher
Comment 4 2015-08-28 05:14:32 PDT
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?
Timothy Hatcher
Comment 5 2015-08-28 10:30:52 PDT
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*
Joseph Pecoraro
Comment 6 2015-08-28 11:37:41 PDT
(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.
Joseph Pecoraro
Comment 7 2015-08-28 11:43:47 PDT
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".
Saam Barati
Comment 8 2015-08-28 12:05:39 PDT
(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.
WebKit Commit Bot
Comment 9 2015-08-28 12:31:54 PDT
Comment on attachment 260126 [details] [PATCH] Proposed Fix Clearing flags on attachment: 260126 Committed r189118: <http://trac.webkit.org/changeset/189118>
WebKit Commit Bot
Comment 10 2015-08-28 12:31:57 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.