Enable to trigger code guideline checks when PR is created (#790)

Enable to trigger code guideline checks for C/C++ source codes when PR is created

.github/workflows/codeing_guildelines.yml

@@ -0,0 +1,39 @@
+# Copyright (C) 2019 Intel Corporation.  All rights reserved.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+name: Coding Guidelines
+
+on:
+  # will be triggered on PR events
+  pull_request:
+  # allow to be triggered manually
+  workflow_dispatch:
+
+# Cancel any in-flight jobs for the same PR/branch so there's only one active
+# at a time
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  # Cancel any in-flight jobs for the same PR/branch so there's only one active
+  # at a time
+  cancel_previous:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Cancel Workflow Action
+        uses: styfle/cancel-workflow-action@0.6.0
+        with:
+          access_token: ${{ github.token }}
+
+  complinace_job:
+    needs: cancel_previous
+    runs-on: ubuntu-latest
+    steps:
+      - name: checkout
+        uses: actions/checkout@v2
+        with:
+          fetch-depth: 0
+      
+      # github.event.pull_request.base.label = ${{github.repository}}/${{github.base_ref}}
+      - name: Run Coding Guidelines Checks
+        run: /usr/bin/env python3 ./ci/coding_guidelines_check.py --commits ${{ github.event.pull_request.base.sha }}..HEAD

ci/coding_guidelines_check.py

@@ -0,0 +1,274 @@
+#!/usr/bin/env python3
+#
+# Copyright (C) 2019 Intel Corporation.  All rights reserved.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+#
+import argparse
+import re
+import pathlib
+import re
+import shlex
+import shutil
+import subprocess
+import sys
+
+CLANG_FORMAT_CMD = "clang-format-12"
+GIT_CLANG_FORMAT_CMD = "git-clang-format-12"
+
+# glob style patterns
+EXCLUDE_PATHS = [
+    "**/.git/*",
+    "**/.github/*",
+    "**/.vscode/*",
+    "**/assembly-script/*",
+    "**/build/*",
+    "**/build-scripts/*",
+    "**/ci/*",
+    "**/core/deps/*",
+    "**/doc/*",
+    "**/samples/wasm-c-api/src/*.*",
+    "**/samples/workload/*",
+    "**/test-tools/wasi-sdk/*",
+    "**/tests/wamr-test-suites/workspace/*",
+    "**/wamr-sdk/*",
+]
+
+C_SUFFIXES = [".c", ".cpp", ".h"]
+INVALID_DIR_NAME_SEGMENT = r"([a-zA-Z0-9]+\_[a-zA-Z0-9]+)"
+INVALID_FILE_NAME_SEGMENT = r"([a-zA-Z0-9]+\-[a-zA-Z0-9]+)"
+
+
+def locate_command(command: str) -> bool:
+    if not shutil.which(command):
+        print(f"Command '{command}'' not found")
+        return False
+
+    return True
+
+
+def is_excluded(path: str) -> bool:
+    path = pathlib.Path(path).resolve()
+    for exclude_path in EXCLUDE_PATHS:
+        if path.match(exclude_path):
+            return True
+    return False
+
+
+def pre_flight_check(root: pathlib) -> bool:
+    def check_aspell(root):
+        return True
+
+    def check_clang_foramt(root: pathlib) -> bool:
+        if not locate_command(CLANG_FORMAT_CMD):
+            return False
+
+        # Quick syntax check for .clang-format
+        try:
+            subprocess.check_output(
+                shlex.split(f"{CLANG_FORMAT_CMD} --dump-config"), cwd=root
+            )
+        except subprocess.CalledProcessError:
+            print(f"Might have a typo in .clang-format")
+            return False
+        return True
+
+    def check_git_clang_format() -> bool:
+        return locate_command(GIT_CLANG_FORMAT_CMD)
+
+    return check_aspell(root) and check_clang_foramt(root) and check_git_clang_format()
+
+
+def run_clang_format(file_path: pathlib, root: pathlib) -> bool:
+    try:
+        subprocess.check_call(
+            shlex.split(
+                f"{CLANG_FORMAT_CMD} --style=file --Werror --dry-run {file_path}"
+            ),
+            cwd=root,
+        )
+        return True
+    except subprocess.CalledProcessError:
+        print(f"{file_path} failed the check of {CLANG_FORMAT_CMD}")
+        return False
+
+
+def run_clang_format_diff(root: pathlib, commits: str) -> bool:
+    """
+    Use `clang-format-12` and `git-clang-format-12` to check code
+    format of the PR, which specificed a commit range. It is required to
+    format code before `git commit` or when failed the PR check:
+
+    ``` shell
+    cd path/to/wamr/root
+    clang-format-12 --style file -i path/to/file
+    ```
+
+    The code wrapped by `/* clang-format off */` and `/* clang-format on */`
+    will not be formatted, you shall use them when the formatted code is not
+    readable or friendly:
+
+    ``` cc
+    /* clang-format off */
+    code snippets
+    /* clang-format on */
+    ```
+
+    """
+    try:
+        before, after = commits.split("..")
+        after = after if after else "HEAD"
+        COMMAND = (
+            f"{GIT_CLANG_FORMAT_CMD} -v --binary "
+            f"{shutil.which(CLANG_FORMAT_CMD)} --style file "
+            f"--extensions c,cpp,h --diff {before} {after}"
+        )
+
+        p = subprocess.Popen(
+            shlex.split(COMMAND),
+            stdout=subprocess.PIPE,
+            stderr=None,
+            stdin=None,
+            universal_newlines=True,
+        )
+
+        stdout, _ = p.communicate()
+        if not stdout.startswith("diff --git"):
+            return True
+
+        diff_content = stdout.split("\n")
+        found = False
+        for summary in [x for x in diff_content if x.startswith("diff --git")]:
+            # b/path/to/file -> path/to/file
+            with_invalid_format = re.split("\s+", summary)[-1][2:]
+            if not is_excluded(with_invalid_format):
+                print(f"--- {with_invalid_format} failed on code style checking.")
+                found = True
+        else:
+            return not found
+    except subprocess.subprocess.CalledProcessError:
+        return False
+
+
+def run_aspell(file_path: pathlib, root: pathlib) -> bool:
+    return True
+
+
+def check_dir_name(path: pathlib, root: pathlib) -> bool:
+    m = re.search(INVALID_DIR_NAME_SEGMENT, str(path.relative_to(root)))
+    if m:
+        print(f"--- found a character '_' in {m.groups()} in {path}")
+
+    return not m
+
+
+def check_file_name(path: pathlib) -> bool:
+    m = re.search(INVALID_FILE_NAME_SEGMENT, path.stem)
+    if m:
+        print(f"--- found a character '-' in {m.groups()} in {path}")
+
+    return not m
+
+
+def parse_commits_range(root: pathlib, commits: str) -> list:
+    GIT_LOG_CMD = f"git log --pretty='%H' {commits}"
+    try:
+        ret = subprocess.check_output(
+            shlex.split(GIT_LOG_CMD), cwd=root, universal_newlines=True
+        )
+        return [x for x in ret.split("\n") if x]
+    except subprocess.CalledProcessError:
+        print(f"can not parse any commit from the range {commits}")
+        return []
+
+
+def analysis_new_item_name(root: pathlib, commit: str) -> bool:
+    """
+    For any file name in the repo, it is required to use '_' to replace '-'.
+
+    For any directory name in the repo,  it is required to use '-' to replace '_'.
+    """
+    GIT_SHOW_CMD = f"git show --oneline --name-status --diff-filter A {commit}"
+    try:
+        invalid_items = True
+        output = subprocess.check_output(
+            shlex.split(GIT_SHOW_CMD), cwd=root, universal_newlines=True
+        )
+        if not output:
+            return True
+
+        NEW_FILE_PATTERN = "^A\s+(\S+)"
+        for line_no, line in enumerate(output.split("\n")):
+            # bypass the first line, usually it is the commit description
+            if line_no == 0:
+                continue
+
+            if not line:
+                continue
+
+            match = re.match(NEW_FILE_PATTERN, line)
+            if not match:
+                continue
+
+            new_item = match.group(1)
+            new_item = pathlib.Path(new_item).resolve()
+
+            if new_item.is_file():
+                if not check_file_name(new_item):
+                    invalid_items = False
+                    continue
+
+                new_item = new_item.parent
+
+            if not check_dir_name(new_item, root):
+                invalid_items = False
+                continue
+        else:
+            return invalid_items
+
+    except subprocess.CalledProcessError:
+        return False
+
+
+def process_entire_pr(root: pathlib, commits: str) -> bool:
+    if not commits:
+        print("Please provide a commits range")
+        return False
+
+    commit_list = parse_commits_range(root, commits)
+    if not commit_list:
+        print(f"Quit since there is no commit to check with")
+        return True
+
+    print(f"there are {len(commit_list)} commits in the PR")
+
+    found = False
+    if not analysis_new_item_name(root, commits):
+        print(f"{analysis_new_item_name.__doc__}")
+        found = True
+
+    if not run_clang_format_diff(root, commits):
+        print(f"{run_clang_format_diff.__doc__}")
+        found = True
+
+    return not found
+
+
+def main() -> int:
+    parser = argparse.ArgumentParser(
+        description="Check if change meets all coding guideline requirements"
+    )
+    parser.add_argument(
+        "-c", "--commits", default=None, help="Commit range in the form: a..b"
+    )
+    options = parser.parse_args()
+
+    wamr_root = pathlib.Path(__file__).parent.joinpath("..").resolve()
+
+    if not pre_flight_check(wamr_root):
+        return False
+
+    return process_entire_pr(wamr_root, options.commits)
+
+
+if __name__ == "__main__":
+    sys.exit(0 if main() else 1)

ci/run_pre_commit_check.py

@@ -1,263 +0,0 @@
-#!/usr/bin/env python3
-#
-# Copyright (C) 2019 Intel Corporation.  All rights reserved.
-# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-#
-
-import json
-import os
-import pathlib
-import queue
-import re
-import shlex
-import shutil
-import subprocess
-import sys
-
-CLANG_CMD = "clang-13"
-CLANG_CPP_CMD = "clang-cpp-13"
-CLANG_FORMAT_CMD = "clang-format-13"
-CLANG_TIDY_CMD = "clang-tidy-13"
-CMAKE_CMD = "cmake"
-
-
-# glob style patterns
-EXCLUDE_PATHS = [
-    "**/.git/",
-    "**/.github/",
-    "**/.vscode/",
-    "**/assembly-script/",
-    "**/build/",
-    "**/build-scripts/",
-    "**/ci/",
-    "**/core/deps/",
-    "**/doc/",
-    "**/samples/workload/",
-    "**/test-tools/",
-    "**/wamr-sdk/",
-    "**/wamr-dev/",
-    "**/wamr-dev-simd/",
-]
-
-C_SUFFIXES = [".c", ".h"]
-
-VALID_DIR_NAME = r"([a-zA-Z0-9]+\-*)+[a-zA-Z0-9]*"
-VALID_FILE_NAME = r"\.?([a-zA-Z0-9]+\_*)+[a-zA-Z0-9]*\.*\w*"
-
-
-def locate_command(command):
-    if not shutil.which(command):
-        print(f"Command '{command}'' not found")
-        return False
-
-    return True
-
-
-def is_excluded(path):
-    for exclude_path in EXCLUDE_PATHS:
-        if path.match(exclude_path):
-            return True
-    return False
-
-
-def pre_flight_check(root):
-    def check_clang_foramt(root):
-        if not locate_command(CLANG_FORMAT_CMD):
-            return False
-
-        # Quick syntax check for .clang-format
-        try:
-            subprocess.check_call(
-                shlex.split(f"{CLANG_FORMAT_CMD} --dump-config"), cwd=root
-            )
-        except subprocess.CalledProcessError:
-            print(f"Might have a typo in .clang-format")
-            return False
-        return True
-
-    def check_clang_tidy(root):
-        if not locate_command(CLANG_TIDY_CMD):
-            return False
-
-        if (
-            not locate_command(CLANG_CMD)
-            or not locate_command(CLANG_CPP_CMD)
-            or not locate_command(CMAKE_CMD)
-        ):
-            return False
-
-        # Quick syntax check for .clang-format
-        try:
-            subprocess.check_call(
-                shlex.split("{CLANG_TIDY_CMD} --dump-config"), cwd=root
-            )
-        except subprocess.CalledProcessError:
-            print(f"Might have a typo in .clang-tidy")
-            return False
-
-        # looking for compile command database
-        return True
-
-    def check_aspell(root):
-        return True
-
-    return check_clang_foramt(root) and check_clang_tidy(root) and check_aspell(root)
-
-
-def run_clang_format(file_path, root):
-    try:
-        subprocess.check_call(
-            shlex.split(
-                f"{CLANG_FORMAT_CMD} --style=file --Werror --dry-run {file_path}"
-            ),
-            cwd=root,
-        )
-        return True
-    except subprocess.CalledProcessError:
-        print(f"{file_path} failed the check of {CLANG_FORMAT_CMD}")
-        return False
-
-
-def generate_compile_commands(compile_command_database, root):
-    CMD = f"{CMAKE_CMD} -DCMAKE_C_COMPILER={shutil.which(CLANG_CMD)} -DCMAKE_CXX_COMPILER={shutil.which(CLANG_CPP_CMD)} -DCMAKE_EXPORT_COMPILE_COMMANDS=ON .."
-
-    try:
-        linux_mini_build = root.joinpath("product-mini/platforms/linux/build").resolve()
-        linux_mini_build.mkdir(exist_ok=True)
-        if subprocess.check_call(shlex.split(CMD), cwd=linux_mini_build):
-            return False
-
-        wamrc_build = root.joinpath("wamr-compiler/build").resolve()
-        wamrc_build.mkdir(exist_ok=True)
-
-        if subprocess.check_call(shlex.split(CMD), cwd=wamrc_build):
-            return False
-
-        with open(linux_mini_build.joinpath("compile_commands.json"), "r") as f:
-            iwasm_compile_commands = json.load(f)
-
-        with open(wamrc_build.joinpath("compile_commands.json"), "r") as f:
-            wamrc_compile_commands = json.load(f)
-
-        all_compile_commands = iwasm_compile_commands + wamrc_compile_commands
-        # TODO: duplication items ?
-        with open(compile_command_database, "w") as f:
-            json.dump(all_compile_commands, f)
-
-        return True
-    except subprocess.CalledProcessError:
-        return False
-
-
-def run_clang_tidy(file_path, root):
-    # preparatoin
-    compile_command_database = pathlib.Path("/tmp/compile_commands.json")
-    if not compile_command_database.exists() and not generate_compile_commands(
-        compile_command_database, root
-    ):
-        return False
-
-    try:
-        if subprocess.check_call(
-            shlex.split(f"{CLANG_TIDY_CMD} -p={compile_command_database} {file_path}"),
-            cwd=root,
-        ):
-            print(f"{file_path} failed the check of {CLANG_TIDY_CMD}")
-    except subprocess.CalledProcessError:
-        print(f"{file_path} failed the check of {CLANG_TIDY_CMD}")
-        return False
-    return True
-
-
-def run_aspell(file_path, root):
-    return True
-
-
-def check_dir_name(path, root):
-    # since we don't want to check the path beyond root.
-    # we hope "-" only will be used in a dir name as separators
-    return all(
-        [
-            re.match(VALID_DIR_NAME, path_part)
-            for path_part in path.relative_to(root).parts
-        ]
-    )
-
-
-def check_file_name(path):
-    # since we don't want to check the path beyond root.
-    # we hope "_" only will be used in a file name as separators
-    return re.match(VALID_FILE_NAME, path.name) is not None
-
-
-def run_pre_commit_check(path, root=None):
-    path = path.resolve()
-    if path.is_dir():
-        if not check_dir_name(path, root):
-            print(f"{path} is not a valid directory name")
-            return False
-        else:
-            return True
-
-    if path.is_file():
-        if not check_file_name(path):
-            print(f"{path} is not a valid file name")
-            return False
-
-        if not path.suffix in C_SUFFIXES:
-            return True
-
-        return (
-            run_clang_format(path, root)
-            and run_clang_tidy(path, root)
-            and run_aspell(path, root)
-        )
-
-    print(f"{path} neither a file nor a directory")
-    return False
-
-
-def main():
-    wamr_root = pathlib.Path(__file__).parent.joinpath("..").resolve()
-
-    if not pre_flight_check(wamr_root):
-        return False
-
-    invalid_file, invalid_directory = 0, 0
-
-    # in order to skip exclude directories ASAP,
-    # will not yield Path.
-    # since we will create every object
-    dirs = queue.Queue()
-    dirs.put(wamr_root)
-    while not dirs.empty():
-        qsize = dirs.qsize()
-        while qsize:
-            current_dir = dirs.get()
-
-            for path in current_dir.iterdir():
-                path = path.resolve()
-
-                if path.is_symlink():
-                    continue
-
-                if path.is_dir() and not is_excluded(path):
-                    invalid_directory += (
-                        0 if run_pre_commit_check(path, wamr_root) else 1
-                    )
-                    dirs.put(path)
-
-                if not path.is_file():
-                    continue
-
-                invalid_file += 0 if run_pre_commit_check(path) else 1
-
-            else:
-                qsize -= 1
-
-    print(f"invalid_directory={invalid_directory}, invalid_file={invalid_file}")
-    return True
-
-
-if __name__ == "__main__":
-    main()