Skip to content

Commit

Permalink
Fixed all shadowed mappings, make those an error
Browse files Browse the repository at this point in the history
  • Loading branch information
@pietervdvn
pietervdvn committed Mar 17, 2022
1 parent aa5e944 commit fe8c63d
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 64 deletions.
11 changes: 7 additions & 4 deletions Models/ThemeConfig/Conversion/Conversion.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,17 @@ export abstract class Conversion<TIn, TOut> {
}

public static strict<T>(fixed: { errors?: string[], warnings?: string[], information?: string[], result?: T }): T {
if (fixed?.errors !== undefined && fixed?.errors?.length > 0) {
throw fixed.errors.join("\n\n");
}

fixed.information?.forEach(i => console.log(" ", i))
const yellow = (s) => "\x1b[33m"+s+"\x1b[0m"
const red = s => '\x1b[31m'+s+'\x1b[0m'

fixed.warnings?.forEach(w => console.warn(red(`<!> `), yellow (w)))

if (fixed?.errors !== undefined && fixed?.errors?.length > 0) {
fixed.errors?.forEach(e => console.error(red(`ERR `+e)))
throw "Detected one or more errors, stopping now"
}

return fixed.result;
}

Expand Down
35 changes: 26 additions & 9 deletions Models/ThemeConfig/Conversion/Validation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,8 +238,25 @@ export class PrevalidateTheme extends Fuse<LayoutConfigJson> {
}

export class DetectShadowedMappings extends DesugaringStep<TagRenderingConfigJson> {
constructor() {
private readonly _calculatedTagNames: string[];
constructor(layerConfig?: LayerConfigJson) {
super("Checks that the mappings don't shadow each other", [], "DetectShadowedMappings");
this._calculatedTagNames = DetectShadowedMappings.extractCalculatedTagNames(layerConfig);
}

/**
*
* DetectShadowedMappings.extractCalculatedTagNames({calculatedTags: ["_abc:=js()"]}) // => ["_abc"]
* DetectShadowedMappings.extractCalculatedTagNames({calculatedTags: ["_abc=js()"]}) // => ["_abc"]
*/
private static extractCalculatedTagNames(layerConfig?: LayerConfigJson){
return layerConfig?.calculatedTags?.map(ct => {
if(ct.indexOf(':=') >= 0){
return ct.split(':=')[0]
}
return ct.split("=")[0]
}) ?? []

}

convert(json: TagRenderingConfigJson, context: string): { result: TagRenderingConfigJson; errors?: string[]; warnings?: string[] } {
Expand All @@ -248,6 +265,10 @@ export class DetectShadowedMappings extends DesugaringStep<TagRenderingConfigJso
if (json.mappings === undefined || json.mappings.length === 0) {
return {result: json}
}
const defaultProperties = {}
for (const calculatedTagName of this._calculatedTagNames) {
defaultProperties[calculatedTagName] = "some_calculated_tag_value_for_"+calculatedTagName
}
const parsedConditions = json.mappings.map(m => {
const ifTags = TagUtils.Tag(m.if);
if(m.hideInAnswer !== undefined && m.hideInAnswer !== false && m.hideInAnswer !== true){
Expand All @@ -263,7 +284,7 @@ export class DetectShadowedMappings extends DesugaringStep<TagRenderingConfigJso
// Yes, it might be shadowed, but running this check is to difficult right now
continue
}
const keyValues = parsedConditions[i].asChange({});
const keyValues = parsedConditions[i].asChange(defaultProperties);
const properties = {}
keyValues.forEach(({k, v}) => {
properties[k] = v
Expand All @@ -289,10 +310,6 @@ export class DetectShadowedMappings extends DesugaringStep<TagRenderingConfigJso

}

// TODO make this errors again
warnings.push(...errors)
errors.splice(0, errors.length)

return {
errors,
warnings,
Expand Down Expand Up @@ -341,9 +358,9 @@ export class DetectMappingsWithImages extends DesugaringStep<TagRenderingConfigJ
}

export class ValidateTagRenderings extends Fuse<TagRenderingConfigJson> {
constructor() {
constructor(layerConfig: LayerConfigJson) {
super("Various validation on tagRenderingConfigs",
new DetectShadowedMappings(),
new DetectShadowedMappings( layerConfig),
new DetectMappingsWithImages()
);
}
Expand Down Expand Up @@ -439,7 +456,7 @@ export class ValidateLayer extends DesugaringStep<LayerConfigJson> {
}
}
if (json.tagRenderings !== undefined) {
const r = new OnEvery("tagRenderings", new ValidateTagRenderings()).convert(json, context)
const r = new OnEvery("tagRenderings", new ValidateTagRenderings(json)).convert(json, context)
warnings.push(...(r.warnings??[]))
errors.push(...(r.errors??[]))
information.push(...(r.information??[]))
Expand Down
6 changes: 1 addition & 5 deletions assets/themes/grb_import/grb_fixme.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,16 +127,12 @@
},
{
"if": "building=apartments",
"then": "An apartment building - highrise for living"
"then": "An apartment building (a highrise building for living)"
},
{
"if": "building=office",
"then": "An office building - highrise for work"
},
{
"if": "building=apartments",
"then": "An apartment building"
},
{
"if": "building=shed",
"then": "A small shed, e.g. in a garden"
Expand Down
85 changes: 39 additions & 46 deletions assets/themes/uk_addresses/uk_addresses.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,13 +256,14 @@
},
"mappings": [
{
"if": "addr:unit=",
"then": "<div class='subtle'>Sub-unit (e.g. \"1\", \"Flat 2\", \"Unit C\")</div>",
"hideInAnswer": true
"if": "not:addr:unit=yes",
"then": "There is no sub-unit within this address",
"addExtraTags": ["addr:unit="]
},
{
"if": "addr:unit=",
"then": "There is no sub-unit within this address"
"then": "<div class='subtle'>Sub-unit (e.g. \"1\", \"Flat 2\", \"Unit C\")</div>",
"hideInAnswer": true
}
],
"condition": {
Expand Down Expand Up @@ -291,17 +292,12 @@
"addr:housename="
]
},
"then": "<div class='subtle'>House or building name</div>",
"hideInAnswer": true
"then": "This building has no housename"
},
{
"if": {
"and": [
"nohousename=yes",
"addr:housename="
]
},
"then": "This building has no housename"
"if": "addr:housename=",
"then": "<div class='subtle'>House or building name</div>",
"hideInAnswer": true
},
{
"#": "By adding nohousenumber!=yes, this option will trigger when first added, but will be untriggered if a housenumber is added, resulting in the question poping up!",
Expand Down Expand Up @@ -330,29 +326,24 @@
},
"mappings": [
{
"if": {
"and": [
"nohousenumber=yes",
"addr:housenumber="
]
},
"if": "nohousenumber=yes",
"then": {
"en": "<div class='subtle'>Number (e.g. 1, 1A, 2)</div>"
"en": "This building has no house number",
"nl": "Dit gebouw heeft geen huisnummer",
"de": "Dieses Gebäude hat keine Hausnummer"
},
"hideInAnswer": true
"addExtraTags": [ "addr:housenumber="]
},
{
"if": {
"and": [
"nohousenumber=yes",
"addr:housenumber="
]
},
"then": {
"en": "This building has no house number",
"nl": "Dit gebouw heeft geen huisnummer",
"de": "Dieses Gebäude hat keine Hausnummer"
}
"en": "<div class='subtle'>Number (e.g. 1, 1A, 2)</div>"
},
"hideInAnswer": true
}
]
},
Expand All @@ -373,17 +364,17 @@
},
"mappings": [
{
"if": "not:addr:substreet=yes",
"then": {
"en": "No extra place name is given or needed"
},
"addExtraTags": ["addr:substreet="]
},{
"if": "addr:substreet=",
"then": {
"en": "<div class='subtle'>Place (e.g. \"Castle Mews\", \"West Business Park\")</div>"
},
"hideInAnswer": true
},
{
"if": "addr:substreet=",
"then": {
"en": "No extra place name is given or needed"
}
}
],
"condition": "addr:parentstreet="
Expand All @@ -404,17 +395,18 @@
},
"mappings": [
{
"if": "addr:substreet=",
"if": "not:addr:substreet=yes",
"then": {
"en": "<div class='subtle'>Place (e.g. \"Castle Mews\", \"West Business Park\")</div>"
"en": "No extra place name is given or needed"
},
"hideInAnswer": true
"addExtraTags": ["addr:substreet="]
},
{
"if": "addr:substreet=",
"then": {
"en": "No extra place name is given or needed"
}
"en": "<div class='subtle'>Place (e.g. \"Castle Mews\", \"West Business Park\")</div>"
},
"hideInAnswer": true
}
],
"condition": {
Expand Down Expand Up @@ -479,15 +471,9 @@
},
"mappings": [
{
"if": "addr:parentstreet=",
"then": {
"en": "<div class='subtle'>Parent street name</div>"
},
"hideInAnswer": true
},
{
"if": "addr:parentstreet=",
"then": "No parent street name is needed within this address"
"if": "not:addr:parentstreet=yes",
"then": "No parent street name is needed within this address",
"addExtraTags": ["addr:parentstreet="]
},
{
"if": "addr:parentstreet:={_closest_street:0:name}",
Expand All @@ -503,6 +489,13 @@
"if": "addr:parentstreet:={_closest_street:2:name}",
"then": "<b>{_closest_street:2:name}</b>",
"hideInAnswer": "_closest_street:2:name="
},
{
"if": "addr:parentstreet=",
"then": {
"en": "<div class='subtle'>Parent street name</div>"
},
"hideInAnswer": true
}
],
"condition": {
Expand Down

0 comments on commit fe8c63d

Please sign in to comment.