From 50ef2ba63674d8e543649b85b0098658ac3f52ed Mon Sep 17 00:00:00 2001 From: Pieter Vander Vennet Date: Mon, 20 Jan 2025 18:35:24 +0100 Subject: [PATCH] Themes: automatically remove filters that are useless because they would filter everything or nothing; fix some logic bugs and add tests --- src/Logic/Tags/And.ts | 15 +++++----- src/Logic/Tags/TagUtils.ts | 5 +++- .../ThemeConfig/Conversion/ExpandFilter.ts | 29 ++++++++++++++++--- 3 files changed, 37 insertions(+), 12 deletions(-) diff --git a/src/Logic/Tags/And.ts b/src/Logic/Tags/And.ts index c2525dee3b..f88c913700 100644 --- a/src/Logic/Tags/And.ts +++ b/src/Logic/Tags/And.ts @@ -124,7 +124,7 @@ export class And extends TagsFilter { * t0.shadows(t0) // => true * t1.shadows(t1) // => true * t2.shadows(t2) // => true - * t0.shadows(t1) // => false + * t0.shadows(t1) // => true * t0.shadows(t2) // => false * t1.shadows(t0) // => false * t1.shadows(t2) // => false @@ -135,13 +135,18 @@ export class And extends TagsFilter { * const t1 = new And([new Tag("shop","clothes"), new Or([new Tag("brand","XYZ"),new Tag("brand:wikidata","Q1234")])]) * const t2 = new And([new RegexTag("shop","mall",true), new Or([TagUtils.Tag("shop~*"), new Tag("craft","shoemaker")])]) * t1.shadows(t2) // => true + * + * const t1 = new Tag("a","b") + * const t2 = new And([new Tag("x","y"), new Tag("a","b")]) + * t2.shadows(t1) // => true + * t1.shadows(t2) // => false */ shadows(other: TagsFilter): boolean { - const phrases: TagsFilter[] = other instanceof And ? other.and : [other] + // The phrases of the _other_ and + const phrases: readonly TagsFilter[] = other instanceof And ? other.and : [other] // A phrase might be shadowed by a certain subsection. We keep track of this here const shadowedOthers = phrases.map(() => false) for (const selfTag of this.and) { - let shadowsSome = false let shadowsAll = true for (let i = 0; i < phrases.length; i++) { const otherTag = phrases[i] @@ -149,7 +154,6 @@ export class And extends TagsFilter { if (doesShadow) { shadowedOthers[i] = true } - shadowsSome ||= doesShadow shadowsAll &&= doesShadow } // If A => X and A => Y, then @@ -157,9 +161,6 @@ export class And extends TagsFilter { if (shadowsAll) { return true } - if (!shadowsSome) { - return false - } } return !shadowedOthers.some((v) => !v) } diff --git a/src/Logic/Tags/TagUtils.ts b/src/Logic/Tags/TagUtils.ts index 7e495ed706..c3933cdca8 100644 --- a/src/Logic/Tags/TagUtils.ts +++ b/src/Logic/Tags/TagUtils.ts @@ -690,6 +690,9 @@ export class TagUtils { return result } + /** + * TagUtils.removeKnownParts(TagUtils.Tag({and: ["vending=excrement_bag"}),TagUtils.Tag({and: ["amenity=waste_basket", "vending=excrement_bag"]}), true) // => true + */ public static removeKnownParts( tag: TagsFilter, known: TagsFilter, @@ -702,7 +705,7 @@ export class TagUtils { if (tagOrBool instanceof And) { return tagOrBool.removePhraseConsideredKnown(known, valueOfKnown) } - return tagOrBool + return new And([tagOrBool]).removePhraseConsideredKnown(known, valueOfKnown) } /** diff --git a/src/Models/ThemeConfig/Conversion/ExpandFilter.ts b/src/Models/ThemeConfig/Conversion/ExpandFilter.ts index 877ba20f3c..1b6fcec390 100644 --- a/src/Models/ThemeConfig/Conversion/ExpandFilter.ts +++ b/src/Models/ThemeConfig/Conversion/ExpandFilter.ts @@ -13,7 +13,7 @@ import { Or } from "../../../Logic/Tags/Or" import Translations from "../../../UI/i18n/Translations" import { FlatTag, OptimizedTag, TagsFilterClosed } from "../../../Logic/Tags/TagTypes" import { TagsFilter } from "../../../Logic/Tags/TagsFilter" -import { And } from "../../../Logic/Tags/And" +import { Translation } from "../../../UI/i18n/Translation" export class PruneFilters extends DesugaringStep { constructor() { @@ -24,11 +24,30 @@ export class PruneFilters extends DesugaringStep { ) } + /** + * Prunes a filter; returns null/undefined if keeping the filter is useless + */ private prune( sourceTags: FlatTag, filter: FilterConfigJson, context: ConversionContext ): FilterConfigJson { + + if (filter.options.length === 1) { + const option = filter.options[0] + const tags = TagUtils.Tag(option.osmTags) + const optimized = TagUtils.removeKnownParts(tags, sourceTags, true) + if (optimized === true) { + context.warn("Removing filter as always known: ", new Translation(option.question).textFor("en")) + return undefined + } + if (optimized === false) { + context.warn("Removing filter as not possible: ", new Translation(option.question).textFor("en")) + return undefined + } + } + + if (!filter.strict) { return filter } @@ -47,7 +66,7 @@ export class PruneFilters extends DesugaringStep { if (!option.osmTags) { return option } - let basetags = TagUtils.Tag(option.osmTags) + const basetags = TagUtils.Tag(option.osmTags) return { ...option, osmTags: (TagUtils.removeKnownParts(basetags, sourceTags)).asJson(), @@ -65,6 +84,8 @@ export class PruneFilters extends DesugaringStep { ")" ) } + + return { ...filter, options: newOptions, strict: undefined } } @@ -78,9 +99,9 @@ export class PruneFilters extends DesugaringStep { const sourceTags = TagUtils.Tag(json.source["osmTags"]) return { ...json, - filter: json.filter?.map((obj) => + filter: Utils.NoNull(json.filter?.map((obj) => this.prune(sourceTags, obj, context) - ), + )), } } }