Web Inspector: Type token positioning should use line/column locations from Esprima...
authormattbaker@apple.com <mattbaker@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 13 Jul 2018 02:06:48 +0000 (02:06 +0000)
committermattbaker@apple.com <mattbaker@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 13 Jul 2018 02:06:48 +0000 (02:06 +0000)
https://bugs.webkit.org/show_bug.cgi?id=187612
<rdar://problem/42131910>

Reviewed by Joseph Pecoraro.

* UserInterface/Controllers/TypeTokenAnnotator.js:
(WI.TypeTokenAnnotator.prototype.insertAnnotations):
(WI.TypeTokenAnnotator.prototype._insertTypeToken):
(WI.TypeTokenAnnotator.prototype._insertToken):
Use line/column locations, instead of offsets, from the AST when calculating
token positions for CodeMirror. Once in CodeMirror's string space, we
can safely convert to/from offsets.

* UserInterface/Models/ScriptSyntaxTree.js:
Retrieve line/column locations for AST nodes, in addition to offsets.
Offsets into the original file are still needed for getting type information
from the profiler in the backend.

(WI.ScriptSyntaxTree):
(WI.ScriptSyntaxTree.prototype.filterByRange):
Filter by positions, which can be safely used from CodeMirror, instead of offsets.

(WI.ScriptSyntaxTree.prototype._createInternalSyntaxTree):
(WI.ScriptSyntaxTree.prototype.filterByRange.filterForNodesInRange): Deleted.

* UserInterface/Models/SourceCodePosition.js:
Add convenience methods for comparing line/column positions, and for
converting to the format expected by CodeMirror. SourceCodePosition could
be made to interoperate with CodeMirror by exposing properties `line`
and `ch`, but making the conversion explicit improves code readability.

(WI.SourceCodePosition.prototype.equals):
(WI.SourceCodePosition.prototype.isBefore):
(WI.SourceCodePosition.prototype.isAfter):
(WI.SourceCodePosition.prototype.isWithin):
(WI.SourceCodePosition.prototype.toCodeMirror):
(WI.SourceCodePosition):

* UserInterface/Views/TextEditor.js:
(WI.TextEditor.prototype.visibleRangePositions):
(WI.TextEditor.prototype.originalPositionToCurrentPosition):
(WI.TextEditor.prototype.currentOffsetToCurrentPosition):
(WI.TextEditor.prototype.currentPositionToCurrentOffset):
(WI.TextEditor.prototype.setInlineWidget):

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

Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Controllers/TypeTokenAnnotator.js
Source/WebInspectorUI/UserInterface/Models/ScriptSyntaxTree.js
Source/WebInspectorUI/UserInterface/Models/SourceCodePosition.js
Source/WebInspectorUI/UserInterface/Views/TextEditor.js

index d84df31..70af80f 100644 (file)
@@ -1,3 +1,51 @@
+2018-07-12  Matt Baker  <mattbaker@apple.com>
+
+        Web Inspector: Type token positioning should use line/column locations from Esprima instead of offsets
+        https://bugs.webkit.org/show_bug.cgi?id=187612
+        <rdar://problem/42131910>
+
+        Reviewed by Joseph Pecoraro.
+
+        * UserInterface/Controllers/TypeTokenAnnotator.js:
+        (WI.TypeTokenAnnotator.prototype.insertAnnotations):
+        (WI.TypeTokenAnnotator.prototype._insertTypeToken):
+        (WI.TypeTokenAnnotator.prototype._insertToken):
+        Use line/column locations, instead of offsets, from the AST when calculating
+        token positions for CodeMirror. Once in CodeMirror's string space, we
+        can safely convert to/from offsets.
+
+        * UserInterface/Models/ScriptSyntaxTree.js:
+        Retrieve line/column locations for AST nodes, in addition to offsets.
+        Offsets into the original file are still needed for getting type information
+        from the profiler in the backend.
+
+        (WI.ScriptSyntaxTree):
+        (WI.ScriptSyntaxTree.prototype.filterByRange):
+        Filter by positions, which can be safely used from CodeMirror, instead of offsets.
+
+        (WI.ScriptSyntaxTree.prototype._createInternalSyntaxTree):
+        (WI.ScriptSyntaxTree.prototype.filterByRange.filterForNodesInRange): Deleted.
+
+        * UserInterface/Models/SourceCodePosition.js:
+        Add convenience methods for comparing line/column positions, and for
+        converting to the format expected by CodeMirror. SourceCodePosition could
+        be made to interoperate with CodeMirror by exposing properties `line`
+        and `ch`, but making the conversion explicit improves code readability.
+
+        (WI.SourceCodePosition.prototype.equals):
+        (WI.SourceCodePosition.prototype.isBefore):
+        (WI.SourceCodePosition.prototype.isAfter):
+        (WI.SourceCodePosition.prototype.isWithin):
+        (WI.SourceCodePosition.prototype.toCodeMirror):
+        (WI.SourceCodePosition):
+
+        * UserInterface/Views/TextEditor.js:
+        (WI.TextEditor.prototype.visibleRangePositions):
+        (WI.TextEditor.prototype.originalPositionToCurrentPosition):
+        (WI.TextEditor.prototype.currentOffsetToCurrentPosition):
+        (WI.TextEditor.prototype.currentPositionToCurrentOffset):
+        (WI.TextEditor.prototype.setInlineWidget):
+
 2018-07-10  Fujii Hironori  <Hironori.Fujii@sony.com>
 
         REGRESSION(r229932) Use of uninitialized value in subroutine entry at copy-user-interface-resources.pl
index bf1cc01..0e5f675 100644 (file)
@@ -55,10 +55,10 @@ WI.TypeTokenAnnotator = class TypeTokenAnnotator extends WI.Annotator
         if (!scriptSyntaxTree.parsedSuccessfully)
             return;
 
-        var {startOffset, endOffset} = this.sourceCodeTextEditor.visibleRangeOffsets();
+        let {startPosition, endPosition} = this.sourceCodeTextEditor.visibleRangePositions();
 
-        var startTime = Date.now();
-        var allNodesInRange = scriptSyntaxTree.filterByRange(startOffset, endOffset);
+        let startTime = Date.now();
+        let allNodesInRange = scriptSyntaxTree.filterByRange(startPosition, endPosition);
         scriptSyntaxTree.updateTypes(allNodesInRange, (nodesWithUpdatedTypes) => {
             // Because this is an asynchronous call, we could have been deactivated before the callback function is called.
             if (!this.isActive())
@@ -86,7 +86,7 @@ WI.TypeTokenAnnotator = class TypeTokenAnnotator extends WI.Annotator
     {
         if (node.type === WI.ScriptSyntaxTree.NodeType.Identifier) {
             if (!node.attachments.__typeToken && node.attachments.types && node.attachments.types.valid)
-                this._insertToken(node.range[0], node, false, WI.TypeTokenView.TitleType.Variable, node.name);
+                this._insertToken(node, false, WI.TypeTokenView.TitleType.Variable, node.name);
 
             if (node.attachments.__typeToken)
                 node.attachments.__typeToken.update(node.attachments.types);
@@ -105,18 +105,18 @@ WI.TypeTokenAnnotator = class TypeTokenAnnotator extends WI.Annotator
         var scriptSyntaxTree = this._script._scriptSyntaxTree;
         if (!node.attachments.__typeToken && (scriptSyntaxTree.containsNonEmptyReturnStatement(node.body) || !functionReturnType.typeSet.isContainedIn(WI.TypeSet.TypeBit.Undefined))) {
             var functionName = node.id ? node.id.name : null;
-            this._insertToken(node.typeProfilingReturnDivot, node, true, WI.TypeTokenView.TitleType.ReturnStatement, functionName);
+            this._insertToken(node, true, WI.TypeTokenView.TitleType.ReturnStatement, functionName);
         }
 
         if (node.attachments.__typeToken)
             node.attachments.__typeToken.update(node.attachments.returnTypes);
     }
 
-    _insertToken(originalOffset, node, shouldTranslateOffsetToAfterParameterList, typeTokenTitleType, functionOrVariableName)
+    _insertToken(node, shouldTranslateOffsetToAfterParameterList, typeTokenTitleType, functionOrVariableName)
     {
-        var tokenPosition = this.sourceCodeTextEditor.originalOffsetToCurrentPosition(originalOffset);
-        var currentOffset = this.sourceCodeTextEditor.currentPositionToCurrentOffset(tokenPosition);
-        var sourceString = this.sourceCodeTextEditor.string;
+        let tokenPosition = this.sourceCodeTextEditor.originalPositionToCurrentPosition(node.startPosition);
+        let currentOffset = this.sourceCodeTextEditor.currentPositionToCurrentOffset(tokenPosition);
+        let sourceString = this.sourceCodeTextEditor.string;
 
         if (shouldTranslateOffsetToAfterParameterList) {
             // Translate the position to the closing parenthesis of the function arguments:
index 5de64e8..26128c9 100644 (file)
@@ -33,7 +33,7 @@ WI.ScriptSyntaxTree = class ScriptSyntaxTree
 
         try {
             let sourceType = this._script.sourceType === WI.Script.SourceType.Module ? "module" : "script";
-            let esprimaSyntaxTree = esprima.parse(sourceText, {range: true, sourceType});
+            let esprimaSyntaxTree = esprima.parse(sourceText, {loc: true, range: true, sourceType});
             this._syntaxTree = this._createInternalSyntaxTree(esprimaSyntaxTree);
             this._parsedSuccessfully = true;
         } catch (error) {
@@ -101,15 +101,13 @@ WI.ScriptSyntaxTree = class ScriptSyntaxTree
         return allNodes;
     }
 
-    filterByRange(startOffset, endOffset)
+    filterByRange(startPosition, endPosition)
     {
         console.assert(this._parsedSuccessfully);
         if (!this._parsedSuccessfully)
             return [];
 
         var allNodes = [];
-        var start = 0;
-        var end = 1;
         function filterForNodesInRange(node, state)
         {
             // program start        range            program end
@@ -118,17 +116,21 @@ WI.ScriptSyntaxTree = class ScriptSyntaxTree
 
             // If a node's range ends before the range we're interested in starts, we don't need to search any of its
             // enclosing ranges, because, by definition, those enclosing ranges are contained within this node's range.
-            if (node.range[end] < startOffset)
+            if (node.endPosition.isBefore(startPosition)) {
                 state.skipChildNodes = true;
+                return;
+            }
 
             // We are only interested in nodes whose start position is within our range.
-            if (startOffset <= node.range[start] && node.range[start] <= endOffset)
+            if (node.startPosition.isWithin(startPosition, endPosition)) {
                 allNodes.push(node);
+                return;
+            }
 
             // Once we see nodes that start beyond our range, we can quit traversing the AST. We can do this safely
             // because we know the AST is traversed using depth first search, so it will traverse into enclosing ranges
             // before it traverses into adjacent ranges.
-            if (node.range[start] > endOffset)
+            if (node.startPosition.isAfter(endPosition))
                 state.shouldStopEarly = true;
         }
 
@@ -1058,6 +1060,10 @@ WI.ScriptSyntaxTree = class ScriptSyntaxTree
             return null;
         }
 
+        let {start, end} = node.loc;
+        result.startPosition = new WI.SourceCodePosition(start.line - 1, start.column);
+        result.endPosition = new WI.SourceCodePosition(end.line - 1, end.column);
+
         result.range = node.range;
         // This is an object for which you can add fields to an AST node without worrying about polluting the syntax-related fields of the node.
         result.attachments = {};
index 628fd11..55b5e79 100644 (file)
@@ -35,4 +35,46 @@ WI.SourceCodePosition = class SourceCodePosition
 
     get lineNumber() { return this._lineNumber; }
     get columnNumber() { return this._columnNumber; }
+
+    equals(position)
+    {
+        return this._lineNumber === position.lineNumber && this._columnNumber === position.columnNumber;
+    }
+
+    isBefore(position)
+    {
+        if (this._lineNumber < position.lineNumber)
+            return true;
+        if (this._lineNumber === position.lineNumber && this._columnNumber < position.columnNumber)
+            return true;
+
+        return false;
+    }
+
+    isAfter(position)
+    {
+        if (this._lineNumber > position.lineNumber)
+            return true;
+        if (this._lineNumber === position.lineNumber && this._columnNumber > position.columnNumber)
+            return true;
+
+        return false;
+    }
+
+    isWithin(startPosition, endPosition)
+    {
+        console.assert(startPosition.isBefore(endPosition) || startPosition.equals(endPosition));
+
+        if (this.equals(startPosition) || this.equals(endPosition))
+            return true;
+        if (this.isAfter(startPosition) && this.isBefore(endPosition))
+            return true;
+
+        return false;
+    }
+
+    toCodeMirror()
+    {
+        return {line: this._lineNumber, ch: this._columnNumber};
+    }
 };
index 5f6e24f..6e5e263 100644 (file)
@@ -720,6 +720,35 @@ WI.TextEditor = class TextEditor extends WI.View
         return {startOffset, endOffset};
     }
 
+    visibleRangePositions()
+    {
+        let visibleRange = this._codeMirror.getViewport();
+        let startLine;
+        let endLine;
+
+        if (this._formatterSourceMap) {
+            startLine = this._formatterSourceMap.formattedToOriginal(Math.max(visibleRange.from - 1, 0), 0).lineNumber;
+            endLine = this._formatterSourceMap.formattedToOriginal(visibleRange.to - 1, 0).lineNumber;
+        } else {
+            startLine = visibleRange.from;
+            endLine = visibleRange.to;
+        }
+
+        return {
+            startPosition: new WI.SourceCodePosition(startLine, 0),
+            endPosition: new WI.SourceCodePosition(endLine, 0)
+        };
+    }
+
+    originalPositionToCurrentPosition(position)
+    {
+        if (!this._formatterSourceMap)
+            return position;
+
+        let {lineNumber, columnNumber} = this._formatterSourceMap.originalToFormatted(position.lineNumber, position.columnNumber);
+        return new WI.SourceCodePosition(lineNumber, columnNumber);
+    }
+
     originalOffsetToCurrentPosition(offset)
     {
         var position = null;
@@ -734,7 +763,8 @@ WI.TextEditor = class TextEditor extends WI.View
 
     currentOffsetToCurrentPosition(offset)
     {
-        return this._codeMirror.getDoc().posFromIndex(offset);
+        let pos = this._codeMirror.getDoc().posFromIndex(offset);
+        return new WI.SourceCodePosition(pos.line, pos.ch);
     }
 
     currentPositionToOriginalOffset(position)
@@ -760,12 +790,12 @@ WI.TextEditor = class TextEditor extends WI.View
 
     currentPositionToCurrentOffset(position)
     {
-        return this._codeMirror.getDoc().indexFromPos(position);
+        return this._codeMirror.getDoc().indexFromPos(position.toCodeMirror());
     }
 
     setInlineWidget(position, inlineElement)
     {
-        return this._codeMirror.setUniqueBookmark(position, {widget: inlineElement});
+        return this._codeMirror.setUniqueBookmark(position.toCodeMirror(), {widget: inlineElement});
     }
 
     addScrollHandler(handler)