Skip to content

Commit

Permalink
MixedCaseNameCheck Enhancements (without AutoFix) (osmlab#546)
Browse files Browse the repository at this point in the history 
* fixing rebase issues.

* Revert "fixing rebase issues."

This reverts commit 4573d4a

* fixing ci issues.

* reverting commit.

* MixedCaseNameCheck Autofix initial draft

* Removed AutoFix, added more handling and unittest

* code smells: reduce number of Cognitive Complexity.

* code smells: reduce number of Cognitive Complexity 2.

* adding more handling to 'n' case.

* minor fix
  • Loading branch information
@vladlemberg
vladlemberg authored May 4, 2021
1 parent 9cc4d42 commit 3844e43
Show file tree
Hide file tree
Showing 4 changed files with 157 additions and 6 deletions.
4 changes: 2 additions & 2 deletions config/configuration.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -696,8 +696,8 @@
"language.keys":["name:en"],
"affixes":["Mc", "Mac", "Mck","Mhic", "Mic"],
"articles": ["a", "an", "the"],
"prepositions": ["and", "from", "to", "of", "by", "upon", "on", "off", "at", "as",
"into", "like", "near", "onto", "per", "till", "up", "via", "with", "for", "in"],
"prepositions": ["and", "from", "to", "of", "by", "upon", "on", "off", "at", "as", "into", "like", "near",
"onto", "per", "till", "up", "via", "with", "for", "in", "de", "la", "del", "du", "van", "der"],
"units":["kv"]
},
"regex.split":" -/&@–",
Expand Down
54 changes: 52 additions & 2 deletions src/main/java/org/openstreetmap/atlas/checks/validation/tag/MixedCaseNameCheck.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@
import org.openstreetmap.atlas.geography.atlas.items.LocationItem;
import org.openstreetmap.atlas.geography.atlas.items.Relation;
import org.openstreetmap.atlas.geography.atlas.walker.OsmWayWalker;
import org.openstreetmap.atlas.tags.AmenityTag;
import org.openstreetmap.atlas.tags.BrandTag;
import org.openstreetmap.atlas.tags.ISOCountryTag;
import org.openstreetmap.atlas.tags.LeisureTag;
import org.openstreetmap.atlas.tags.ShopTag;
import org.openstreetmap.atlas.tags.annotations.validation.Validators;
import org.openstreetmap.atlas.tags.names.NameTag;
import org.openstreetmap.atlas.utilities.configuration.Configuration;
Expand All @@ -40,7 +44,7 @@ public class MixedCaseNameCheck extends BaseCheck<String>
private static final List<String> LANGUAGE_NAME_TAGS_DEFAULT = Arrays.asList("name:en");
private static final List<String> LOWER_CASE_PREPOSITIONS_DEFAULT = Arrays.asList("and", "from",
"to", "of", "by", "upon", "on", "off", "at", "as", "into", "like", "near", "onto",
"per", "till", "up", "via", "with", "for", "in");
"per", "till", "up", "via", "with", "for", "in", "de", "la", "del", "du", "van", "der");
private static final List<String> LOWER_CASE_ARTICLES_DEFAULT = Arrays.asList("a", "an", "the");
private static final String SPLIT_CHARACTERS_DEFAULT = " -/&@–";
private static final List<String> NAME_AFFIXES_DEFAULT = Arrays.asList("Mc", "Mac", "Mck",
Expand Down Expand Up @@ -69,6 +73,8 @@ public class MixedCaseNameCheck extends BaseCheck<String>
private final Pattern mixedCaseUnitsPattern;
private final Pattern mixedCaseApostrophePattern;
private final Pattern nonFirstCapitalPattern;
private final Pattern iCasePattern;
private final Pattern dashAndDashPattern;

/**
* The default constructor that must be supplied. The Atlas Checks framework will generate the
Expand Down Expand Up @@ -106,6 +112,8 @@ public MixedCaseNameCheck(final Configuration configuration)
.compile("([^\\p{Ll}]+'\\p{Ll})|([^\\p{Ll}]+\\p{Ll}')");
this.nonFirstCapitalPattern = Pattern.compile(String.format(
"(\\p{L}.*(?<!'|%1$s)(\\p{Lu}))|(\\p{L}.*(?<=')\\p{Lu}(?!.))", this.nameAffixes));
this.iCasePattern = Pattern.compile("^i[A-Z]");
this.dashAndDashPattern = Pattern.compile("^n$|^n'$|^'n$|^'n'$");
}

/**
Expand All @@ -127,6 +135,10 @@ public boolean validCheckForObject(final AtlasObject object)
.contains(object.tag(ISOCountryTag.KEY).toUpperCase())
// And have a name tag
&& Validators.hasValuesFor(object, NameTag.class)
// Excluding names with following tags to reduce number of legit
// MixedCaseNames
&& !Validators.hasValuesFor(object, BrandTag.class, ShopTag.class,
AmenityTag.class, LeisureTag.class)
// And if an Edge, is a main edge
&& (!(object instanceof Edge) || ((Edge) object).isMainEdge())
// Or it must have a specific language name tag from languageNameTags
Expand Down Expand Up @@ -191,6 +203,44 @@ protected List<String> getFallbackInstructions()
return FALLBACK_INSTRUCTIONS;
}

/**
* Tests if {@link String} matches one of the patterns.
*
* @param word
* {@link String} to test
* @return true if {@code word} matches the {@code dashAndDashPattern} or {@code iCasePattern}.
*/
private boolean isAlowedPatternHandling(final String word)
{
return this.isMixedCaseUnit(word) || this.isICase(word) || this.isDashAndDashCase(word);
}

/**
* Tests if {@link String} is "n" or "'n"
*
* @param word
* {@link String} to test
* @return true if {@code word} matches the {@code dashAndDashPattern}.
*/
private boolean isDashAndDashCase(final String word)
{
// return true for following cases: Rock-n-Roll, Rock n' Roll
return this.dashAndDashPattern.matcher(word).find();
}

/**
* Tests if {@link String} starts with lower "i" character followed by capital letter.
*
* @param word
* {@link String} to test
* @return true if {@code word} matches the {@code iCasePattern}.
*/
private boolean isICase(final String word)
{
// return true for following cases: iExample, iCode
return this.iCasePattern.matcher(word).find();
}

/**
* Tests each word in a string for proper use of case in a name.
*
Expand All @@ -210,7 +260,7 @@ private boolean isMixedCase(final String value)
for (final String word : wordArray)
{
// Check if the word is intentionally mixed case
if (!this.isMixedCaseUnit(word))
if (!this.isAlowedPatternHandling(word))
{
final Matcher firstLetterMatcher = this.anyLetterPattern.matcher(word);
// If the word is not in the list of prepositions, and the
Expand Down
34 changes: 34 additions & 0 deletions src/test/java/org/openstreetmap/atlas/checks/validation/tag/MixedCaseNameCheckTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,22 @@ public void invalidNameEdgeTest()
this.verifier.globallyVerify(flags -> Assert.assertEquals(1, flags.size()));
}

@Test
public void invalidNameICase()
{
this.verifier.actual(this.setup.invalidNameICase(),
new MixedCaseNameCheck(this.inlineConfiguration));
this.verifier.globallyVerify(flags -> Assert.assertEquals(1, flags.size()));
}

@Test
public void invalidNameICaseMiddle()
{
this.verifier.actual(this.setup.invalidNameICaseMiddle(),
new MixedCaseNameCheck(this.inlineConfiguration));
this.verifier.globallyVerify(flags -> Assert.assertEquals(1, flags.size()));
}

@Test
public void invalidNameLineTest()
{
Expand Down Expand Up @@ -135,6 +151,15 @@ public void invalidNamePointTest()
this.verifier.globallyVerify(flags -> Assert.assertEquals(1, flags.size()));
}

@Test
public void validNameICase()
{
// Example: iRobot
this.verifier.actual(this.setup.validNameICase(),
new MixedCaseNameCheck(this.inlineConfiguration));
this.verifier.globallyVerify(flags -> Assert.assertEquals(0, flags.size()));
}

@Test
public void validNamePointAffixTest()
{
Expand Down Expand Up @@ -199,6 +224,15 @@ public void validNamePointChnTest()
this.verifier.globallyVerify(flags -> Assert.assertEquals(0, flags.size()));
}

@Test
public void validNamePointDashAndDashTest()
{
// example: Rock-n-Roll
this.verifier.actual(this.setup.validNamePointDashAndDash(),
new MixedCaseNameCheck(this.inlineConfiguration));
this.verifier.globallyVerify(flags -> Assert.assertEquals(0, flags.size()));
}

@Test
public void validNamePointGreekElTest()
{
Expand Down
71 changes: 69 additions & 2 deletions src/test/java/org/openstreetmap/atlas/checks/validation/tag/MixedCaseNameCheckTestRule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,34 @@ public class MixedCaseNameCheckTestRule extends CoreTestRule

@TestAtlas(
// points
points = { @Point(coordinates = @Loc(value = TEST_1), tags = { "iso_country_code=USA",
"name=Test of Test" }) })
points = {
@Point(coordinates = @Loc(value = TEST_1), tags = { "iso_country_code=USA",
"name=Test-n-Test" }),
@Point(coordinates = @Loc(value = TEST_2), tags = { "iso_country_code=USA",
"name=Test n' Test" }),
@Point(coordinates = @Loc(value = TEST_2), tags = { "iso_country_code=USA",
"name=Test 'n Test" }),
@Point(coordinates = @Loc(value = TEST_2), tags = { "iso_country_code=USA",
"name=Test 'n' Test" }) })
private Atlas validNamePointDashAndDash;

@TestAtlas(
// points
points = {
@Point(coordinates = @Loc(value = TEST_1), tags = { "iso_country_code=USA",
"name=Test of Test" }),
@Point(coordinates = @Loc(value = TEST_1), tags = { "iso_country_code=USA",
"name=Plaza de la Test" }),
@Point(coordinates = @Loc(value = TEST_1), tags = { "iso_country_code=USA",
"name=Via del Test" }),
@Point(coordinates = @Loc(value = TEST_1), tags = { "iso_country_code=USA",
"name=Test de Test" }),
@Point(coordinates = @Loc(value = TEST_1), tags = { "iso_country_code=USA",
"name=Test of Test" }),
@Point(coordinates = @Loc(value = TEST_1), tags = { "iso_country_code=USA",
"name=Test van der Test" }),
@Point(coordinates = @Loc(value = TEST_1), tags = { "iso_country_code=USA",
"name=Test du Test" }) })
private Atlas validNamePointLowerCasePrepositionAtlas;

@TestAtlas(
Expand Down Expand Up @@ -210,6 +236,27 @@ public class MixedCaseNameCheckTestRule extends CoreTestRule
"name:en=Test of Test", "name=tEst TeSt" }) })
private Atlas validNamePointChnAtlas;

@TestAtlas(
// points
points = {
@Point(coordinates = @Loc(value = TEST_1), tags = { "iso_country_code=USA",
"name:en=iTest", "name=iTest" }),
@Point(coordinates = @Loc(value = TEST_2), tags = { "iso_country_code=USA",
"name=Test of iTest Middle" }) })
private Atlas validNameICase;

@TestAtlas(
// points
points = { @Point(coordinates = @Loc(value = TEST_1), tags = { "iso_country_code=USA",
"name=itest Corp" }) })
private Atlas invalidNameICase;

@TestAtlas(
// points
points = { @Point(coordinates = @Loc(value = TEST_1), tags = { "iso_country_code=USA",
"name=MiddleiTest" }) })
private Atlas invalidNameICaseMiddle;

public Atlas invalidNameAreaAtlas()
{
return this.invalidNameAreaAtlas;
Expand All @@ -220,6 +267,16 @@ public Atlas invalidNameEdgeAtlas()
return this.invalidNameEdgeAtlas;
}

public Atlas invalidNameICase()
{
return this.invalidNameICase;
}

public Atlas invalidNameICaseMiddle()
{
return this.invalidNameICaseMiddle;
}

public Atlas invalidNameLineAtlas()
{
return this.invalidNameLineAtlas;
Expand Down Expand Up @@ -280,6 +337,11 @@ public Atlas invalidNamePointOneWordAtlas()
return this.invalidNamePointOneWordAtlas;
}

public Atlas validNameICase()
{
return this.validNameICase;
}

public Atlas validNamePointAffixAtlas()
{
return this.validNamePointAffixAtlas;
Expand Down Expand Up @@ -320,6 +382,11 @@ public Atlas validNamePointChnAtlas()
return this.validNamePointChnAtlas;
}

public Atlas validNamePointDashAndDash()
{
return this.validNamePointDashAndDash;
}

public Atlas validNamePointGreekAtlas()
{
return this.validNamePointGreekAtlas;
Expand Down

0 comments on commit 3844e43

Please sign in to comment.