Fix clang-tidy warnings. (#4149)

* Upgrade gtest for clang-tidy.
* Use CMake to install GTest instead of mv.
* Don't enforce clang-tidy to return 0 due to errors in thrust.
* Add a small test for tidy itself.

* Reformat.

tests/ci_build/clang_tidy.sh

@@ -1,12 +1,18 @@
 #!/bin/bash
 
-rm -rf gtest googletest-release-1.7.0
-wget -nc https://github.com/google/googletest/archive/release-1.7.0.zip
-unzip -n release-1.7.0.zip
-mv googletest-release-1.7.0 gtest && cd gtest
-cmake . && make
-mkdir lib && mv libgtest.a lib
-cd ..
-rm -rf release-1.7.0.zip*
+export GTEST_PKG_NAME=release-1.8.1
+export GTEST_DIR_NAME=googletest-${GTEST_PKG_NAME}  # uncompressed directory
+export GTEST_ZIP_FILE=${GTEST_PKG_NAME}.zip	    # downloaded zip ball name
 
-python3 tests/ci_build/tidy.py --gtest-path=${PWD}/gtest
+rm -rf gtest googletest-release*
+
+wget -nc https://github.com/google/googletest/archive/${GTEST_ZIP_FILE}
+unzip -n ${GTEST_ZIP_FILE}
+mv ${GTEST_DIR_NAME} gtest && cd gtest
+cmake . -DCMAKE_INSTALL_PREFIX=./ins && make
+make install
+
+cd ..
+rm ${GTEST_ZIP_FILE}
+
+python3 tests/ci_build/tidy.py --gtest-path=${PWD}/gtest/ins

tests/ci_build/test_tidy.cc

@@ -0,0 +1,11 @@
+#include <iostream>
+#include <vector>
+
+struct Foo {
+  int bar_;
+};
+
+int main() {
+  std::vector<Foo> values;
+  values.push_back(Foo());
+}

tests/ci_build/tidy.py

@@ -5,23 +5,24 @@ import json
 from multiprocessing import Pool, cpu_count
 import shutil
 import os
+import sys
 import re
 import argparse
 
 
 def call(args):
     '''Subprocess run wrapper.'''
-    completed = subprocess.run(args, stdout=subprocess.PIPE,
-                               stderr=subprocess.DEVNULL)
+    completed = subprocess.run(args,
+                               stdout=subprocess.PIPE,
+                               stderr=subprocess.PIPE)
     error_msg = completed.stdout.decode('utf-8')
-    matched = re.match('.*xgboost.*warning.*', error_msg,
-                       re.MULTILINE | re.DOTALL)
+    matched = re.search('(src|tests)/.*warning:', error_msg,
+                        re.MULTILINE)
     if matched is None:
         return_code = 0
     else:
-        print(error_msg, '\n')
         return_code = 1
-    return completed.returncode | return_code
+    return (completed.returncode, return_code, error_msg)
 
 
 class ClangTidy(object):
@@ -69,8 +70,8 @@ class ClangTidy(object):
 
     def _configure_flags(self, path, command):
         common_args = ['clang-tidy',
-                       # "-header-filter='(xgboost\\/src|xgboost\\/include)'",
-                       '-config='+str(self.clang_tidy)]
+                       "-header-filter='(xgboost\\/src|xgboost\\/include)'",
+                       '-config='+self.clang_tidy]
         common_args.append(path)
         common_args.append('--')
 
@@ -112,7 +113,10 @@ class ClangTidy(object):
         def should_lint(path):
             if not self.cpp_lint and path.endswith('.cc'):
                 return False
-            return True
+            isxgb = path.find('rabit') == -1
+            isxgb = isxgb and path.find('dmlc-core') == -1
+            if isxgb:
+                return True
 
         cdb_file = os.path.join(self.cdb_path, 'compile_commands.json')
         with open(cdb_file, 'r') as fd:
@@ -120,6 +124,7 @@ class ClangTidy(object):
         tidy_file = os.path.join(self.root_path, '.clang-tidy')
         with open(tidy_file) as fd:
             self.clang_tidy = yaml.load(fd)
+            self.clang_tidy = str(self.clang_tidy)
         all_files = []
         for entry in self.compile_commands:
             path = entry['file']
@@ -132,15 +137,59 @@ class ClangTidy(object):
     def run(self):
         '''Run clang-tidy.'''
         all_files = self._configure()
+        passed = True
+        BAR = '-'*32
         with Pool(cpu_count()) as pool:
             results = pool.map(call, all_files)
-        passed = True
-        if 1 in results:
+            for (process_status, tidy_status, msg) in results:
+                # Don't enforce clang-tidy to pass for now due to namespace
+                # for cub in thrust is not correct.
+                if tidy_status == 1:
+                    passed = False
+                    print(BAR, '\n'
+                          'Process return code:', process_status, ', ',
+                          'Tidy result code:', tidy_status, ', ',
+                          'Message:\n', msg,
+                          BAR, '\n')
+        if not passed:
             print('Please correct clang-tidy warnings.')
-            passed = False
         return passed
 
 
+def test_tidy():
+    '''See if clang-tidy and our regex is working correctly.  There are
+many subtleties we need to be careful.  For instances:
+
+    * Is the string re-directed to pipe encoded as UTF-8? or is it
+bytes?
+
+    * On Jenkins there's no 'xgboost' directory, are we catching the
+right keywords?
+
+    * Should we use re.DOTALL?
+
+    * Should we use re.MULTILINE?
+
+    Tests here are not thorough, at least we want to guarantee tidy is
+    not missing anything on Jenkins.
+
+    '''
+    root_path = os.path.abspath(os.path.curdir)
+    tidy_file = os.path.join(root_path, '.clang-tidy')
+    test_file_path = os.path.join(root_path,
+                                  'tests', 'ci_build', 'test_tidy.cc')
+
+    with open(tidy_file) as fd:
+        tidy_config = fd.read()
+        tidy_config = str(tidy_config)
+    tidy_config = '-config='+tidy_config
+    args = ['clang-tidy', tidy_config, test_file_path]
+    (proc_code, tidy_status, error_msg) = call(args)
+    assert proc_code == 0
+    assert tidy_status == 1
+    print('clang-tidy is working.')
+
+
 if __name__ == '__main__':
     parser = argparse.ArgumentParser(description='Run clang-tidy.')
     parser.add_argument('--cpp', type=int, default=1)
@@ -148,8 +197,10 @@ if __name__ == '__main__':
     parser.add_argument('--gtest-path', required=True,
                         help='Full path of Google Test library directory')
     args = parser.parse_args()
+
+    test_tidy()
+
     with ClangTidy(args.gtest_path, args.cpp, args.cuda) as linter:
         passed = linter.run()
-    # Uncomment it once the code base is clang-tidy conformant.
-    # if not passed:
-    #     sys.exit(1)
+    if not passed:
+        sys.exit(1)