From b9cc56bbc544a94c3895205deae6319a3bf26478 Mon Sep 17 00:00:00 2001 From: pietervdvn Date: Thu, 9 Jun 2022 16:50:53 +0200 Subject: [PATCH] Improve error messages --- Logic/Tags/TagUtils.ts | 54 ++++++++++----------- Models/ThemeConfig/Conversion/Conversion.ts | 2 +- Models/ThemeConfig/Conversion/Validation.ts | 7 +-- Models/ThemeConfig/Json/LayerConfigJson.ts | 2 +- Models/ThemeConfig/TagRenderingConfig.ts | 14 ++++-- 5 files changed, 43 insertions(+), 36 deletions(-) diff --git a/Logic/Tags/TagUtils.ts b/Logic/Tags/TagUtils.ts index 884e81e4a..b249291de 100644 --- a/Logic/Tags/TagUtils.ts +++ b/Logic/Tags/TagUtils.ts @@ -216,11 +216,11 @@ export class TagUtils { * * TagUtils.Tag("xyz<5").matchesProperties({xyz: 4}) // => true * TagUtils.Tag("xyz<5").matchesProperties({xyz: 5}) // => false - * + * * // RegexTags must match values with newlines * TagUtils.Tag("note~.*aed.*").matchesProperties({note: "Hier bevindt zich wss een defibrillator. \\n\\n De aed bevindt zich op de 5de verdieping"}) // => true * TagUtils.Tag("note~i~.*aed.*").matchesProperties({note: "Hier bevindt zich wss een defibrillator. \\n\\n De AED bevindt zich op de 5de verdieping"}) // => true - * + * * // Must match case insensitive * TagUtils.Tag("name~i~somename").matchesProperties({name: "SoMeName"}) // => true */ @@ -286,7 +286,7 @@ export class TagUtils { private static TagUnsafe(json: AndOrTagConfigJson | string, context: string = ""): TagsFilter { if (json === undefined) { - throw `Error while parsing a tag: 'json' is undefined in ${context}. Make sure all the tags are defined and at least one tag is present in a complex expression` + throw new Error(`Error while parsing a tag: 'json' is undefined in ${context}. Make sure all the tags are defined and at least one tag is present in a complex expression`) } if (typeof (json) != "string") { if (json.and !== undefined && json.or !== undefined) { @@ -335,7 +335,7 @@ export class TagUtils { return new ComparingTag(split[0], f, operator + val) } } - + if (tag.indexOf("~~") >= 0) { const split = Utils.SplitFirst(tag, "~~"); if (split[1] === "*") { @@ -346,9 +346,9 @@ export class TagUtils { new RegExp("^" + split[1] + "$", "s") ); } - + const withRegex = TagUtils.parseRegexOperator(tag) - if(withRegex != null) { + if (withRegex != null) { if (withRegex.value === "*" && withRegex.invert) { throw `Don't use 'key!~*' - use 'key=' instead (empty string as value (in the tag ${tag} while parsing ${context})` } @@ -362,7 +362,7 @@ export class TagUtils { } return new RegexTag( withRegex.key, - new RegExp("^"+value+"$", "s"+withRegex.modifier), + new RegExp("^" + value + "$", "s" + withRegex.modifier), withRegex.invert ); } @@ -459,35 +459,35 @@ export class TagUtils { /** * Returns 'true' is opposite tags are detected. * Note that this method will never work perfectly - * + * * // should be false for some simple cases * TagUtils.ContainsOppositeTags([new Tag("key", "value"), new Tag("key0", "value")]) // => false * TagUtils.ContainsOppositeTags([new Tag("key", "value"), new Tag("key", "value0")]) // => false - * + * * // should detect simple cases * TagUtils.ContainsOppositeTags([new Tag("key", "value"), new RegexTag("key", "value", true)]) // => true * TagUtils.ContainsOppositeTags([new Tag("key", "value"), new RegexTag("key", /value/, true)]) // => true */ - public static ContainsOppositeTags(tags: (TagsFilter)[]) : boolean{ - for (let i = 0; i < tags.length; i++){ + public static ContainsOppositeTags(tags: (TagsFilter)[]): boolean { + for (let i = 0; i < tags.length; i++) { const tag = tags[i]; - if(!(tag instanceof Tag || tag instanceof RegexTag)){ + if (!(tag instanceof Tag || tag instanceof RegexTag)) { continue } - for (let j = i + 1; j < tags.length; j++){ + for (let j = i + 1; j < tags.length; j++) { const guard = tags[j]; - if(!(guard instanceof Tag || guard instanceof RegexTag)){ + if (!(guard instanceof Tag || guard instanceof RegexTag)) { continue } - if(guard.key !== tag.key) { + if (guard.key !== tag.key) { // Different keys: they can _never_ be opposites continue } - if((guard.value["source"] ?? guard.value) !== (tag.value["source"] ?? tag.value)){ + if ((guard.value["source"] ?? guard.value) !== (tag.value["source"] ?? tag.value)) { // different values: the can _never_ be opposites continue } - if( (guard["invert"] ?? false) !== (tag["invert"] ?? false) ) { + if ((guard["invert"] ?? false) !== (tag["invert"] ?? false)) { // The 'invert' flags are opposite, the key and value is the same for both // This means we have found opposite tags! return true @@ -502,28 +502,28 @@ export class TagUtils { * Returns a filtered version of 'listToFilter'. * For a list [t0, t1, t2], If `blackList` contains an equivalent (or broader) match of any `t`, this respective `t` is dropped from the returned list * Ignores nested ORS and ANDS - * + * * TagUtils.removeShadowedElementsFrom([new Tag("key","value")], [new Tag("key","value"), new Tag("other_key","value")]) // => [new Tag("other_key","value")] */ - public static removeShadowedElementsFrom(blacklist: TagsFilter[], listToFilter: TagsFilter[] ) : TagsFilter[] { + public static removeShadowedElementsFrom(blacklist: TagsFilter[], listToFilter: TagsFilter[]): TagsFilter[] { return listToFilter.filter(tf => !blacklist.some(guard => guard.shadows(tf))) } /** * Returns a filtered version of 'listToFilter', where no duplicates and no equivalents exists. - * + * * TagUtils.removeEquivalents([new RegexTag("key", /^..*$/), new Tag("key","value")]) // => [new Tag("key", "value")] */ - public static removeEquivalents( listToFilter: (Tag | RegexTag)[]) : TagsFilter[] { + public static removeEquivalents(listToFilter: (Tag | RegexTag)[]): TagsFilter[] { const result: TagsFilter[] = [] - outer: for (let i = 0; i < listToFilter.length; i++){ + outer: for (let i = 0; i < listToFilter.length; i++) { const tag = listToFilter[i]; - for (let j = 0; j < listToFilter.length; j++){ - if(i === j){ + for (let j = 0; j < listToFilter.length; j++) { + if (i === j) { continue } const guard = listToFilter[j]; - if(guard.shadows(tag)) { + if (guard.shadows(tag)) { // the guard 'kills' the tag: we continue the outer loop without adding the tag continue outer; } @@ -532,7 +532,7 @@ export class TagUtils { } return result } - + /** * Returns `true` if at least one element of the 'guards' shadows one element of the 'listToFilter'. * @@ -540,7 +540,7 @@ export class TagUtils { * TagUtils.containsEquivalents([new Tag("key","value")], [ new Tag("other_key","value")]) // => false * TagUtils.containsEquivalents([new Tag("key","value")], [ new Tag("key","other_value")]) // => false */ - public static containsEquivalents( guards: TagsFilter[], listToFilter: TagsFilter[] ) : boolean { + public static containsEquivalents(guards: TagsFilter[], listToFilter: TagsFilter[]): boolean { return listToFilter.some(tf => guards.some(guard => guard.shadows(tf))) } diff --git a/Models/ThemeConfig/Conversion/Conversion.ts b/Models/ThemeConfig/Conversion/Conversion.ts index 5b6b72c00..bc5ff9f64 100644 --- a/Models/ThemeConfig/Conversion/Conversion.ts +++ b/Models/ThemeConfig/Conversion/Conversion.ts @@ -226,7 +226,7 @@ export class Fuse extends DesugaringStep { break; } }catch(e){ - console.error("Step "+step.name+" failed due to "+e); + console.error("Step "+step.name+" failed due to ",e,e.stack); throw e } } diff --git a/Models/ThemeConfig/Conversion/Validation.ts b/Models/ThemeConfig/Conversion/Validation.ts index 96f08f33b..aa4d4d53a 100644 --- a/Models/ThemeConfig/Conversion/Validation.ts +++ b/Models/ThemeConfig/Conversion/Validation.ts @@ -321,8 +321,9 @@ export class DetectShadowedMappings extends DesugaringStep { - const ifTags = TagUtils.Tag(m.if); + const parsedConditions = json.mappings.map((m,i) => { + const ctx = `${context}.mappings[${i}]` + const ifTags = TagUtils.Tag(m.if, ctx); if(m.hideInAnswer !== undefined && m.hideInAnswer !== false && m.hideInAnswer !== true){ let conditionTags = TagUtils.Tag( m.hideInAnswer) // Merge the condition too! @@ -407,7 +408,7 @@ export class DetectMappingsWithImages extends DesugaringStep=0 - const images = Utils.Dedup(Translations.T(mapping.then).ExtractImages()) + const images = Utils.Dedup(Translations.T(mapping.then)?.ExtractImages() ?? []) const ctx = `${context}.mappings[${i}]` if (images.length > 0) { if(!ignore){ diff --git a/Models/ThemeConfig/Json/LayerConfigJson.ts b/Models/ThemeConfig/Json/LayerConfigJson.ts index 09285eb40..4264d3565 100644 --- a/Models/ThemeConfig/Json/LayerConfigJson.ts +++ b/Models/ThemeConfig/Json/LayerConfigJson.ts @@ -287,7 +287,7 @@ export interface LayerConfigJson { */ tagRenderings?: (string - | { builtin: string, override: any } + | { builtin: string | string[], override: any } | QuestionableTagRenderingConfigJson | RewritableConfigJson<(string | { builtin: string, override: any } | QuestionableTagRenderingConfigJson)[]> ) [], diff --git a/Models/ThemeConfig/TagRenderingConfig.ts b/Models/ThemeConfig/TagRenderingConfig.ts index 95076cff7..e68014357 100644 --- a/Models/ThemeConfig/TagRenderingConfig.ts +++ b/Models/ThemeConfig/TagRenderingConfig.ts @@ -172,15 +172,21 @@ export default class TagRenderingConfig { this.mappings = json.mappings.map((mapping, i) => { const ctx = `${translationKey}.mappings.${i}` + if (mapping.if === undefined) { + throw `${ctx}: Invalid mapping: "if" is not defined in ${JSON.stringify(mapping)}` + } if (mapping.then === undefined) { - throw `${ctx}: Invalid mapping: if without body` + if(mapping["render"] !== undefined){ + throw `${ctx}: Invalid mapping: no 'then'-clause found. You might have typed 'render' instead of 'then', change it in ${JSON.stringify(mapping)}` + } + throw `${ctx}: Invalid mapping: no 'then'-clause found in ${JSON.stringify(mapping)}` } if (mapping.ifnot !== undefined && !this.multiAnswer) { - throw `${ctx}: Invalid mapping: ifnot defined, but the tagrendering is not a multianswer` + throw `${ctx}: Invalid mapping: 'ifnot' is defined, but the tagrendering is not a multianswer. Either remove ifnot or set 'multiAnswer:true' to enable checkboxes instead of radiobuttons` } - if (mapping.if === undefined) { - throw `${ctx}: Invalid mapping: "if" is not defined, but the tagrendering is not a multianswer` + if(mapping["render"] !== undefined){ + throw `${ctx}: Invalid mapping: a 'render'-key is present, this is probably a bug: ${JSON.stringify(mapping)}` } if (typeof mapping.if !== "string" && mapping.if["length"] !== undefined) { throw `${ctx}: Invalid mapping: "if" is defined as an array. Use {"and": } or {"or": } instead`