Web Inspector: Should be able to pretty print module code (import / export statements)
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 13 Sep 2016 18:37:12 +0000 (18:37 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 13 Sep 2016 18:37:12 +0000 (18:37 +0000)
https://bugs.webkit.org/show_bug.cgi?id=161891
<rdar://problem/28272784>

Patch by Joseph Pecoraro <pecoraro@apple.com> on 2016-09-13
Reviewed by Yusuke Suzuki.

Source/WebInspectorUI:

* Tools/Formatting/EsprimaFormatterDebug.js:
* Tools/Formatting/index.html:
Update the formatting tool to toggle between source type modes.

* UserInterface/Proxies/FormatterWorkerProxy.js:
* UserInterface/Workers/Formatter/FormatterWorker.js:
(FormatterWorker.prototype.formatJavaScript):
Provide a flag to parse the input as a module instead of a script/program.

* UserInterface/Views/TextEditor.js:
(WebInspector.TextEditor.prototype._startWorkerPrettyPrint):
Using the formatter here, we may have module scripts in the future.

* UserInterface/Views/SourceCodeTextEditor.js:
(WebInspector.SourceCodeTextEditor.prototype._showPopoverForFunction.didGetDetails):
Using the formatter for a function is not module source.

* UserInterface/Workers/Formatter/ESTreeWalker.js:
(ESTreeWalker.prototype._walkChildren):
Visit children of Export/Import nodes.

* UserInterface/Workers/Formatter/EsprimaFormatter.js:
(EsprimaFormatter.prototype._handleTokenAtNode):
Output tokens with appropriate whitespace.

LayoutTests:

* inspector/formatting/formatting-javascript-expected.txt:
* inspector/formatting/formatting-javascript.html:
* inspector/formatting/resources/javascript-tests/modules-expected.js: Added.
* inspector/formatting/resources/javascript-tests/modules.js: Added.
Include a new test for modules.

* inspector/formatting/formatting-json.html:
All of these are non-module source code.

* inspector/formatting/resources/utilities.js:
Determine if module or not based on the test name.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@205866 268f45cc-cd09-0410-ab3c-d52691b4dbfc

16 files changed:
LayoutTests/ChangeLog
LayoutTests/inspector/formatting/formatting-javascript-expected.txt
LayoutTests/inspector/formatting/formatting-javascript.html
LayoutTests/inspector/formatting/formatting-json.html
LayoutTests/inspector/formatting/resources/javascript-tests/modules-expected.js [new file with mode: 0644]
LayoutTests/inspector/formatting/resources/javascript-tests/modules.js [new file with mode: 0644]
LayoutTests/inspector/formatting/resources/utilities.js
Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/Tools/Formatting/EsprimaFormatterDebug.js
Source/WebInspectorUI/Tools/Formatting/index.html
Source/WebInspectorUI/UserInterface/Proxies/FormatterWorkerProxy.js
Source/WebInspectorUI/UserInterface/Views/SourceCodeTextEditor.js
Source/WebInspectorUI/UserInterface/Views/TextEditor.js
Source/WebInspectorUI/UserInterface/Workers/Formatter/ESTreeWalker.js
Source/WebInspectorUI/UserInterface/Workers/Formatter/EsprimaFormatter.js
Source/WebInspectorUI/UserInterface/Workers/Formatter/FormatterWorker.js

index 7c66cf6..304bd67 100644 (file)
@@ -1,3 +1,23 @@
+2016-09-13  Joseph Pecoraro  <pecoraro@apple.com>
+
+        Web Inspector: Should be able to pretty print module code (import / export statements)
+        https://bugs.webkit.org/show_bug.cgi?id=161891
+        <rdar://problem/28272784>
+
+        Reviewed by Yusuke Suzuki.
+
+        * inspector/formatting/formatting-javascript-expected.txt:
+        * inspector/formatting/formatting-javascript.html:
+        * inspector/formatting/resources/javascript-tests/modules-expected.js: Added.
+        * inspector/formatting/resources/javascript-tests/modules.js: Added.
+        Include a new test for modules.
+
+        * inspector/formatting/formatting-json.html:
+        All of these are non-module source code.
+
+        * inspector/formatting/resources/utilities.js:
+        Determine if module or not based on the test name.
+
 2016-09-13  Ryan Haddad  <ryanhaddad@apple.com>
 
         Marking http/tests/security/cross-origin-cached-scripts-parallel.html as flaky.
index 4b6d5e7..017cc56 100644 (file)
@@ -80,3 +80,6 @@ PASS
 -- Running test case: EsprimaFormatter.JavaScript.with-statement.js
 PASS
 
+-- Running test case: EsprimaFormatter.JavaScript.modules.js
+PASS
+
index f06f75d..33e12bb 100644 (file)
@@ -35,6 +35,9 @@ function test()
         "resources/javascript-tests/variable-declaration.js",
         "resources/javascript-tests/while-statement.js",
         "resources/javascript-tests/with-statement.js",
+
+        // Modules
+        "resources/javascript-tests/modules.js",
     ]);
 
     suite.runTestCasesAndFinish();
index ded5ec2..3745c67 100644 (file)
@@ -34,7 +34,8 @@ function test()
 
     let suite = InspectorTest.createAsyncSuite("EsprimaFormatter.JSON");
 
-    let indentString = "    ";
+    const isModule = false;
+    const indentString = "    ";
     let workerProxy = WebInspector.FormatterWorkerProxy.singleton();
 
     suite.addTestCase({
@@ -44,7 +45,7 @@ function test()
             let validJSON = JSON.stringify({"a":123,"b":[1,2,3],"c":{"d":"e"}});
             InspectorTest.log("JSON: " + doubleQuotedString(validJSON));
             ensureJSON(validJSON, "valid");
-            workerProxy.formatJavaScript(validJSON, indentString, ({formattedText, sourceMapData}) => {
+            workerProxy.formatJavaScript(validJSON, isModule, indentString, ({formattedText, sourceMapData}) => {
                 InspectorTest.log("FORMATTED:");
                 InspectorTest.log(formattedText);
                 resolve();
@@ -60,7 +61,7 @@ function test()
             InspectorTest.log("JSON: " + doubleQuotedString(invalidJSON));
             ensureJSON(invalidJSON, "invalid");
             ensureEval(invalidJSON, "object");
-            workerProxy.formatJavaScript(invalidJSON, indentString, ({formattedText, sourceMapData}) => {
+            workerProxy.formatJavaScript(invalidJSON, isModule, indentString, ({formattedText, sourceMapData}) => {
                 InspectorTest.log("FORMATTED:");
                 InspectorTest.log(formattedText);
                 resolve();
@@ -76,7 +77,7 @@ function test()
             InspectorTest.log("JSON: " + doubleQuotedString(invalidJSON));
             ensureJSON(invalidJSON, "invalid");
             ensureEval(invalidJSON, "object");
-            workerProxy.formatJavaScript(invalidJSON, indentString, ({formattedText, sourceMapData}) => {
+            workerProxy.formatJavaScript(invalidJSON, isModule, indentString, ({formattedText, sourceMapData}) => {
                 InspectorTest.log("FORMATTED:");
                 InspectorTest.log(formattedText);
                 resolve();
@@ -92,7 +93,7 @@ function test()
             InspectorTest.log("INPUT: " + doubleQuotedString(invalid));
             ensureJSON(invalid, "invalid");
             ensureEval(invalid, "bad");
-            workerProxy.formatJavaScript(invalid, indentString, ({formattedText, sourceMapData}) => {
+            workerProxy.formatJavaScript(invalid, isModule, indentString, ({formattedText, sourceMapData}) => {
                 InspectorTest.expectThat(formattedText === null, "Response should be null.");
                 resolve();
             });
diff --git a/LayoutTests/inspector/formatting/resources/javascript-tests/modules-expected.js b/LayoutTests/inspector/formatting/resources/javascript-tests/modules-expected.js
new file mode 100644 (file)
index 0000000..87a22dc
--- /dev/null
@@ -0,0 +1,40 @@
+import "module";
+import x from "module";
+import { x } from "module";
+import { a, b, c } from "module";
+import { a, b, c,  } from "module";
+import { a as A } from "module";
+import { a, a as A, a } from "module";
+import { a as A, a as A } from "module";
+import a, { a } from "module";
+import a, { a as A } from "module";
+import x, * as A from "module";
+import * as A from "module";
+
+export { x };
+export let x;
+export var x;
+export const x = 1;
+export let x = 1,
+    y = 1;
+export let {x, y, z} = o;
+export let [x, y, z] = a;
+export { x as X };
+export { x as X, a as A };
+export * from "module";
+export { x } from "module";
+export { x, y } from "module";
+export { x as A } from "module";
+export { x as default };
+export default x;
+export default 1;
+export default {};
+export default (1);
+export default [1];
+export default /r/;
+export default function() {}
+export default function f() {}
+export default function* () {}
+export default function* f() {}
+export default class {}
+export default class X {}
diff --git a/LayoutTests/inspector/formatting/resources/javascript-tests/modules.js b/LayoutTests/inspector/formatting/resources/javascript-tests/modules.js
new file mode 100644 (file)
index 0000000..936e355
--- /dev/null
@@ -0,0 +1,39 @@
+import"module";
+import x from"module";
+import{x}from"module";
+import{a,b,c}from"module";
+import{a,b,c,}from"module";
+import{a as A}from"module";
+import{a,a as A,a}from"module";
+import{a as A,a as A}from"module";
+import a,{a}from"module";
+import a,{a as A}from"module";
+import x,*as A from"module";
+import*as A from"module";
+
+export{x};
+export let x;
+export var x;
+export const x=1;
+export let x=1,y=1;
+export let{x,y,z}=o;
+export let[x,y,z]=a;
+export{x as X};
+export{x as X,a as A};
+export*from"module";
+export{x}from"module";
+export{x,y}from"module";
+export{x as A}from"module";
+export{x as default};
+export default x;
+export default 1;
+export default{};
+export default(1);
+export default[1];
+export default/r/;
+export default function(){}
+export default function f(){}
+export default function*(){}
+export default function*f(){}
+export default class{}
+export default class X{}
index b987047..b164e9d 100644 (file)
@@ -15,7 +15,8 @@ TestPage.registerInitializer(function() {
             return new Promise(function(resolve, reject) {
                 const indentString = "    ";
                 let workerProxy = WebInspector.FormatterWorkerProxy.singleton();
-                workerProxy.formatJavaScript(testText, indentString, ({formattedText, sourceMapData}) => {
+                let isModule = /^module/.test(testName);
+                workerProxy.formatJavaScript(testText, isModule, indentString, ({formattedText, sourceMapData}) => {
                     let pass = formattedText === expectedText;
                     InspectorTest.log(pass ? "PASS" : "FAIL");
 
index c3d52df..1473110 100644 (file)
@@ -1,3 +1,36 @@
+2016-09-13  Joseph Pecoraro  <pecoraro@apple.com>
+
+        Web Inspector: Should be able to pretty print module code (import / export statements)
+        https://bugs.webkit.org/show_bug.cgi?id=161891
+        <rdar://problem/28272784>
+
+        Reviewed by Yusuke Suzuki.
+
+        * Tools/Formatting/EsprimaFormatterDebug.js:
+        * Tools/Formatting/index.html:
+        Update the formatting tool to toggle between source type modes.
+
+        * UserInterface/Proxies/FormatterWorkerProxy.js:
+        * UserInterface/Workers/Formatter/FormatterWorker.js:
+        (FormatterWorker.prototype.formatJavaScript):
+        Provide a flag to parse the input as a module instead of a script/program.
+
+        * UserInterface/Views/TextEditor.js:
+        (WebInspector.TextEditor.prototype._startWorkerPrettyPrint):
+        Using the formatter here, we may have module scripts in the future.
+
+        * UserInterface/Views/SourceCodeTextEditor.js:
+        (WebInspector.SourceCodeTextEditor.prototype._showPopoverForFunction.didGetDetails):
+        Using the formatter for a function is not module source.
+
+        * UserInterface/Workers/Formatter/ESTreeWalker.js:
+        (ESTreeWalker.prototype._walkChildren):
+        Visit children of Export/Import nodes.
+
+        * UserInterface/Workers/Formatter/EsprimaFormatter.js:
+        (EsprimaFormatter.prototype._handleTokenAtNode):
+        Output tokens with appropriate whitespace.
+
 2016-09-13  Chris Dumez  <cdumez@apple.com>
 
         Drop support for <isindex>
index 90e89a7..6a67408 100644 (file)
@@ -1,8 +1,8 @@
 EsprimaFormatterDebug = class EsprimaFormatterDebug
 {
-    constructor(sourceText)
+    constructor(sourceText, sourceType)
     {
-        let tree = esprima.parse(sourceText, {attachComment: true, range: true, tokens: true});
+        let tree = esprima.parse(sourceText, {attachComment: true, range: true, tokens: true, sourceType});
         let walker = new ESTreeWalker(this._before.bind(this), this._after.bind(this));
 
         this._statistics = {
index 3765af5..f93462e 100644 (file)
     <!-- Controls -->
     <button id="populate">Populate</button>
     <select id="load-individual-test"><option>-- Load Test --</option></select>
+    <small>
+        <label><input id="program" type="radio" name="program-or-module"> Program</label>
+        <label><input id="module" type="radio" name="program-or-module" checked> Module</label>
+    </small>
     <button id="run-tests">Run All Tests</button>
     <button id="clear">Clear</button>
     <button id="select-output">Select Output</button>
@@ -65,7 +69,7 @@
         "sample-webinspector-object.js",
         "sample-normal-utilities.js",
         "sample-jquery.js",
-        // FIXME: Add a modules test when we support testing module source code.
+        "modules.js",
     ];
 
     // Initial values from URL.
@@ -83,6 +87,8 @@
     let content = "(function(){let a=1;return a+1;})();";
     if (queryParams.content)
         content = queryParams.content || "";
+    if (queryParams.program)
+        document.getElementById("program").checked = true;
 
     // Setup CodeMirror.
     let cm = CodeMirror.fromTextArea(document.getElementById("code"), {lineNumbers: true});
         option.value = test;
     }
 
+    // Program / Module checkboxes.
+    let programCheckbox = document.getElementById("program");
+    let moduleCheckbox = document.getElementById("module");
+    programCheckbox.addEventListener("change", refresh);
+    moduleCheckbox.addEventListener("change", refresh);
+
     // Run Tests button.
     document.getElementById("run-tests").addEventListener("click", function(event) {
         cm.setValue("/* Running Tests... */");
     // Save as URL button.
     document.getElementById("save-as-url").addEventListener("click", function(event) {
         let content = cm.getValue();
-        window.location.search = "?content=" + window.encodeURIComponent(content);
+        let queryString = "?content=" + window.encodeURIComponent(content);
+        if (programCheckbox.checked)
+            queryString += "&program";
+        window.location.search = queryString;
     });
 
     // Refresh after changes after a short delay.
     let prettyPre = document.getElementById("pretty");
     let debugPre = document.getElementById("debug");
 
+    // Current value of checkboxes.
+    function currentSourceType() {
+        if (programCheckbox.checked)
+            return EsprimaFormatter.SourceType.Script;
+        if (moduleCheckbox.checked)
+            return EsprimaFormatter.SourceType.Module;
+        console.assert(false, "Program or Module radio button should be checked");
+        return undefined;
+    }
+
     function refresh() {
         if (timer)
             clearTimeout(timer);
 
         // Time the formatter.
         let startTime = Date.now();
-        let formatter = new EsprimaFormatter(cm.getValue());
+        let formatter = new EsprimaFormatter(cm.getValue(), currentSourceType());
         let endTime = Date.now();
 
         // Show debug info.
         let debugText;
         try {
-            let debugFormatter = new EsprimaFormatterDebug(cm.getValue());
+            let debugFormatter = new EsprimaFormatterDebug(cm.getValue(), currentSourceType());
             debugText = debugFormatter.debugText;
         } catch (error) {
             debugText = "Parse error";
 
     // Testing.
 
+    function isModuleTest(test) {
+        return test === "modules.js";
+    }
+
     function loadIndividualTest(test) {
         let testURL = testURLPrefix + test;
         let xhr = new XMLHttpRequest;
         xhr.open("GET", testURL, true);
         xhr.onload = () => { cm.setValue(xhr.responseText); setTimeout(refresh); }
         xhr.send();
+
+        if (isModuleTest(test))
+            moduleCheckbox.checked = true;
+        else
+            programCheckbox.checked = true;
     }
 
     function runAllTests() {
             let expectedData = xhr2.responseText;
 
             // Run the test.
-            let formatter = new EsprimaFormatter(testData);
+            let sourceType = isModuleTest(test) ? EsprimaFormatter.SourceType.Module : EsprimaFormatter.SourceType.Script;
+            let formatter = new EsprimaFormatter(testData, sourceType);
 
             // Compare results.
             let pass = formatter.formattedText === expectedData;
index 8de51d5..96159fb 100644 (file)
@@ -47,7 +47,7 @@ WebInspector.FormatterWorkerProxy = class FormatterWorkerProxy extends WebInspec
 
     // Actions
 
-    formatJavaScript(sourceText, indentString, includeSourceMapData)
+    formatJavaScript(sourceText, isModule, indentString, includeSourceMapData)
     {
         this.performAction("formatJavaScript", ...arguments);
     }
index 699e448..8557275 100644 (file)
@@ -1523,8 +1523,9 @@ WebInspector.SourceCodeTextEditor = class SourceCodeTextEditor extends WebInspec
             });
 
             // FIXME: <rdar://problem/10593948> Provide a way to change the tab width in the Web Inspector
+            const isModule = false;
             let workerProxy = WebInspector.FormatterWorkerProxy.singleton();
-            workerProxy.formatJavaScript(data.description, "    ", false, ({formattedText}) => {
+            workerProxy.formatJavaScript(data.description, isModule, "    ", false, ({formattedText}) => {
                 codeMirror.setValue(formattedText || data.description);
             });
 
index 49c20b4..b82a636 100644 (file)
@@ -792,8 +792,11 @@ WebInspector.TextEditor = class TextEditor extends WebInspector.View
         let sourceText = this._codeMirror.getValue();
         let includeSourceMapData = true;
 
+        // FIXME: Properly pass if this is a module or script.
+        const isModule = false;
+
         let workerProxy = WebInspector.FormatterWorkerProxy.singleton();
-        workerProxy.formatJavaScript(sourceText, indentString, includeSourceMapData, ({formattedText, sourceMapData}) => {
+        workerProxy.formatJavaScript(sourceText, isModule, indentString, includeSourceMapData, ({formattedText, sourceMapData}) => {
             // Handle if formatting failed, which is possible for invalid programs.
             if (formattedText === null) {
                 callback();
index ec70373..bd5dd92 100644 (file)
@@ -240,6 +240,36 @@ ESTreeWalker = class ESTreeWalker
             this._walk(node.argument, node);
             break;
 
+        case "ExportAllDeclaration":
+            this._walk(node.source, node);
+            break;
+        case "ExportNamedDeclaration":
+            this._walk(node.declaration, node);
+            this._walkArray(node.specifiers, node);
+            this._walk(node.source, node);
+            break;
+        case "ExportDefaultDeclaration":
+            this._walk(node.declaration, node);
+            break;
+        case "ExportSpecifier":
+            this._walk(node.local, node);
+            this._walk(node.exported, node);
+            break;
+        case "ImportDeclaration":
+            this._walkArray(node.specifiers, node);
+            this._walk(node.source, node);
+            break;
+        case "ImportDefaultSpecifier":
+            this._walk(node.local, node);
+            break;
+        case "ImportNamespaceSpecifier":
+            this._walk(node.local, node);
+            break;
+        case "ImportSpecifier":
+            this._walk(node.imported, node);
+            this._walk(node.local, node);
+            break;
+
         // Special case. We want to walk in program order,
         // so walk quasi, expression, quasi, expression, etc.
         case "TemplateLiteral":
index d7aad2b..9cdc16f 100644 (file)
 
 EsprimaFormatter = class EsprimaFormatter
 {
-    constructor(sourceText, indentString = "    ")
+    constructor(sourceText, sourceType, indentString = "    ")
     {
         this._success = false;
 
         let tree = (function() {
             try {
-                // FIXME: Support "module" sourceType as well.
-                return esprima.parse(sourceText, {attachComment: true, range: true, tokens: true});
+                return esprima.parse(sourceText, {attachComment: true, range: true, tokens: true, sourceType});
             } catch (error) {
                 return null;
             }
@@ -738,7 +737,8 @@ EsprimaFormatter = class EsprimaFormatter
 
         if (nodeType === "ClassBody") {
             if (tokenValue === "{") {
-                builder.appendSpace();
+                if (node.parent.id)
+                    builder.appendSpace();
                 builder.appendToken(tokenValue, tokenOffset);
                 if (node.body.length)
                     builder.appendNewline();
@@ -778,6 +778,30 @@ EsprimaFormatter = class EsprimaFormatter
             return;
         }
 
+        if (nodeType === "ImportDeclaration" || nodeType === "ExportNamedDeclaration") {
+            if (tokenValue === "}" || (tokenType === "Identifier" && tokenValue === "from"))
+                builder.appendSpace();
+            builder.appendToken(tokenValue, tokenOffset);
+            if (tokenValue !== "}")
+                builder.appendSpace();
+            return;
+        }
+
+        if (nodeType === "ExportSpecifier" || nodeType === "ImportSpecifier") {
+            if (tokenType === "Identifier" && tokenValue === "as")
+                builder.appendSpace();
+            builder.appendToken(tokenValue, tokenOffset);
+            builder.appendSpace();
+            return;            
+        }
+
+        if (nodeType === "ExportAllDeclaration" || nodeType === "ExportDefaultDeclaration" || nodeType === "ImportDefaultSpecifier" || nodeType === "ImportNamespaceSpecifier") {
+            builder.appendToken(tokenValue, tokenOffset);
+            if (tokenValue !== "(" && tokenValue !== ")")
+                builder.appendSpace();
+            return;
+        }
+
         // Include these here so we get only get warnings about unhandled nodes.
         if (nodeType === "ExpressionStatement"
             || nodeType === "SpreadElement"
@@ -876,3 +900,8 @@ EsprimaFormatter = class EsprimaFormatter
         }
     }
 };
+
+EsprimaFormatter.SourceType = {
+    Script: "script",
+    Module: "module",
+};
index 9bc892e..9c056f1 100644 (file)
@@ -40,10 +40,12 @@ FormatterWorker = class FormatterWorker
 
     // Actions
 
-    formatJavaScript(sourceText, indentString, includeSourceMapData)
+    formatJavaScript(sourceText, isModule, indentString, includeSourceMapData)
     {
+        let sourceType = isModule ? EsprimaFormatter.SourceType.Module : EsprimaFormatter.SourceType.Script;
+
         // Format a JavaScript program.
-        let formatter = new EsprimaFormatter(sourceText, indentString);
+        let formatter = new EsprimaFormatter(sourceText, sourceType, indentString);
         if (formatter.success) {
             let result = {formattedText: formatter.formattedText};
             if (includeSourceMapData) {
@@ -74,7 +76,7 @@ FormatterWorker = class FormatterWorker
         // Likewise, an unnamed function's toString() produces a "function() { ... }", which is not
         // a valid program on its own. Wrap it in parenthesis to make it a function expression,
         // which is a valid program.
-        let invalidJSONFormatter = new EsprimaFormatter("(" + sourceText + ")", indentString);
+        let invalidJSONFormatter = new EsprimaFormatter("(" + sourceText + ")", sourceType, indentString);
         if (invalidJSONFormatter.success) {
             let formattedTextWithParens = invalidJSONFormatter.formattedText;
             let result = {formattedText: formattedTextWithParens.substring(1, formattedTextWithParens.length - 2)}; // Remove "(" and ")\n".