Improve parser flow coverage (#5766)

y9127 · jasonLaster · commit ddcb297c613e · 2018-03-26T19:35:59.000-04:00
diff --git a/src/actions/preview.js b/src/actions/preview.js
@@ -24,7 +24,7 @@ import { getMappedExpression } from "./expressions";
 import { isEqual } from "lodash";
 
 import type { ThunkArgs } from "./types";
-import type { AstLocation } from "../workers/parser";
+import type { Range, Position } from "../types";
 
 async function getReactProps(evaluate) {
   const reactDisplayName = await evaluate(
@@ -142,8 +142,8 @@ export function updatePreview(target: HTMLElement, editor: any) {
 
 export function setPreview(
   expression: string,
-  location: AstLocation,
-  tokenPos: AstLocation,
+  location: Range,
+  tokenPos: Position,
   cursorPos: any
 ) {
   return async ({ dispatch, getState, client, sourceMaps }: ThunkArgs) => {
diff --git a/src/components/Editor/index.js b/src/components/Editor/index.js
@@ -41,7 +41,7 @@ import EmptyLines from "./EmptyLines";
 import GutterMenu from "./GutterMenu";
 import EditorMenu from "./EditorMenu";
 import ConditionalPanel from "./ConditionalPanel";
-import type { SymbolDeclarations } from "../../workers/parser/types";
+import type { SymbolDeclarations } from "../../workers/parser";
 
 import {
   showSourceText,
diff --git a/src/reducers/ast.js b/src/reducers/ast.js
@@ -26,7 +26,7 @@ import type { Action } from "../actions/types";
 import type { Record } from "../utils/makeRecord";
 
 type EmptyLinesType = number[];
-export type Symbols = SymbolDeclarations | { loading: true };
+export type Symbols = SymbolDeclarations | {| loading: true |};
 export type SymbolsMap = Map<string, Symbols>;
 export type EmptyLinesMap = Map<string, EmptyLinesType>;
 
@@ -148,10 +148,7 @@ function update(
 // https://github.com/devtools-html/debugger.html/blob/master/src/reducers/sources.js#L179-L185
 type OuterState = { ast: Record<ASTState> };
 
-export function getSymbols(
-  state: OuterState,
-  source: Source
-): ?SymbolDeclarations {
+export function getSymbols(state: OuterState, source: Source): ?Symbols {
   if (!source) {
     return null;
   }
@@ -166,7 +163,7 @@ export function hasSymbols(state: OuterState, source: Source): boolean {
     return false;
   }
 
-  return !symbols.loading;
+  return !symbols.hasOwnProperty("loading");
 }
 
 export function isSymbolsLoading(state: OuterState, source: Source): boolean {
@@ -175,7 +172,7 @@ export function isSymbolsLoading(state: OuterState, source: Source): boolean {
     return false;
   }
 
-  return !!symbols.loading;
+  return symbols.hasOwnProperty("loading");
 }
 
 export function isEmptyLineInSource(
diff --git a/src/types.js b/src/types.js
@@ -60,6 +60,13 @@ export type Location = {
   sourceUrl?: string
 };
 
+export type Position = {
+  line: number,
+  column: ?number
+};
+
+export type Range = { end: Position, start: Position };
+
 export type PendingLocation = {
   line: number,
   column: ?number,
@@ -332,7 +339,3 @@ export type Worker = {
   type: number,
   url: string
 };
-
-export type Position = { line: number, column: number };
-
-export type Range = { end: Position, start: Position };
diff --git a/src/utils/breakpoint/astBreakpointLocation.js b/src/utils/breakpoint/astBreakpointLocation.js
@@ -7,14 +7,15 @@
 import { getSymbols } from "../../workers/parser";
 
 import type {
-  Scope,
   AstPosition,
-  SymbolDeclarations
+  AstLocation,
+  SymbolDeclarations,
+  SymbolDeclaration
 } from "../../workers/parser";
 
 import type { Location, Source, ASTLocation } from "../../types";
 
-export function containsPosition(a: AstPosition, b: AstPosition) {
+export function containsPosition(a: AstLocation, b: AstPosition) {
   const startsBefore =
     a.start.line < b.line ||
     (a.start.line === b.line && a.start.column <= b.column);
@@ -24,11 +25,17 @@ export function containsPosition(a: AstPosition, b: AstPosition) {
   return startsBefore && endsAfter;
 }
 
-export function findClosestScope(functions: Scope[], location: Location) {
+export function findClosestScope(
+  functions: SymbolDeclaration[],
+  location: Location
+) {
   return functions.reduce((found, currNode) => {
     if (
       currNode.name === "anonymous" ||
-      !containsPosition(currNode.location, location)
+      !containsPosition(currNode.location, {
+        line: location.line,
+        column: location.column || 0
+      })
     ) {
       return found;
     }
diff --git a/src/utils/editor/get-expression.js b/src/utils/editor/get-expression.js
@@ -4,10 +4,7 @@
 
 // @flow
 
-type Pos = {
-  line: number,
-  column: number
-};
+import type { Position } from "../../types";
 
 type Token = {
   startColumn: number,
@@ -17,7 +14,7 @@ type Token = {
 
 export function tokenAtTextPosition(
   cm: any,
-  { line, column }: Pos
+  { line, column }: Position
 ): Token | null {
   if (line < 0 || line >= cm.lineCount()) {
     return null;
@@ -33,7 +30,7 @@ export function tokenAtTextPosition(
 
 // The strategy of querying codeMirror tokens was borrowed
 // from Chrome's inital implementation in JavaScriptSourceFrame.js#L414
-export function getExpressionFromCoords(cm: any, coord: Pos) {
+export function getExpressionFromCoords(cm: any, coord: Position) {
   const token = tokenAtTextPosition(cm, coord);
   if (!token) {
     return null;
diff --git a/src/utils/editor/get-token-location.js b/src/utils/editor/get-token-location.js
@@ -3,8 +3,12 @@
  * file, You can obtain one at <http://mozilla.org/MPL/2.0/>. */
 
 // @flow
+import type { Position } from "../../types";
 
-export function getTokenLocation(codeMirror: any, tokenEl: HTMLElement) {
+export function getTokenLocation(
+  codeMirror: any,
+  tokenEl: HTMLElement
+): Position {
   const { left, top, width, height } = tokenEl.getBoundingClientRect();
   const { line, ch } = codeMirror.coordsChar({
     left: left + width / 2,
diff --git a/src/utils/editor/index.js b/src/utils/editor/index.js
@@ -16,8 +16,9 @@ import { findNext, findPrev } from "./source-search";
 import { isWasm, lineToWasmOffset, wasmOffsetToLine } from "../wasm";
 import { isOriginalId } from "devtools-source-map";
 
-import type { AstPosition, AstLocation } from "../../workers/parser/types";
+import type { AstLocation } from "../../workers/parser";
 import type { EditorPosition, EditorRange } from "../editor/types";
+import type { Location } from "../../types";
 
 export function shouldShowPrettyPrint(selectedSource) {
   if (!selectedSource) {
@@ -60,7 +61,7 @@ export function toEditorLine(sourceId: string, lineOrOffset: number): number {
   return lineOrOffset ? lineOrOffset - 1 : 1;
 }
 
-export function toEditorPosition(location: AstPosition): EditorPosition {
+export function toEditorPosition(location: Location): EditorPosition {
   return {
     line: toEditorLine(location.sourceId, location.line),
     column: isWasm(location.sourceId) || !location.column ? 0 : location.column
@@ -121,16 +122,6 @@ function isVisible(codeMirror: any, top: number, left: number) {
   return inXView && inYView;
 }
 
-export function toSourceLocation(
-  sourceId: string,
-  location: EditorPosition
-): AstPosition {
-  return {
-    line: toSourceLine(sourceId, location.line),
-    column: isWasm(sourceId) ? undefined : location.column
-  };
-}
-
 export function markText(editor: any, className, { start, end }: EditorRange) {
   return editor.codeMirror.markText(
     { ch: start.column, line: start.line },
diff --git a/src/utils/editor/tests/editor.spec.js b/src/utils/editor/tests/editor.spec.js
@@ -7,7 +7,6 @@ import {
   toEditorPosition,
   toEditorRange,
   toSourceLine,
-  toSourceLocation,
   scrollToColumn,
   markText,
   lineAtHeight,
@@ -123,17 +122,6 @@ describe("toSourceLine", () => {
   });
 });
 
-describe("toSourceLocation", () => {
-  const testId = "test-123";
-  const loc = { line: 100, column: 30 };
-  it("returns a source location", () => {
-    expect(toSourceLocation(testId, loc)).toEqual({
-      line: 101,
-      column: 30
-    });
-  });
-});
-
 const codeMirror = {
   doc: {
     iter: jest.fn((_, __, cb) => cb())
diff --git a/src/utils/quick-open.js b/src/utils/quick-open.js
@@ -8,11 +8,9 @@ import { isPretty, getSourcePath } from "./source";
 
 import type { Location as BabelLocation } from "@babel/types";
 import type { SourcesMap } from "../reducers/sources";
+import type { Symbols } from "../reducers/ast";
 import type { QuickOpenType } from "../reducers/quick-open";
-import type {
-  SymbolDeclaration,
-  SymbolDeclarations
-} from "../workers/parser/types";
+import type { SymbolDeclaration } from "../workers/parser";
 
 export const MODIFIERS = {
   "@": "functions",
@@ -75,10 +73,8 @@ export function formatSymbol(symbol: SymbolDeclaration): QuickOpenResult {
   };
 }
 
-export function formatSymbols(
-  symbols: ?SymbolDeclarations
-): FormattedSymbolDeclarations {
-  if (!symbols) {
+export function formatSymbols(symbols: ?Symbols): FormattedSymbolDeclarations {
+  if (!symbols || symbols.loading) {
     return { variables: [], functions: [] };
   }
 
diff --git a/src/utils/source.js b/src/utils/source.js
@@ -17,7 +17,7 @@ import { parse as parseURL } from "url";
 export { isMinified } from "./isMinified";
 
 import type { Source, SourceRecord } from "../types";
-import type { SymbolDeclarations } from "../workers/parser/types";
+import type { SymbolDeclarations } from "../workers/parser";
 
 type transformUrlCallback = string => string;
 
diff --git a/src/workers/parser/index.js b/src/workers/parser/index.js
@@ -47,6 +47,6 @@ export type {
   SymbolDeclarations,
   AstLocation,
   AstPosition,
-  Scope
+  Scope,
+  PausePoint
 } from "./types";
-export type { PausePoint, PausePoints } from "./pausePoints";
diff --git a/src/workers/parser/pausePoints.js b/src/workers/parser/pausePoints.js
@@ -1,13 +1,5 @@
 import { traverseAst } from "./utils/ast";
 import * as t from "@babel/types";
-import type { AstLocation } from "./types";
-
-export type PausePoint = {|
-  location: AstLocation,
-  types: {| breakpoint: boolean, stepOver: boolean |}
-|};
-
-export type PausePoints = PausePoint[];
 
 const isControlFlow = node =>
   t.isForStatement(node) || t.isWhileStatement(node) || t.isIfStatement(node);
diff --git a/src/workers/parser/steps.js b/src/workers/parser/steps.js
@@ -7,7 +7,7 @@
 import * as t from "@babel/types";
 import type { SimplePath } from "./utils/simple-path";
 import type { SourceId } from "../../types";
-import { AstPosition } from "./types";
+import type { AstPosition } from "./types";
 import { getClosestPath } from "./utils/closest";
 import { isAwaitExpression, isYieldExpression } from "./utils/helpers";
 
@@ -27,7 +27,7 @@ export function getNextStep(sourceId: SourceId, pausedPosition: AstPosition) {
     );
   }
 
-  return _getNextStep(currentStatement, pausedPosition);
+  return _getNextStep(currentStatement, sourceId, pausedPosition);
 }
 
 function getSteppableExpression(
@@ -49,12 +49,16 @@ function getSteppableExpression(
   );
 }
 
-function _getNextStep(statement: SimplePath, position: AstPosition) {
+function _getNextStep(
+  statement: SimplePath,
+  sourceId: string,
+  position: AstPosition
+) {
   const nextStatement = statement.getSibling(1);
   if (nextStatement) {
     return {
       ...nextStatement.node.loc.start,
-      sourceId: position.sourceId
+      sourceId: sourceId
     };
   }
 
diff --git a/src/workers/parser/tests/__snapshots__/closest.spec.js.snap b/src/workers/parser/tests/__snapshots__/closest.spec.js.snap
diff --git a/src/workers/parser/tests/closest.spec.js b/src/workers/parser/tests/closest.spec.js
diff --git a/src/workers/parser/types.js b/src/workers/parser/types.js
diff --git a/src/workers/parser/utils/closest.js b/src/workers/parser/utils/closest.js
diff --git a/src/workers/parser/worker.js b/src/workers/parser/worker.js

Original file line number	Diff line number	Diff line change
`@@ -26,7 +26,7 @@ import type { Action } from "../actions/types";`
`26`	`26`	`import type { Record } from "../utils/makeRecord";`
`27`	`27`
`28`	`28`	`type EmptyLinesType = number[];`
`29`		`-export type Symbols = SymbolDeclarations \| { loading: true };`
	`29`	`+export type Symbols = SymbolDeclarations \| {\| loading: true \|};`
`30`	`30`	`export type SymbolsMap = Map<string, Symbols>;`
`31`	`31`	`export type EmptyLinesMap = Map<string, EmptyLinesType>;`
`32`	`32`
`@@ -148,10 +148,7 @@ function update(`
`148`	`148`	`// https://github.com/devtools-html/debugger.html/blob/master/src/reducers/sources.js#L179-L185`
`149`	`149`	`type OuterState = { ast: Record<ASTState> };`
`150`	`150`
`151`		`-export function getSymbols(`
`152`		`- state: OuterState,`
`153`		`- source: Source`
`154`		`-): ?SymbolDeclarations {`
	`151`	`+export function getSymbols(state: OuterState, source: Source): ?Symbols {`
`155`	`152`	`if (!source) {`
`156`	`153`	`return null;`
`157`	`154`	`}`
`@@ -166,7 +163,7 @@ export function hasSymbols(state: OuterState, source: Source): boolean {`
`166`	`163`	`return false;`
`167`	`164`	`}`
`168`	`165`
`169`		`- return !symbols.loading;`
	`166`	`+ return !symbols.hasOwnProperty("loading");`
`170`	`167`	`}`
`171`	`168`
`172`	`169`	`export function isSymbolsLoading(state: OuterState, source: Source): boolean {`
`@@ -175,7 +172,7 @@ export function isSymbolsLoading(state: OuterState, source: Source): boolean {`
`175`	`172`	`return false;`
`176`	`173`	`}`
`177`	`174`
`178`		`- return !!symbols.loading;`
	`175`	`+ return symbols.hasOwnProperty("loading");`
`179`	`176`	`}`
`180`	`177`
`181`	`178`	`export function isEmptyLineInSource(`