Boundary overlap validation improvements:

- Improve overlap validation script to be less buggy
- Account for all acceptable boundary overlaps.
- Add linting rule to require a description for each expected overlap.
- Remove diffs from building script that avoided overlaps
- Fix Broken_Hill timezone boundary source
- Fix ogr2ogr output command

Fixes #41

6 files changed, 202 insertions, 123 deletions

diff --git a/expectedZoneOverlaps.json b/expectedZoneOverlaps.json
index 96d2d6c..7906ce3 100644
--- a/expectedZoneOverlaps.json
+++ b/expectedZoneOverlaps.json
@@ -1,3 +1,116 @@
 {
-  "Asia/Shanghai-Asia/Urumqi": [[73.4, 34.3, 96.4, 49.2]]
+  "Africa/Juba-Africa/Khartoum": [
+    {
+      "bounds": [27.8, 9.3, 29.1, 10.2],
+      "description": "Allow disputed Abyei Area (https://en.wikipedia.org/wiki/Abyei) to overlap."
+    }, {
+      "bounds": [23.4, 8.6, 24.9, 9.9],
+      "description": "Allow disputed Kafia Kingi region (https://en.wikipedia.org/wiki/Kafia_Kingi) to overlap."
+    }
+  ],
+  "America/Argentina/Rio_Gallegos-America/Punta_Arenas": [
+    {
+      "bounds": [-73.6, -49.8, -73, -49.2],
+      "description": "Allow disputed Southern Patagonia Ice Field are (https://en.wikipedia.org/wiki/Argentina%E2%80%93Chile_relations#Border_issues) to overlap."
+    }
+  ],
+  "America/La_Paz-America/Porto_Velho": [
+    {
+      "bounds": [-65.4, -10.9, -65.3, -10.7],
+      "description": "Allow disputed area of Isla Suárez (https://en.wikipedia.org/wiki/Isla_Su%C3%A1rez) to overlap."
+    }
+  ],
+  "America/Moncton-America/New_York": [
+    {
+      "bounds": [-67.4, 44.2, -66.9, 44.8],
+      "description": "Allow disputed area near Machias Seal Island (https://en.wikipedia.org/wiki/Machias_Seal_Island) to overlap."
+    }
+  ],
+  "Asia/Hebron-Asia/Jerusalem": [
+    {
+      "bounds": [35.1, 31.7, 35.3, 31.9],
+      "description": "Allow disputed areas in Jerusalem (https://en.wikipedia.org/wiki/Borders_of_Israel#Status_of_Jerusalem) to overlap."
+    }
+  ],
+  "Asia/Ho_Chi_Minh-Asia/Manila": [
+    {
+      "bounds": [114.1, 10.9, 114.2, 11],
+      "description": "Allow disputed area near Gaven Reefs (https://en.wikipedia.org/wiki/Gaven_Reefs) to overlap."
+    }
+  ],
+  "Asia/Ho_Chi_Minh-Asia/Shanghai": [
+    {
+      "bounds": [114, 10.1, 114.3, 11],
+      "description": "Allow disputed area near Gaven Reefs (https://en.wikipedia.org/wiki/Gaven_Reefs) to overlap."
+    }, {
+      "bounds": [114.4, 9.8, 114.6, 10],
+      "description": "Allow disputed area near Hughes Reef (https://en.wikipedia.org/wiki/Hughes_Reef) to overlap."
+    }, {
+      "bounds": [115.4, 9.8, 115.6, 10],
+      "description": "Allow disputed area near Mischief Reef (https://en.wikipedia.org/wiki/Mischief_Reef) to overlap."
+    }, {
+      "bounds": [114.2, 9.6, 114.4, 9.8],
+      "description": "Allow disputed area near Johnson South Reef (https://en.wikipedia.org/wiki/Johnson_South_Reef) to overlap."
+    }, {
+      "bounds": [112.8, 9.5, 113.1, 9.8],
+      "description": "Allow disputed area near Fiery Cross Reef (https://en.wikipedia.org/wiki/Fiery_Cross_Reef) to overlap."
+    }, {
+      "bounds": [112.7, 8.8, 112.9, 8.9],
+      "description": "Allow disputed area near Cuarteron Reef (https://en.wikipedia.org/wiki/Cuarteron_Reef) to overlap."
+    }, {
+      "bounds": [110.9, 15.5, 113, 17.4],
+      "description": "Allow disputed area near Paracel Islands (https://en.wikipedia.org/wiki/Paracel_Islands) to overlap."
+    }
+  ],
+  "Asia/Manila-Asia/Shanghai": [
+    {
+      "bounds": [114.1, 10.9, 114.2, 11],
+      "description": "Allow disputed area near Gaven Reefs (https://en.wikipedia.org/wiki/Gaven_Reefs) to overlap."
+    }
+  ],
+  "Asia/Shanghai-Asia/Taipei": [
+    {
+      "bounds": [119.4, 24.9, 119.5, 25.1],
+      "description": "Allow disputed area near Wuqiu Township (https://en.wikipedia.org/wiki/Wuqiu,_Kinmen) to overlap."
+    }, {
+      "bounds": [118.1, 24.3, 118.6, 24.6],
+      "description": "Allow disputed islands of Great Kinmen and Lesser Kinmen (https://en.wikipedia.org/wiki/Kinmen) to overlap."
+    }, {
+      "bounds": [118.2, 24.1, 118.3, 24.2],
+      "description": "Allow disputed area near Dongding Island (https://en.wikipedia.org/wiki/Jincheng,_Kinmen) to overlap."
+    }, {
+      "bounds": [119.8, 25.9, 120.6, 26.5],
+      "description": "Allow disputed area near Matsu Islands (https://en.wikipedia.org/wiki/Matsu_Islands) to overlap."
+    }
+  ],
+  "Asia/Shanghai-Asia/Urumqi": [
+    {
+      "bounds": [73.4, 34.3, 96.4, 49.2],
+      "description": "Xinjiang time might be used by some locals in Xinjiang.  https://en.wikipedia.org/wiki/Xinjiang_Time"
+    }
+  ],
+  "Europe/Amsterdam-Europe/Berlin": [
+    {
+      "bounds": [6.3, 53.3, 7.2, 53.8],
+      "description": "Allow dispupted maritime border near the Ems estuary to overlap.  https://en.wikipedia.org/wiki/Germany%E2%80%93Netherlands_border#Disputes"
+    }
+  ],
+  "Europe/Belgrade-Europe/Zagreb": [
+    {
+      "bounds": [18.8, 45.2, 19.3, 46],
+      "description": "Allow disupted borders of Croatia and Serbia to overlap.  https://en.wikipedia.org/wiki/Croatia%E2%80%93Serbia_border_dispute"
+    }
+  ],
+  "Europe/Berlin-Europe/Luxembourg": [
+    {
+      "bounds": [6.1, 49.4, 6.6, 50.2],
+      "description": "Allow entire border of Germany and Luxembourg to overlap as most of it is a condominium.  https://opinionator.blogs.nytimes.com/2012/01/23/the-worlds-most-exclusive-condominium/"
+    }
+  ],
+  "Europe/Paris-Europe/Rome": [
+    {
+      "bounds": [6.8, 45.8, 6.9, 45.9],
+      "description": "Allow disputed areas near Mont Blanc (https://en.wikipedia.org/wiki/Mont_Blanc#Ownership_of_the_summit) and Dôme du Goûter (https://en.wikipedia.org/wiki/D%C3%B4me_du_Go%C3%BBter) to overlap."
+    }
+  ]
 }
diff --git a/index.js b/index.js
index e1dee6a..e2c54c5 100644
--- a/index.js
+++ b/index.js
@@ -207,7 +207,11 @@ var downloadOsmBoundary = function (boundaryId, boundaryCallback) {
       // union all multi-polygons / polygons into one
       for (var i = data.features.length - 1; i >= 0; i--) {
         var curOsmGeom = data.features[i].geometry
-        if (curOsmGeom.type === 'Polygon' || curOsmGeom.type === 'MultiPolygon') {
+        const curOsmProps = data.features[i].properties
+        if (
+          (curOsmGeom.type === 'Polygon' || curOsmGeom.type === 'MultiPolygon') &&
+          curOsmProps.type === 'boundary' // need to make sure enclaves aren't unioned
+        ) {
           console.log('combining border')
           let errors = geojsonhint.hint(curOsmGeom)
           if (errors && errors.length > 0) {
@@ -388,6 +392,23 @@ var getDistZoneGeom = function (tzid) {
   return distZones[tzid]
 }
 
+var roundDownToTenth = function (n) {
+  return Math.floor(n * 10) / 10
+}
+
+var roundUpToTenth = function (n) {
+  return Math.ceil(n * 10) / 10
+}
+
+var formatBounds = function (bounds) {
+  let boundsStr = '['
+  boundsStr += roundDownToTenth(bounds[0]) + ', '
+  boundsStr += roundDownToTenth(bounds[1]) + ', '
+  boundsStr += roundUpToTenth(bounds[2]) + ', '
+  boundsStr += roundUpToTenth(bounds[3]) + ']'
+  return boundsStr
+}
+
 var validateTimezoneBoundaries = function () {
   console.log('do validation... this may take a few minutes')
   var allZonesOk = true
@@ -426,25 +447,52 @@ var validateTimezoneBoundaries = function () {
             // overlaps
             let overlapsPolygons
             switch (overlapsGeoJson.type) {
+              case 'MultiPolygon':
+                overlapsPolygons = overlapsGeoJson.coordinates.map(
+                  polygonCoords => ({
+                    coordinates: polygonCoords,
+                    type: 'Polygon'
+                  })
+                )
+                break
               case 'Polygon':
                 overlapsPolygons = [overlapsGeoJson]
                 break
+              case 'GeometryCollection':
+                overlapsPolygons = []
+                overlapsGeoJson.geometries.forEach(geom => {
+                  if (geom.type === 'Polygon') {
+                    overlapsPolygons.push(geom)
+                  } else if (geom.type === 'MultiPolygon') {
+                    geom.coordinates.forEach(polygonCoords => {
+                      overlapsPolygons.push({
+                        coordinates: polygonCoords,
+                        type: 'Polygon'
+                      })
+                    })
+                  }
+                })
+                break
               default:
+                console.error('unexpected geojson overlap type')
+                console.log(overlapsGeoJson)
                 break
             }
 
             let allOverlapsOk = true
             overlapsPolygons.forEach((polygon, idx) => {
               const bounds = bbox(polygon)
+              const polygonArea = area.geometry(polygon)
               if (
+                polygonArea > 10 && // ignore small polygons
                 !allowedOverlapBounds.some(allowedBounds =>
-                  allowedBounds[0] <= bounds[0] && // minX
-                    allowedBounds[1] <= bounds[1] && // minY
-                    allowedBounds[2] >= bounds[2] && // maxX
-                    allowedBounds[3] >= bounds[3] // maxY
+                  allowedBounds.bounds[0] <= bounds[0] && // minX
+                    allowedBounds.bounds[1] <= bounds[1] && // minY
+                    allowedBounds.bounds[2] >= bounds[2] && // maxX
+                    allowedBounds.bounds[3] >= bounds[3] // maxY
                 )
               ) {
-                console.error('Unexpected intersection with bounds: ', bounds)
+                console.error(`Unexpected intersection (${polygonArea} area) with bounds: ${formatBounds(bounds)}`)
                 allOverlapsOk = false
               }
             })
@@ -452,9 +500,9 @@ var validateTimezoneBoundaries = function () {
             if (allOverlapsOk) continue
           }
 
-          // not an expected overlap, output an error
+          // at least one unexpected overlap found, output an error and write debug file
           console.error('Validation error: ' + tzid + ' intersects ' + compareTzid + ' area: ' + intersectedArea)
-          const debugFilename = tzid.replace('/', '-') + '-' + compareTzid.replace('/', '-') + '-overlap.json'
+          const debugFilename = tzid.replace(/\//g, '-') + '-' + compareTzid.replace(/\//g, '-') + '-overlap.json'
           fs.writeFileSync(
             debugFilename,
             JSON.stringify(overlapsGeoJson)
@@ -617,7 +665,7 @@ asynclib.auto({
     console.log('convert from geojson to shapefile')
     rimraf.sync('dist/combined-shapefile.*')
     exec(
-      'ogr2ogr -nlt MULTIPOLYGON dist/combined-shapefile.shp dist/combined.json OGRGeoJSON',
+      'ogr2ogr -f "ESRI Shapefile" dist/combined-shapefile.shp dist/combined.json OGRGeoJSON',
       function (err, stdout, stderr) {
         if (err) { return cb(err) }
         exec('zip dist/timezones.shapefile.zip dist/combined-shapefile.*', cb)
diff --git a/lint-json.js b/lint-json.js
index 9884ccf..95f2f55 100644
--- a/lint-json.js
+++ b/lint-json.js
@@ -1,5 +1,6 @@
 const osmBoundarySources = require('./osmBoundarySources.json')
 const zoneCfg = require('./timezones.json')
+const expectedZoneOverlaps = require('./expectedZoneOverlaps.json')
 
 let numErrors = 0
 
@@ -37,6 +38,16 @@ Object.keys(sourcesUsage).forEach(source => {
   }
 })
 
+// Make sure all expected zone overlaps have a description
+Object.keys(expectedZoneOverlaps).forEach(zoneOverlap => {
+  expectedZoneOverlaps[zoneOverlap].forEach((overlapBounds, idx) => {
+    if (!overlapBounds.description || overlapBounds.description.length < 3) {
+      numErrors++
+      console.error(`Expected overlap #${idx} of zones ${zoneOverlap} missing description`)
+    }
+  })
+})
+
 if (numErrors > 0) {
   console.error(`${numErrors} errors found`)
   process.exit(1)
diff --git a/osmBoundarySources.json b/osmBoundarySources.json
index 8956f92..09c192e 100644
--- a/osmBoundarySources.json
+++ b/osmBoundarySources.json
@@ -167,6 +167,9 @@
   "British Virgin Islands": {
     "ISO3166-1": "VG"
   },
+  "Broken_Hill-tz": {
+    "timezone": "Australia/Broken_Hill"
+  },
   "Brunei": {
     "ISO3166-1": "BN"
   },
@@ -1349,9 +1352,6 @@
   "Ust-Maysky": {
     "name:en": "Ust-Maysky Ulus"
   },
-  "Uttarakhand": {
-    "ISO3166-2": "IN-UT"
-  },
   "Uzbekistan": {
     "ISO3166-1": "UZ"
   },
@@ -1414,9 +1414,6 @@
   "Xinjiang": {
     "ISO3166-2": "CN-65"
   },
-  "Yancowinna County": {
-    "name": "NSW Central Time Zone"
-  },
   "Yap": {
     "ISO3166-2": "FM-YAP"
   },
diff --git a/package.json b/package.json
index 5038cd9..72f5a3c 100644
--- a/package.json
+++ b/package.json
@@ -23,7 +23,7 @@
     "@turf/helpers": "^6.1.4",
     "async": "^2.1.5",
     "jsts": "^1.3.0",
-    "query-overpass": "^1.1.3",
+    "query-overpass": "^1.5.3",
     "rimraf": "^2.6.1",
     "shpjs": "^3.3.2"
   }
diff --git a/timezones.json b/timezones.json
index de4a3d7..6b721fc 100644
--- a/timezones.json
+++ b/timezones.json
@@ -201,15 +201,11 @@
       "op": "union",
       "source": "manual-polygon",
       "data": [[[35.3,5.2],[35.9,4.6],[34,4.6],[35.3,5.2]]],
-      "description": "Initially add Ilemi Triangle to South Sudan area.  Whatever OSM thinks Kenya's border is is then used as seen in the next operation."
+      "description": "Initially add Ilemi Triangle to South Sudan area.  Whatever OSM thinks Kenya's border is is then used as seen in the next operation.  This avoids having holes."
     }, {
       "op": "difference",
       "source": "overpass",
       "id": "Kenya"
-    }, {
-      "op": "difference",
-      "source": "overpass",
-      "id": "Sudan"
     }
   ],
   "Africa/Kampala": [
@@ -341,10 +337,6 @@
       "op": "init",
       "source": "overpass",
       "id": "Mozambique"
-    }, {
-      "op": "difference",
-      "source": "overpass",
-      "id": "Malawi"
     }
   ],
   "Africa/Maseru": [
@@ -1788,11 +1780,6 @@
       "source": "manual-polygon",
       "data": [[[-66.681,48.01],[-66.685,48.0158],[-66.6864,48.0166],[-66.723,48.029],[-66.7179,48.0363],[-66.7407,48.0623],[-66.733,48.0728],[-66.756,48.1022],[-66.802,48.086],[-66.754,48.0305],[-66.737,48.0274],[-66.73,48.0309],[-66.71,47.996],[-66.681,48.01]]],
       "description": "Include terroritory belonging to Listuguj Mi'gmaq First Nation"
-    }, {
-      "op": "difference",
-      "source": "overpass",
-      "id": "United States of America",
-      "description": "Exclude USA territory that is overlapping near Maine"
     }
   ],
   "America/Monterrey": [
@@ -2056,11 +2043,6 @@
       "op": "init",
       "source": "overpass",
       "id": "Rondônia"
-    }, {
-      "op": "difference",
-      "source": "overpass",
-      "id": "Bolivia",
-      "description": "There is a territorial dispute over Isla Suárez, but apparently Bolivia controls it (https://es.wikipedia.org/wiki/Isla_Su%C3%A1rez).  Therefore this zone is diffed againest Bolivia."
     }
   ],
   "America/Puerto_Rico": [
@@ -2081,11 +2063,6 @@
       "source": "overpass",
       "id": "Magallanes Region"
     }, {
-      "op": "difference",
-      "source": "manual-polygon",
-      "data": [[[-70,-50],[-73.4511111,-49.7894444],[-73.0975128,-49.2838889],[-70,-50]]],
-      "description": "In disputed Viedma Glacier area on Argentina-Chile border, assume that a straight line is drawn between last agreed upon boundary points.  This operation removes area in Chile to compensate for the straight line rule."
-    }, {
       "op": "union",
       "source": "manual-polygon",
       "data": [[[-64,-64],[-57.3,-63.4],[-58.8,-62.25],[-59,-61.8],[-63,-62.7],[-64,-64]]],
@@ -2373,6 +2350,14 @@
       "source": "manual-polygon",
       "data": [[[-66.681,48.01],[-66.685,48.0158],[-66.6864,48.0166],[-66.723,48.029],[-66.7179,48.0363],[-66.7407,48.0623],[-66.733,48.0728],[-66.756,48.1022],[-66.802,48.086],[-66.754,48.0305],[-66.737,48.0274],[-66.73,48.0309],[-66.71,47.996],[-66.681,48.01]]],
       "description": "Exclude terroritory belonging to Listuguj Mi'gmaq First Nation"
+    }, {
+      "op": "difference",
+      "source": "overpass",
+      "id": "Nipigon-tz"
+    }, {
+      "op": "difference",
+      "source": "overpass",
+      "id": "Thunder Bay-tz"
     }
   ],
   "America/Tortola": [
@@ -2683,14 +2668,6 @@
       "op": "init",
       "source": "overpass",
       "id": "Kyrgyzstan"
-    }, {
-      "op": "difference",
-      "source": "overpass",
-      "id": "Uzbekistan"
-    }, {
-      "op": "difference",
-      "source": "overpass",
-      "id": "Tajikistan"
     }
   ],
   "Asia/Brunei": [
@@ -2758,10 +2735,6 @@
       "op": "init",
       "source": "overpass",
       "id": "Tajikistan"
-    }, {
-      "op": "difference",
-      "source": "overpass",
-      "id": "Uzbekistan"
     }
   ],
   "Asia/Famagusta": [
@@ -2813,6 +2786,11 @@
       "op": "init",
       "source": "overpass",
       "id": "Vietnam"
+    }, {
+      "op": "difference",
+      "source": "manual-polygon",
+      "data": [[[117.76,16.2],[116,14],[119,14],[117.76,16.2]]],
+      "description": "Exclude Scarborough Shoal because it's not claimed by Vietnam (https://en.wikipedia.org/wiki/Scarborough_Shoal)"
     }
   ],
   "Asia/Hong_Kong": [
@@ -2902,10 +2880,6 @@
       "op": "init",
       "source": "overpass",
       "id": "Israel"
-    }, {
-      "op": "difference",
-      "source": "overpass",
-      "id": "Palestinian Territories"
     }
   ],
   "Asia/Kabul": [
@@ -2934,10 +2908,6 @@
       "op": "init",
       "source": "overpass",
       "id": "Nepal"
-    }, {
-      "op": "difference",
-      "source": "overpass",
-      "id": "Uttarakhand"
     }
   ],
   "Asia/Khandyga": [
@@ -2956,10 +2926,6 @@
       "op": "init",
       "source": "overpass",
       "id": "India"
-    }, {
-      "op": "difference",
-      "source": "overpass",
-      "id": "Bangladesh"
     }
   ],
   "Asia/Krasnoyarsk": [
@@ -3066,10 +3032,6 @@
       "op": "init",
       "source": "overpass",
       "id": "Oman"
-    }, {
-      "op": "difference",
-      "source": "overpass",
-      "id": "United Arab Emirates"
     }
   ],
   "Asia/Nicosia": [
@@ -3217,20 +3179,6 @@
       "op": "difference",
       "source": "overpass",
       "id": "Macau"
-    }, {
-      "op": "difference",
-      "source": "overpass",
-      "id": "Republic of China"
-    }, {
-      "op": "difference",
-      "source": "manual-polygon",
-      "data": [[[117.76,16.2],[116,14],[119,14],[117.76,16.2]]],
-      "description": "Exclude Scarborough Shoal so it becomes part of Asia/Manila"
-    }, {
-      "op": "difference",
-      "source": "manual-polygon",
-      "data": [[[114.08,10.99],[114.13,10.9],[114.14,11],[114.08,10.99]]],
-      "description": "Clip part of area off of Subi Reef that encroaches on territory claimed by Philippines."
     }
   ],
   "Asia/Singapore": [
@@ -3445,10 +3393,6 @@
       "op": "init",
       "source": "overpass",
       "id": "Armenia"
-    }, {
-      "op": "difference",
-      "source": "overpass",
-      "id": "Azerbaijan"
     }
   ],
   "Atlantic/Azores": [
@@ -3559,7 +3503,7 @@
     {
       "op": "init",
       "source": "overpass",
-      "id": "Yancowinna County"
+      "id": "Broken_Hill-tz"
     }
   ],
   "Australia/Currie": [
@@ -3670,7 +3614,7 @@
     }, {
       "op": "difference",
       "source": "overpass",
-      "id": "Yancowinna County"
+      "id": "Broken_Hill-tz"
     }, {
       "op": "difference",
       "source": "manual-polygon",
@@ -3693,11 +3637,6 @@
       "source": "manual-polygon",
       "data": [[[9,55],[9,48],[-2,52],[9,55]]],
       "description": "Intersect to include only European area of The Netherlands."
-    }, {
-      "op": "difference",
-      "source": "overpass",
-      "id": "Belgium",
-      "description": "Difference against Belgium to make sure that a lot of the Belgian exclaves are properly recognized."
     }
   ],
   "Europe/Andorra": [
@@ -3748,14 +3687,6 @@
       "id": "Germany"
     }, {
       "op": "difference",
-      "source": "overpass",
-      "id": "The Netherlands"
-    }, {
-      "op": "difference",
-      "source": "overpass",
-      "id": "Luxemburg"
-    }, {
-      "op": "difference",
       "source": "manual-polygon",
       "data": [[[8.7,47.75],[8.75,47.63],[8.6,47.7],[8.7,47.75]]],
       "description": "Exclude area belonging to Busingen timezone."
@@ -3982,10 +3913,6 @@
       "op": "init",
       "source": "overpass",
       "id": "Belarus"
-    }, {
-      "op": "difference",
-      "source": "overpass",
-      "id": "Moscow-tz"
     }
   ],
   "Europe/Monaco": [
@@ -4076,10 +4003,6 @@
       "op": "init",
       "source": "overpass",
       "id": "Italy"
-    }, {
-      "op": "difference",
-      "source": "overpass",
-      "id": "France"
     }
   ],
   "Europe/Samara": [
@@ -4221,19 +4144,6 @@
       "op": "init",
       "source": "overpass",
       "id": "Croatia"
-    }, {
-      "op": "union",
-      "source": "manual-polygon",
-      "data": [[[19,46],[18.6,46],[18.7,45.5],[19.2,45.5],[19,46]]],
-      "description": "filling area of border dispute and then diffing it against Serbia"
-    }, {
-      "op": "difference",
-      "source": "overpass",
-      "id": "Serbia"
-    }, {
-      "op": "difference",
-      "source": "overpass",
-      "id": "Hungary"
     }
   ],
   "Europe/Zaporozhye": [