#396 HighwayToFerryTagCheck fix suggestions (#524)

* Added fix suggestion for HighwayToFerryTagCheck

- implemented
- added new verifiers for this in existing unit tests

* Changed complete entity to shallow version with tags

* Extended fix suggestion to all edges

src/main/java/org/openstreetmap/atlas/checks/validation/tag/HighwayToFerryTagCheck.java

@@ -4,10 +4,15 @@
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 import org.openstreetmap.atlas.checks.atlas.predicates.TypePredicates;
 import org.openstreetmap.atlas.checks.base.BaseCheck;
 import org.openstreetmap.atlas.checks.flag.CheckFlag;
+import org.openstreetmap.atlas.geography.atlas.change.FeatureChange;
+import org.openstreetmap.atlas.geography.atlas.complete.CompleteEdge;
+import org.openstreetmap.atlas.geography.atlas.complete.CompleteEntity;
+import org.openstreetmap.atlas.geography.atlas.items.AtlasEntity;
 import org.openstreetmap.atlas.geography.atlas.items.AtlasObject;
 import org.openstreetmap.atlas.geography.atlas.items.Edge;
 import org.openstreetmap.atlas.geography.atlas.walker.OsmWayWalker;
@@ -83,13 +88,23 @@ protected Optional<CheckFlag> flag(final AtlasObject object)
         if (Validators.hasValuesFor(object, FerryTag.class))
         {
             final int instructionIndex = hasSameHighwayClassification ? 1 : 0;
-            return Optional.of(this.createFlag(edges,
-                    this.getLocalizedInstruction(instructionIndex, object.getOsmIdentifier())));
+            return Optional.of(this
+                    .createFlag(edges,
+                            this.getLocalizedInstruction(instructionIndex,
+                                    object.getOsmIdentifier()))
+                    .addFixSuggestions(edges.stream().map(
+                            edge -> this.getFixSuggestion(edge, true, hasSameHighwayClassification))
+                            .collect(Collectors.toSet())));
         }
         else
         {
-            return Optional.of(this.createFlag(edges,
-                    this.getLocalizedInstruction(2, object.getOsmIdentifier())));
+            return Optional.of(this
+                    .createFlag(edges, this.getLocalizedInstruction(2, object.getOsmIdentifier()))
+                    .addFixSuggestions(
+                            edges.stream()
+                                    .map(edge -> this.getFixSuggestion(edge, false,
+                                            hasSameHighwayClassification))
+                                    .collect(Collectors.toSet())));
         }
     }
 
@@ -99,6 +114,36 @@ protected List<String> getFallbackInstructions()
         return FALLBACK_INSTRUCTIONS;
     }
 
+    /**
+     * Returns the appropriate fix suggestion based on the highway and ferry tags.
+     * 
+     * @param object
+     *            {@link AtlasObject} whose fix suggestion needs to be established
+     * @param hasFerryTag
+     *            does the object have a ferry tag
+     * @param hasSameHighwayClassification
+     *            do the ferry and highway tags have the same value
+     * @return {@link FeatureChange} with the appropriate fix suggestion
+     */
+    private FeatureChange getFixSuggestion(final AtlasObject object, final boolean hasFerryTag,
+            final boolean hasSameHighwayClassification)
+    {
+        final CompleteEntity<CompleteEdge> completeEntity = ((CompleteEntity) CompleteEntity
+                .shallowFrom((AtlasEntity) object)).withTags(object.getTags());
+        if (hasFerryTag && !hasSameHighwayClassification)
+        {
+            completeEntity.withReplacedTag(FerryTag.KEY, FerryTag.KEY, object.tag(HighwayTag.KEY));
+
+        }
+        else if (!hasFerryTag)
+        {
+            completeEntity.withAddedTag(FerryTag.KEY, object.tag(HighwayTag.KEY));
+
+        }
+        completeEntity.withRemovedTag(HighwayTag.KEY);
+        return FeatureChange.add((AtlasEntity) completeEntity, object.getAtlas());
+    }
+
     /**
      * Verifies if the {@link FerryTag} value is the same as the {@link HighwayTag} value
      *

src/test/java/org/openstreetmap/atlas/checks/validation/tag/HighwayToFerryTagCheckTest.java

@@ -5,6 +5,9 @@
 import org.junit.Test;
 import org.openstreetmap.atlas.checks.configuration.ConfigurationResolver;
 import org.openstreetmap.atlas.checks.validation.verifier.ConsumerBasedExpectedCheckVerifier;
+import org.openstreetmap.atlas.geography.atlas.change.FeatureChange;
+import org.openstreetmap.atlas.tags.FerryTag;
+import org.openstreetmap.atlas.tags.HighwayTag;
 
 /**
  * {@link HighwayToFerryTagCheckTest} Unit Test
@@ -25,8 +28,17 @@ public void testAtlasWithDifferentFerryAndHighwayTags()
         this.verifier.actual(this.setup.getDifferentFerryHighwayAtlas(),
                 new HighwayToFerryTagCheck(ConfigurationResolver.emptyConfiguration()));
         this.verifier.globallyVerify(flags -> Assert.assertEquals(1, flags.size()));
-        this.verifier.verify(flag -> Assert.assertTrue(flag.getInstructions().contains(
-                "has a Ferry and a Highway tag for a ferry route. Please verify and update the Ferry tag with the Highway tag value and remove the highway tag.")));
+        this.verifier.verify(flag ->
+        {
+            Assert.assertTrue(flag.getInstructions().contains(
+                    "has a Ferry and a Highway tag for a ferry route. Please verify and update the Ferry tag with the Highway tag value and remove the highway tag."));
+            Assert.assertFalse(flag.getFixSuggestions().isEmpty());
+            final FeatureChange fixSuggestion = flag.getFixSuggestions().iterator().next();
+            Assert.assertEquals(2, fixSuggestion.getTags().size());
+            Assert.assertEquals(HighwayTag.TRUNK.toString().toLowerCase(),
+                    fixSuggestion.getTags().get(FerryTag.KEY));
+        });
+
     }
 
     @Test
@@ -35,8 +47,15 @@ public void testAtlasWithNoFerryTag()
         this.verifier.actual(this.setup.getHighwayAtlas(),
                 new HighwayToFerryTagCheck(ConfigurationResolver.emptyConfiguration()));
         this.verifier.globallyVerify(flags -> Assert.assertEquals(4, flags.size()));
-        this.verifier.verify(flag -> Assert.assertTrue(flag.getInstructions()
-                .contains("has a Highway tag for a ferry route instead of a Ferry tag")));
+        this.verifier.verify(flag ->
+        {
+            Assert.assertTrue(flag.getInstructions()
+                    .contains("has a Highway tag for a ferry route instead of a Ferry tag"));
+            Assert.assertFalse(flag.getFixSuggestions().isEmpty());
+            final FeatureChange fixSuggestion = flag.getFixSuggestions().iterator().next();
+            Assert.assertTrue(fixSuggestion.getTag(HighwayTag.KEY).isEmpty());
+            Assert.assertTrue(fixSuggestion.getTag(FerryTag.KEY).isPresent());
+        });
     }
 
     @Test
@@ -45,8 +64,14 @@ public void testAtlasWithSameFerryAndHighwayTags()
         this.verifier.actual(this.setup.getSameFerryHighwayAtlas(),
                 new HighwayToFerryTagCheck(ConfigurationResolver.emptyConfiguration()));
         this.verifier.globallyVerify(flags -> Assert.assertEquals(1, flags.size()));
-        this.verifier.verify(flag -> Assert.assertTrue(flag.getInstructions().contains(
-                "has a Ferry and a Highway tag for a ferry route. Please verify and remove the highway tag.")));
+        this.verifier.verify(flag ->
+        {
+            Assert.assertTrue(flag.getInstructions().contains(
+                    "has a Ferry and a Highway tag for a ferry route. Please verify and remove the highway tag."));
+            Assert.assertFalse(flag.getFixSuggestions().isEmpty());
+            Assert.assertTrue(
+                    flag.getFixSuggestions().iterator().next().getTag(HighwayTag.KEY).isEmpty());
+        });
     }
 
     @Test