Skip to content

Commit

Permalink
Simplify ParsingContext, add some internal docs
Browse files Browse the repository at this point in the history
  • Loading branch information
Anand Thakker committed Aug 5, 2017
1 parent 0da019a commit cab3b58
Show file tree
Hide file tree
Showing 9 changed files with 60 additions and 34 deletions.
9 changes: 6 additions & 3 deletions src/style-spec/function/compound_expression.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class CompoundExpression implements Expression {
// First parse all the args
const parsedArgs: Array<Expression> = [];
for (const arg of args.slice(1)) {
const parsed = parseExpression(arg, context.concat(1 + parsedArgs.length, op));
const parsed = parseExpression(arg, context.concat(1 + parsedArgs.length));
if (!parsed) return null;
parsedArgs.push(parsed);
}
Expand All @@ -78,7 +78,10 @@ class CompoundExpression implements Expression {
let signatureContext: ParsingContext = (null: any);

for (const [params, compileFromArgs] of overloads) {
signatureContext = new ParsingContext(context.definitions, context.path, context.ancestors, context.scope);
// Use a fresh context for each attempted signature so that, if
// we eventually succeed, we haven't polluted `context.errors`.
signatureContext = new ParsingContext(context.definitions, context.path, context.scope);

if (Array.isArray(params)) {
if (params.length !== parsedArgs.length) {
signatureContext.error(`Expected ${params.length} arguments, but found ${parsedArgs.length} instead.`);
Expand All @@ -89,7 +92,7 @@ class CompoundExpression implements Expression {
for (let i = 0; i < parsedArgs.length; i++) {
const expected = Array.isArray(params) ? params[i] : params.type;
const arg = parsedArgs[i];
match(expected, arg.type, signatureContext.concat(i + 1, op));
match(expected, arg.type, signatureContext.concat(i + 1));
}

if (signatureContext.errors.length === 0) {
Expand Down
2 changes: 1 addition & 1 deletion src/style-spec/function/definitions/array.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class ArrayAssertion implements Expression {

const type = array(itemType, N);

const input = parseExpression(args[args.length - 1], context.concat(args.length - 1, 'array'), ValueType);
const input = parseExpression(args[args.length - 1], context.concat(args.length - 1), ValueType);
if (!input) return null;

return new ArrayAssertion(context.key, type, input);
Expand Down
4 changes: 2 additions & 2 deletions src/style-spec/function/definitions/at.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ class At implements Expression {
if (args.length !== 3)
return context.error(`Expected 2 arguments, but found ${args.length - 1} instead.`);

const index = parseExpression(args[1], context.concat(1, 'at'), NumberType);
const input = parseExpression(args[2], context.concat(2, 'at'), array(expectedType || ValueType));
const index = parseExpression(args[1], context.concat(1), NumberType);
const input = parseExpression(args[2], context.concat(2), array(expectedType || ValueType));

if (!index || !input) return null;

Expand Down
6 changes: 3 additions & 3 deletions src/style-spec/function/definitions/case.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,18 @@ class Case implements Expression {

const branches = [];
for (let i = 0; i < args.length - 1; i += 2) {
const test = parseExpression(args[i], context.concat(i, 'case'), BooleanType);
const test = parseExpression(args[i], context.concat(i), BooleanType);
if (!test) return null;

const result = parseExpression(args[i + 1], context.concat(i + 1, 'case'), outputType);
const result = parseExpression(args[i + 1], context.concat(i + 1), outputType);
if (!result) return null;

branches.push([test, result]);

outputType = outputType || result.type;
}

const otherwise = parseExpression(args[args.length - 1], context.concat(args.length, 'case'), outputType);
const otherwise = parseExpression(args[args.length - 1], context.concat(args.length), outputType);
if (!otherwise) return null;

assert(outputType);
Expand Down
2 changes: 1 addition & 1 deletion src/style-spec/function/definitions/coalesce.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class Coalesce implements Expression {
let outputType = expectedType;
const parsedArgs = [];
for (const arg of args) {
const argContext = context.concat(1 + parsedArgs.length, 'coalesce');
const argContext = context.concat(1 + parsedArgs.length);
const parsed = parseExpression(arg, argContext, outputType);
if (!parsed) return null;
outputType = outputType || parsed.type;
Expand Down
4 changes: 2 additions & 2 deletions src/style-spec/function/definitions/curve.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class Curve implements Expression {
return context.error(`Unknown interpolation type ${String(interpolation[0])}`, 1, 0);
}

input = parseExpression(input, context.concat(2, 'curve'), NumberType);
input = parseExpression(input, context.concat(2), NumberType);
if (!input) return null;

const stops: Stops = [];
Expand All @@ -91,7 +91,7 @@ class Curve implements Expression {
return context.error('Input/output pairs for "curve" expressions must be arranged with input values in strictly ascending order.', i + 3);
}

const parsed = parseExpression(value, context.concat(i + 4, 'curve'), outputType);
const parsed = parseExpression(value, context.concat(i + 4), outputType);
if (!parsed) return null;
outputType = outputType || parsed.type;
stops.push([label, parsed]);
Expand Down
4 changes: 2 additions & 2 deletions src/style-spec/function/definitions/let.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,13 @@ class Let implements Expression {
if (context.definitions[name])
return context.error(`"${name}" is reserved, so it cannot not be used as a "let" binding.`, i + 1);

const value = parseExpression(args[i + 1], context.concat(i + 2, 'let.binding'));
const value = parseExpression(args[i + 1], context.concat(i + 2));
if (!value) return null;

bindings.push([name, value]);
}

const resultContext = context.concat(args.length, 'let.result', bindings);
const resultContext = context.concat(args.length, bindings);
const result = parseExpression(args[args.length - 1], resultContext);
if (!result) return null;

Expand Down
8 changes: 4 additions & 4 deletions src/style-spec/function/definitions/match.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class Match implements Expression {
labels = [labels];
}

const labelContext = context.concat(i + 1, 'match');
const labelContext = context.concat(i + 1);
if (labels.length === 0) {
return labelContext.error('Expected at least one branch label.');
}
Expand All @@ -60,17 +60,17 @@ class Match implements Expression {
}
}

const result = parseExpression(value, context.concat(i + 1, 'match'), outputType);
const result = parseExpression(value, context.concat(i + 1), outputType);
if (!result) return null;
outputType = outputType || result.type;

branches.push([(labels: any), result]);
}

const input = parseExpression(args[0], context.concat(1, 'match'), inputType);
const input = parseExpression(args[0], context.concat(1), inputType);
if (!input) return null;

const otherwise = parseExpression(args[args.length - 1], context.concat(args.length, 'match'), outputType);
const otherwise = parseExpression(args[args.length - 1], context.concat(args.length), outputType);
if (!otherwise) return null;

assert(inputType && outputType);
Expand Down
55 changes: 39 additions & 16 deletions src/style-spec/function/expression.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ class ParsingError extends Error {
}
}

/**
* Tracks `let` bindings during expression parsing.
* @private
*/
class Scope {
parent: ?Scope;
bindings: {[string]: Expression};
Expand Down Expand Up @@ -68,42 +72,62 @@ class Scope {
}
}

/**
* State associated parsing at a given point in an expression tree.
* @private
*/
class ParsingContext {
key: string;
path: Array<number>;
ancestors: Array<string>;
definitions: {[string]: Class<Expression>};
path: Array<number>;
key: string;
scope: Scope;
errors: Array<ParsingError>;

constructor(definitions: *, path: Array<number> = [], ancestors: * = [], scope: Scope = new Scope(), errors: Array<ParsingError> = []) {
constructor(definitions: *, path: Array<number> = [], scope: Scope = new Scope(), errors: Array<ParsingError> = []) {
this.definitions = definitions;
this.path = path;
this.key = path.map(part => `[${part}]`).join('');
this.ancestors = ancestors;
this.scope = scope;
this.errors = errors;
}

// Returns a copy of this context suitable for parsing the subexpression at
// index `index`, optionally appending to 'let' binding map.
//
// Note that `errors` property, intended for collecting errors while
// parsing, is copied by reference rather than cloned.
concat(index: number, expressionName: string, bindings?: Array<[string, Expression]>) {
/**
* Returns a copy of this context suitable for parsing the subexpression at
* index `index`, optionally appending to 'let' binding map.
*
* Note that `errors` property, intended for collecting errors while
* parsing, is copied by reference rather than cloned.
* @private
*/
concat(index: number, bindings?: Array<[string, Expression]>) {
const path = typeof index === 'number' ? this.path.concat(index) : this.path;
const ancestors = expressionName ? this.ancestors.concat(expressionName) : this.ancestors;
const scope = bindings ? this.scope.concat(bindings) : this.scope;
return new ParsingContext(this.definitions, path, ancestors, scope, this.errors);
return new ParsingContext(this.definitions, path, scope, this.errors);
}

/**
* Push a parsing (or type checking) error into the `this.errors`
* @param error The message
* @param keys Optionally specify the source of the error at a child
* of the current expression at `this.key`.
* @private
*/
error(error: string, ...keys: Array<number>) {
const key = `${this.key}${keys.map(k => `[${k}]`).join('')}`;
this.errors.push(new ParsingError(key, error));
return null;
}
}

/**
* Parse the given JSON expression.
*
* @param expectedType If provided, the parsed expression will be checked
* against this type. Additionally, `expectedType` will be pssed to
* Expression#parse(), wherein it may be used to infer child expression types
*
* @private
*/
function parseExpression(expr: mixed, context: ParsingContext, expectedType?: ?Type) : ?Expression {
if (expr === null || typeof expr === 'string' || typeof expr === 'boolean' || typeof expr === 'number') {
expr = ['literal', expr];
Expand Down Expand Up @@ -144,9 +168,8 @@ function parseExpression(expr: mixed, context: ParsingContext, expectedType?: ?T
/**
* Returns null if the type matches, or an error message if not.
*
* Also populate the given typenames context when a generic type is successfully
* matched against a concrete one, with `scope` controlling whether type names
* from `expected` or `t` are to be bound.
* If `context` is provided, then also push the error to it via
* `context.error()`
*
* @private
*/
Expand Down

0 comments on commit cab3b58

Please sign in to comment.