- Revision
- 233792
- Author
- mattba...@apple.com
- Date
- 2018-07-12 19:06:48 -0700 (Thu, 12 Jul 2018)
Log Message
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):
Modified Paths
Diff
Modified: trunk/Source/WebInspectorUI/ChangeLog (233791 => 233792)
--- trunk/Source/WebInspectorUI/ChangeLog 2018-07-13 01:20:40 UTC (rev 233791)
+++ trunk/Source/WebInspectorUI/ChangeLog 2018-07-13 02:06:48 UTC (rev 233792)
@@ -1,3 +1,51 @@
+2018-07-12 Matt Baker <mattba...@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.fu...@sony.com>
REGRESSION(r229932) Use of uninitialized value in subroutine entry at copy-user-interface-resources.pl
Modified: trunk/Source/WebInspectorUI/UserInterface/Controllers/TypeTokenAnnotator.js (233791 => 233792)
--- trunk/Source/WebInspectorUI/UserInterface/Controllers/TypeTokenAnnotator.js 2018-07-13 01:20:40 UTC (rev 233791)
+++ trunk/Source/WebInspectorUI/UserInterface/Controllers/TypeTokenAnnotator.js 2018-07-13 02:06:48 UTC (rev 233792)
@@ -55,10 +55,10 @@
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 @@
{
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,7 +105,7 @@
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)
@@ -112,11 +112,11 @@
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:
Modified: trunk/Source/WebInspectorUI/UserInterface/Models/ScriptSyntaxTree.js (233791 => 233792)
--- trunk/Source/WebInspectorUI/UserInterface/Models/ScriptSyntaxTree.js 2018-07-13 01:20:40 UTC (rev 233791)
+++ trunk/Source/WebInspectorUI/UserInterface/Models/ScriptSyntaxTree.js 2018-07-13 02:06:48 UTC (rev 233792)
@@ -33,7 +33,7 @@
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,7 +101,7 @@
return allNodes;
}
- filterByRange(startOffset, endOffset)
+ filterByRange(startPosition, endPosition)
{
console.assert(this._parsedSuccessfully);
if (!this._parsedSuccessfully)
@@ -108,8 +108,6 @@
return [];
var allNodes = [];
- var start = 0;
- var end = 1;
function filterForNodesInRange(node, state)
{
// program start range program end
@@ -118,17 +116,21 @@
// 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 @@
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 = {};
Modified: trunk/Source/WebInspectorUI/UserInterface/Models/SourceCodePosition.js (233791 => 233792)
--- trunk/Source/WebInspectorUI/UserInterface/Models/SourceCodePosition.js 2018-07-13 01:20:40 UTC (rev 233791)
+++ trunk/Source/WebInspectorUI/UserInterface/Models/SourceCodePosition.js 2018-07-13 02:06:48 UTC (rev 233792)
@@ -35,4 +35,46 @@
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};
+ }
};
Modified: trunk/Source/WebInspectorUI/UserInterface/Views/TextEditor.js (233791 => 233792)
--- trunk/Source/WebInspectorUI/UserInterface/Views/TextEditor.js 2018-07-13 01:20:40 UTC (rev 233791)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/TextEditor.js 2018-07-13 02:06:48 UTC (rev 233792)
@@ -720,6 +720,35 @@
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 @@
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 @@
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)
