ARROW-3313: [R] Move .clang-format to top level. Add r/lint.sh script…

… for linting R C++ files in Travis CI This also skips the R Travis job if there are no changes affecting it. Author: Wes McKinney <[email protected]> Closes #2628 from wesm/clang-format-raise and squashes the following commits: 8b7efea <Wes McKinney> slashdot 1a15762 <Wes McKinney> Move .clang-format to top level. Add r/lint.sh script for linting R files
apache · Sep 26, 2018 · 2a6c0cb · 2a6c0cb
1 parent 9d007b1
commit 2a6c0cb
Show file tree

Hide file tree

Showing 20 changed files with 288 additions and 213 deletions.
diff --git a/cpp/.clang-format → .clang-format b/cpp/.clang-format → .clang-format
diff --git a/cpp/.clang-tidy → .clang-tidy b/cpp/.clang-tidy → .clang-tidy
diff --git a/cpp/.clang-tidy-ignore → .clang-tidy-ignore b/cpp/.clang-tidy-ignore → .clang-tidy-ignore
diff --git a/.travis.yml b/.travis.yml
@@ -249,11 +249,14 @@ matrix:
   - language: r
     cache: packages
     latex: false
+    before_script:
+    - if [ $ARROW_CI_R_AFFECTED != "1" ]; then exit; fi
     before_install:
     - $TRAVIS_BUILD_DIR/ci/travis_install_linux.sh
     - $TRAVIS_BUILD_DIR/ci/travis_before_script_cpp.sh --only-library
     - export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$TRAVIS_BUILD_DIR/cpp-install/lib
     - export PKG_CONFIG_PATH=$PKG_CONFIG_PATH:$TRAVIS_BUILD_DIR/cpp-install/lib/pkgconfig
+    - $TRAVIS_BUILD_DIR/ci/travis_lint.sh
     - pushd ${TRAVIS_BUILD_DIR}/r
 
 

diff --git a/ci/detect-changes.py b/ci/detect-changes.py
@@ -26,7 +26,8 @@
 
 perr = functools.partial(print, file=sys.stderr)
 
-LANGUAGE_TOPICS = ['c_glib', 'cpp', 'go', 'java', 'js', 'python', 'ruby', 'rust']
+LANGUAGE_TOPICS = ['c_glib', 'cpp', 'go', 'java', 'js', 'python',
+                   'r', 'ruby', 'rust']
 
 ALL_TOPICS = LANGUAGE_TOPICS + ['integration', 'site', 'dev']
 
@@ -66,7 +67,8 @@ def get_travis_head_commit():
 
 def get_travis_commit_range():
     cr = os.environ['TRAVIS_COMMIT_RANGE']
-    # See https://github.com/travis-ci/travis-ci/issues/4596#issuecomment-139811122
+    # See
+    # https://github.com/travis-ci/travis-ci/issues/4596#issuecomment-139811122
     return cr.replace('...', '..')
 
 
@@ -136,16 +138,16 @@ def get_affected_topics(affected_files):
             break
         elif p in ('cpp', 'format'):
             # Test C++ and bindings to the C++ library
-            for k in ('cpp', 'python', 'c_glib', 'ruby', 'integration'):
+            for k in ('cpp', 'python', 'c_glib', 'r', 'ruby', 'integration'):
                 affected[k] = True
         elif p in ('java', 'js'):
             affected[p] = True
             affected['integration'] = True
         elif p in ('c_glib'):
             affected[p] = True
             affected['ruby'] = True
-        elif p in ('go', 'integration', 'python', 'ruby', 'rust', 'site',
-                   'dev'):
+        elif p in ('go', 'integration', 'python', 'r', 'ruby', 'rust',
+                   'site', 'dev'):
             affected[p] = True
 
     return affected
@@ -174,8 +176,8 @@ def get_windows_shell_eval(env):
 
 def run_from_travis():
     if (os.environ['TRAVIS_REPO_SLUG'] == 'apache/arrow' and
-        os.environ['TRAVIS_BRANCH'] == 'master' and
-        os.environ['TRAVIS_EVENT_TYPE'] != 'pull_request'):
+            os.environ['TRAVIS_BRANCH'] == 'master' and
+            os.environ['TRAVIS_EVENT_TYPE'] != 'pull_request'):
         # Never skip anything on master builds in the official repository
         affected = dict.fromkeys(ALL_TOPICS, True)
     else:
@@ -219,14 +221,14 @@ def run_from_appveyor():
     if os.environ.get('TRAVIS'):
         try:
             print(run_from_travis())
-        except:
+        except Exception:
             # Make sure the enclosing eval will return an error
             print("exit 1")
             raise
     elif os.environ.get('APPVEYOR'):
         try:
             print(run_from_appveyor())
-        except:
+        except Exception:
             print("exit 1")
             raise
     else:

diff --git a/ci/travis_lint.sh b/ci/travis_lint.sh
@@ -65,3 +65,9 @@ if [ "$ARROW_CI_PYTHON_AFFECTED" != "0" ]; then
                    --config=$ARROW_PYTHON_DIR/.flake8.cython \
                    $ARROW_PYTHON_DIR
 fi
+
+if [ "$ARROW_CI_R_AFFECTED" != "0" ]; then
+  pushd $ARROW_R_DIR
+  ./lint.sh
+  popd
+fi
diff --git a/cpp/build-support/clang_format_exclusions.txt b/cpp/build-support/clang_format_exclusions.txt
@@ -8,3 +8,4 @@
 *thirdparty/ae/*
 *xxhash.cc
 *xxhash.h
+*RcppExports.cpp*
diff --git a/cpp/build-support/run_clang_format.py b/cpp/build-support/run_clang_format.py
@@ -54,7 +54,9 @@
         fullpaths = (os.path.join(directory, filename)
                      for filename in filenames)
         source_files = [x for x in fullpaths
-                        if x.endswith(".h") or x.endswith(".cc")]
+                        if x.endswith(".h") or
+                        x.endswith(".cc") or
+                        x.endswith(".cpp")]
         formatted_filenames.extend(
             # Filter out files that match the globs in the globs file
             [filename for filename in source_files

diff --git a/r/README.md b/r/README.md
@@ -38,15 +38,15 @@ tf <- tempfile()
 #> # A tibble: 10 x 2
 #>        x       y
 #>    <int>   <dbl>
-#>  1     1 -0.255 
-#>  2     2 -0.162 
-#>  3     3 -0.614 
-#>  4     4 -0.322 
+#>  1     1 -0.255
+#>  2     2 -0.162
+#>  3     3 -0.614
+#>  4     4 -0.322
 #>  5     5  0.0693
-#>  6     6 -0.920 
-#>  7     7 -1.08  
-#>  8     8  0.658 
-#>  9     9  0.821 
+#>  6     6 -0.920
+#>  7     7 -1.08
+#>  8     8  0.658
+#>  9     9  0.821
 #> 10    10  0.539
 arrow::write_arrow(tib, tf)
 
@@ -56,14 +56,30 @@ as_tibble(pa$open_file(tf)$read_pandas())
 #> # A tibble: 10 x 2
 #>        x       y
 #>    <int>   <dbl>
-#>  1     1 -0.255 
-#>  2     2 -0.162 
-#>  3     3 -0.614 
-#>  4     4 -0.322 
+#>  1     1 -0.255
+#>  2     2 -0.162
+#>  3     3 -0.614
+#>  4     4 -0.322
 #>  5     5  0.0693
-#>  6     6 -0.920 
-#>  7     7 -1.08  
-#>  8     8  0.658 
-#>  9     9  0.821 
+#>  6     6 -0.920
+#>  7     7 -1.08
+#>  8     8  0.658
+#>  9     9  0.821
 #> 10    10  0.539
 ```
+
+## Development
+
+### Code style
+
+We use Google C++ style in our C++ code. Check for style errors with
+
+```
+./lint.sh
+```
+
+You can fix the style issues with
+
+```
+./lint.sh --fix
+```
diff --git a/r/lint.sh b/r/lint.sh
@@ -0,0 +1,28 @@
+#!/bin/bash
+
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+SOURCE_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+CPP_BUILD_SUPPORT=$SOURCE_DIR/../cpp/build-support
+
+LLVM_VERSION=6.0
+CLANG_FORMAT=clang-format-$LLVM_VERSION
+
+$CPP_BUILD_SUPPORT/run_clang_format.py $CLANG_FORMAT \
+                                       $CPP_BUILD_SUPPORT/clang_format_exclusions.txt \
+                                       $SOURCE_DIR/src --quiet $1
diff --git a/r/src/ArrayData.cpp b/r/src/ArrayData.cpp
@@ -20,21 +20,22 @@
 using namespace Rcpp;
 
 // [[Rcpp::export]]
-std::shared_ptr<arrow::DataType> ArrayData__get_type(const std::shared_ptr<arrow::ArrayData>& x){
+std::shared_ptr<arrow::DataType> ArrayData__get_type(
+    const std::shared_ptr<arrow::ArrayData>& x) {
   return x->type;
 }
 
 // [[Rcpp::export]]
-int ArrayData__get_length(const std::shared_ptr<arrow::ArrayData>& x){
+int ArrayData__get_length(const std::shared_ptr<arrow::ArrayData>& x) {
   return x->length;
 }
 
 // [[Rcpp::export]]
-int ArrayData__get_null_count(const std::shared_ptr<arrow::ArrayData>& x){
+int ArrayData__get_null_count(const std::shared_ptr<arrow::ArrayData>& x) {
   return x->null_count;
 }
 
 // [[Rcpp::export]]
-int ArrayData__get_offset(const std::shared_ptr<arrow::ArrayData>& x){
+int ArrayData__get_offset(const std::shared_ptr<arrow::ArrayData>& x) {
   return x->offset;
 }
diff --git a/r/src/ChunkedArray.cpp b/r/src/ChunkedArray.cpp
@@ -21,34 +21,33 @@ using namespace Rcpp;
 using namespace arrow;
 
 template <int RTYPE>
-inline SEXP simple_ChunkedArray_to_Vector(const std::shared_ptr<arrow::ChunkedArray>& chunked_array){
+inline SEXP simple_ChunkedArray_to_Vector(
+    const std::shared_ptr<arrow::ChunkedArray>& chunked_array) {
   using stored_type = typename Rcpp::Vector<RTYPE>::stored_type;
   Rcpp::Vector<RTYPE> out = no_init(chunked_array->length());
   auto p = out.begin();
 
   int k = 0;
-  for (int i=0; i<chunked_array->num_chunks(); i++) {
+  for (int i = 0; i < chunked_array->num_chunks(); i++) {
     auto chunk = chunked_array->chunk(i);
     auto n = chunk->length();
 
     // copy the data
     auto q = p;
     p = std::copy_n(
-      reinterpret_cast<const stored_type*>(
-        chunk->data()->buffers[1]->data() + chunk->offset() * sizeof(stored_type)
-      ),
-      n, p);
+        reinterpret_cast<const stored_type*>(chunk->data()->buffers[1]->data() +
+                                             chunk->offset() * sizeof(stored_type)),
+        n, p);
 
     // set NA using the bitmap
     auto bitmap_data = chunk->null_bitmap();
     if (bitmap_data && RTYPE != RAWSXP) {
-      arrow::internal::BitmapReader bitmap_reader(
-          bitmap_data->data(), chunk->offset(), n
-      );
+      arrow::internal::BitmapReader bitmap_reader(bitmap_data->data(), chunk->offset(),
+                                                  n);
 
-      for (int j=0; j<n; j++, bitmap_reader.Next()){
+      for (int j = 0; j < n; j++, bitmap_reader.Next()) {
         if (bitmap_reader.IsNotSet()) {
-          q[k+j] = Rcpp::Vector<RTYPE>::get_na();
+          q[k + j] = Rcpp::Vector<RTYPE>::get_na();
         }
       }
     }
@@ -58,43 +57,47 @@ inline SEXP simple_ChunkedArray_to_Vector(const std::shared_ptr<arrow::ChunkedAr
   return out;
 }
 
-
 // [[Rcpp::export]]
-int ChunkedArray__length(const std::shared_ptr<arrow::ChunkedArray>& chunked_array){
+int ChunkedArray__length(const std::shared_ptr<arrow::ChunkedArray>& chunked_array) {
   return chunked_array->length();
 }
 
 // [[Rcpp::export]]
-int ChunkedArray__null_count(const std::shared_ptr<arrow::ChunkedArray>& chunked_array){
+int ChunkedArray__null_count(const std::shared_ptr<arrow::ChunkedArray>& chunked_array) {
   return chunked_array->null_count();
 }
 
 // [[Rcpp::export]]
-int ChunkedArray__num_chunks(const std::shared_ptr<arrow::ChunkedArray>& chunked_array){
+int ChunkedArray__num_chunks(const std::shared_ptr<arrow::ChunkedArray>& chunked_array) {
   return chunked_array->num_chunks();
 }
 
 // [[Rcpp::export]]
-std::shared_ptr<arrow::Array> ChunkedArray__chunk(const std::shared_ptr<arrow::ChunkedArray>& chunked_array, int i){
+std::shared_ptr<arrow::Array> ChunkedArray__chunk(
+    const std::shared_ptr<arrow::ChunkedArray>& chunked_array, int i) {
   return chunked_array->chunk(i);
 }
 
 // [[Rcpp::export]]
-List ChunkedArray__chunks(const std::shared_ptr<arrow::ChunkedArray>& chunked_array){
+List ChunkedArray__chunks(const std::shared_ptr<arrow::ChunkedArray>& chunked_array) {
   return wrap(chunked_array->chunks());
 }
 
 // [[Rcpp::export]]
-std::shared_ptr<arrow::DataType> ChunkedArray__type(const std::shared_ptr<arrow::ChunkedArray>& chunked_array){
+std::shared_ptr<arrow::DataType> ChunkedArray__type(
+    const std::shared_ptr<arrow::ChunkedArray>& chunked_array) {
   return chunked_array->type();
 }
 
 // [[Rcpp::export]]
-SEXP ChunkedArray__as_vector(const std::shared_ptr<arrow::ChunkedArray>& chunked_array){
-  switch(chunked_array->type()->id()){
-    case Type::INT8: return simple_ChunkedArray_to_Vector<RAWSXP>(chunked_array);
-    case Type::INT32: return simple_ChunkedArray_to_Vector<INTSXP>(chunked_array);
-    case Type::DOUBLE: return simple_ChunkedArray_to_Vector<REALSXP>(chunked_array);
+SEXP ChunkedArray__as_vector(const std::shared_ptr<arrow::ChunkedArray>& chunked_array) {
+  switch (chunked_array->type()->id()) {
+    case Type::INT8:
+      return simple_ChunkedArray_to_Vector<RAWSXP>(chunked_array);
+    case Type::INT32:
+      return simple_ChunkedArray_to_Vector<INTSXP>(chunked_array);
+    case Type::DOUBLE:
+      return simple_ChunkedArray_to_Vector<REALSXP>(chunked_array);
     default:
       break;
   }
@@ -104,19 +107,21 @@ SEXP ChunkedArray__as_vector(const std::shared_ptr<arrow::ChunkedArray>& chunked
 }
 
 // [[Rcpp::export]]
-std::shared_ptr<arrow::ChunkedArray> ChunkArray__Slice1( const std::shared_ptr<arrow::ChunkedArray>& chunked_array, int offset) {
+std::shared_ptr<arrow::ChunkedArray> ChunkArray__Slice1(
+    const std::shared_ptr<arrow::ChunkedArray>& chunked_array, int offset) {
   return chunked_array->Slice(offset);
 }
 
 // [[Rcpp::export]]
-std::shared_ptr<arrow::ChunkedArray> ChunkArray__Slice2( const std::shared_ptr<arrow::ChunkedArray>& chunked_array, int offset, int length) {
+std::shared_ptr<arrow::ChunkedArray> ChunkArray__Slice2(
+    const std::shared_ptr<arrow::ChunkedArray>& chunked_array, int offset, int length) {
   return chunked_array->Slice(offset, length);
 }
 
 // [[Rcpp::export]]
-std::shared_ptr<arrow::ChunkedArray> ChunkedArray__from_list(List chunks){
+std::shared_ptr<arrow::ChunkedArray> ChunkedArray__from_list(List chunks) {
   std::vector<std::shared_ptr<arrow::Array>> vec;
-  for ( SEXP chunk: chunks) {
+  for (SEXP chunk : chunks) {
     vec.push_back(Array__from_vector(chunk));
   }
   return std::make_shared<arrow::ChunkedArray>(std::move(vec));

diff --git a/r/src/Column.cpp b/r/src/Column.cpp
@@ -28,11 +28,13 @@ int Column__null_count(const std::shared_ptr<arrow::Column>& column) {
 }
 
 // [[Rcpp::export]]
-std::shared_ptr<arrow::DataType> Column__type(const std::shared_ptr<arrow::Column>& column) {
+std::shared_ptr<arrow::DataType> Column__type(
+    const std::shared_ptr<arrow::Column>& column) {
   return column->type();
 }
 
 // [[Rcpp::export]]
-std::shared_ptr<arrow::ChunkedArray> Column__data(const std::shared_ptr<arrow::Column>& column) {
+std::shared_ptr<arrow::ChunkedArray> Column__data(
+    const std::shared_ptr<arrow::Column>& column) {
   return column->data();
 }