Download and use clang-format 7 from Mason if 7 is not found locally (v…

…alhalla#1275)

Update formatting for `clang-format` 7.0, and make `./scripts/format.sh` download the correct clang-format version if it's not found locally installed on the `PATH` already (Linux/macOS).

.circleci/config.yml

@@ -8,7 +8,7 @@ jobs:
           LD_LIBRARY_PATH: /usr/local/lib:/lib/x86_64-linux-gnu:/usr/lib/x86_64-linux-gnu:/lib32:/usr/lib32
     steps:
       - checkout
-      - run: apt-get update && apt-get -y install clang-format clang-tidy-5.0 clang-5.0 git
+      - run: apt-get update && apt-get -y install clang-tidy-5.0 clang-5.0 curl git
       - run: ./scripts/format.sh && ./scripts/error_on_dirty.sh
       - run: ./.circleci/make_test.sh && run-clang-tidy-5.0.py -header-filter "^$(pwd)/(src|valhalla/(baldr|midgard|sif|odin|thor|skadi|tyr|loki|mjolnir)/.*" && ./scripts/error_on_dirty.sh
   build:

.circleci/make_test.sh

@@ -5,7 +5,7 @@ set -e
 apt-get install -y software-properties-common
 add-apt-repository -y ppa:valhalla-core/valhalla
 apt-get update
-apt-get install -y autoconf automake make libtool pkg-config g++ gcc jq lcov locales coreutils protobuf-compiler vim-common libboost-all-dev libcurl4-openssl-dev libgeos-dev libgeos++-dev liblua5.2-dev libprime-server0.6.3-dev libprotobuf-dev libspatialite-dev libsqlite3-dev libsqlite3-mod-spatialite python-all-dev zlib1g-dev liblz4-dev lua5.2 prime-server0.6.3-bin ccache bear clang-tidy-5.0 clang-5.0 jq
+apt-get install -y autoconf automake make libtool pkg-config g++ gcc jq lcov locales coreutils protobuf-compiler vim-common libboost-all-dev libcurl4-openssl-dev libgeos-dev libgeos++-dev liblua5.2-dev libprime-server0.6.3-dev libprotobuf-dev libspatialite-dev libsqlite3-dev libsqlite3-mod-spatialite python-all-dev zlib1g-dev liblz4-dev lua5.2 prime-server0.6.3-bin ccache bear clang-tidy-5.0 clang-5.0 jq curl
 
 # test some easier stuff
 for f in *.json; do

.clang-format

@@ -32,11 +32,11 @@ BraceWrapping:
   BeforeCatch:     false
   BeforeElse:      false
   IndentBraces:    false
-BreakBeforeBinaryOperators: false
+BreakBeforeBinaryOperators: None
 BreakBeforeBraces: Attach
 BreakBeforeTernaryOperators: true
 BreakConstructorInitializersBeforeComma: false
-ColumnLimit:     100
+ColumnLimit:     102
 CommentPragmas:  '^ IWYU pragma:'
 ConstructorInitializerAllOnOneLineOrOnePerLine: false
 ConstructorInitializerIndentWidth: 4
@@ -64,7 +64,7 @@ NamespaceIndentation: None
 ObjCBlockIndentWidth: 2
 ObjCSpaceAfterProperty: false
 ObjCSpaceBeforeProtocolList: true
-PenaltyBreakBeforeFirstCallParameter: 19
+PenaltyBreakBeforeFirstCallParameter: 1000000
 PenaltyBreakComment: 300
 PenaltyBreakFirstLessLess: 120
 PenaltyBreakString: 1000
@@ -86,5 +86,6 @@ SpacesInSquareBrackets: false
 Standard:        Cpp11
 TabWidth:        8
 UseTab:          Never
+BreakStringLiterals: false
 ...
 

README.md

@@ -157,7 +157,11 @@ Contributing
 
 We welcome contributions to valhalla. If you would like to report an issue, or even better fix an existing one, please use the [valhalla issue tracker](https://github.com/valhalla/valhalla/issues) on GitHub.
 
-If you would like to make an improvement to the code, please be aware that all valhalla projects are written mostly in C++11, in the K&R (1TBS variant) with two spaces as indentation. We welcome contributions as pull requests to the [repository](https://github.com/valhalla/valhalla) and highly recommend that your pull request include a test to validate the addition/change of functionality.
+If you would like to make an improvement to the code, please be aware that all valhalla projects are written mostly in C++11.  We use `clang-format` v7.0 to format the code. We welcome contributions as pull requests to the [repository](https://github.com/valhalla/valhalla) and highly recommend that your pull request include a test to validate the addition/change of functionality.
+
+Note that our CI system checks that code formatting is consistent, and the build will fail if formatting rules aren't followed.  Please run `./scripts/format.sh` over your code before committing, to auto-format it in the projects preferred style.
+
+Also note that we run some `clang-tidy` linting over the code as well (see `.clang-tidy` for the list of rules enforced).  You can run `./scripts/tidy.sh` over the code before committing to ensure you haven't added any of the common problems we check for (Note: `./scripts/tidy.sh` requires the exitence of a `compile_commands.json` database.  You can generate this file by running `bear make` instead of just `make`.  The `bear` tool is installable on Ubuntu-based systems with `apt-get install bear`, and on macOS with `brew install bear`).
 
 Tests
 -----

scripts/format.sh

@@ -19,19 +19,41 @@ elif [[ ${OS} = "Darwin" ]] ; then
 fi
 
 # Discover clang-format
-if type clang-format-3.8 2> /dev/null ; then
-    CLANG_FORMAT=clang-format-3.8
+if type clang-format-7.0 2> /dev/null ; then
+    CLANG_FORMAT=clang-format-7.0
 elif type clang-format 2> /dev/null ; then
     # Clang format found, but need to check version
     CLANG_FORMAT=clang-format
     V=$(clang-format --version)
-    if [[ $V != *3.8* ]] ; then
-        echo "clang-format is not 3.8 (returned ${V})"
+    if [[ $V != *6.0* ]] ; then
+        echo "Installed clang-format is not version 7.0"
+        if [ ! -f $(pwd)/mason_packages/.link/bin/clang-format ] ; then
+            echo "Installing clang-format 7.0 via mason"
+            if [ ! -d mason ] ; then
+                mkdir -p ./mason
+                curl -sSfL https://github.com/mapbox/mason/archive/v0.18.0.tar.gz | tar --gunzip --extract --strip-components=1 --exclude="*md" --exclude="test*" --directory=./mason
+            fi
+            ./mason/mason install clang-format 7.0.0
+            ./mason/mason link clang-format 7.0.0
+        fi
+        echo "Using clang-format 7.0.0 from $(pwd)/mason_packages/.link/bin"
+        PATH="$(pwd)/mason_packages/.link/bin:$PATH"
         #exit 1
     fi
 else
-    echo "No appropriate clang-format found (expected clang-format-3.8, or clang-format)"
-    exit 1
+    echo "No clang-format found"
+    if [ ! -f $(pwd)/mason_packages/.link/bin/clang-format ] ; then
+        echo "Installing clang-format 7.0.0 via mason"
+        if [ ! -d mason ] ; then
+            mkdir -p ./mason
+            curl -sSfL https://github.com/mapbox/mason/archive/v0.18.0.tar.gz | tar --gunzip --extract --strip-components=1 --exclude="*md" --exclude="test*" --directory=./mason
+        fi
+        ./mason/mason install clang-format 7.0.0
+        ./mason/mason link clang-format 7.0.0
+    fi
+    echo "Using clang-format 7.0.0 from $(pwd)/mason_packages/.link/bin"
+    CLANG_FORMAT=clang-format
+    PATH="$(pwd)/mason_packages/.link/bin:$PATH"
 fi
 
 find src valhalla test -type f -name '*.h' -o -name '*.cc' \

src/baldr/connectivity_map.cc

@@ -112,9 +112,7 @@ std::string to_feature_collection(const std::unordered_map<size_t, polygon_t>& b
 polygon_t to_boundary(const std::pair<size_t, std::unordered_set<uint32_t>>& region,
                       const Tiles<PointLL>& tiles) {
   // do we have this tile in this region
-  auto member = [&region](int32_t tile) {
-    return region.second.find(tile) != region.second.cend();
-  };
+  auto member = [&region](int32_t tile) { return region.second.find(tile) != region.second.cend(); };
   // get the neighbor tile giving -1 if no neighbor
   auto neighbor = [&tiles](int32_t tile, int side) -> int32_t {
     if (tile == -1) {

src/baldr/datetime.cc

@@ -94,23 +94,24 @@ boost::local_time::local_date_time get_ldt(const boost::gregorian::date& date,
                                            const boost::local_time::time_zone_ptr& time_zone) {
 
   boost::posix_time::time_duration td = time_duration;
-  boost::local_time::local_date_time in_local_time(
-      date, td, time_zone, boost::local_time::local_date_time::NOT_DATE_TIME_ON_ERROR);
+  boost::local_time::local_date_time
+      in_local_time(date, td, time_zone, boost::local_time::local_date_time::NOT_DATE_TIME_ON_ERROR);
 
   // create not-a-date-time if invalid (eg: in dst transition)
   if (in_local_time.is_not_a_date_time()) {
 
     if (time_zone->dst_local_start_time(date.year()).date() == date) {
       td += time_zone->dst_offset(); // clocks ahead.
-      in_local_time = boost::local_time::local_date_time(
-          date, td, time_zone, boost::local_time::local_date_time::NOT_DATE_TIME_ON_ERROR);
+      in_local_time = boost::local_time::
+          local_date_time(date, td, time_zone,
+                          boost::local_time::local_date_time::NOT_DATE_TIME_ON_ERROR);
     } else {
       // Daylight Savings Results are ambiguous: time given: 2016-Nov-06 01:00:00
       boost::posix_time::time_duration time_dur = time_zone->dst_offset();
 
-      in_local_time = boost::local_time::local_date_time(
-          date, td + time_dur, time_zone,
-          boost::local_time::local_date_time::NOT_DATE_TIME_ON_ERROR);
+      in_local_time = boost::local_time::
+          local_date_time(date, td + time_dur, time_zone,
+                          boost::local_time::local_date_time::NOT_DATE_TIME_ON_ERROR);
       in_local_time -= time_dur;
     }
   }
@@ -1028,18 +1029,18 @@ bool is_restricted(const bool type,
       }
 
       if (b_week && b_week <= 5) { // kNthDow
-        boost::gregorian::nth_day_of_the_week_in_month nthdow(
-            static_cast<boost::gregorian::nth_day_of_the_week_in_month::week_num>(b_week),
-            b_day_dow - 1, b_month);
+        boost::gregorian::nth_day_of_the_week_in_month
+            nthdow(static_cast<boost::gregorian::nth_day_of_the_week_in_month::week_num>(b_week),
+                   b_day_dow - 1, b_month);
         begin_date = nthdow.get_date(b_year);
       } else { // YMD
         begin_date = boost::gregorian::date(b_year, b_month, b_day_dow);
       }
 
       if (e_week && e_week <= 5) { // kNthDow
-        boost::gregorian::nth_day_of_the_week_in_month nthdow(
-            static_cast<boost::gregorian::nth_day_of_the_week_in_month::week_num>(e_week),
-            e_day_dow - 1, e_month);
+        boost::gregorian::nth_day_of_the_week_in_month
+            nthdow(static_cast<boost::gregorian::nth_day_of_the_week_in_month::week_num>(e_week),
+                   e_day_dow - 1, e_month);
         end_date = nthdow.get_date(e_year);
       } else {                                                         // YMD
         end_date = boost::gregorian::date(e_year, e_month, e_day_dow); // Dec 5 to Mar 3
@@ -1223,8 +1224,7 @@ std::vector<uint64_t> get_time_range(const std::string& str) {
     boost::algorithm::trim(condition);
 
     // Holidays and school hours skip for now
-    if (condition.size() >= 2 &&
-        (condition.substr(0, 2) == "PH" || condition.substr(0, 2) == "SH")) {
+    if (condition.size() >= 2 && (condition.substr(0, 2) == "PH" || condition.substr(0, 2) == "SH")) {
       return time_domains;
     }
 
@@ -1288,21 +1288,21 @@ std::vector<uint64_t> get_time_range(const std::string& str) {
           } else {
 
             // Feb 16-Oct 15 09:00-18:30
-            regex = re::regex(
-                "(?:(January|February|March|April|May|June|July|"
-                "August|September|October|November|December|Jan|Feb|Mar|Apr|May|Jun|Jul|Aug|"
-                "Sep|Sept|Oct|Nov|Dec)) (\\d{1,2})",
-                re::regex_constants::icase);
+            regex = re::
+                regex("(?:(January|February|March|April|May|June|July|"
+                      "August|September|October|November|December|Jan|Feb|Mar|Apr|May|Jun|Jul|Aug|"
+                      "Sep|Sept|Oct|Nov|Dec)) (\\d{1,2})",
+                      re::regex_constants::icase);
 
             if (RegexFound(condition, regex)) {
               condition = FormatCondition(condition, regex, "$1#$2");
             } else {
               // Feb 2-14
-              regex = re::regex(
-                  "(?:(January|February|March|April|May|June|July|"
-                  "August|September|October|November|December|Jan|Feb|Mar|Apr|May|Jun|Jul|Aug|"
-                  "Sep|Sept|Oct|Nov|Dec)) (\\d{1,2})-(\\d{1,2})",
-                  re::regex_constants::icase);
+              regex = re::
+                  regex("(?:(January|February|March|April|May|June|July|"
+                        "August|September|October|November|December|Jan|Feb|Mar|Apr|May|Jun|Jul|Aug|"
+                        "Sep|Sept|Oct|Nov|Dec)) (\\d{1,2})-(\\d{1,2})",
+                        re::regex_constants::icase);
 
               if (RegexFound(condition, regex)) {
                 condition = FormatCondition(condition, regex, "$1#$2-$1#$3");

src/baldr/graphreader.cc

@@ -358,17 +358,15 @@ bool GraphReader::AreEdgesConnectedForward(const GraphId& edge1,
 
   // Check if edge2's Id is an outgoing directed edge of the node
   const NodeInfo* node = tile->node(endnode);
-  return (node->edge_index() <= edge2.id() &&
-          edge2.id() < (node->edge_index() + node->edge_count()));
+  return (node->edge_index() <= edge2.id() && edge2.id() < (node->edge_index() + node->edge_count()));
 }
 
 // Get the shortcut edge that includes this edge.
 GraphId GraphReader::GetShortcut(const GraphId& id) {
   // Lambda to get continuing edge at a node. Skips the specified edge Id
   // transition edges, shortcut edges, and transit connections. Returns
   // nullptr if more than one edge remains or no continuing edge is found.
-  auto continuing_edge = [](const GraphTile* tile, const GraphId& edgeid,
-                            const NodeInfo* nodeinfo) {
+  auto continuing_edge = [](const GraphTile* tile, const GraphId& edgeid, const NodeInfo* nodeinfo) {
     uint32_t idx = nodeinfo->edge_index();
     const DirectedEdge* continuing_edge = static_cast<const DirectedEdge*>(nullptr);
     const DirectedEdge* directededge = tile->directededge(idx);
@@ -506,8 +504,8 @@ std::unordered_set<GraphId> GraphReader::GetTileSet(const uint8_t level) const {
     } // or individually on disk
   } else {
     // crack open this level of tiles directory
-    boost::filesystem::path root_dir(tile_dir_ + filesystem::path_separator +
-                                     std::to_string(level) + filesystem::path_separator);
+    boost::filesystem::path root_dir(tile_dir_ + filesystem::path_separator + std::to_string(level) +
+                                     filesystem::path_separator);
     if (boost::filesystem::exists(root_dir) && boost::filesystem::is_directory(root_dir)) {
       // iterate over all the files in the directory and turn into GraphIds
       for (boost::filesystem::recursive_directory_iterator i(root_dir), end; i != end; ++i) {