Add more consistency checks, slight refactoring of theme conversions

Logic/Tags/TagUtils.ts

@@ -285,6 +285,10 @@ export class TagUtils {
             throw `Error while parsing tag '${tag}' in ${context}: no key part and value part were found`
 
         }
+        
+        if(json.and !== undefined && json.or !== undefined){
+            throw `Error while parsing a TagConfig: got an object where both 'and' and 'or' are defined`}
+        
         if (json.and !== undefined) {
             return new And(json.and.map(t => TagUtils.Tag(t, context)));
         }

Models/ThemeConfig/Conversion/Conversion.ts

@@ -18,9 +18,9 @@ export abstract class Conversion<TIn, TOut> {
         this.name = name ?? this.constructor.name
     }
 
-    public static strict<T>(fixed: { errors: string[], warnings: string[], result?: T }): T {
+    public static strict<T>(fixed: { errors?: string[], warnings?: string[], result?: T }): T {
-        if (fixed?.errors?.length > 0) {
+        if (fixed?.errors !== undefined &&  fixed?.errors?.length > 0) {
-            throw fixed.errors.join("\n");
+            throw fixed.errors.join("\n\n");
         }
         fixed.warnings?.forEach(w => console.warn(w))
         return fixed.result;
@@ -31,9 +31,12 @@ export abstract class Conversion<TIn, TOut> {
         return DesugaringStep.strict(fixed)
     }
 
-    abstract convert(state: DesugaringContext, json: TIn, context: string): { result: TOut, errors: string[], warnings: string[] }
+    abstract convert(state: DesugaringContext, json: TIn, context: string): { result: TOut, errors?: string[], warnings?: string[] }
 
     public convertAll(state: DesugaringContext, jsons: TIn[], context: string): { result: TOut[], errors: string[], warnings: string[] } {
+        if(jsons === undefined){
+            throw "convertAll received undefined - don't do this (at "+context+")"
+        }
         const result = []
         const errors = []
         const warnings = []
@@ -41,8 +44,8 @@ export abstract class Conversion<TIn, TOut> {
             const json = jsons[i];
             const r = this.convert(state, json, context + "[" + i + "]")
             result.push(r.result)
-            errors.push(...r.errors)
+            errors.push(...r.errors ?? [])
-            warnings.push(...r.warnings)
+            warnings.push(...r.warnings ?? [])
         }
         return {
             result,
@@ -66,7 +69,7 @@ export class OnEvery<X, T> extends DesugaringStep<T> {
         this.key = key;
     }
 
-    convert(state: DesugaringContext, json: T, context: string): { result: T; errors: string[]; warnings: string[] } {
+    convert(state: DesugaringContext, json: T, context: string): { result: T; errors?: string[]; warnings?: string[] } {
         json = {...json}
         const step = this.step
         const key = this.key;
@@ -132,8 +135,8 @@ export class Fuse<T> extends DesugaringStep<T> {
         for (let i = 0; i < this.steps.length; i++) {
             const step = this.steps[i];
             let r = step.convert(state, json, "While running step " +step.name + ": " + context)
-            errors.push(...r.errors)
+            errors.push(...r.errors ?? [])
-            warnings.push(...r.warnings)
+            warnings.push(...r.warnings ?? [])
             json = r.result
             if (errors.length > 0) {
                 break;

Models/ThemeConfig/Conversion/CreateNoteImportLayer.ts

@@ -45,6 +45,9 @@ export default class CreateNoteImportLayer extends Conversion<LayerConfigJson, L
 
         const pointRenderings = (layerJson.mapRendering ?? []).filter(r => r !== null && r["location"] !== undefined);
         const firstRender = <PointRenderingConfigJson>(pointRenderings [0])
+        if(firstRender === undefined){
+            throw `Layer ${layerJson.id} does not have a pointRendering: `+context
+        }
         const icon = firstRender.icon
         const iconBadges = []
         const title = layer.presets[0].title

Models/ThemeConfig/Conversion/LegacyJsonConvert.ts

@@ -1,11 +1,7 @@
 import {LayoutConfigJson} from "../Json/LayoutConfigJson";
-import LayerConfig from "../LayerConfig";
-import {Translation} from "../../../UI/i18n/Translation";
-import LayoutConfig from "../LayoutConfig";
 import {Utils} from "../../../Utils";
 import LineRenderingConfigJson from "../Json/LineRenderingConfigJson";
 import {LayerConfigJson} from "../Json/LayerConfigJson";
-import Constants from "../../Constants";
 import {DesugaringContext, DesugaringStep, Fuse, OnEvery} from "./Conversion";
 
 export class UpdateLegacyLayer extends DesugaringStep<LayerConfigJson | string | { builtin, override }> {
@@ -161,232 +157,3 @@ export class FixLegacyTheme extends Fuse<LayoutConfigJson> {
         );
     }
 }
-
-export class ValidateLayer extends DesugaringStep<LayerConfigJson> {
-    /**
-     * The paths where this layer is originally saved. Triggers some extra checks
-     * @private
-     */
-    private readonly _path?: string;
-    private readonly knownImagePaths?: Set<string>;
-    private readonly _isBuiltin: boolean;
-
-    constructor(knownImagePaths: Set<string>, path: string, isBuiltin: boolean) {
-        super("Doesn't change anything, but emits warnings and errors", []);
-        this.knownImagePaths = knownImagePaths;
-        this._path = path;
-        this._isBuiltin = isBuiltin;
-    }
-
-    convert(state: DesugaringContext, json: LayerConfigJson, context: string): { result: LayerConfigJson; errors: string[]; warnings: string[] } {
-        const errors = []
-        const warnings = []
-
-        if (typeof json === "string") {
-            errors.push(context + ": This layer hasn't been expanded: " + json)
-            return {
-                result: null,
-                warnings: [],
-                errors
-            }
-        }
-
-        if (json["builtin"] !== undefined) {
-            errors.push(context + ": This layer hasn't been expanded: " + json)
-            return {
-                result: null,
-                warnings: [],
-                errors
-            }
-        }
-
-        try {
-            {
-                // Some checks for legacy elements
-
-                if (json["overpassTags"] !== undefined) {
-                    errors.push("Layer " + json.id + "still uses the old 'overpassTags'-format. Please use \"source\": {\"osmTags\": <tags>}' instead of \"overpassTags\": <tags> (note: this isn't your fault, the custom theme generator still spits out the old format)")
-                }
-                const forbiddenTopLevel = ["icon", "wayHandling", "roamingRenderings", "roamingRendering", "label", "width", "color", "colour", "iconOverlays"]
-                for (const forbiddenKey of forbiddenTopLevel) {
-                    if (json[forbiddenKey] !== undefined)
-                        errors.push(context + ": layer " + json.id + " still has a forbidden key " + forbiddenKey)
-                }
-                if (json["hideUnderlayingFeaturesMinPercentage"] !== undefined) {
-                    errors.push(context + ": layer " + json.id + " contains an old 'hideUnderlayingFeaturesMinPercentage'")
-                }
-            }
-            {
-                const layer = new LayerConfig(json, "test", true)
-                const images = Array.from(layer.ExtractImages())
-                const remoteImages = images.filter(img => img.indexOf("http") == 0)
-                for (const remoteImage of remoteImages) {
-                    errors.push("Found a remote image: " + remoteImage + " in layer " + layer.id + ", please download it. You can use the fixTheme script to automate this")
-                }
-                for (const image of images) {
-                    if (image.indexOf("{") >= 0) {
-                        warnings.push("Ignoring image with { in the path: ", image)
-                        continue
-                    }
-
-                    if (this.knownImagePaths !== undefined && !this.knownImagePaths.has(image)) {
-                        const ctx = context === undefined ? "" : ` in a layer defined in the theme ${context}`
-                        errors.push(`Image with path ${image} not found or not attributed; it is used in ${layer.id}${ctx}`)
-                    }
-                }
-
-            }
-            {
-                // CHeck location
-                const expected: string = `assets/layers/${json.id}/${json.id}.json`
-                if (this._path != undefined && this._path.indexOf(expected) < 0) {
-                    errors.push("Layer is in an incorrect place. The path is " + this._path + ", but expected " + expected)
-                }
-            }
-
-
-            if (this._isBuiltin) {
-                if (json.tagRenderings?.some(tr => tr["id"] === "")) {
-                    const emptyIndexes: number[] = []
-                    for (let i = 0; i < json.tagRenderings.length; i++) {
-                        const tagRendering = json.tagRenderings[i];
-                        if (tagRendering["id"] === "") {
-                            emptyIndexes.push(i)
-                        }
-                    }
-                    errors.push(`Some tagrendering-ids are empty or have an emtpy string; this is not allowed (at ${context}.tagRenderings.[${emptyIndexes.join(",")}])`)
-                }
-
-                const duplicateIds = Utils.Dupiclates((json.tagRenderings ?? [])?.map(f => f["id"]).filter(id => id !== "questions"))
-                if (duplicateIds.length > 0 && !Utils.runningFromConsole) {
-                    errors.push(`Some tagRenderings have a duplicate id: ${duplicateIds} (at ${context}.tagRenderings)`)
-                }
-
-
-                if (json.description === undefined) {
-
-                    if (Constants.priviliged_layers.indexOf(json.id) >= 0) {
-                        errors.push(
-                            context + ": A priviliged layer must have a description"
-                        )
-                    } else {
-                        warnings.push(
-                            context + ": A builtin layer should have a description"
-                        )
-                    }
-                }
-            }
-        } catch (e) {
-            errors.push(e)
-        }
-        return {
-            result: undefined,
-            errors,
-            warnings
-        };
-    }
-}
-
-class ValidateLanguageCompleteness extends DesugaringStep<any> {
-    private readonly _languages: string[];
-
-    constructor(...languages: string[]) {
-        super("Checks that the given object is fully translated in the specified languages", []);
-        this._languages = languages;
-    }
-
-    convert(state: DesugaringContext, obj: any, context: string): { result: LayerConfig; errors: string[]; warnings: string[] } {
-        const errors = []
-        const translations = Translation.ExtractAllTranslationsFrom(
-            obj
-        )
-        for (const neededLanguage of this._languages) {
-            translations
-                .filter(t => t.tr.translations[neededLanguage] === undefined && t.tr.translations["*"] === undefined)
-                .forEach(missing => {
-                    errors.push(context + "A theme should be translation-complete for " + neededLanguage + ", but it lacks a translation for " + missing.context + ".\n\tThe english translation is " + missing.tr.textFor('en'))
-                })
-        }
-
-        return {
-            result: obj,
-            warnings: [], errors
-        };
-    }
-}
-
-class ValidateTheme extends DesugaringStep<LayoutConfigJson> {
-    /**
-     * The paths where this layer is originally saved. Triggers some extra checks
-     * @private
-     */
-    private readonly _path?: string;
-    private readonly knownImagePaths: Set<string>;
-    private readonly _isBuiltin: boolean;
-
-    constructor(knownImagePaths: Set<string>, path: string, isBuiltin: boolean) {
-        super("Doesn't change anything, but emits warnings and errors", []);
-        this.knownImagePaths = knownImagePaths;
-        this._path = path;
-        this._isBuiltin = isBuiltin;
-    }
-
-    convert(state: DesugaringContext, json: LayoutConfigJson, context: string): { result: LayoutConfigJson; errors: string[]; warnings: string[] } {
-        const errors = []
-        const warnings = []
-        {
-            // Legacy format checks  
-            if (this._isBuiltin) {
-                if (json["units"] !== undefined) {
-                    errors.push("The theme " + json.id + " has units defined - these should be defined on the layer instead. (Hint: use overrideAll: { '+units': ... }) ")
-                }
-                if (json["roamingRenderings"] !== undefined) {
-                    errors.push("Theme " + json.id + " contains an old 'roamingRenderings'. Use an 'overrideAll' instead")
-                }
-            }
-        }
-
-        try {
-            const theme = new LayoutConfig(json, true, "test")
-            if (theme.id !== theme.id.toLowerCase()) {
-                errors.push("Theme ids should be in lowercase, but it is " + theme.id)
-            }
-
-            const filename = this._path.substring(this._path.lastIndexOf("/") + 1, this._path.length - 5)
-            if (theme.id !== filename) {
-                errors.push("Theme ids should be the same as the name.json, but we got id: " + theme.id + " and filename " + filename + " (" + this._path + ")")
-            }
-            if (!this.knownImagePaths.has(theme.icon)) {
-                errors.push("The theme image " + theme.icon + " is not attributed or not saved locally")
-            }
-            const dups = Utils.Dupiclates(json.layers.map(layer => layer["id"]))
-            if (dups.length > 0) {
-                errors.push(`The theme ${json.id} defines multiple layers with id ${dups.join(", ")}`)
-            }
-            if (json["mustHaveLanguage"] !== undefined) {
-                const checked = new ValidateLanguageCompleteness(...json["mustHaveLanguage"])
-                    .convert(state, theme, theme.id)
-                errors.push(...checked.errors)
-                warnings.push(...checked.warnings)
-            }
-
-        } catch (e) {
-            errors.push(e)
-        }
-
-        return {
-            result: json,
-            errors,
-            warnings
-        };
-    }
-}
-
-export class ValidateThemeAndLayers extends Fuse<LayoutConfigJson> {
-    constructor(knownImagePaths: Set<string>, path: string, isBuiltin: boolean) {
-        super("Validates a theme and the contained layers",
-            new ValidateTheme(knownImagePaths, path, isBuiltin),
-            new OnEvery("layers", new ValidateLayer(knownImagePaths, undefined, false))
-        );
-    }
-}

Models/ThemeConfig/Conversion/PrepareTheme.ts

@@ -10,21 +10,35 @@ import LayerConfig from "../LayerConfig";
 import {TagRenderingConfigJson} from "../Json/TagRenderingConfigJson";
 import {SubstitutedTranslation} from "../../../UI/SubstitutedTranslation";
 import DependencyCalculator from "../DependencyCalculator";
+import {ValidateThemeAndLayers} from "./Validation";
 
 class SubstituteLayer extends Conversion<(string | LayerConfigJson), LayerConfigJson[]> {
     constructor() {
         super("Converts the identifier of a builtin layer into the actual layer, or converts a 'builtin' syntax with override in the fully expanded form", []);
     }
-
+    
     convert(state: DesugaringContext, json: string | LayerConfigJson, context: string): { result: LayerConfigJson[]; errors: string[]; warnings: string[] } {
         const errors = []
         const warnings = []
+        
+        function reportNotFound(name: string){
+            const knownLayers = Array.from(state.sharedLayers.keys())
+            const withDistance = knownLayers.map(lname => [lname,  Utils.levenshteinDistance(name, lname)])
+            withDistance.sort((a, b) => a[1] - b[1])
+            const ids = withDistance.map(n => n[0])
+           // Known builtin layers are "+.join(",")+"\n    For more information, see "
+            errors.push(`${context}: The layer with name ${name} was not found as a builtin layer. Perhaps you meant ${ids[0]}, ${ids[1]} or ${ids[2]}?
+ For an overview of all available layers, refer to https://github.com/pietervdvn/MapComplete/blob/develop/Docs/BuiltinLayers.md`)
+        }
+        
+        
         if (typeof json === "string") {
             const found = state.sharedLayers.get(json)
             if (found === undefined) {
+                reportNotFound(json)
                 return {
                     result: null,
-                    errors: [context + ": The layer with name " + json + " was not found as a builtin layer"],
+                    errors,
                     warnings
                 }
             }
@@ -43,7 +57,7 @@ class SubstituteLayer extends Conversion<(string | LayerConfigJson), LayerConfig
             for (const name of names) {
                 const found = Utils.Clone(state.sharedLayers.get(name))
                 if (found === undefined) {
-                    errors.push(context + ": The layer with name " + JSON.stringify(json) + " was not found as a builtin layer. Known builtin layers are "+Array.from(state.sharedLayers.keys()).join(",")+"\n    For more information, see https://github.com/pietervdvn/MapComplete/blob/develop/Docs/BuiltinLayers.md" )
+                    reportNotFound(name)
                     continue
                 }
                 if (json["override"]["tagRenderings"] !== undefined && (found["tagRenderings"] ?? []).length > 0) {

Models/ThemeConfig/Conversion/Validation.ts

@@ -0,0 +1,334 @@
+import {DesugaringContext, DesugaringStep, Fuse, OnEvery} from "./Conversion";
+import {LayerConfigJson} from "../Json/LayerConfigJson";
+import LayerConfig from "../LayerConfig";
+import {Utils} from "../../../Utils";
+import Constants from "../../Constants";
+import {Translation} from "../../../UI/i18n/Translation";
+import {LayoutConfigJson} from "../Json/LayoutConfigJson";
+import LayoutConfig from "../LayoutConfig";
+import {TagRenderingConfigJson} from "../Json/TagRenderingConfigJson";
+import {TagUtils} from "../../../Logic/Tags/TagUtils";
+import {parseString} from "xml2js";
+
+
+class ValidateLanguageCompleteness extends DesugaringStep<any> {
+    private readonly _languages: string[];
+
+    constructor(...languages: string[]) {
+        super("Checks that the given object is fully translated in the specified languages", []);
+        this._languages = languages;
+    }
+
+    convert(state: DesugaringContext, obj: any, context: string): { result: LayerConfig; errors: string[]; warnings: string[] } {
+        const errors = []
+        const translations = Translation.ExtractAllTranslationsFrom(
+            obj
+        )
+        for (const neededLanguage of this._languages) {
+            translations
+                .filter(t => t.tr.translations[neededLanguage] === undefined && t.tr.translations["*"] === undefined)
+                .forEach(missing => {
+                    errors.push(context + "A theme should be translation-complete for " + neededLanguage + ", but it lacks a translation for " + missing.context + ".\n\tThe english translation is " + missing.tr.textFor('en'))
+                })
+        }
+
+        return {
+            result: obj,
+            warnings: [], errors
+        };
+    }
+}
+
+class ValidateTheme extends DesugaringStep<LayoutConfigJson> {
+    /**
+     * The paths where this layer is originally saved. Triggers some extra checks
+     * @private
+     */
+    private readonly _path?: string;
+    private readonly knownImagePaths: Set<string>;
+    private readonly _isBuiltin: boolean;
+
+    constructor(knownImagePaths: Set<string>, path: string, isBuiltin: boolean) {
+        super("Doesn't change anything, but emits warnings and errors", []);
+        this.knownImagePaths = knownImagePaths;
+        this._path = path;
+        this._isBuiltin = isBuiltin;
+    }
+
+    convert(state: DesugaringContext, json: LayoutConfigJson, context: string): { result: LayoutConfigJson; errors: string[]; warnings: string[] } {
+        const errors = []
+        const warnings = []
+        {
+            // Legacy format checks  
+            if (this._isBuiltin) {
+                if (json["units"] !== undefined) {
+                    errors.push("The theme " + json.id + " has units defined - these should be defined on the layer instead. (Hint: use overrideAll: { '+units': ... }) ")
+                }
+                if (json["roamingRenderings"] !== undefined) {
+                    errors.push("Theme " + json.id + " contains an old 'roamingRenderings'. Use an 'overrideAll' instead")
+                }
+            }
+        }
+
+        try {
+            const theme = new LayoutConfig(json, true, "test")
+            if (theme.id !== theme.id.toLowerCase()) {
+                errors.push("Theme ids should be in lowercase, but it is " + theme.id)
+            }
+
+            const filename = this._path.substring(this._path.lastIndexOf("/") + 1, this._path.length - 5)
+            if (theme.id !== filename) {
+                errors.push("Theme ids should be the same as the name.json, but we got id: " + theme.id + " and filename " + filename + " (" + this._path + ")")
+            }
+            if (!this.knownImagePaths.has(theme.icon)) {
+                errors.push("The theme image " + theme.icon + " is not attributed or not saved locally")
+            }
+            const dups = Utils.Dupiclates(json.layers.map(layer => layer["id"]))
+            if (dups.length > 0) {
+                errors.push(`The theme ${json.id} defines multiple layers with id ${dups.join(", ")}`)
+            }
+            if (json["mustHaveLanguage"] !== undefined) {
+                const checked = new ValidateLanguageCompleteness(...json["mustHaveLanguage"])
+                    .convert(state, theme, theme.id)
+                errors.push(...checked.errors)
+                warnings.push(...checked.warnings)
+            }
+
+        } catch (e) {
+            errors.push(e)
+        }
+
+        return {
+            result: json,
+            errors,
+            warnings
+        };
+    }
+}
+
+export class ValidateThemeAndLayers extends Fuse<LayoutConfigJson> {
+    constructor(knownImagePaths: Set<string>, path: string, isBuiltin: boolean) {
+        super("Validates a theme and the contained layers",
+            new ValidateTheme(knownImagePaths, path, isBuiltin),
+            new OnEvery("layers", new ValidateLayer(knownImagePaths, undefined, false))
+        );
+    }
+}
+
+
+class OverrideShadowingCheck extends DesugaringStep<LayoutConfigJson>{
+    
+    constructor() {
+        super("Checks that an 'overrideAll' does not override a single override");
+    }
+
+    convert(state: DesugaringContext, json: LayoutConfigJson, context: string): { result: LayoutConfigJson; errors?: string[]; warnings?: string[] } {
+        
+        const overrideAll = json.overrideAll;
+        if(overrideAll === undefined){
+            return {result: json}
+        }
+        
+        const errors = []
+        const withOverride = json.layers.filter(l =>  l["override"] !== undefined)
+
+        for (const layer of withOverride) {
+            for (const key in overrideAll) {
+               if(layer["override"][key] !== undefined || layer["override"]["="+key] !== undefined){
+                  const w = "The override of layer "+JSON.stringify(layer["builtin"])+" has a shadowed property: "+key+" is overriden by overrideAll of the theme";
+                errors.push(w)
+               } 
+            }
+        }
+        
+        return  {result: json, errors}
+    }
+    
+    
+    
+}
+
+export class PrevalidateTheme extends Fuse<LayoutConfigJson>{
+
+    constructor() {
+        super("Various consistency checks on the raw JSON",
+            new OverrideShadowingCheck()
+            );
+        
+    }
+
+
+}
+
+export class DetectShadowedMappings extends DesugaringStep<TagRenderingConfigJson>{
+    constructor() {
+        super("Checks that the mappings don't shadow each other");
+    }
+    
+    convert(state: DesugaringContext, json: TagRenderingConfigJson, context: string): { result: TagRenderingConfigJson; errors?: string[]; warnings?: string[] } {
+        const errors = []
+        if(json.mappings === undefined || json.mappings.length === 0){
+            return {result: json}
+        }
+        const parsedConditions = json.mappings.map(m => TagUtils.Tag(m.if))
+        for (let i = 0; i < json.mappings.length; i++){
+            if(!parsedConditions[i].isUsableAsAnswer()){
+                continue
+            }
+            const keyValues = parsedConditions[i].asChange({});
+            const properties = []
+            keyValues.forEach(({k, v}) => {
+                properties[k] = v
+            })
+            for (let j = 0; j < i; j++){
+                const doesMatch = parsedConditions[j].matchesProperties(properties)
+                if(doesMatch){
+                    // The current mapping is shadowed!
+                    errors.push(`Mapping ${i} is shadowed by mapping ${j} and will thus never be shown:
+    The mapping ${parsedConditions[i].asHumanString(false,false, {})} is fully matched by a previous mapping, which matches:
+    ${parsedConditions[j].asHumanString(false, false, {})}.
+    
+    Move the mapping up to fix this problem
+`)
+                }
+            }
+
+        }
+         
+        return {
+            errors,
+            result: json
+        };
+    }
+}
+
+export class ValidateLayer extends DesugaringStep<LayerConfigJson> {
+    /**
+     * The paths where this layer is originally saved. Triggers some extra checks
+     * @private
+     */
+    private readonly _path?: string;
+    private readonly knownImagePaths?: Set<string>;
+    private readonly _isBuiltin: boolean;
+
+    constructor(knownImagePaths: Set<string>, path: string, isBuiltin: boolean) {
+        super("Doesn't change anything, but emits warnings and errors", []);
+        this.knownImagePaths = knownImagePaths;
+        this._path = path;
+        this._isBuiltin = isBuiltin;
+    }
+
+    convert(state: DesugaringContext, json: LayerConfigJson, context: string): { result: LayerConfigJson; errors: string[]; warnings?: string[] } {
+        const errors = []
+        const warnings = []
+
+        if (typeof json === "string") {
+            errors.push(context + ": This layer hasn't been expanded: " + json)
+            return {
+                result: null,
+                errors
+            }
+        }
+
+        if (json["builtin"] !== undefined) {
+            errors.push(context + ": This layer hasn't been expanded: " + json)
+            return {
+                result: null,
+                errors
+            }
+        }
+
+        try {
+            {
+                // Some checks for legacy elements
+
+                if (json["overpassTags"] !== undefined) {
+                    errors.push("Layer " + json.id + "still uses the old 'overpassTags'-format. Please use \"source\": {\"osmTags\": <tags>}' instead of \"overpassTags\": <tags> (note: this isn't your fault, the custom theme generator still spits out the old format)")
+                }
+                const forbiddenTopLevel = ["icon", "wayHandling", "roamingRenderings", "roamingRendering", "label", "width", "color", "colour", "iconOverlays"]
+                for (const forbiddenKey of forbiddenTopLevel) {
+                    if (json[forbiddenKey] !== undefined)
+                        errors.push(context + ": layer " + json.id + " still has a forbidden key " + forbiddenKey)
+                }
+                if (json["hideUnderlayingFeaturesMinPercentage"] !== undefined) {
+                    errors.push(context + ": layer " + json.id + " contains an old 'hideUnderlayingFeaturesMinPercentage'")
+                }
+            }
+            {
+                // Check for remote images
+                const layer = new LayerConfig(json, "test", true)
+                const images = Array.from(layer.ExtractImages())
+                const remoteImages = images.filter(img => img.indexOf("http") == 0)
+                for (const remoteImage of remoteImages) {
+                    errors.push("Found a remote image: " + remoteImage + " in layer " + layer.id + ", please download it. You can use the fixTheme script to automate this")
+                }
+                for (const image of images) {
+                    if (image.indexOf("{") >= 0) {
+                        warnings.push("Ignoring image with { in the path: ", image)
+                        continue
+                    }
+
+                    if (this.knownImagePaths !== undefined && !this.knownImagePaths.has(image)) {
+                        const ctx = context === undefined ? "" : ` in a layer defined in the theme ${context}`
+                        errors.push(`Image with path ${image} not found or not attributed; it is used in ${layer.id}${ctx}`)
+                    }
+                }
+
+            }
+            {
+                // CHeck location of layer file
+                const expected: string = `assets/layers/${json.id}/${json.id}.json`
+                if (this._path != undefined && this._path.indexOf(expected) < 0) {
+                    errors.push("Layer is in an incorrect place. The path is " + this._path + ", but expected " + expected)
+                }
+            }
+            if (this._isBuiltin) {
+                // Check for correct IDs
+                if (json.tagRenderings?.some(tr => tr["id"] === "")) {
+                    const emptyIndexes: number[] = []
+                    for (let i = 0; i < json.tagRenderings.length; i++) {
+                        const tagRendering = json.tagRenderings[i];
+                        if (tagRendering["id"] === "") {
+                            emptyIndexes.push(i)
+                        }
+                    }
+                    errors.push(`Some tagrendering-ids are empty or have an emtpy string; this is not allowed (at ${context}.tagRenderings.[${emptyIndexes.join(",")}])`)
+                }
+
+                const duplicateIds = Utils.Dupiclates((json.tagRenderings ?? [])?.map(f => f["id"]).filter(id => id !== "questions"))
+                if (duplicateIds.length > 0 && !Utils.runningFromConsole) {
+                    errors.push(`Some tagRenderings have a duplicate id: ${duplicateIds} (at ${context}.tagRenderings)`)
+                }
+
+
+                if (json.description === undefined) {
+
+                    if (Constants.priviliged_layers.indexOf(json.id) >= 0) {
+                        errors.push(
+                            context + ": A priviliged layer must have a description"
+                        )
+                    } else {
+                        warnings.push(
+                            context + ": A builtin layer should have a description"
+                        )
+                    }
+                }
+            }
+            if(json.tagRenderings !== undefined){
+            new DetectShadowedMappings().convertAll(state, <TagRenderingConfigJson[]> json.tagRenderings, context+".tagRenderings")
+            }
+            
+        } catch (e) {
+            errors.push(e)
+        }
+        
+        
+        
+        
+        return {
+            result: json,
+            errors,
+            warnings
+        };
+    }
+}

Utils.ts

@@ -301,6 +301,10 @@ In the case that MapComplete is pointed to the testing grounds, the edit will be
      * @return the second parameter as is
      */
     static Merge(source: any, target: any) {
+        if (target === null) {
+            return source
+        }
+        
         for (const key in source) {
             if (!source.hasOwnProperty(key)) {
                 continue
@@ -327,10 +331,7 @@ In the case that MapComplete is pointed to the testing grounds, the edit will be
                 continue;
             }
 
-            const sourceV = source[key];
+            const sourceV = source[key]
-            if (target === null) {
-                return source
-            }
             const targetV = target[key]
             if (typeof sourceV === "object") {
                 if (sourceV === null) {
@@ -668,5 +669,27 @@ In the case that MapComplete is pointed to the testing grounds, the edit will be
             b: parseInt(hex.substr(5, 2), 16),
         }
     }
+
+    public static levenshteinDistance (str1: string, str2: string) {
+        const track = Array(str2.length + 1).fill(null).map(() =>
+            Array(str1.length + 1).fill(null));
+        for (let i = 0; i <= str1.length; i += 1) {
+            track[0][i] = i;
+        }
+        for (let j = 0; j <= str2.length; j += 1) {
+            track[j][0] = j;
+        }
+        for (let j = 1; j <= str2.length; j += 1) {
+            for (let i = 1; i <= str1.length; i += 1) {
+                const indicator = str1[i - 1] === str2[j - 1] ? 0 : 1;
+                track[j][i] = Math.min(
+                    track[j][i - 1] + 1, // deletion
+                    track[j - 1][i] + 1, // insertion
+                    track[j - 1][i - 1] + indicator, // substitution
+                );
+            }
+        }
+        return track[str2.length][str1.length];
+    }
 }
 

test/LegacyThemeLoader.spec.ts

@@ -4,6 +4,7 @@ import LayoutConfig from "../Models/ThemeConfig/LayoutConfig";
 import {LayerConfigJson} from "../Models/ThemeConfig/Json/LayerConfigJson";
 import {TagRenderingConfigJson} from "../Models/ThemeConfig/Json/TagRenderingConfigJson";
 import {AddMiniMap} from "../Models/ThemeConfig/Conversion/PrepareTheme";
+import {DetectShadowedMappings} from "../Models/ThemeConfig/Conversion/Validation";
 
 export default class LegacyThemeLoaderSpec extends T {
 
@@ -235,18 +236,18 @@ export default class LegacyThemeLoaderSpec extends T {
                         "question": {
                             "nl": "Wat is het type inzamelpunt?"
                         },
-                        "mappings":[
+                        "mappings": [
                             {
-                                "if":"recycling_type=container",
+                                "if": "recycling_type=container",
-                                "then":"Container of vat"
+                                "then": "Container of vat"
                             },
                             {
-                                "if":"recycling_type=centre",
+                                "if": "recycling_type=centre",
-                                "then":"Verwerkingsplaats of containerpark"
+                                "then": "Verwerkingsplaats of containerpark"
                             },
                             {
-                                "if":"recycling_type=dump",
+                                "if": "recycling_type=dump",
-                                "then":"Composthoop"
+                                "then": "Composthoop"
                             }
 
                         ],
@@ -256,28 +257,28 @@ export default class LegacyThemeLoaderSpec extends T {
                         "question": {
                             "nl": "Wie mag hier organisch afval bezorgen?"
                         },
-                        "mappings":[
+                        "mappings": [
                             {
-                                "if":"access=yes",
+                                "if": "access=yes",
-                                "then":"Publiek toegankelijk"
+                                "then": "Publiek toegankelijk"
                             },
                             {
-                                "if":"access=no",
+                                "if": "access=no",
-                                "then":"Privaat"
+                                "then": "Privaat"
                             },
                             {
-                                "if":"access=permessive",
+                                "if": "access=permessive",
-                                "then":"Mogelijks toegelaten tot nader order"
+                                "then": "Mogelijks toegelaten tot nader order"
                             },
                             {
-                                "if":"access=",
+                                "if": "access=",
-                                "then":"Waarschijnlijk publiek toegankelijk",
+                                "then": "Waarschijnlijk publiek toegankelijk",
-                                "hideInAnswer":true
+                                "hideInAnswer": true
                             },
                             {
-                                "if":"access=residents",
+                                "if": "access=residents",
-                                "then":"Bewoners van gemeente",
+                                "then": "Bewoners van gemeente",
-                                "hideInAnswer":"recycling_type!=centre"
+                                "hideInAnswer": "recycling_type!=centre"
                             }
 
                         ],
@@ -294,15 +295,15 @@ export default class LegacyThemeLoaderSpec extends T {
                         "render": {
                             "nl": "De naam van dit compost-inzamelpunt is {name}"
                         },
-                        "mappings":[
+                        "mappings": [
                             {
-                                "if":{"and":["noname=yes","name="]},
+                                "if": {"and": ["noname=yes", "name="]},
-                                "then":"Heeft geen naam"
+                                "then": "Heeft geen naam"
                             },
                             {
-                                "if":"name=",
+                                "if": "name=",
-                                "then":"Geen naam bekend",
+                                "then": "Geen naam bekend",
-                                "hideInAnswer":true
+                                "hideInAnswer": true
                             }
                         ],
                         "id": "Composthoekjes-name"
@@ -410,7 +411,39 @@ export default class LegacyThemeLoaderSpec extends T {
                         render: "Some random value {minimap}"
                     })
 
-                }]
+                }],
+                ["Shadowed mappings are detected",
+                    () => {
+                        const r = new DetectShadowedMappings().convert(undefined, {
+                            mappings: [
+                                {
+                                    if: {or: ["key=value", "x=y"]},
+                                    then: "Case A"
+                                },
+                                {
+                                    if: "key=value",
+                                    then: "Shadowed"
+                                }
+                            ]
+                        }, "test");
+                    T.isTrue(r.errors.length > 0, "Failing case is not detected")
+
+                        const r0 = new DetectShadowedMappings().convert(undefined, {
+                            mappings: [
+                                {
+                                    if: {or: ["key=value", "x=y"]},
+                                    then: "Case A"
+                                },
+                                {
+                                    if: {and: ["key=value", "x=y"]},
+                                    then: "Shadowed"
+                                }
+                            ]
+                        }, "test");
+                        T.isTrue(r0.errors.length > 0, "Failing case is not detected")
+                    }
+
+                ]
             ]
         );
     }