fix: False positive for StopZoneIdValidator (#1082)

RULES.md

@@ -607,7 +607,7 @@ Missing `stop_time.arrival_time` or `stop_time.departure_time`
 
 #### StopWithoutZoneIdNotice
 
-If `fare_rules.txt` is provided, then all stops and platforms (location_type = 0) must have `stops.zone_id` assigned.
+If `fare_rules.txt` is provided, and `fare_rules.txt` uses at least one column among `origin_id`, `destination_id`, and `contains_id`, then all stops and platforms (location_type = 0) must have `stops.zone_id` assigned. 
 
 ##### References:
 * [GTFS stops.txt specification](https://gtfs.org/reference/static#stopstxt)

main/src/main/java/org/mobilitydata/gtfsvalidator/validator/StopZoneIdValidator.java

@@ -21,14 +21,22 @@
 import org.mobilitydata.gtfsvalidator.notice.NoticeContainer;
 import org.mobilitydata.gtfsvalidator.notice.SeverityLevel;
 import org.mobilitydata.gtfsvalidator.notice.ValidationNotice;
+import org.mobilitydata.gtfsvalidator.table.GtfsFareRule;
 import org.mobilitydata.gtfsvalidator.table.GtfsFareRuleTableContainer;
 import org.mobilitydata.gtfsvalidator.table.GtfsLocationType;
 import org.mobilitydata.gtfsvalidator.table.GtfsStop;
 import org.mobilitydata.gtfsvalidator.table.GtfsStopTableContainer;
 
 /**
- * Check that if {@code fare_rules.txt} is provided, then all stops and platforms (location_type =
- * 0) have {@code stops.zone_id} assigned.
+ * Checks that all stops and platforms (location_type = 0) have {@code stops.zone_id} assigned.
+ * assigned if {@code fare_rules.txt} is provided and at least one of the following fields is
+ * provided:
+ *
+ * <ul>
+ *   <li>{@code fare_rules.origin_id}
+ *   <li>{@code fare_rules.contains_id}
+ *   <li>{@code fare_rules.destination_id}
+ * </ul>
  *
  * <p>Generated notice: {@link StopWithoutZoneIdNotice}.
  */
@@ -49,13 +57,36 @@ public void validate(NoticeContainer noticeContainer) {
     if (fareRuleTable.getEntities().isEmpty()) {
       return;
     }
+    if (!hasFareZoneStructure(fareRuleTable)) {
+      return;
+    }
     for (GtfsStop stop : stopTable.getEntities()) {
-      if (stop.locationType().equals(GtfsLocationType.STOP) && !stop.hasZoneId()) {
+      if (!stop.locationType().equals(GtfsLocationType.STOP)) {
+        continue;
+      }
+      if (!stop.hasZoneId()) {
         noticeContainer.addValidationNotice(new StopWithoutZoneIdNotice(stop));
       }
     }
   }
 
+  /**
+   * Checks if the {@code GtfsFareRuleTableContainer} provided as parameter has a fare structure
+   * that uses zones.
+   *
+   * @param fareRuleTable the {@code GtfsFareRuleTableContainer} to be checked
+   * @return true if the {@code GtfsFareRuleTableContainer} provided as parameter has a fare
+   *     structure that uses zones; false otherwise.
+   */
+  private static boolean hasFareZoneStructure(GtfsFareRuleTableContainer fareRuleTable) {
+    for (GtfsFareRule fareRule : fareRuleTable.getEntities()) {
+      if (fareRule.hasContainsId() || fareRule.hasDestinationId() || fareRule.hasOriginId()) {
+        return true;
+      }
+    }
+    return false;
+  }
+
   /**
    * A {@code GtfsShape} should be referred to at least once in {@code GtfsTripTableContainer}
    * station).

main/src/test/java/org/mobilitydata/gtfsvalidator/validator/StopZoneIdValidatorTest.java

@@ -45,10 +45,19 @@ private static GtfsStop createStop(
         .build();
   }
 
-  private static GtfsFareRule createFareRule(long csvRowNumber) {
+  private static GtfsFareRule createFareRuleWithZoneStructure(long csvRowNumber) {
     return new GtfsFareRule.Builder()
         .setCsvRowNumber(csvRowNumber)
         .setFareId(toFareRuleId(csvRowNumber))
+        .setOriginId("origin id")
+        .build();
+  }
+
+  private static GtfsFareRule createFareRuleWithoutZoneStructure(long csvRowNumber) {
+    return new GtfsFareRule.Builder()
+        .setCsvRowNumber(csvRowNumber)
+        .setFareId(toFareRuleId(csvRowNumber))
+        .setRouteId("route id value")
         .build();
   }
 
@@ -71,18 +80,25 @@ private static List<ValidationNotice> generateNotices(
   }
 
   @Test
-  public void stop_zoneIdNotProvided_yieldsNotice() {
+  public void stop_zoneIdNotProvided_zoneStructure_yieldsNotice() {
     ImmutableList<GtfsStop> stops = ImmutableList.of(createStop(3, GtfsLocationType.STOP, null));
-    assertThat(generateNotices(stops, ImmutableList.of(createFareRule(5))))
+    assertThat(generateNotices(stops, ImmutableList.of(createFareRuleWithZoneStructure(5))))
         .containsExactly(new StopWithoutZoneIdNotice(stops.get(0)));
   }
 
+  @Test
+  public void stop_zoneIdNotProvided_noZoneStructure_yieldsNotice() {
+    ImmutableList<GtfsStop> stops = ImmutableList.of(createStop(3, GtfsLocationType.STOP, null));
+    assertThat(generateNotices(stops, ImmutableList.of(createFareRuleWithoutZoneStructure(5))))
+        .isEmpty();
+  }
+
   @Test
   public void stop_zoneIdProvided_noNotice() {
     assertThat(
             generateNotices(
                 ImmutableList.of(createStop(3, GtfsLocationType.STOP, "zone id value")),
-                ImmutableList.of(createFareRule(5))))
+                ImmutableList.of(createFareRuleWithZoneStructure(5))))
         .isEmpty();
   }
 
@@ -91,7 +107,7 @@ public void station_zoneIdNotProvided_noNotice() {
     assertThat(
             generateNotices(
                 ImmutableList.of(createStop(3, GtfsLocationType.STATION, null)),
-                ImmutableList.of(createFareRule(5))))
+                ImmutableList.of(createFareRuleWithZoneStructure(5))))
         .isEmpty();
   }
 
@@ -100,7 +116,7 @@ public void station_zoneIdProvided_noNotice() {
     assertThat(
             generateNotices(
                 ImmutableList.of(createStop(3, GtfsLocationType.STATION, "zone id value")),
-                ImmutableList.of(createFareRule(5))))
+                ImmutableList.of(createFareRuleWithZoneStructure(5))))
         .isEmpty();
   }
 
@@ -109,7 +125,7 @@ public void entrance_zoneIdNotProvided_noNotice() {
     assertThat(
             generateNotices(
                 ImmutableList.of(createStop(3, GtfsLocationType.ENTRANCE, null)),
-                ImmutableList.of(createFareRule(5))))
+                ImmutableList.of(createFareRuleWithZoneStructure(5))))
         .isEmpty();
   }
 
@@ -118,7 +134,7 @@ public void entrance_zoneIdProvided_noNotice() {
     assertThat(
             generateNotices(
                 ImmutableList.of(createStop(3, GtfsLocationType.ENTRANCE, "zone id value")),
-                ImmutableList.of(createFareRule(5))))
+                ImmutableList.of(createFareRuleWithZoneStructure(5))))
         .isEmpty();
   }
 
@@ -127,7 +143,7 @@ public void genericNode_zoneIdNotProvided_noNotice() {
     assertThat(
             generateNotices(
                 ImmutableList.of(createStop(3, GtfsLocationType.GENERIC_NODE, null)),
-                ImmutableList.of(createFareRule(5))))
+                ImmutableList.of(createFareRuleWithZoneStructure(5))))
         .isEmpty();
   }
 
@@ -136,7 +152,7 @@ public void genericNode_zoneIdProvided_noNotice() {
     assertThat(
             generateNotices(
                 ImmutableList.of(createStop(3, GtfsLocationType.GENERIC_NODE, "zone id value")),
-                ImmutableList.of(createFareRule(5))))
+                ImmutableList.of(createFareRuleWithZoneStructure(5))))
         .isEmpty();
   }
 
@@ -145,7 +161,7 @@ public void boardingArea_zoneIdNotProvided_noNotice() {
     assertThat(
             generateNotices(
                 ImmutableList.of(createStop(3, GtfsLocationType.BOARDING_AREA, null)),
-                ImmutableList.of(createFareRule(5))))
+                ImmutableList.of(createFareRuleWithZoneStructure(5))))
         .isEmpty();
   }
 
@@ -154,7 +170,7 @@ public void boardingArea_zoneIdProvided_noNotice() {
     assertThat(
             generateNotices(
                 ImmutableList.of(createStop(3, GtfsLocationType.BOARDING_AREA, "zone id value")),
-                ImmutableList.of(createFareRule(5))))
+                ImmutableList.of(createFareRuleWithZoneStructure(5))))
         .isEmpty();
   }