Fixing fixup of source locations, mostly. (#2356)

Summary:
Release notes: Fixing source map support.

I kept getting seemingly garbage source locations in error messages, and looked into that.
I found various issues:
- We in-place update positions, but they are actually shared between locations, and thus we may revisit positions. When we do, we map them again, and so on...
- Locations are also shared between nodes, so we kept revisiting and rewriting yet again.
- The actual mapping doesn't pay attention to the filename, so we apply the wrong mapping altogether, especially in the presence of multiple input files. I am not fixing this, but added a TODO, and opened #2353.
- Another remaining but not blocking issue is that something goes wrong with end-positions: They are sometimes mapped to some seemingly random position in the right line. If anyone knows anything about this, please let me know...
Pull Request resolved: https://github.com/facebook/prepack/pull/2356

Differential Revision: D9141809

Pulled By: NTillmann

fbshipit-source-id: d765e99706d69e3c792fba4c553d4110963067eb
This commit is contained in:
Nikolai Tillmann 2018-08-06 18:40:08 -07:00 committed by Facebook Github Bot
parent b965400a31
commit 9d02cbc575
7 changed files with 281 additions and 87 deletions

View File

@ -14,7 +14,161 @@
*/
declare module 'source-map' {
declare module.exports: any;
// Adapted from https://raw.githubusercontent.com/mozilla/source-map/c73baa52dedcbb77af97d90390d9def4d594c75f/source-map.d.ts
// Type definitions for source-map
// Project: https://github.com/mozilla/source-map
// Definitions by: Morten Houston Ludvigsen <https://github.com/MortenHoustonLudvigsen>,
// Ron Buckton <https://github.com/rbuckton>,
// John Vilk <https://github.com/jvilk>
// Definitions: https://github.com/mozilla/source-map
declare type SourceMapUrl = string;
declare interface Position {
line: number;
column: number;
}
declare interface NullablePosition {
line: number | null;
column: number | null;
lastColumn: number | null;
}
declare interface MappedPosition {
source: string;
line: number;
column: number;
name?: string;
}
declare interface NullableMappedPosition {
source: string | null;
line: number | null;
column: number | null;
name: string | null;
}
declare interface MappingItem {
source: string;
generatedLine: number;
generatedColumn: number;
originalLine: number;
originalColumn: number;
name: string;
}
declare interface Mapping {
generated: Position;
original: Position;
source: string;
name?: string;
}
declare class SourceMapConsumer {
constructor (rawSourceMap: string, sourceMapUrl?: SourceMapUrl): void;
/**
* Compute the last column for each generated mapping. The last column is
* inclusive.
*/
computeColumnSpans(): void;
/**
* Returns the original source, line, and column information for the generated
* source's line and column positions provided. The only argument is an object
* with the following properties:
*
* - line: The line number in the generated source.
* - column: The column number in the generated source.
* - bias: Either 'SourceMapConsumer.GREATEST_LOWER_BOUND' or
* 'SourceMapConsumer.LEAST_UPPER_BOUND'. Specifies whether to return the
* closest element that is smaller than or greater than the one we are
* searching for, respectively, if the exact element cannot be found.
* Defaults to 'SourceMapConsumer.GREATEST_LOWER_BOUND'.
*
* and an object is returned with the following properties:
*
* - source: The original source file, or null.
* - line: The line number in the original source, or null.
* - column: The column number in the original source, or null.
* - name: The original identifier, or null.
*/
originalPositionFor(generatedPosition: Position & { bias?: number }): NullableMappedPosition;
/**
* Returns the generated line and column information for the original source,
* line, and column positions provided. The only argument is an object with
* the following properties:
*
* - source: The filename of the original source.
* - line: The line number in the original source.
* - column: The column number in the original source.
* - bias: Either 'SourceMapConsumer.GREATEST_LOWER_BOUND' or
* 'SourceMapConsumer.LEAST_UPPER_BOUND'. Specifies whether to return the
* closest element that is smaller than or greater than the one we are
* searching for, respectively, if the exact element cannot be found.
* Defaults to 'SourceMapConsumer.GREATEST_LOWER_BOUND'.
*
* and an object is returned with the following properties:
*
* - line: The line number in the generated source, or null.
* - column: The column number in the generated source, or null.
*/
generatedPositionFor(originalPosition: MappedPosition & { bias?: number }): NullablePosition;
/**
* Returns all generated line and column information for the original source,
* line, and column provided. If no column is provided, returns all mappings
* corresponding to a either the line we are searching for or the next
* closest line that has any mappings. Otherwise, returns all mappings
* corresponding to the given line and either the column we are searching for
* or the next closest column that has any offsets.
*
* The only argument is an object with the following properties:
*
* - source: The filename of the original source.
* - line: The line number in the original source.
* - column: Optional. the column number in the original source.
*
* and an array of objects is returned, each with the following properties:
*
* - line: The line number in the generated source, or null.
* - column: The column number in the generated source, or null.
*/
allGeneratedPositionsFor(originalPosition: MappedPosition): NullablePosition[];
/**
* Return true if we have the source content for every source in the source
* map, false otherwise.
*/
hasContentsOfAllSources(): boolean;
/**
* Returns the original source content. The only argument is the url of the
* original source file. Returns null if no original source content is
* available.
*/
sourceContentFor(source: string, returnNullOnMissing?: boolean): string | null;
/**
* Iterate over each mapping between an original source/line/column and a
* generated line/column in this source map.
*
* @param callback
* The function that is called with each mapping.
* @param context
* Optional. If specified, this object will be the value of `this` every
* time that `aCallback` is called.
* @param order
* Either `SourceMapConsumer.GENERATED_ORDER` or
* `SourceMapConsumer.ORIGINAL_ORDER`. Specifies whether you want to
* iterate over the mappings sorted by the generated file's line/column
* order or the original's source/line/column order, respectively. Defaults to
* `SourceMapConsumer.GENERATED_ORDER`.
*/
eachMapping(callback: (mapping: MappingItem) => void, context?: any, order?: number): void;
}
}
/**

View File

@ -51,7 +51,7 @@ import { TypesDomain, ValuesDomain } from "./domains/index.js";
import PrimitiveValue from "./values/PrimitiveValue.js";
import { createOperationDescriptor } from "./utils/generator.js";
const sourceMap = require("source-map");
import { SourceMapConsumer, type NullableMappedPosition } from "source-map";
function deriveGetBinding(realm: Realm, binding: Binding) {
let types = TypesDomain.topVal;
@ -1154,12 +1154,10 @@ export class LexicalEnvironment {
let sourceMapContents = source.sourceMapContents;
if (sourceMapContents && sourceMapContents.length > 0) {
this.realm.statistics.fixupSourceLocations.measure(() =>
this.fixup_source_locations(node, sourceMapContents)
);
this.realm.statistics.fixupSourceLocations.measure(() => this.fixupSourceLocations(node, sourceMapContents));
}
this.realm.statistics.fixupFilenames.measure(() => this.fixup_filenames(node));
this.realm.statistics.fixupFilenames.measure(() => this.fixupFilenames(node));
asts = asts.concat(node.program.body);
code[source.filePath] = source.fileContents;
@ -1244,7 +1242,7 @@ export class LexicalEnvironment {
invariant(partialAST.type === "File");
let fileAst = ((partialAST: any): BabelNodeFile);
let prog = t.program(fileAst.program.body, ast.program.directives);
this.fixup_filenames(prog);
this.fixupFilenames(prog);
// The type signature for generate is not complete, hence the any
return generate(prog, { sourceMaps: options.sourceMaps }, (code: any));
}
@ -1272,8 +1270,8 @@ export class LexicalEnvironment {
throw e;
}
if (onParse) onParse(ast);
if (map.length > 0) this.fixup_source_locations(ast, map);
this.fixup_filenames(ast);
if (map.length > 0) this.fixupSourceLocations(ast, map);
this.fixupFilenames(ast);
res = this.evaluateCompletion(ast, false);
} finally {
this.realm.popContext(context);
@ -1289,65 +1287,117 @@ export class LexicalEnvironment {
return Environment.GetValue(this.realm, res);
}
fixup_source_locations(ast: BabelNode, map: string): void {
const smc = new sourceMap.SourceMapConsumer(map);
fixupSourceLocations(ast: BabelNode, map: string): void {
invariant(ast.loc);
const source = ast.loc.source;
invariant(source !== undefined);
const positionInfos = new Map();
const smc = new SourceMapConsumer(map);
traverseFast(ast, node => {
let loc = node.loc;
if (!loc) return false;
fixup(loc, loc.start);
fixup(loc, loc.end);
fixup_comments(node.leadingComments);
fixup_comments(node.innerComments);
fixup_comments(node.trailingComments);
fixupLocation(node.loc);
fixupComments(node.leadingComments);
fixupComments(node.innerComments);
fixupComments(node.trailingComments);
return false;
function fixup(new_loc: BabelNodeSourceLocation, new_pos: BabelNodePosition) {
let old_pos = smc.originalPositionFor({ line: new_pos.line, column: new_pos.column });
if (old_pos.source === null) return;
new_pos.line = old_pos.line;
new_pos.column = old_pos.column;
new_loc.source = old_pos.source;
}
function fixup_comments(comments: ?Array<BabelNodeComment>) {
if (!comments) return;
for (let c of comments) {
let cloc = c.loc;
if (!cloc) continue;
fixup(cloc, cloc.start);
fixup(cloc, cloc.end);
}
}
});
type PositionInfo = {
originalPosition: NullableMappedPosition,
newLine: number,
newColumn: number,
rewritten: boolean,
};
function getPositionInfo(position: BabelNodePosition): PositionInfo {
let info = positionInfos.get(position);
if (info === undefined)
positionInfos.set(
position,
(info = {
originalPosition: smc.originalPositionFor(position),
newLine: position.line,
newColumn: position.column,
rewritten: false,
})
);
return info;
}
function fixupPosition(pos: BabelNodePosition, posInfo: PositionInfo, otherInfo: PositionInfo): void {
if (posInfo.rewritten) return;
let posOriginalPosition = posInfo.originalPosition;
if (posOriginalPosition.source == null) {
invariant(otherInfo.originalPosition.source != null);
let deltaLine = posInfo.newLine - otherInfo.newLine;
pos.line = Math.max(1, otherInfo.originalPosition.line + deltaLine);
let deltaColumn = posInfo.newColumn - otherInfo.newColumn;
pos.column = Math.max(0, otherInfo.originalPosition.column + deltaColumn);
} else {
invariant(typeof posOriginalPosition.line === "number");
pos.line = posOriginalPosition.line;
invariant(typeof posOriginalPosition.column === "number");
pos.column = posOriginalPosition.column;
}
posInfo.rewritten = true;
}
function fixupLocation(loc: ?BabelNodeSourceLocation): void {
if (loc == null) return;
// Bail out when location already got fixed up or doesn't have source
if (loc.source === undefined || loc.source !== source) return;
let locStart = loc.start;
let locEnd = loc.end;
let startInfo = getPositionInfo(locStart);
let endInfo = getPositionInfo(locEnd);
let startOriginalPosition = startInfo.originalPosition;
let endOriginalPosition = endInfo.originalPosition;
// Sanity checks on the positions supplied directly by the Babel parser
invariant(startInfo.newLine <= endInfo.newLine);
invariant(startInfo.newLine !== endInfo.newLine || startInfo.newColumn <= endInfo.newColumn);
let originalSource = startOriginalPosition.source || endOriginalPosition.source;
if (originalSource) {
fixupPosition(locStart, startInfo, endInfo);
fixupPosition(locEnd, endInfo, startInfo);
// NOTE: Babel only persists the start position of most nodes in source maps
// (only block statements also get their end positions persisted).
// Thus, end positions tend to be mostly wrong (in fact often so wrong
// that they point before the start position).
// The best way to deal with that is to never print end positions in user-facing
// messages, or use them for any reason.
invariant(loc.source !== originalSource);
loc.source = originalSource;
}
}
function fixupComments(comments: ?Array<BabelNodeComment>) {
if (!comments) return;
for (let c of comments) fixupLocation(c.loc);
}
}
fixup_filenames(ast: BabelNode): void {
fixupFilenames(ast: BabelNode): void {
traverseFast(ast, node => {
let loc = node.loc;
if (!loc || !loc.source) {
node.leadingComments = null;
node.innerComments = null;
node.trailingComments = null;
node.loc = null;
} else {
let filename = loc.source;
(loc: any).filename = filename;
fixup_comments(node.leadingComments, filename);
fixup_comments(node.innerComments, filename);
fixup_comments(node.trailingComments, filename);
}
if (loc && loc.source) (loc: any).filename = loc.source;
else node.loc = null;
fixupComments(node.leadingComments);
fixupComments(node.innerComments);
fixupComments(node.trailingComments);
return false;
function fixup_comments(comments: ?Array<BabelNodeComment>, filename: string) {
if (!comments) return;
for (let c of comments) {
if (c.loc) {
(c.loc: any).filename = filename;
c.loc.source = filename;
}
}
}
});
function fixupComments(comments: ?Array<BabelNodeComment>): void {
if (!comments) return;
for (let c of comments) {
let loc = c.loc;
if (loc && loc.source) (loc: any).filename = loc.source;
else (c: any).loc = null;
}
}
}
evaluate(ast: BabelNode, strictCode: boolean, metadata?: any): Value | Reference {

View File

@ -35,7 +35,6 @@ import {
hardModifyReactObjectPropertyBinding,
getComponentName,
getComponentTypeFromRootValue,
getLocationFromValue,
getProperty,
getReactSymbol,
getValueFromFunctionCall,
@ -78,6 +77,7 @@ import { Logger } from "../utils/logger.js";
import type { ClassComponentMetadata, ReactComponentTreeConfig, ReactHint } from "../types.js";
import { handleReportedSideEffect } from "../serializer/utils.js";
import { createOperationDescriptor } from "../utils/generator.js";
import { optionalStringOfLocation } from "../utils/babelhelpers.js";
type ComponentResolutionStrategy =
| "NORMAL"
@ -1388,7 +1388,7 @@ export class Reconciler {
} else if (value instanceof ObjectValue && isReactElement(value)) {
return this._resolveReactElement(componentType, value, context, branchStatus, evaluatedNode, needsKey);
} else {
let location = getLocationFromValue(value.expressionLocation);
let location = optionalStringOfLocation(value.expressionLocation);
throw new ExpectedBailOut(`invalid return value from render${location}`);
}
}

View File

@ -825,15 +825,6 @@ function isEventProp(name: string): boolean {
return name.length > 2 && name[0].toLowerCase() === "o" && name[1].toLowerCase() === "n";
}
export function getLocationFromValue(expressionLocation: any): string {
// if we can't get a value, then it's likely that the source file was not given
// (this happens in React tests) so instead don't print any location
return expressionLocation
? ` at location: ${expressionLocation.start.line}:${expressionLocation.start.column} ` +
`- ${expressionLocation.end.line}:${expressionLocation.end.line}`
: "";
}
export function createNoopFunction(realm: Realm): ECMAScriptSourceFunctionValue {
if (realm.react.noopFunction !== undefined) {
return realm.react.noopFunction;

View File

@ -35,7 +35,7 @@ import { convertConfigObjectToReactComponentTreeConfig, valueIsKnownReactAbstrac
import { optimizeReactComponentTreeRoot } from "../react/optimizing.js";
import { handleReportedSideEffect } from "./utils.js";
import type { ArgModel } from "../types.js";
import { stringOfLocation } from "../utils/babelhelpers";
import { optionalStringOfLocation } from "../utils/babelhelpers";
import { Properties, Utils } from "../singletons.js";
type AdditionalFunctionEntry = {
@ -96,10 +96,7 @@ export class Functions {
};
}
let location = value.expressionLocation
? `${value.expressionLocation.start.line}:${value.expressionLocation.start.column} ` +
`${value.expressionLocation.end.line}:${value.expressionLocation.end.line}`
: "location unknown";
let location = optionalStringOfLocation(value.expressionLocation);
realm.handleError(
new CompilerDiagnostic(
`Optimized Function Value ${location} is an not a function or react element`,
@ -287,7 +284,7 @@ export class Functions {
for (let fun1 of additionalFunctions) {
invariant(fun1 instanceof FunctionValue);
let fun1Location = fun1.expressionLocation;
let fun1Name = fun1.getDebugName() || `(unknown function ${fun1Location ? stringOfLocation(fun1Location) : ""})`;
let fun1Name = fun1.getDebugName() || optionalStringOfLocation(fun1Location);
// Also do argument validation here
let additionalFunctionEffects = this.writeEffects.get(fun1);
invariant(additionalFunctionEffects !== undefined);
@ -307,8 +304,7 @@ export class Functions {
if (fun1 === fun2) continue;
invariant(fun2 instanceof FunctionValue);
let fun2Location = fun2.expressionLocation;
let fun2Name =
fun2.getDebugName() || `(unknown function ${fun2Location ? stringOfLocation(fun2Location) : ""})`;
let fun2Name = fun2.getDebugName() || optionalStringOfLocation(fun2Location);
let reportFn = () => {
this.reportWriteConflicts(
fun1Name,
@ -353,13 +349,13 @@ export class Functions {
key?: string,
originalLocation: BabelNodeSourceLocation | void | null
) => {
let firstLocationString = originalLocation ? `${stringOfLocation(originalLocation)}` : "";
let secondLocationString = `${stringOfLocation(location)}`;
let firstLocationString = optionalStringOfLocation(originalLocation);
let secondLocationString = optionalStringOfLocation(location);
let propString = key ? ` "${key}"` : "";
let objectString = object ? ` on object "${object}" ` : "";
if (!objectString && key) objectString = " on <unnamed object> ";
let error = new CompilerDiagnostic(
`Write to property${propString}${objectString}at optimized function ${f1name}[${firstLocationString}] conflicts with access in function ${f2name}[${secondLocationString}]`,
`Write to property${propString}${objectString}at optimized function ${f1name}${firstLocationString} conflicts with access in function ${f2name}${secondLocationString}`,
location,
"PP1003",
"RecoverableError"

View File

@ -28,7 +28,7 @@ import { Logger } from "../utils/logger.js";
import { Generator } from "../utils/generator.js";
import type { AdditionalFunctionEffects } from "./types";
import type { Binding } from "../environment.js";
import { getLocationFromValue } from "../react/utils.js";
import { optionalStringOfLocation } from "../utils/babelhelpers.js";
/**
* Get index property list length by searching array properties list for the max index key value plus 1.
@ -210,7 +210,7 @@ export function handleReportedSideEffect(
): void {
// This causes an infinite recursion because creating a callstack causes internal-only side effects
if (binding && binding.object && binding.object.intrinsicName === "__checkedBindings") return;
let location = getLocationFromValue(expressionLocation);
let location = optionalStringOfLocation(expressionLocation);
if (sideEffectType === "MODIFIED_BINDING") {
let name = binding ? `"${((binding: any): Binding).name}"` : "unknown";

View File

@ -59,8 +59,11 @@ export function memberExpressionHelper(
return t.memberExpression(object, propertyExpression, computed);
}
export function stringOfLocation(location: BabelNodeSourceLocation): string {
return `${location.source || "(unknown source file)"}(${location.start.line}:${location.start.column} ${
location.end.line
}:${location.end.column})`;
export function optionalStringOfLocation(location: ?BabelNodeSourceLocation): string {
// if we can't get a value, then it's likely that the source file was not given
return location ? ` at location ${stringOfLocation(location)}` : "";
}
export function stringOfLocation(location: BabelNodeSourceLocation): string {
return `${location.source || "(unknown source file)"}[${location.start.line}:${location.start.column}]`;
}