diff --git a/Models/ThemeConfig/Conversion/Validation.ts b/Models/ThemeConfig/Conversion/Validation.ts index e7624b0dce..f8174c5b0e 100644 --- a/Models/ThemeConfig/Conversion/Validation.ts +++ b/Models/ThemeConfig/Conversion/Validation.ts @@ -248,25 +248,32 @@ export class DetectShadowedMappings extends DesugaringStep TagUtils.Tag(m.if)) + const parsedConditions = json.mappings.map(m => { + const ifTags = TagUtils.Tag(m.if); + if(m.hideInAnswer !== undefined && m.hideInAnswer !== false && m.hideInAnswer !== true){ + let conditionTags = TagUtils.Tag( m.hideInAnswer) + // Merge the condition too! + return new And([conditionTags, ifTags]) + } + return ifTags + }) for (let i = 0; i < json.mappings.length; i++) { - if(json.mappings[i].hideInAnswer === true){ + if(!parsedConditions[i].isUsableAsAnswer()){ + // There is no straightforward way to convert this mapping.if into a properties-object, so we simply skip this one + // Yes, it might be shadowed, but running this check is to difficult right now continue } const keyValues = parsedConditions[i].asChange({}); - const properties = [] + const properties = {} keyValues.forEach(({k, v}) => { properties[k] = v }) for (let j = 0; j < i; j++) { - if(json.mappings[j].hideInAnswer === true){ - continue - } const doesMatch = parsedConditions[j].matchesProperties(properties) if (doesMatch) { // The current mapping is shadowed! errors.push(`At ${context}: 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: + The mapping ${parsedConditions[i].asHumanString(false, false, {})} is fully matched by a previous mapping (namely ${j}), which matches: ${parsedConditions[j].asHumanString(false, false, {})}. Move the mapping up to fix this problem @@ -276,6 +283,10 @@ export class DetectShadowedMappings extends DesugaringStep 0) { if(!ignore){ - errors.push(`${ctx}: A mapping has an image in the 'then'-clause. Remove the image there and use \`"icon": \` instead. The images found are ${images.join(", ")}. (Ignore this warning by adding "#": "${ignoreToken}" to the mapping`) + errors.push(`${ctx}: A mapping has an image in the 'then'-clause. Remove the image there and use \`"icon": \` instead. The images found are ${images.join(", ")}. (This check can be turned of by adding "#": "${ignoreToken}" in the mapping, but this is discouraged`) }else{ - information.push(`${ctx}: Ignored images in then`) + information.push(`${ctx}: Ignored image ${images.join(", ")} in 'then'-clause of a mapping as this check has been disabled`) } }else if (ignore){ warnings.push(`${ctx}: unused '${ignoreToken}' - please remove this`) } } - return { - errors,warnings,information, + return { + errors, + warnings, + information, result: json }; } @@ -324,8 +337,7 @@ export class DetectMappingsWithImages extends DesugaringStep { constructor() { super("Various validation on tagRenderingConfigs", - // TODO enable these checks again - // new DetectShadowedMappings(), + new DetectShadowedMappings(), new DetectMappingsWithImages() ); } diff --git a/test/LegacyThemeLoader.spec.ts b/test/LegacyThemeLoader.spec.ts index e3b66ff655..8b1972120d 100644 --- a/test/LegacyThemeLoader.spec.ts +++ b/test/LegacyThemeLoader.spec.ts @@ -422,8 +422,8 @@ export default class LegacyThemeLoaderSpec extends T { } ] }, "test"); - T.isTrue(r.errors.length > 0, "Failing case 0 is not detected") - + T.isTrue(r.warnings.length > 0, "Failing case 0 is not detected") + T.isTrue(r.warnings[0].indexOf("The mapping key=value is fully matched by a previous mapping (namely 0)") >= 0, "Error message does not contain tag and indices") const r0 = new DetectShadowedMappings().convert({ mappings: [ { @@ -436,7 +436,7 @@ export default class LegacyThemeLoaderSpec extends T { } ] }, "test"); - T.isTrue(r0.errors.length > 0, "Failing case 1 is not detected") + T.isTrue(r0.warnings.length > 0, "Failing case 1 is not detected") } ], ["Images are rewritten", () => {