Fixed lint errors (vesoft-inc#102)

* Fixed lint errors * Fixed lint errors in src/dataman * Enhance pre-commit.sh * Fixed miss-fixing * Check on *.inl files
tong-hao · Jan 15, 2019 · 86426a4 · 86426a4
1 parent 8e8d1ed
commit 86426a4
Show file tree

Hide file tree

Showing 56 changed files with 318 additions and 267 deletions.
diff --git a/cpplint/bin/pre-commit.sh b/cpplint/bin/pre-commit.sh
@@ -5,23 +5,44 @@
 # This source code is licensed under Apache 2.0 License
 #  (found in the LICENSE.Apache file in the root directory)
 
-CPPLINT_FILE=`dirname $0`/../../cpplint/cpplint.py
+CPPLINT=`dirname $0`/../../cpplint/cpplint.py
 
 if [ $# -eq 0 ];then
-  CHECK_FILES=$(git diff --name-only HEAD)
+    # Since cpplint.py could only apply on our working tree,
+    # so we forbid committing with unstaged changes present.
+    # Otherwise, lints can be bypassed via changing without commit.
+    if ! git diff-files --exit-code --quiet
+    then
+        echo "You have unstaged changes, please stage or stash them first."
+        exit 1
+    fi
+    CHECK_FILES=$(git diff --name-only --diff-filter=d HEAD | egrep '.*\.cpp$|.*\.h$|.*\.inl$')
 else
-  CHECK_FILES=$(find $1 -not \( -path src/CMakeFiles -prune \)  -not \( -path src/interface/gen-cpp2 -prune \) -name "*.[h]" -o -name "*.cpp" | grep -v 'GraphScanner.*' | grep -v 'GraphParser.*')
+    CHECK_FILES=$(find $@ -not \( -path src/CMakeFiles -prune \) \
+                          -not \( -path src/interface/gen-cpp2 -prune \) \
+                          -name "*.[h]" -o -name "*.cpp" -o -name '*.inl' \
+                          | grep -v 'GraphScanner.*' | grep -v 'GraphParser.*')
 fi
 
-CPPLINT_EXTENS=cpp,h
-CPPLINT_FITER=-whitespace/indent,-build/include_what_you_use,-readability/todo,-build/include,-build/header_guard,-runtime/references
-python $CPPLINT_FILE --quiet --extensions=$CPPLINT_EXTENS --filter=$CPPLINT_FITER --linelength=100 $CHECK_FILES 2>&1 
+# No changes on interested files
+if [[ -z $CHECK_FILES ]]
+then
+    exit 0
+fi
+
+echo "Performing C++ linters..."
+
+CPPLINT_EXTENS=cpp,h,inl
+CPPLINT_FILTER=-whitespace/indent,-build/include_what_you_use,-readability/todo,-build/include,-build/header_guard,-runtime/references,-build/c++11
+
+python $CPPLINT --quiet --extensions=$CPPLINT_EXTENS \
+                --filter=$CPPLINT_FILTER --linelength=100 $CHECK_FILES 2>&1
 
 result=$?
 if [ $result -eq 0 ]
 then
-  exit 0
+    exit 0
 else
-  echo "cpplint code style check failed, please fix and recommit."
-  exit 1
+    echo "cpplint code style check failed, please fix and recommit."
+    exit 1
 fi
diff --git a/src/common/base/Base.h b/src/common/base/Base.h
@@ -64,7 +64,7 @@
 #include <folly/RWSpinLock.h>
 
 #include "thread/NamedThread.h"
-//#include "base/StringUnorderedMap.h"
+// #include "base/StringUnorderedMap.h"
 
 #define MUST_USE_RESULT                 __attribute__((warn_unused_result))
 #define DONT_OPTIMIZE                   __attribute__((optimize("O0")))

diff --git a/src/common/base/Cord.cpp b/src/common/base/Cord.cpp
@@ -75,7 +75,7 @@ void Cord::clear() {
 }
 
 
-bool Cord::applyTo(std::function<bool (const char*, int32_t)> visitor) const {
+bool Cord::applyTo(std::function<bool(const char*, int32_t)> visitor) const {
     char* next = head_;
     while (next != tail_) {
         if (!visitor(next, blockContentSize_)) {

diff --git a/src/common/base/Cord.h b/src/common/base/Cord.h
@@ -22,7 +22,7 @@ class Cord {
 
     // Apply each block to the visitor until the end or the visitor
     // returns false
-    bool applyTo(std::function<bool (const char*, int32_t)> visitor) const;
+    bool applyTo(std::function<bool(const char*, int32_t)> visitor) const;
 
     // Append the cord content to the given string
     size_t appendTo(std::string& str) const;

diff --git a/src/common/base/StatusOr.h b/src/common/base/StatusOr.h
@@ -75,15 +75,15 @@ class StatusOr final {
     // Copy/move construct from `Status'
     // Not explicit to allow construct from a `Status', e.g. in the `return' statement
     template <typename U>
-    StatusOr(U &&status, std::enable_if_t<is_status_v<U>>* = nullptr)
+    StatusOr(U &&status, std::enable_if_t<is_status_v<U>>* = nullptr)   // NOLINT
         : variant_(std::forward<U>(status)) {
         state_ = kStatus;
     }
 
     // Copy/move construct with a value of any compatible type
     // Not explicit to allow construct from a value, e.g. in the `return' statement
     template <typename U>
-    StatusOr(U &&value, std::enable_if_t<is_initializable_v<U>>* = nullptr)
+    StatusOr(U &&value, std::enable_if_t<is_initializable_v<U>>* = nullptr)     // NOLINT
         : variant_(std::forward<U>(value)) {
         state_ = kValue;
     }

diff --git a/src/common/base/StringUnorderedMap.h b/src/common/base/StringUnorderedMap.h
@@ -92,7 +92,7 @@ class StringUnorderedMap {
     std::pair<iterator, bool> insert_or_assign(key_type&& key, V&& v);
 
     template< class... Args >
-    std::pair<iterator,bool> emplace( Args&&... args );
+    std::pair<iterator, bool> emplace(Args&&... args);
 
     iterator erase(const_iterator pos);
     iterator erase(const_iterator first, const_iterator last);
@@ -112,13 +112,13 @@ class StringUnorderedMap {
     T& operator[](const key_type& key);
     T& operator[](key_type&& key);
 
-    size_type count(const key_type& key ) const;
+    size_type count(const key_type& key) const;
 
     iterator find(const key_type& key);
     const_iterator find(const key_type& key) const;
 
-    std::pair<iterator,iterator> equal_range(const key_type& key);
-    std::pair<const_iterator,const_iterator> equal_range(const key_type& key) const;
+    std::pair<iterator, iterator> equal_range(const key_type& key);
+    std::pair<const_iterator, const_iterator> equal_range(const key_type& key) const;
 
 private:
     std::unordered_map<uint64_t, T> valueMap_;

diff --git a/src/common/base/StringUnorderedMap.inl b/src/common/base/StringUnorderedMap.inl
diff --git a/src/common/base/test/CordBenchmark.cpp b/src/common/base/test/CordBenchmark.cpp
@@ -8,7 +8,7 @@
 #include <folly/Benchmark.h>
 #include "base/Cord.h"
 
-using namespace nebula;
+using nebula::Cord;
 
 BENCHMARK(sstream_10k_string, iters) {
     for (auto i = 0u; i < iters; i++) {

diff --git a/src/common/base/test/CordTest.cpp b/src/common/base/test/CordTest.cpp
@@ -8,7 +8,7 @@
 #include <gtest/gtest.h>
 #include "base/Cord.h"
 
-using namespace nebula;
+namespace nebula {
 
 TEST(Cord, write) {
     Cord cord;
@@ -194,6 +194,8 @@ TEST(Cord, cordStream) {
     EXPECT_EQ(str1 + str2, c1.str());
 }
 
+}   // namespace nebula
+
 
 int main(int argc, char** argv) {
     testing::InitGoogleTest(&argc, argv);

diff --git a/src/common/fs/FileUtils.cpp b/src/common/fs/FileUtils.cpp
@@ -98,7 +98,7 @@ StatusOr<std::string> FileUtils::readLink(const char *path) {
 
 std::string FileUtils::dirname(const char *path) {
     DCHECK(path != nullptr && *path != '\0');
-    if (::strcmp("/", path) == 0) { // root only
+    if (::strcmp("/", path) == 0) {     // root only
         return "/";
     }
     static const std::regex pattern("(.*)/([^/]+)/?");
@@ -228,7 +228,7 @@ std::string FileUtils::joinPath(const folly::StringPiece dir,
     std::size_t len = dir.size();
     if (len == 0) {
         buf.resize(filename.size() + 2);
-        strcpy(&(buf[0]), "./");
+        strcpy(&(buf[0]), "./");    // NOLINT
         strncpy(&(buf[2]), filename.begin(), filename.size());
         return std::move(buf);
     }

diff --git a/src/common/fs/FileUtils.h b/src/common/fs/FileUtils.h
@@ -213,7 +213,6 @@ class FileUtils final {
         }
 
     private:
-
         void openFileOrDirectory();
         void dirNext();
         void fileNext();

diff --git a/src/common/fs/TempDir.cpp b/src/common/fs/TempDir.cpp
@@ -14,7 +14,7 @@ TempDir::TempDir(const char* pathTemplate, bool deleteOnDestroy)
         : deleteOnDestroy_(deleteOnDestroy) {
     auto len = strlen(pathTemplate);
     std::unique_ptr<char[]> name(new char[len + 1]);
-    strcpy(name.get(), pathTemplate);
+    strcpy(name.get(), pathTemplate);   // NOLINT
 
     VLOG(2) << "Trying to create the temp directory with pattern \""
             << name.get() << "\"";

diff --git a/src/common/fs/TempFile.cpp b/src/common/fs/TempFile.cpp
@@ -14,7 +14,7 @@ TempFile::TempFile(const char *path, bool autoDelete) {
     autoDelete_ = autoDelete;
     auto len = ::strlen(path);
     path_ = std::make_unique<char[]>(len + 1);
-    ::strcpy(path_.get(), path);
+    ::strcpy(path_.get(), path);    // NOLINT
 
     auto fd = ::mkstemp(path_.get());
     if (fd == -1) {

diff --git a/src/common/fs/test/FileUtilsTest.cpp b/src/common/fs/test/FileUtilsTest.cpp
@@ -8,7 +8,8 @@
 #include <gtest/gtest.h>
 #include "fs/FileUtils.h"
 
-using namespace nebula::fs;
+namespace nebula {
+namespace fs {
 
 TEST(FileUtils, readLink) {
     {
@@ -325,6 +326,9 @@ TEST(FileUtilsIterator, File) {
     ASSERT_FALSE(iter.valid());
 }
 
+}   // namespace fs
+}   // namespace nebula
+
 
 int main(int argc, char** argv) {
     testing::InitGoogleTest(&argc, argv);

diff --git a/src/common/fs/test/TempDirTest.cpp b/src/common/fs/test/TempDirTest.cpp
@@ -9,7 +9,8 @@
 #include "fs/TempDir.h"
 #include "fs/FileUtils.h"
 
-using namespace nebula::fs;
+namespace nebula {
+namespace fs {
 
 TEST(TempDir, AutoRemoval) {
     std::string dirpath;
@@ -56,7 +57,7 @@ TEST(TempDir, CreateFiles) {
         TempDir td("/tmp/", "TempDirTest.XXXXXX");
         ASSERT_FALSE(!td.path());
         dirpath = td.path();
-    
+
         int fd = open(FileUtils::joinPath(dirpath, "testfile.txt").c_str(),
                       O_CREAT | O_RDWR | O_EXCL,
                       0644);
@@ -74,6 +75,9 @@ TEST(TempDir, NegativeTest) {
     EXPECT_TRUE(!td.path());
 }
 
+}   // namespace fs
+}   // namespace nebula
+
 
 int main(int argc, char** argv) {
     testing::InitGoogleTest(&argc, argv);

diff --git a/src/common/network/NetworkUtils.cpp b/src/common/network/NetworkUtils.cpp
@@ -189,16 +189,16 @@ std::string NetworkUtils::intToIPv4(uint32_t ip) {
 
     char buf[16];
     char* pt = buf;
-    strcpy(pt, f4.c_str());
+    strcpy(pt, f4.c_str());     // NOLINT
     pt += f4.size();
     *pt++ = '.';
-    strcpy(pt, f3.c_str());
+    strcpy(pt, f3.c_str());     // NOLINT
     pt += f3.size();
     *pt++ = '.';
-    strcpy(pt, f2.c_str());
+    strcpy(pt, f2.c_str());     // NOLINT
     pt += f2.size();
     *pt++ = '.';
-    strcpy(pt, f1.c_str());
+    strcpy(pt, f1.c_str());     // NOLINT
     pt += f1.size();
 
     return buf;

diff --git a/src/common/network/test/NetworkUtilsBenchmark.cpp b/src/common/network/test/NetworkUtilsBenchmark.cpp
@@ -8,7 +8,7 @@
 #include <folly/Benchmark.h>
 #include "network/NetworkUtils.h"
 
-using namespace nebula::network;
+using nebula::network::NetworkUtils;
 
 bool ipToInt(const std::string& ipStr, uint32_t& ip) {
     std::vector<std::string> parts;

diff --git a/src/common/network/test/NetworkUtilsTest.cpp b/src/common/network/test/NetworkUtilsTest.cpp
@@ -8,8 +8,8 @@
 #include <gtest/gtest.h>
 #include "network/NetworkUtils.h"
 
-using namespace nebula::network;
-
+namespace nebula {
+namespace network {
 
 TEST(NetworkUtils, getHostname) {
     std::string hostname = NetworkUtils::getHostname();
@@ -73,6 +73,9 @@ TEST(NetworkUtils, intIPv4Conversion) {
     EXPECT_EQ(converted, ip);
 }
 
+}   // namespace network
+}   // namespace nebula
+
 
 int main(int argc, char** argv) {
     testing::InitGoogleTest(&argc, argv);

diff --git a/src/common/thread/GenericWorker.cpp b/src/common/thread/GenericWorker.cpp
@@ -91,7 +91,7 @@ void GenericWorker::notify() {
     if (notifier_ == nullptr) {
         return;
     }
-    DCHECK(evfd_ != -1);
+    DCHECK_NE(-1, evfd_);
     auto one = 1UL;
     auto len = ::write(evfd_, &one, sizeof(one));
     DCHECK(len == sizeof(one));

diff --git a/src/common/thread/NamedThread.cpp b/src/common/thread/NamedThread.cpp
@@ -11,6 +11,7 @@ namespace nebula {
 namespace thread {
 
 namespace detail {
+
 class TLSThreadID {
 public:
     TLSThreadID() {
@@ -28,7 +29,8 @@ class TLSThreadID {
 private:
     pid_t                           tid_;
 };
-}
+
+}   // namespace detail
 
 pid_t gettid() {
     static thread_local detail::TLSThreadID tlstid;

diff --git a/src/common/thread/NamedThread.h b/src/common/thread/NamedThread.h
@@ -46,7 +46,7 @@ class NamedThread final : public std::thread {
 public:
     class Nominator {
     public:
-        Nominator(const std::string &name) {
+        explicit Nominator(const std::string &name) {
             get(prevName_);
             set(name);
         }
@@ -89,7 +89,7 @@ template <typename F, typename...Args>
 NamedThread::NamedThread(const std::string &name, F &&f, Args&&...args)
     : std::thread(hook, this, name,
                   std::bind(std::forward<F>(f), std::forward<Args>(args)...)) {
-};
+}
 
 }   // namespace thread
 }   // namespace nebula

diff --git a/src/common/thread/test/GenericThreadPoolTest.cpp b/src/common/thread/test/GenericThreadPoolTest.cpp
@@ -108,8 +108,8 @@ TEST(GenericThreadPool, addDelayTask) {
         ASSERT_EQ(1, pool.addDelayTask(50, cb).get());
         ASSERT_GE(shared.use_count(), 2);
         ASSERT_TRUE(msAboutEqual(50, clock.elapsedInUSec() / 1000));
-        ::usleep(5 * 1000); // ensure all internal resources are released
-        ASSERT_EQ(2, shared.use_count()); // two ref: `shared' and `cb'
+        ::usleep(5 * 1000);     // ensure all internal resources are released
+        ASSERT_EQ(2, shared.use_count());   // two ref: `shared' and `cb'
     }
 }
 

diff --git a/src/common/thread/test/GenericWorkerTest.cpp b/src/common/thread/test/GenericWorkerTest.cpp
@@ -108,8 +108,8 @@ TEST(GenericWorker, addDelayTask) {
         ASSERT_EQ(1, worker.addDelayTask(50, cb).get());
         ASSERT_GE(shared.use_count(), 2);
         ASSERT_TRUE(msAboutEqual(50, clock.elapsedInUSec() / 1000));
-        ::usleep(5 * 1000); // ensure all internal resources are released
-        ASSERT_EQ(2, shared.use_count()); // two ref: `shared' and `cb'
+        ::usleep(5 * 1000);     // ensure all internal resources are released
+        ASSERT_EQ(2, shared.use_count());   // two ref: `shared' and `cb'
     }
 }
 

diff --git a/src/common/thrift/ThriftClientManager.inl b/src/common/thrift/ThriftClientManager.inl
@@ -15,15 +15,13 @@ DECLARE_int32(conn_timeout_ms);
 namespace nebula {
 namespace thrift {
 
-using namespace network;
-
 template<class ClientType>
 std::shared_ptr<ClientType> ThriftClientManager<ClientType>::getClient(
         const HostAddr& host, folly::EventBase* evb) {
     static ThriftClientManager manager;
 
     VLOG(2) << "Getting a client to "
-            << NetworkUtils::intToIPv4(host.first)
+            << network::NetworkUtils::intToIPv4(host.first)
             << ":" << host.second;
 
     if (evb == nullptr) {
@@ -36,7 +34,7 @@ std::shared_ptr<ClientType> ThriftClientManager<ClientType>::getClient(
     }
 
     // Need to create a new client
-    auto ipAddr = NetworkUtils::intToIPv4(host.first);
+    auto ipAddr = network::NetworkUtils::intToIPv4(host.first);
     auto port = host.second;
     VLOG(2) << "There is no existing client to "
             << ipAddr << ":" << port