Update appearance validation (internal-6344)

GitOrigin-RevId: 2859f84ea7d57598e95573300ddd224b31cee3c2

Author: ibesora

src/style-spec/validate/validate_appearance.ts

@@ -2,8 +2,10 @@ import validateObject from './validate_object';
 import ValidationError from '../error/validation_error';
 import validateProperty from './validate_property';
 import {unbundle} from '../util/unbundle_jsonlint';
+import validateExpression from './validate_expression';
+import latest from '../reference/latest';
 
-import type {StyleSpecification, LayerSpecification} from '../types';
+import type {StyleSpecification, LayerSpecification, AppearanceSpecification} from '../types';
 import type {StyleReference} from '../reference/latest';
 
 export type AppearanceValidatorOptions = {
@@ -19,7 +21,9 @@ export type AppearanceValidatorOptions = {
 
 export default function validateAppearance(options: AppearanceValidatorOptions): Array<ValidationError> {
     const {key, layer, layerType} = options;
-    const value = unbundle(options.value);
+    const value = unbundle(options.value) as AppearanceSpecification;
+    const name = unbundle(value.name);
+    const condition = unbundle(value.condition);
 
     const errors = validateObject({
         key,
@@ -28,10 +32,15 @@ export default function validateAppearance(options: AppearanceValidatorOptions):
         style: options.style,
         styleSpec: options.styleSpec,
         objectElementValidators: {
+            condition: (options) => validateCondition(Object.assign({layer, layerType}, options)),
             properties: (options) => validateProperties(Object.assign({layer, layerType}, options)),
         }
     });
 
+    if (name !== 'hidden' && !condition) {
+        errors.push(new ValidationError(options.key, 'name', `Appearance with name different than "hidden" must have a condition`));
+    }
+
     return errors;
 }
 
@@ -73,3 +82,20 @@ function validateProperties(options: AppearanceValidatorOptions): Array<Validati
 
     return errors;
 }
+
+function validateCondition(options: AppearanceValidatorOptions): Array<ValidationError> {
+    const errors: Array<ValidationError> = [];
+
+    const appearance = options.object as AppearanceSpecification;
+    const condition = appearance.condition;
+
+    errors.push(...validateExpression({
+        key: options.key,
+        value: condition,
+        // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access
+        valueSpec: latest['appearance']['condition'],
+        expressionContext: 'appearance'
+    }));
+
+    return errors;
+}

src/style-spec/validate/validate_expression.ts

@@ -4,10 +4,10 @@ import {deepUnbundle} from '../util/unbundle_jsonlint';
 import {isStateConstant, isGlobalPropertyConstant, isFeatureConstant} from '../expression/is_constant';
 import CompoundExpression from '../expression/compound_expression';
 
+import type {StylePropertySpecification} from '../../style-spec/style-spec';
 import type {Expression} from '../expression/expression';
 import type {StyleReference} from '../reference/latest';
 import type {StyleSpecification} from '../types';
-import type {StylePropertySpecification} from '../style-spec';
 
 export type ExpressionValidatorOptions = {
     key: string;
@@ -17,7 +17,7 @@ export type ExpressionValidatorOptions = {
     propertyType?: 'layout' | 'paint' | 'filter';
     style?: Partial<StyleSpecification>;
     styleSpec?: StyleReference;
-    expressionContext?: 'property' | 'filter' | 'cluster-initial' | 'cluster-reduce' | 'cluster-map';
+    expressionContext?: 'property' | 'filter' | 'cluster-initial' | 'cluster-reduce' | 'cluster-map' | 'appearance';
 };
 
 export default function validateExpression(options: ExpressionValidatorOptions): ValidationError[] {
@@ -48,6 +48,11 @@ export default function validateExpression(options: ExpressionValidatorOptions):
         return disallowedFilterParameters(expressionObj, options);
     }
 
+    if (options.expressionContext === 'appearance') {
+        // eslint-disable-next-line @typescript-eslint/no-unsafe-argument
+        return checkDisallowedParameters(expressionObj, options);
+    }
+
     if (options.expressionContext && options.expressionContext.indexOf('cluster') === 0) {
         // eslint-disable-next-line @typescript-eslint/no-unsafe-argument
         if (!isGlobalPropertyConstant(expressionObj, ['zoom', 'feature-state'])) {
@@ -97,3 +102,34 @@ export function disallowedFilterParameters(e: Expression, options: any): Validat
 
     return errors;
 }
+
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
+function checkDisallowedParameters(e: Expression, options: any): ValidationError[] {
+    const allowedParameters = new Set<string>();
+
+    // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
+    if (options.valueSpec && options.valueSpec.expression) {
+        // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
+        for (const param of options.valueSpec.expression.parameters) {
+            // eslint-disable-next-line @typescript-eslint/no-unsafe-argument
+            allowedParameters.add(param);
+        }
+    }
+
+    if (allowedParameters.size === 0) {
+        return [];
+    }
+    const errors: ValidationError[] = [];
+
+    if (e instanceof CompoundExpression) {
+        if (!allowedParameters.has(e.name)) {
+            // eslint-disable-next-line @typescript-eslint/no-unsafe-argument, @typescript-eslint/no-unsafe-member-access
+            return [new ValidationError(options.key, options.value, `["${e.name}"] is not an allowed parameter`)];
+        }
+    }
+    e.eachChild((arg) => {
+        errors.push(...checkDisallowedParameters(arg, options));
+    });
+
+    return errors;
+}

src/style-spec/validate/validate_layer.ts

@@ -160,14 +160,32 @@ export default function validateLayer(options: LayerValidatorOptions): Validatio
                 });
             },
             appearances(options) {
-                return validateArray({
+                const validationErrors = validateArray({
                     key: options.key,
                     value: options.value,
+                    // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
                     valueSpec: options.valueSpec,
                     style: options.style,
                     styleSpec: options.styleSpec,
                     arrayElementValidator: (options) => validateAppearance(Object.assign({layerType: type, layer}, options) as AppearanceValidatorOptions)
                 });
+                // Check non-repeated names on a given layer
+                const appearances = Array.isArray(options.value) ? options.value : [];
+                const dedupedNames = new Set<string>();
+                appearances.forEach((a, index) => {
+                    // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
+                    const name: string | undefined = unbundle(a.name) as string | undefined;
+                    if (name) {
+                        if (dedupedNames.has(name)) {
+                            const layerId = unbundle((layer as LayerSpecification).id) as string;
+                            validationErrors.push(new ValidationError(options.key, name, `Duplicated appearance name "${name}" for layer "${layerId}"`));
+                        } else {
+                            dedupedNames.add(name);
+                        }
+                    }
+                });
+
+                return validationErrors;
             }
         }
     }));

src/style-spec/validate/validate_object.ts

@@ -11,6 +11,8 @@ type ObjectElementValidatorOptions = {
     valueSpec?: unknown;
     style: Partial<StyleSpecification>;
     styleSpec: StyleReference;
+    object?: object;
+    objectKey?: string;
 };
 
 type ObjectValidatorOptions = {

test/unit/style-spec/fixture/appearances.input.json

@@ -36,6 +36,15 @@
             "icon-opacity": 1,
             "text-color": "red"
           }
+        },
+        {
+          "name": "hidden",
+          "properties": {
+            "icon-image": ["image", "poi", {"params": {"fill": "red"}}],
+            "icon-size": 1.3,
+            "icon-opacity": 1,
+            "text-color": "red"
+          }
         }
       ]
     },
@@ -51,6 +60,46 @@
             "bad": true,
             "icon-opacity": -1
           }
+        },
+        {
+          "name": "not-hidden",
+          "properties": {
+            "icon-opacity": 0.5
+          }
+        },
+        {
+          "condition": ["==", ["line-progress"], 16],
+          "properties": {
+            "icon-opacity": 0.5
+          }
+        }
+      ]
+    },
+    {
+      "id": "appearances-with-repeated-names",
+      "type": "symbol",
+      "source": "vector",
+      "source-layer": "source-layer",
+      "appearances": [
+        {
+          "name": "A",
+          "condition": [">=", ["zoom"], 16],
+          "properties": {
+            "icon-opacity": 1
+          }
+        },
+        {
+          "condition": [">=", ["zoom"], 16],
+          "properties": {
+            "icon-opacity": 1
+          }
+        },
+        {
+          "name": "A",
+          "condition": [">=", ["zoom"], 16],
+          "properties": {
+            "icon-opacity": 1
+          }
         }
       ]
     }

test/unit/style-spec/fixture/appearances.output-api-supported.json

@@ -4,6 +4,16 @@
   },
   {
     "message": "layers[1].appearances[0].properties.icon-opacity: -1 is less than the minimum value 0",
-    "line": 52
+    "line": 61
+  },
+  {
+    "message": "layers[1].appearances[1]: Appearance with name different than \"hidden\" must have a condition"
+  },
+  {
+    "message": "layers[1].appearances[2].condition: [\"line-progress\"] is not an allowed parameter",
+    "line": 71
+  },
+  {
+    "message": "layers[2].appearances: Duplicated appearance name \"A\" for layer \"appearances-with-repeated-names\""
   }
 ]

test/unit/style-spec/fixture/appearances.output.json

@@ -4,6 +4,16 @@
   },
   {
     "message": "layers[1].appearances[0].properties.icon-opacity: -1 is less than the minimum value 0",
-    "line": 52
+    "line": 61
+  },
+  {
+    "message": "layers[1].appearances[1]: Appearance with name different than \"hidden\" must have a condition"
+  },
+  {
+    "message": "layers[1].appearances[2].condition: [\"line-progress\"] is not an allowed parameter",
+    "line": 71
+  },
+  {
+    "message": "layers[2].appearances: Duplicated appearance name \"A\" for layer \"appearances-with-repeated-names\""
   }
 ]