CI: add pre-commit.ci to apply clang-format and clang-tidy fixes (#4225)

* CI: add pre-commit.ci to apply clang-format and clang-tidy fixes * do pre-commit fixes first * install clang utils * apply fixes only on changed files * fetch historical commits to get the changed files * test if pre-commit could fix sabotaged format * update pre-commit config * apply fixes * commit changes after apply them * commit changes using pre-commit.ci lite * try to fix SSL error using pre-commit.ci lite * [pre-commit.ci lite] apply automatic fixes * format arguments * add docs on pre-commit hook * Update clang-format config. ABACUS uses some functions with a long function name and a long parameter list. Current setup will generates ugly parameter list with each parameters indented by many spaces. Plus, it is unable to use a customized layout for e.g. putting x, y, and z value in the same line. After discussion, we agree to disable those settings, leaving developers free to chose their own flavor. * Update clang-tidy config. #4015 * [pre-commit.ci lite] apply automatic fixes * Update .pre-commit-config.yaml * Update .clang-tidy according to the meeting --------- Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Co-authored-by: Mohan Chen <[email protected]> Co-authored-by: Haozhi Han <[email protected]>
deepmodeling · Jun 13, 2024 · 10c5281 · 10c5281
1 parent f7a75d2
commit 10c5281
Show file tree

Hide file tree

Showing 7 changed files with 116 additions and 67 deletions.
diff --git a/.clang-format b/.clang-format
@@ -4,10 +4,10 @@ BasedOnStyle: Microsoft
 
 AlwaysBreakTemplateDeclarations: Yes
 
-AllowAllArgumentsOnNextLine: false
-AllowAllParametersOfDeclarationOnNextLine: false
-BinPackArguments: false
-BinPackParameters: false
+AllowAllArgumentsOnNextLine: true
+AllowAllParametersOfDeclarationOnNextLine: true
+BinPackArguments: true
+BinPackParameters: true
 
 BreakBeforeBinaryOperators: All
 BreakBeforeTernaryOperators: true

diff --git a/.clang-tidy b/.clang-tidy
@@ -1,35 +1,44 @@
 ---
 Checks: '
-    -*,
-    bugprone-*,
-    clang-analyzer-*,
-    clang-diagnostic-*,
-    cppcoreguidelines-avoid-non-const-global-variables,
-    cppcoreguidelines-macro-usage,
-    cppcoreguidelines-pro-type-member-init,
-    fuchsia-multiple-inheritance,
-    google-*,
-    -google-runtime-references,
     llvm-header-guard,
-    misc-*,
     modernize-deprecated-headers,
     modernize-redundant-void-arg,
-    modernize-use-nullptr,
     modernize-use-bool-literals,
-    modernize-use-auto,
-    modernize-use-std-numbers,
-    modernize-pass-by-value,
-    modernize-shrink-to-fit,
-    mpi-*,
-    performance-*,
-    -performance-avoid-endl,
-    readability-*,
-    -readability-magic-numbers,
-    -readability-isolate-declaration,
-    -readability-braces-around-statements,
-    -readability-implicit-bool-conversion,
-    -google-readability-braces-around-statements,
-    -misc-unused-parameters,
+    modernize-use-nullptr,
+    readability-avoid-return-with-void-value
+    
+
+    # bugprone-*,
+    # clang-analyzer-*,
+    # clang-diagnostic-*,
+    # cppcoreguidelines-avoid-non-const-global-variables,
+    # cppcoreguidelines-macro-usage,
+    # cppcoreguidelines-pro-type-member-init,
+    # fuchsia-multiple-inheritance,
+    # google-*,
+    # -google-runtime-references,
+    # misc-*,
+    # modernize-shrink-to-fit,
+    # modernize-use-starts-ends-with,
+    # mpi-*,
+    # performance-*,
+    # -performance-avoid-endl,
+    # readability-*,
+    # -readability-else-after-return,
+    # -readability-magic-numbers,
+    # -readability-isolate-declaration,
+    # -readability-braces-around-statements,
+    # -readability-implicit-bool-conversion,
+    # -google-readability-braces-around-statements,
+    # -misc-unused-parameters,
+    # -modernize-pass-by-value,
+    # -modernize-use-auto,
+    # -google-readability-casting,
+    # -readability-make-member-function-const,
+    # -cppcoreguidelines-avoid-non-const-global-variables,
+    # -readability-function-cognitive-complexity,
+    # -google-build-using-namespace,
+    # -bugprone-narrowing-conversions,
 '
 FormatStyle: file
 ...
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -21,19 +21,30 @@ jobs:
         uses: actions/checkout@v4
         with:
           submodules: recursive
+          fetch-depth: 0
 
-      - name: Install Ccache
+      - name: Install CI tools
         run: |
           sudo apt-get update
-          sudo apt-get install -y ccache
+          sudo apt-get install -y ccache clang-format clang-tidy ca-certificates
 
-      - name: Build
+      - name: Configure
         run: |
+          cmake -B build -DBUILD_TESTING=ON -DENABLE_DEEPKS=ON -DENABLE_LIBXC=ON -DENABLE_LIBRI=ON -DENABLE_PAW=ON -DENABLE_GOOGLEBENCH=ON -DENABLE_RAPIDJSON=ON  -DCMAKE_EXPORT_COMPILE_COMMANDS=1
+
+      - uses: actions/setup-python@v3
+      - uses: pre-commit/[email protected]
+        with:
+          extra_args:
+            --from-ref ${{ github.event.pull_request.base.sha }}
+              --to-ref ${{ github.event.pull_request.head.sha }}
+        continue-on-error: true
+      - uses: pre-commit-ci/[email protected]
 
-          cmake -B build -DBUILD_TESTING=ON -DENABLE_DEEPKS=ON -DENABLE_LIBXC=ON -DENABLE_LIBRI=ON -DENABLE_PAW=ON -DENABLE_GOOGLEBENCH=ON -DENABLE_RAPIDJSON=ON
+      - name: Build
+        run: |
           cmake --build build -j8
           cmake --install build
-
       - name: Test
         env:
           GTEST_COLOR: 'yes'

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -0,0 +1,14 @@
+fail_fast: false
+repos:
+  - repo: https://github.com/pocc/pre-commit-hooks
+    rev: v1.3.5
+    hooks:
+      - id: clang-format
+        args: [-i]
+      # - id: clang-tidy
+      #   args: [-p=build, --fix-errors]
+      # - id: oclint
+      # - id: uncrustify
+      # - id: cppcheck
+      # - id: cpplint
+      # - id: include-what-you-use
diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md
@@ -164,6 +164,23 @@ We use `clang-format` as our code formatter. The `.clang-format` file in root di
 For Visual Studio Code developers, the [official extension of C/C++](https://marketplace.visualstudio.com/items?itemName=ms-vscode.cpptools) provided by Microsoft can help you format your codes following the rules. With this extension installed, format your code with `shift+command/alt+f`.
 Configure your VS Code settings as `"C_Cpp.clang_format_style": "file"` (you can look up this option by pasting it into the search box of VS Code settings page), and all this stuff will take into effect. You may also set `"editor.formatOnSave": true` to avoid formatting files everytime manually.
 
+We use <https://pre-commit.ci/> to format the code.
+It is performed after pushing new commits to a PR. You might need to pull the changes before adding new commits.
+
+To use pre-commit locally (**generally not required**):
+Please install the pre-commit tool by running the following command:
+
+```bash
+pip install pre-commit
+pip install clang-tidy clang-format # if you haven't installed them
+```
+
+Then, run the following command to install the pre-commit hooks:
+
+```bash
+pre-commit install
+```
+
 ## Adding a unit test
 
 We use [GoogleTest](https://github.com/google/googletest) as our test framework. Write your test under the corresponding module folder at `abacus-develop/tests`, then append the test to `tests/CMakeLists.txt`. If there are currently no unit tests provided for the module, do as follows. `module_base` provides a simple demonstration.
@@ -386,33 +403,35 @@ To run a subset of unit test, use `ctest -R <test-match-pattern>` to perform tes
     git push origin my-fix-branch
     ```
 
-6. In GitHub, send a pull request (PR) with `deepmodeling/abacus-develop:develop` as the base repository. It is required to document your PR following [our guidelines](#commit-message-guidelines).
+6. In GitHub, send a pull request (PR) with `deepmodeling/abacus-develop:develop` as the base repository. It is **required** to document your PR following [our guidelines](#commit-message-guidelines).
 
-7. After your pull request is merged, you can safely delete your branch and sync the changes from the main (upstream) repository:
+7. If more changes are needed, you can add more commits to your branch and push them to GitHub. Your PR will be updated automatically.
 
-- Delete the remote branch on GitHub either [through the GitHub web UI](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-branches-in-your-repository/deleting-and-restoring-branches-in-a-pull-request#deleting-a-branch-used-for-a-pull-request) or your local shell as follows:
+8. After your pull request is merged, you can safely delete your branch and sync the changes from the main (upstream) repository:
 
-    ```shell
-    git push origin --delete my-fix-branch
-    ```
+    - Delete the remote branch on GitHub either [through the GitHub web UI](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-branches-in-your-repository/deleting-and-restoring-branches-in-a-pull-request#deleting-a-branch-used-for-a-pull-request) or your local shell as follows:
 
-- Check out the master branch:
+        ```shell
+        git push origin --delete my-fix-branch
+        ```
 
-    ```shell
-    git checkout develop -f
-    ```
+    - Check out the master branch:
 
-- Delete the local branch:
+        ```shell
+        git checkout develop -f
+        ```
 
-    ```shell
-    git branch -D my-fix-branch
-    ```
+    - Delete the local branch:
 
-- Update your master with the latest upstream version:
+        ```shell
+        git branch -D my-fix-branch
+        ```
 
-    ```shell
-    git pull --ff upstream develop
-    ```
+    - Update your master with the latest upstream version:
+
+        ```shell
+        git pull --ff upstream develop
+        ```
 
 ## Commit message guidelines
 

diff --git a/source/driver.cpp b/source/driver.cpp
@@ -16,7 +16,7 @@ Driver::Driver()
 
 Driver::~Driver()
 {
-    // Release the device memory within singleton object GlobalC::ppcell 
+    // Release the device memory within singleton object GlobalC::ppcell
     // before the main function exits.
     GlobalC::ppcell.release_memory();
 }
@@ -25,31 +25,30 @@ void Driver::init()
 {
     ModuleBase::TITLE("Driver", "init");
 
-    time_t time_start = std::time(NULL);
+    time_t time_start = std::time(nullptr);
     ModuleBase::timer::start();
 
     // (1) read the input parameters.
     // INPUT should be initalized here and then pass to atomic world, mohan 2024-05-12
     // INPUT should not be GlobalC, mohan 2024-05-12
-    this->reading();
+    Driver::reading();
 
     // (2) welcome to the atomic world!
     this->atomic_world();
 
     // (3) output information
-    time_t time_finish = std::time(NULL);
+    time_t time_finish = std::time(nullptr);
     Print_Info::print_time(time_start, time_finish);
 
     // (4) close all of the running logs
     INPUT.close_log();
 
     // (5) output the json file
-    //Json::create_Json(&GlobalC::ucell.symm,GlobalC::ucell.atoms,&INPUT);
-    Json::create_Json(&GlobalC::ucell,&INPUT);
-    return;
+    // Json::create_Json(&GlobalC::ucell.symm,GlobalC::ucell.atoms,&INPUT);
+    Json::create_Json(&GlobalC::ucell, &INPUT);
 }
 
-void Driver::reading(void)
+void Driver::reading()
 {
     ModuleBase::timer::tick("Driver", "reading");
 
@@ -83,10 +82,9 @@ void Driver::reading(void)
     // ModuleBase::GlobalFunc::DONE(GlobalV::ofs_running,"READING CARDS");
 
     ModuleBase::timer::tick("Driver", "reading");
-    return;
 }
 
-void Driver::atomic_world(void)
+void Driver::atomic_world()
 {
     ModuleBase::TITLE("Driver", "atomic_world");
     //--------------------------------------------------
@@ -101,6 +99,4 @@ void Driver::atomic_world(void)
 
     ModuleBase::timer::finish(GlobalV::ofs_running);
     ModuleBase::Memory::print_all(GlobalV::ofs_running);
-
-    return;
 }
diff --git a/source/driver.h b/source/driver.h
@@ -19,12 +19,12 @@ class Driver
      * This function read the parameter in "INPUT", "STRU" etc,
      * and split the MPI world into different groups.
      */
-    void reading();
+    static void reading();
 
     /**
      * @brief An interface function.
      * This function calls "this->driver_run()" to do calculation,
-	 * and log the time and  memory consumed during calculation.
+     * and log the time and  memory consumed during calculation.
      */
     void atomic_world();