Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify text-font static output validation #9355

Merged
merged 1 commit into from
Mar 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/style-spec/expression/compound_expression.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ class CompoundExpression implements Expression {
this.args.forEach(fn);
}

possibleOutputs() {
return [undefined];
outputDefined() {
return false;
}

serialize(): Array<mixed> {
Expand Down
5 changes: 2 additions & 3 deletions src/style-spec/expression/definitions/assertion.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import {typeOf} from '../values';
import type {Expression} from '../expression';
import type ParsingContext from '../parsing_context';
import type EvaluationContext from '../evaluation_context';
import type {Value} from '../values';
import type {Type} from '../types';

const types = {
Expand Down Expand Up @@ -105,8 +104,8 @@ class Assertion implements Expression {
this.args.forEach(fn);
}

possibleOutputs(): Array<Value | void> {
return [].concat(...this.args.map((arg) => arg.possibleOutputs()));
outputDefined(): boolean {
return this.args.every(arg => arg.outputDefined());
}

serialize(): Array<mixed> {
Expand Down
4 changes: 2 additions & 2 deletions src/style-spec/expression/definitions/at.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ class At implements Expression {
fn(this.input);
}

possibleOutputs() {
return [undefined];
outputDefined() {
return false;
}

serialize() {
Expand Down
7 changes: 2 additions & 5 deletions src/style-spec/expression/definitions/case.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {BooleanType} from '../types';
import type {Expression} from '../expression';
import type ParsingContext from '../parsing_context';
import type EvaluationContext from '../evaluation_context';
import type {Value} from '../values';
import type {Type} from '../types';

type Branches = Array<[Expression, Expression]>;
Expand Down Expand Up @@ -72,10 +71,8 @@ class Case implements Expression {
fn(this.otherwise);
}

possibleOutputs(): Array<Value | void> {
return []
.concat(...this.branches.map(([_, out]) => out.possibleOutputs()))
.concat(this.otherwise.possibleOutputs());
outputDefined(): boolean {
return this.branches.every(([_, out]) => out.outputDefined()) && this.otherwise.outputDefined();
}

serialize() {
Expand Down
5 changes: 2 additions & 3 deletions src/style-spec/expression/definitions/coalesce.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import ResolvedImage from '../types/resolved_image';
import type {Expression} from '../expression';
import type ParsingContext from '../parsing_context';
import type EvaluationContext from '../evaluation_context';
import type {Value} from '../values';
import type {Type} from '../types';

class Coalesce implements Expression {
Expand Down Expand Up @@ -80,8 +79,8 @@ class Coalesce implements Expression {
this.args.forEach(fn);
}

possibleOutputs(): Array<Value | void> {
return [].concat(...this.args.map((arg) => arg.possibleOutputs()));
outputDefined(): boolean {
return this.args.every(arg => arg.outputDefined());
}

serialize() {
Expand Down
5 changes: 2 additions & 3 deletions src/style-spec/expression/definitions/coercion.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import ResolvedImage from '../types/resolved_image';
import type {Expression} from '../expression';
import type ParsingContext from '../parsing_context';
import type EvaluationContext from '../evaluation_context';
import type {Value} from '../values';
import type {Type} from '../types';

const types = {
Expand Down Expand Up @@ -112,8 +111,8 @@ class Coercion implements Expression {
this.args.forEach(fn);
}

possibleOutputs(): Array<Value | void> {
return [].concat(...this.args.map((arg) => arg.possibleOutputs()));
outputDefined(): boolean {
return this.args.every(arg => arg.outputDefined());
}

serialize() {
Expand Down
8 changes: 4 additions & 4 deletions src/style-spec/expression/definitions/collator.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,12 @@ export default class CollatorExpression implements Expression {
}
}

possibleOutputs() {
outputDefined() {
// Technically the set of possible outputs is the combinatoric set of Collators produced
// by all possibleOutputs of locale/caseSensitive/diacriticSensitive
// by all possible outputs of locale/caseSensitive/diacriticSensitive
// But for the primary use of Collators in comparison operators, we ignore the Collator's
// possibleOutputs anyway, so we can get away with leaving this undefined for now.
return [undefined];
// possible outputs anyway, so we can get away with leaving this false for now.
return false;
}

serialize() {
Expand Down
4 changes: 2 additions & 2 deletions src/style-spec/expression/definitions/comparison.js
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,8 @@ function makeComparison(op: ComparisonOperator, compareBasic, compareWithCollato
}
}

possibleOutputs() {
return [true, false];
outputDefined(): boolean {
return true;
}

serialize() {
Expand Down
4 changes: 2 additions & 2 deletions src/style-spec/expression/definitions/format.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,10 @@ export default class FormatExpression implements Expression {
}
}

possibleOutputs() {
outputDefined() {
// Technically the combinatoric set of all children
// Usually, this.text will be undefined anyway
return [undefined];
return false;
}

serialize() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import assert from 'assert';
import type {Expression} from '../expression';
import type EvaluationContext from '../evaluation_context';
import type {Value} from '../values';
import type {Type} from '../types';
import type {ZoomConstantExpression} from '../../expression';
import {NullType} from '../types';
Expand Down Expand Up @@ -43,8 +42,8 @@ export default class FormatSectionOverride<T> implements Expression {
}

// Cannot be statically evaluated, as the output depends on the evaluation context.
possibleOutputs(): Array<Value | void> {
return [undefined];
outputDefined() {
return false;
}

serialize() {
Expand Down
4 changes: 2 additions & 2 deletions src/style-spec/expression/definitions/image.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ export default class ImageExpression implements Expression {
fn(this.input);
}

possibleOutputs() {
outputDefined() {
// The output of image is determined by the list of available images in the evaluation context
return [undefined];
return false;
}

serialize() {
Expand Down
4 changes: 2 additions & 2 deletions src/style-spec/expression/definitions/in.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ class In implements Expression {
fn(this.haystack);
}

possibleOutputs() {
return [true, false];
outputDefined() {
return true;
}

serialize() {
Expand Down
5 changes: 2 additions & 3 deletions src/style-spec/expression/definitions/interpolate.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import type {Stops} from '../stops';
import type {Expression} from '../expression';
import type ParsingContext from '../parsing_context';
import type EvaluationContext from '../evaluation_context';
import type {Value} from '../values';
import type {Type} from '../types';

export type InterpolationType =
Expand Down Expand Up @@ -187,8 +186,8 @@ class Interpolate implements Expression {
}
}

possibleOutputs(): Array<Value | void> {
return [].concat(...this.outputs.map((output) => output.possibleOutputs()));
outputDefined(): boolean {
return this.outputs.every(out => out.outputDefined());
}

serialize(): Array<mixed> {
Expand Down
4 changes: 2 additions & 2 deletions src/style-spec/expression/definitions/length.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ class Length implements Expression {
fn(this.input);
}

possibleOutputs() {
return [undefined];
outputDefined() {
return false;
}

serialize() {
Expand Down
4 changes: 2 additions & 2 deletions src/style-spec/expression/definitions/let.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ class Let implements Expression {
return new Let(bindings, result);
}

possibleOutputs() {
return this.result.possibleOutputs();
outputDefined() {
return this.result.outputDefined();
}

serialize() {
Expand Down
4 changes: 2 additions & 2 deletions src/style-spec/expression/definitions/literal.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ class Literal implements Expression {

eachChild() {}

possibleOutputs() {
return [this.value];
outputDefined() {
return true;
}

serialize(): Array<mixed> {
Expand Down
7 changes: 2 additions & 5 deletions src/style-spec/expression/definitions/match.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {ValueType, type Type} from '../types';
import type {Expression} from '../expression';
import type ParsingContext from '../parsing_context';
import type EvaluationContext from '../evaluation_context';
import type {Value} from '../values';

// Map input label values to output expression index
type Cases = {[number | string]: number};
Expand Down Expand Up @@ -112,10 +111,8 @@ class Match implements Expression {
fn(this.otherwise);
}

possibleOutputs(): Array<Value | void> {
return []
.concat(...this.outputs.map((out) => out.possibleOutputs()))
.concat(this.otherwise.possibleOutputs());
outputDefined(): boolean {
return this.outputs.every(out => out.outputDefined()) && this.otherwise.outputDefined();
}

serialize(): Array<mixed> {
Expand Down
4 changes: 2 additions & 2 deletions src/style-spec/expression/definitions/number_format.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ export default class NumberFormat implements Expression {
}
}

possibleOutputs() {
return [undefined];
outputDefined() {
return false;
}

serialize() {
Expand Down
5 changes: 2 additions & 3 deletions src/style-spec/expression/definitions/step.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import type {Stops} from '../stops';
import type {Expression} from '../expression';
import type ParsingContext from '../parsing_context';
import type EvaluationContext from '../evaluation_context';
import type {Value} from '../values';
import type {Type} from '../types';

class Step implements Expression {
Expand Down Expand Up @@ -102,8 +101,8 @@ class Step implements Expression {
}
}

possibleOutputs(): Array<Value | void> {
return [].concat(...this.outputs.map((output) => output.possibleOutputs()));
outputDefined(): boolean {
return this.outputs.every(out => out.outputDefined());
}

serialize() {
Expand Down
4 changes: 2 additions & 2 deletions src/style-spec/expression/definitions/var.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ class Var implements Expression {

eachChild() {}

possibleOutputs() {
return [undefined];
outputDefined() {
return false;
}

serialize() {
Expand Down
6 changes: 2 additions & 4 deletions src/style-spec/expression/expression.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// @flow

import type {Type} from './types';
import type {Value} from './values';
import type ParsingContext from './parsing_context';
import type EvaluationContext from './evaluation_context';

Expand All @@ -16,10 +15,9 @@ export interface Expression {

/**
* Statically analyze the expression, attempting to enumerate possible outputs. Returns
* an array of values plus the sentinel value `undefined`, used to indicate that the
* complete set of outputs is statically undecidable.
* false if the complete set of outputs is statically undecidable, otherwise true.
*/
possibleOutputs(): Array<Value | void>;
outputDefined(): boolean;

serialize(): SerializedExpression;
}
Expand Down
2 changes: 1 addition & 1 deletion src/style-spec/validate/validate_expression.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export default function validateExpression(options: any): Array<ValidationError>
const expressionObj = (expression.value: any).expression || (expression.value: any)._styleExpression.expression;

if (options.expressionContext === 'property' && (options.propertyKey === 'text-font') &&
expressionObj.possibleOutputs().indexOf(undefined) !== -1) {
!expressionObj.outputDefined()) {
return [new ValidationError(options.key, options.value, `Invalid data expression for "${options.propertyKey}". Output values must be contained as literals within the expression.`)];
}

Expand Down