elgaard/MapComplete
1
0
Fork 0
forked from MapComplete/MapComplete
Code Pull requests Activity

Fix naughty bug in tag optimization by adding better typing

Browse source
This commit is contained in:
Pieter Vander Vennet 2024-07-29 14:38:50 +02:00
parent b98245fafb
commit fd16e165c4
11 changed files with 223 additions and 93 deletions
Download patch file Download diff file Expand all files Collapse all files

42
src/Logic/Tags/And.ts
View file

@ -6,6 +6,7 @@ import { RegexTag } from "./RegexTag"
import { TagConfigJson } from "../../Models/ThemeConfig/Json/TagConfigJson"
import { ExpressionSpecification } from "maplibre-gl"
import ComparingTag from "./ComparingTag"
import { FlatTag, OptimizedTag, TagsFilterClosed, TagTypes } from "./TagTypes"
export class And extends TagsFilter {
public and: TagsFilter[]
@ -15,6 +16,8 @@ export class And extends TagsFilter {
this.and = and
}
public static construct(and: TagsFilter[]): TagsFilter
public static construct(and: (FlatTag | (Or & OptimizedTag))[]): TagsFilterClosed & OptimizedTag
public static construct(and: TagsFilter[]): TagsFilter {
if (and.length === 1) {
return and[0]
@ -175,6 +178,10 @@ export class And extends TagsFilter {
* When the evaluation hits (A=B & X=Y), we know _for sure_ that X=Y does _not_ match, as it would have matched the first clause otherwise.
* This means that the entire 'AND' is considered FALSE
*
* @return only phrases that should be kept.
* @param knownExpression The expression which is known in the subexpression and for which calculations can be done
* @param value the given knownExpression is considered to have this value, namely 'true' or 'false'
*
* new And([ new Tag("key","value") ,new Tag("other_key","value")]).removePhraseConsideredKnown(new Tag("key","value"), true) // => new Tag("other_key","value")
* new And([ new Tag("key","value") ,new Tag("other_key","value")]).removePhraseConsideredKnown(new Tag("key","value"), false) // => false
* new And([ new RegexTag("key",/^..*$/) ,new Tag("other_key","value")]).removePhraseConsideredKnown(new Tag("key","value"), true) // => new Tag("other_key","value")
@ -187,13 +194,14 @@ export class And extends TagsFilter {
* const expr = <And> TagUtils.Tag({and: ["sport=climbing", {or:["club~*", "office~*"]}]} )
* expr.removePhraseConsideredKnown(new Tag("club","climbing"), false) // => expr
*/
removePhraseConsideredKnown(knownExpression: TagsFilter, value: boolean): TagsFilter | boolean {
removePhraseConsideredKnown(knownExpression: TagsFilter, value: boolean): (TagsFilterClosed & OptimizedTag) | boolean {
const newAnds: TagsFilter[] = []
for (const tag of this.and) {
if (tag instanceof And) {
throw "Optimize expressions before using removePhraseConsideredKnown. Found an AND in an AND: "+this.asHumanString()
throw "Optimize expressions before using removePhraseConsideredKnown. Found an AND in an AND: " + this.asHumanString()
}
if (tag instanceof Or) {
// Second try
const r = tag.removePhraseConsideredKnown(knownExpression, value)
if (r === true) {
continue
@ -232,7 +240,7 @@ export class And extends TagsFilter {
if (newAnds.length === 0) {
return true
}
return And.construct(newAnds)
return And.construct(newAnds).optimize()
}
/**
@ -265,7 +273,7 @@ export class And extends TagsFilter {
* const parsed = TagUtils.Tag(orig)
* parsed.optimize().asJson() // => orig
*/
optimize(): TagsFilter | boolean {
optimize(): (TagsFilterClosed & OptimizedTag) | boolean {
if (this.and.length === 0) {
return true
}
@ -276,7 +284,7 @@ export class And extends TagsFilter {
// We have an AND with a contained false: this is always 'false'
return false
}
const optimized = <TagsFilter[]>optimizedRaw
const optimized = <(TagsFilterClosed & OptimizedTag)[]>optimizedRaw
for (let i = 0; i < optimized.length; i++) {
for (let j = i + 1; j < optimized.length; j++) {
@ -299,7 +307,7 @@ export class And extends TagsFilter {
}
{
// Conflicting keys do return false
// Conflicting keys do return false. We build a 'known' set and check for conflicts
const properties: Record<string, string> = {}
for (const opt of optimized) {
if (opt instanceof Tag) {
@ -366,12 +374,12 @@ export class And extends TagsFilter {
}
}
const newAnds: TagsFilter[] = []
const newAnds: (FlatTag | (Or & OptimizedTag))[] = []
let containedOrs: (Or & OptimizedTag)[] = []
let containedOrs: Or[] = []
for (const tf of optimized) {
if (tf instanceof And) {
newAnds.push(...tf.and)
newAnds.push(...TagTypes.safeAnd(tf))
} else if (tf instanceof Or) {
containedOrs.push(tf)
} else {
@ -382,10 +390,10 @@ export class And extends TagsFilter {
{
let dirty = false
do {
const cleanedContainedOrs: Or[] = []
const cleanedContainedOrs: (Or & OptimizedTag)[] = []
outer: for (let containedOr of containedOrs) {
for (const known of newAnds) {
// input for optimazation: (K=V & (X=Y | K=V))
// input for optimization: (K=V & (X=Y | K=V))
// containedOr: (X=Y | K=V)
// newAnds (and thus known): (K=V) --> true
const cleaned = containedOr.removePhraseConsideredKnown(known, true)
@ -401,11 +409,17 @@ export class And extends TagsFilter {
containedOr = cleaned
continue
}
if (cleaned instanceof And) {
// An optimized 'And' should not contain 'Ands', we can safely cast
newAnds.push(...TagTypes.safeAnd(cleaned))
continue
}
// the 'or' dissolved into a normal tag -> it has to be added to the newAnds
newAnds.push(cleaned)
dirty = true // rerun this algo later on
continue outer
}
cleanedContainedOrs.push(containedOr)
}
containedOrs = cleanedContainedOrs
@ -433,9 +447,9 @@ export class And extends TagsFilter {
if (commonValues.length === 0) {
newAnds.push(...containedOrs)
} else {
const newOrs: TagsFilter[] = []
const newOrs: TagsFilterClosed[] = []
for (const containedOr of containedOrs) {
const elements = containedOr.or.filter(
const elements: (FlatTag | (And & OptimizedTag))[] = TagTypes.safeOr( containedOr).filter(
(candidate) => !commonValues.some((cv) => cv.shadows(candidate))
)
if (elements.length > 0) {
@ -450,6 +464,8 @@ export class And extends TagsFilter {
return false
} else if (result === true) {
// neutral element: skip
}else if(result instanceof And) {
newAnds.push(...TagTypes.safeAnd(result))
} else {
newAnds.push(result)
}

5
src/Logic/Tags/ComparingTag.ts
View file

@ -1,6 +1,7 @@
import { TagsFilter } from "./TagsFilter"
import { Tag } from "./Tag"
import { ExpressionSpecification } from "maplibre-gl"
import { OptimizedTag } from "./TagTypes"
export default class ComparingTag extends TagsFilter {
public readonly key: string
@ -121,8 +122,8 @@ export default class ComparingTag extends TagsFilter {
return this.key + this._representation + this._boundary
}
optimize(): TagsFilter | boolean {
return this
optimize(): (ComparingTag & OptimizedTag) | boolean {
return <any> this
}
isNegative(): boolean {

63
src/Logic/Tags/Or.ts
View file

@ -3,6 +3,11 @@ import { TagUtils } from "./TagUtils"
import { And } from "./And"
import { TagConfigJson } from "../../Models/ThemeConfig/Json/TagConfigJson"
import { ExpressionSpecification } from "maplibre-gl"
import { Tag } from "./Tag"
import { RegexTag } from "./RegexTag"
import SubstitutingTag from "./SubstitutingTag"
import ComparingTag from "./ComparingTag"
import { FlatTag, OptimizedTag, TagsFilterClosed, TagTypes } from "./TagTypes"
export class Or extends TagsFilter {
public or: TagsFilter[]
@ -12,6 +17,9 @@ export class Or extends TagsFilter {
this.or = or
}
public static construct(or: TagsFilter[]): TagsFilter
public static construct<T extends TagsFilter>(or: [T]): T
public static construct(or: ((And & OptimizedTag) | FlatTag)[]): (TagsFilterClosed & OptimizedTag)
public static construct(or: TagsFilter[]): TagsFilter {
if (or.length === 1) {
return or[0]
@ -59,7 +67,7 @@ export class Or extends TagsFilter {
return this.or
.map((t) => {
let e = t.asHumanString(linkToWiki, shorten, properties)
if (t["and"]) {
if (t["and"] || t["or"]) {
e = "(" + e + ")"
}
return e
@ -115,13 +123,13 @@ export class Or extends TagsFilter {
* new Or([ new Tag("key","value") ,new Tag("other_key","value")]).removePhraseConsideredKnown(new Tag("key","value"), false) // => new Tag("other_key","value")
* new Or([ new Tag("key","value") ]).removePhraseConsideredKnown(new Tag("key","value"), true) // => true
* new Or([ new Tag("key","value") ]).removePhraseConsideredKnown(new Tag("key","value"), false) // => false
* new Or([new RegexTag("x", "y", true),new RegexTag("c", "d")]).removePhraseConsideredKnown(new Tag("foo","bar"), false) // => new Or([new RegexTag("x", "y", true),new RegexTag("c", "d")])
* new Or([new RegexTag("x", "y", true),new RegexTag("c", "d")]).removePhraseConsideredKnown(new Tag("foo","bar"), false) // => new Or([new RegexTag("c", "d"), new RegexTag("x", "y", true)])
*/
removePhraseConsideredKnown(knownExpression: TagsFilter, value: boolean): TagsFilter | boolean {
removePhraseConsideredKnown(knownExpression: TagsFilter, value: boolean): (TagsFilterClosed & OptimizedTag) | boolean {
const newOrs: TagsFilter[] = []
for (const tag of this.or) {
if (tag instanceof Or) {
throw "Optimize expressions before using removePhraseConsideredKnown. Found an OR in an OR: "+this.asHumanString(false, false, {})
throw "Optimize expressions before using removePhraseConsideredKnown. Found an OR in an OR: " + this.asHumanString()
}
if (tag instanceof And) {
const r = tag.removePhraseConsideredKnown(knownExpression, value)
@ -160,7 +168,7 @@ export class Or extends TagsFilter {
if (newOrs.length === 0) {
return false
}
return Or.construct(newOrs)
return Or.construct(newOrs).optimize()
}
/**
@ -169,7 +177,7 @@ export class Or extends TagsFilter {
* parsed.optimize().asJson() // => {"and":["leisure=playground","playground!=forest"]}
*
*/
optimize(): TagsFilter | boolean {
optimize(): (TagsFilterClosed & OptimizedTag) | boolean {
if (this.or.length === 0) {
return false
}
@ -181,26 +189,28 @@ export class Or extends TagsFilter {
// We have an OR with a contained true: this is always 'true'
return true
}
const optimized = <TagsFilter[]>optimizedRaw
const optimized = optimizedRaw
const newOrs: TagsFilter[] = []
let containedAnds: And[] = []
const newOrs: ((And & OptimizedTag) | FlatTag)[] = []
let containedAnds: (And & OptimizedTag)[] = []
for (const tf of optimized) {
if (tf["or"]) {
if (tf instanceof Or) {
// expand all the nested ors...
newOrs.push(...tf["or"])
for (const clauseOr of TagTypes.safeOr(tf)) {
newOrs.push(clauseOr)
}
} else if (tf instanceof And) {
// partition of all the ands
containedAnds.push(tf)
} else {
newOrs.push(tf)
newOrs.push(<(Tag | (And & OptimizedTag) | RegexTag | SubstitutingTag | ComparingTag)>tf)
}
}
{
let dirty = false
do {
const cleanedContainedANds: And[] = []
const cleanedContainedANds: (And & OptimizedTag)[] = []
outer: for (let containedAnd of containedAnds) {
for (const known of newOrs) {
// input for optimization: (K=V | (X=Y & K=V))
@ -219,8 +229,16 @@ export class Or extends TagsFilter {
containedAnd = cleaned
continue // clean up with the other known values
}
if(cleaned instanceof Or){
// An optimized 'or' should not contain 'ors', we can safely cast
newOrs.push(...TagTypes.safeOr(cleaned))
continue
}
const noAnd: OptimizedTag & (Tag | RegexTag | SubstitutingTag | ComparingTag) = cleaned
// the 'and' dissolved into a normal tag -> it has to be added to the newOrs
newOrs.push(cleaned)
newOrs.push(noAnd)
dirty = true // rerun this algo later on
continue outer
}
@ -243,16 +261,16 @@ export class Or extends TagsFilter {
if (commonValues.length === 0) {
newOrs.push(...containedAnds)
} else {
const newAnds: TagsFilter[] = []
const newAnds: TagsFilterClosed[] = []
for (const containedAnd of containedAnds) {
const elements = containedAnd.and.filter(
const elements: (FlatTag | (Or & OptimizedTag))[] = TagTypes.safeAnd(containedAnd).filter(
(candidate) => !commonValues.some((cv) => cv.shadows(candidate))
)
if (elements.length == 0) {
continue
if (elements.length > 0) {
newAnds.push(And.construct(elements))
}
newAnds.push(And.construct(elements))
}
if (newAnds.length > 0) {
commonValues.push(Or.construct(newAnds))
}
@ -262,9 +280,10 @@ export class Or extends TagsFilter {
return true
} else if (result === false) {
// neutral element: skip
} else if (commonValues.length > 0) {
newOrs.push(And.construct(commonValues))
}
} else if(result instanceof Or){
newOrs.push(...TagTypes.safeOr(result))
}else
newOrs.push(result)
}
}

5
src/Logic/Tags/RegexTag.ts
View file

@ -2,6 +2,7 @@ import { Tag } from "./Tag"
import { TagsFilter } from "./TagsFilter"
import { TagConfigJson } from "../../Models/ThemeConfig/Json/TagConfigJson"
import { ExpressionSpecification } from "maplibre-gl"
import { OptimizedTag } from "./TagTypes"
export class RegexTag extends TagsFilter {
public readonly key: RegExp | string
@ -354,8 +355,8 @@ export class RegexTag extends TagsFilter {
return []
}
optimize(): TagsFilter | boolean {
return this
optimize(): (RegexTag & OptimizedTag) | boolean {
return <any> this
}
isNegative(): boolean {

5
src/Logic/Tags/SubstitutingTag.ts
View file

@ -3,6 +3,7 @@ import { Tag } from "./Tag"
import { Utils } from "../../Utils"
import { TagConfigJson } from "../../Models/ThemeConfig/Json/TagConfigJson"
import { ExpressionSpecification } from "maplibre-gl"
import { OptimizedTag } from "./TagTypes"
/**
* The substituting-tag uses the tags of a feature a variables and replaces them.
@ -111,8 +112,8 @@ export default class SubstitutingTag extends TagsFilter {
return [{ k: this._key, v: v }]
}
optimize(): TagsFilter | boolean {
return this
optimize(): (SubstitutingTag & OptimizedTag) | boolean {
return <any> this
}
isNegative(): boolean {

7
src/Logic/Tags/Tag.ts
View file

@ -3,6 +3,7 @@ import { TagsFilter } from "./TagsFilter"
import { TagConfigJson } from "../../Models/ThemeConfig/Json/TagConfigJson"
import { ExpressionSpecification } from "maplibre-gl"
import { RegexTag } from "./RegexTag"
import { OptimizedTag } from "./TagTypes"
export class Tag extends TagsFilter {
public key: string
@ -66,7 +67,7 @@ export class Tag extends TagsFilter {
asOverpass(): string[] {
if (this.value === "") {
// NOT having this key
return ['[!"' + this.key + '"]']
return ["[!\"" + this.key + "\"]"]
}
return [`["${this.key}"="${this.value}"]`]
}
@ -164,8 +165,8 @@ export class Tag extends TagsFilter {
return [{ k: this.key, v: this.value }]
}
optimize(): TagsFilter | boolean {
return this
optimize(): (Tag & OptimizedTag) | boolean {
return <any>this
}
isNegative(): boolean {

35
src/Logic/Tags/TagTypes.ts Normal file
View file

@ -0,0 +1,35 @@
import { TagsFilter } from "./TagsFilter"
import { Tag } from "./Tag"
import SubstitutingTag from "./SubstitutingTag"
import { And } from "./And"
import { RegexTag } from "./RegexTag"
import ComparingTag from "./ComparingTag"
import { Or } from "./Or"
declare const __is_optimized: unique symbol
type Brand<B> = { [__is_optimized]: B }
/**
* A marker class, no actual content
*/
export type OptimizedTag = Brand<TagsFilter>
export type UploadableTag = Tag | SubstitutingTag | And
/**
* Not nested
*/
export type FlatTag = Tag | RegexTag | SubstitutingTag | ComparingTag
export type TagsFilterClosed = FlatTag | And | Or
export class TagTypes {
static safeAnd(and: And & OptimizedTag): ((FlatTag | (Or & OptimizedTag)) & OptimizedTag)[]{
return <any> and.and
}
static safeOr(or: Or & OptimizedTag): ((FlatTag | (And & OptimizedTag)) & OptimizedTag)[]{
return <any> or.or
}
}

6
src/Logic/Tags/TagUtils.ts
View file

@ -10,9 +10,9 @@ import { TagConfigJson } from "../../Models/ThemeConfig/Json/TagConfigJson"
import key_counts from "../../assets/key_totals.json"
import { ConversionContext } from "../../Models/ThemeConfig/Conversion/ConversionContext"
import { TagsFilterClosed, UploadableTag } from "./TagTypes"
type Tags = Record<string, string>
export type UploadableTag = Tag | SubstitutingTag | And
export class TagUtils {
public static readonly comparators: ReadonlyArray<
@ -476,7 +476,7 @@ export class TagUtils {
* regex.matchesProperties({maxspeed: "50 mph"}) // => true
*/
public static Tag(json: TagConfigJson, context: string | ConversionContext = ""): TagsFilter {
public static Tag(json: TagConfigJson, context: string | ConversionContext = ""): TagsFilterClosed {
try {
const ctx = typeof context === "string" ? context : context.path.join(".")
return this.ParseTagUnsafe(json, ctx)
@ -712,7 +712,7 @@ export class TagUtils {
return Utils.NoNull(spec)
}
private static ParseTagUnsafe(json: TagConfigJson, context: string = ""): TagsFilter {
private static ParseTagUnsafe(json: TagConfigJson, context: string = ""): TagsFilterClosed {
if (json === undefined) {
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`

3
src/Logic/Tags/TagsFilter.ts
View file

@ -1,5 +1,6 @@
import { TagConfigJson } from "../../Models/ThemeConfig/Json/TagConfigJson"
import { ExpressionSpecification } from "maplibre-gl"
import { OptimizedTag, TagsFilterClosed } from "./TagTypes"
export abstract class TagsFilter {
abstract asOverpass(): string[]
@ -50,7 +51,7 @@ export abstract class TagsFilter {
/**
* Returns an optimized version (or self) of this tagsFilter
*/
abstract optimize(): TagsFilter | boolean
abstract optimize(): (OptimizedTag & TagsFilterClosed) | boolean
/**
* Returns 'true' if the tagsfilter might select all features (i.e. the filter will return everything from OSM, except a few entries).