Skip to content

Commit

Permalink
harden parser: acknowledge other "yes"-like values such as "permissiv…
Browse files Browse the repository at this point in the history
…e" etc
  • Loading branch information
westnordost committed Dec 10, 2022
1 parent ea0d653 commit af1ad7e
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,14 @@ fun SeparateCycleway.applyTo(tags: Tags) {
when (this) {
PATH -> {
tags["highway"] = "path"
if (tags["foot"] == "no" || tags["foot"] == "designated") tags["foot"] = "yes"
if (tags["bicycle"] == "no" || tags["bicycle"] == "designated") tags["bicycle"] = "yes"

// only re-tag to "yes" if defined and not some kind of "yes" value
if (tags.containsKey("foot") && tags["foot"] !in yesButNotDesignated) {
tags["foot"] = "yes"
}
if (tags.containsKey("bicycle") && tags["bicycle"] !in yesButNotDesignated) {
tags["bicycle"] = "yes"
}
}
NOT_ALLOWED, ALLOWED_ON_FOOTWAY, NON_DESIGNATED -> {
// not a cycleway if not designated as one!
Expand All @@ -26,9 +32,15 @@ fun SeparateCycleway.applyTo(tags: Tags) {
}

when (this) {
NOT_ALLOWED -> tags["bicycle"] = "no"
ALLOWED_ON_FOOTWAY -> tags["bicycle"] = "yes"
else -> if (tags["bicycle"] == "designated") tags.remove("bicycle")
NOT_ALLOWED -> {
if (tags["bicycle"] !in noCycling) tags["bicycle"] = "no"
}
ALLOWED_ON_FOOTWAY -> {
if (tags["bicycle"] !in yesButNotDesignated) tags["bicycle"] = "yes"
}
else -> {
if (tags["bicycle"] == "designated") tags.remove("bicycle")
}
}
if (tags["foot"] == "no") {
tags["foot"] = "yes"
Expand All @@ -38,7 +50,8 @@ fun SeparateCycleway.applyTo(tags: Tags) {
if (!isCycleway || tags.containsKey("bicycle")) {
tags["bicycle"] = "designated"
}
if (isCycleway || tags.containsKey("foot")) {
// do not retag highway=cycleway + foot=yes
if ((isCycleway || tags.containsKey("foot")) && tags["foot"] !in yesButNotDesignated) {
tags["foot"] = "designated"
}
}
Expand Down Expand Up @@ -88,3 +101,11 @@ fun SeparateCycleway.applyTo(tags: Tags) {
tags.updateCheckDateForKey("bicycle")
}
}

private val noCycling = setOf(
"no", "dismount"
)

private val yesButNotDesignated = setOf(
"yes", "permissive", "private", "destination", "customers", "permit"
)
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,27 @@ fun createSeparateCycleway(tags: Map<String, String>): SeparateCycleway? {
else -> null // only happens if highway=cycleway
}

if (bicycle == "no") return NOT_ALLOWED
if (bicycle in noCycling) return NOT_ALLOWED

if (bicycle == "yes" && foot == "yes") return PATH
if (bicycle in yesButNotDesignated && foot in yesButNotDesignated) return PATH

if (bicycle == "yes" && foot == "designated") return ALLOWED_ON_FOOTWAY
if (bicycle in yesButNotDesignated && foot == "designated") return ALLOWED_ON_FOOTWAY

if (bicycle != "designated") return NON_DESIGNATED

val hasSidewalk = createSidewalkSides(tags)?.any { it == Sidewalk.YES } == true || tags["sidewalk"] == "yes"
if (hasSidewalk) return EXCLUSIVE_WITH_SIDEWALK

if (foot !in listOf("yes", "designated")) return EXCLUSIVE
if (foot == "no" || foot == null) return EXCLUSIVE

val segregated = tags["segregated"] == "yes"
return if (segregated) SEGREGATED else NON_SEGREGATED
}

private val noCycling = setOf(
"no", "dismount"
)

private val yesButNotDesignated = setOf(
"yes", "permissive", "private", "destination", "customers", "permit"
)
4 changes: 2 additions & 2 deletions app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1582,8 +1582,8 @@ Partially means that a wheelchair can enter and use the restroom, but no handrai
<string name="separate_cycleway_allowed">Cyclists are allowed</string>
<!-- highway=path / foot=yes + bicycle=yes -->
<string name="separate_cycleway_path">Path or trail</string>
<!-- highway=footway / foot=designated -->
<string name="separate_cycleway_no_or_allowed">Sidewalk or foot path</string>
<!-- highway=footway / foot=designated and anything else that is not designated for cyclists-->
<string name="separate_cycleway_no_or_allowed">Not designated for cyclists (cycling may still be allowed)</string>
<!-- foot=designated + bicycle=designated -->
<string name="separate_cycleway_non_segregated">Shared-use path</string>
<!-- foot=designated + bicycle=designated + segregated=yes -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,16 @@ class SeparateCyclewayCreatorKtTest {
)
}

@Test fun `apply none does not change bicycle=dismount`() {
verifyAnswer(
mapOf("highway" to "cycleway", "bicycle" to "dismount"),
NOT_ALLOWED,
arrayOf(
StringMapEntryModify("highway", "cycleway", "path"),
)
)
}

@Test fun `apply none re-tags cycleway and adds foot=yes if foot was no before`() {
verifyAnswer(
mapOf("highway" to "cycleway", "foot" to "no"),
Expand Down Expand Up @@ -92,6 +102,24 @@ class SeparateCyclewayCreatorKtTest {
ALLOWED_ON_FOOTWAY,
arrayOf(StringMapEntryAdd("bicycle", "yes"))
)
verifyAnswer(
mapOf("highway" to "footway", "bicycle" to "no"),
ALLOWED_ON_FOOTWAY,
arrayOf(StringMapEntryModify("bicycle", "no", "yes"))
)
}

@Test fun `apply allowed does not re-tag bicycle=permissive etc`() {
verifyAnswer(
mapOf("highway" to "footway", "bicycle" to "permissive"),
ALLOWED_ON_FOOTWAY,
arrayOf(StringMapEntryAdd("check_date:bicycle", nowAsCheckDateString()))
)
verifyAnswer(
mapOf("highway" to "footway", "bicycle" to "private"),
ALLOWED_ON_FOOTWAY,
arrayOf(StringMapEntryAdd("check_date:bicycle", nowAsCheckDateString()))
)
}

@Test fun `apply allowed re-tags cycleway and adds foot=yes if foot was not yes before`() {
Expand Down Expand Up @@ -263,7 +291,6 @@ class SeparateCyclewayCreatorKtTest {
NON_SEGREGATED,
arrayOf(
StringMapEntryAdd("bicycle", "designated"),
StringMapEntryModify("foot", "yes", "designated"),
StringMapEntryAdd("segregated", "no"),
)
)
Expand All @@ -286,6 +313,23 @@ class SeparateCyclewayCreatorKtTest {
)
}

@Test fun `apply non-segregated does not re-tag any yes-like value`() {
verifyAnswer(
mapOf("highway" to "cycleway", "foot" to "yes"),
NON_SEGREGATED,
arrayOf(
StringMapEntryAdd("segregated", "no"),
)
)
verifyAnswer(
mapOf("highway" to "cycleway", "foot" to "permissive"),
NON_SEGREGATED,
arrayOf(
StringMapEntryAdd("segregated", "no"),
)
)
}

@Test fun `apply non-segregated removes sidewalk tags`() {
verifyAnswer(
mapOf(
Expand Down Expand Up @@ -336,15 +380,6 @@ class SeparateCyclewayCreatorKtTest {
StringMapEntryAdd("segregated", "yes"),
)
)
verifyAnswer(
mapOf("highway" to "footway", "foot" to "yes"),
SEGREGATED,
arrayOf(
StringMapEntryAdd("bicycle", "designated"),
StringMapEntryModify("foot", "yes", "designated"),
StringMapEntryAdd("segregated", "yes"),
)
)
verifyAnswer(
mapOf("highway" to "cycleway"),
SEGREGATED,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,30 @@ class SeparateCyclewayParserKtTest {
assertEquals(PATH, parse("highway" to "path"))
assertEquals(PATH, parse("highway" to "cycleway", "foot" to "yes", "bicycle" to "yes"))
assertEquals(PATH, parse("highway" to "footway", "foot" to "yes", "bicycle" to "yes"))
assertEquals(PATH, parse("highway" to "path", "foot" to "permissive", "bicycle" to "permissive"))
assertEquals(PATH, parse("highway" to "path", "foot" to "customers", "bicycle" to "destination"))
assertEquals(PATH, parse("highway" to "path", "bicycle" to "permissive"))
assertEquals(PATH, parse("highway" to "path", "bicycle" to "private"))
}

@Test fun `parse not designated for cyclists`() {
assertEquals(NON_DESIGNATED, parse("highway" to "footway"))
assertEquals(NON_DESIGNATED, parse("highway" to "path", "bicycle" to "permissive"))
assertEquals(NON_DESIGNATED, parse("highway" to "path", "bicycle" to "private"))
assertEquals(NON_DESIGNATED, parse("highway" to "cycleway", "bicycle" to "yes"))

assertEquals(NON_DESIGNATED, parse("highway" to "cycleway", "bicycle" to "yes", "foot" to "no"))
}

@Test fun `parse no bicyclists allowed on path`() {
assertEquals(NOT_ALLOWED, parse("highway" to "path", "bicycle" to "no"))
assertEquals(NOT_ALLOWED, parse("highway" to "footway", "bicycle" to "no"))
assertEquals(NOT_ALLOWED, parse("highway" to "cycleway", "bicycle" to "no"))
assertEquals(NOT_ALLOWED, parse("highway" to "cycleway", "bicycle" to "dismount"))
}

@Test fun `parse bicyclists allowed on footway`() {
assertEquals(ALLOWED_ON_FOOTWAY, parse("highway" to "footway", "bicycle" to "yes"))
assertEquals(ALLOWED_ON_FOOTWAY, parse("highway" to "footway", "bicycle" to "destination"))
assertEquals(ALLOWED_ON_FOOTWAY, parse("highway" to "path", "foot" to "designated", "bicycle" to "yes"))
assertEquals(ALLOWED_ON_FOOTWAY, parse("highway" to "path", "foot" to "designated", "bicycle" to "permissive"))
}

@Test fun `parse cyclists on non-segregated path`() {
Expand Down

0 comments on commit af1ad7e

Please sign in to comment.