[Github][CI] Introduce doc8 to code-lint-helper.py

[zeyi2]

Closes #167098

[llvmbot]

@llvm/pr-subscribers-clang-tools-extra
@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-github-workflow

Author: mitchell (zeyi2)

Changes

Closes #167098

---

Full diff: https://github.com/llvm/llvm-project/pull/172123.diff

3 Files Affected:

• (modified) .github/workflows/containers/github-action-ci-tooling/Dockerfile (+4-1)
• (modified) .github/workflows/pr-code-lint.yml (+3-3)
• (modified) llvm/utils/git/code-lint-helper.py (+60-1)

diff --git a/.github/workflows/containers/github-action-ci-tooling/Dockerfile b/.github/workflows/containers/github-action-ci-tooling/Dockerfile
index b78c99efb9be3..47dedba5194e2 100644
--- a/.github/workflows/containers/github-action-ci-tooling/Dockerfile
+++ b/.github/workflows/containers/github-action-ci-tooling/Dockerfile
@@ -94,6 +94,10 @@ COPY --from=llvm-downloader /llvm-extract/LLVM-${LLVM_VERSION}-Linux-X64/bin/cla
 COPY clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py ${LLVM_SYSROOT}/bin/clang-tidy-diff.py
 
 # Install dependencies for 'pr-code-lint.yml' job
+RUN apt-get update && \
+    DEBIAN_FRONTEND=noninteractive apt-get install -y python3-doc8 && \
+    apt-get clean && \
+    rm -rf /var/lib/apt/lists/*
 COPY llvm/utils/git/requirements_linting.txt requirements_linting.txt
 RUN pip install -r requirements_linting.txt --break-system-packages && \
     rm requirements_linting.txt
@@ -119,4 +123,3 @@ RUN git clone https://github.com/universal-ctags/ctags.git && \
     ./configure && \
     sudo make install && \
     rm -Rf ../ctags
-
diff --git a/.github/workflows/pr-code-lint.yml b/.github/workflows/pr-code-lint.yml
index ea4f8217cd003..ed0b371064b80 100644
--- a/.github/workflows/pr-code-lint.yml
+++ b/.github/workflows/pr-code-lint.yml
@@ -20,7 +20,7 @@ jobs:
       run:
         shell: bash
     container:
-      image: 'ghcr.io/llvm/ci-ubuntu-24.04-lint'
+      image: 'ghcr.io/llvm/ci-ubuntu-24.04-lint-doc8'
     timeout-minutes: 60
     concurrency:
       group: ${{ github.workflow }}-${{ github.ref }}
@@ -71,12 +71,12 @@ jobs:
                 -DLLVM_INCLUDE_TESTS=OFF \
                 -DCLANG_INCLUDE_TESTS=OFF \
                 -DCMAKE_BUILD_TYPE=Release
-          
+
           ninja -C build \
                 clang-tablegen-targets \
                 genconfusable               # for "ConfusableIdentifierCheck.h"
 
-      - name: Run code linter
+      - name: Run code linters
         env:
           GITHUB_PR_NUMBER: ${{ github.event.pull_request.number }}
           CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }}
diff --git a/llvm/utils/git/code-lint-helper.py b/llvm/utils/git/code-lint-helper.py
index a53549fb69a77..aca88e2be5a8c 100644
--- a/llvm/utils/git/code-lint-helper.py
+++ b/llvm/utils/git/code-lint-helper.py
@@ -36,6 +36,7 @@ class LintArgs:
     issue_number: int = 0
     build_path: str = "build"
     clang_tidy_binary: str = "clang-tidy"
+    doc8_binary: str = "doc8"
 
     def __init__(self, args: argparse.Namespace) -> None:
         if args is not None:
@@ -50,6 +51,7 @@ def __init__(self, args: argparse.Namespace) -> None:
             self.verbose = args.verbose
             self.build_path = args.build_path
             self.clang_tidy_binary = args.clang_tidy_binary
+            self.doc8_binary = args.doc8_binary
 
 
 class LintHelper:
@@ -289,8 +291,59 @@ def _clean_clang_tidy_output(self, output: str) -> str:
         return ""
 
 
+class Doc8LintHelper(LintHelper):
+    name: Final = "doc8"
+    friendly_name: Final = "documentation linter"
 
-ALL_LINTERS = (ClangTidyLintHelper(),)
+    def instructions(self, files_to_lint: Sequence[str], args: LintArgs) -> str:
+        files_str = " ".join(files_to_lint)
+        return f"doc8 -q {files_str}"
+
+    def filter_changed_files(self, changed_files: Sequence[str]) -> Sequence[str]:
+        filtered_files = []
+        for filepath in changed_files:
+            _, ext = os.path.splitext(filepath)
+            if ext != ".rst":
+                continue
+            if not filepath.startswith("clang-tools-extra/docs/clang-tidy/"):
+                continue
+            if os.path.exists(filepath):
+                filtered_files.append(filepath)
+        return filtered_files
+
+    def run_linter_tool(self, files_to_lint: Sequence[str], args: LintArgs) -> str:
+        if not files_to_lint:
+            return ""
+
+        doc8_cmd = [args.doc8_binary, "-q"]
+        doc8_cmd.extend(files_to_lint)
+
+        if args.verbose:
+            print(f"Running doc8: {' '.join(doc8_cmd)}")
+
+        proc = subprocess.run(
+            doc8_cmd,
+            stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE,
+            text=True,
+            check=False,
+        )
+
+        if proc.returncode == 0:
+            return ""
+
+        output = proc.stdout.strip()
+        if output:
+            return output
+
+        error_output = proc.stderr.strip()
+        if error_output:
+            return error_output
+
+        return f"doc8 exited with return code {proc.returncode} but no output."
+
+
+ALL_LINTERS = (ClangTidyLintHelper(), Doc8LintHelper())
 
 
 if __name__ == "__main__":
@@ -331,6 +384,12 @@ def _clean_clang_tidy_output(self, output: str) -> str:
         default="clang-tidy",
         help="Path to clang-tidy binary",
     )
+    parser.add_argument(
+        "--doc8-binary",
+        type=str,
+        default="doc8",
+        help="Path to doc8 binary",
+    )
     parser.add_argument(
         "--verbose", action="store_true", default=True, help="Verbose output"
     )

[zeyi2]

I plan to test the following scenarios:

• doc8 and clang-tidy both succ
• doc8 and clang-tidy both fail
• doc8 fails, check whether the comments can be updated
• clang-tidy fails, check whether the comments can be updated

---

case 1: both no modified files:

running linter clang-tidy
got changed files: ['.github/workflows/containers/github-action-ci-tooling/Dockerfile', '.github/workflows/pr-code-lint.yml', 'llvm/utils/git/code-lint-helper.py']
no modified files found
running linter doc8
got changed files: ['.github/workflows/containers/github-action-ci-tooling/Dockerfile', '.github/workflows/pr-code-lint.yml', 'llvm/utils/git/code-lint-helper.py']
no modified files found

Working as expected.

---

case 2: Only doc8 fails

:warning: RST documentation linter, doc8 found issues in your code. :warning:

<details>
<summary>
You can test this locally with the following command:
</summary>

doc8 -q clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst

</details>

<details>
<summary>
View the output from doc8 here.
</summary>

clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst:6: D001 Line too long

comment update:

:white_check_mark: With the latest revision this PR passed the RST documentation linter.

working as expected.

---

case 3: Only clang-tidy fails

:warning: C/C++ code linter, clang-tidy found issues in your code. :warning:

<details>
<summary>
You can test this locally with the following command:
</summary>

git diff -U0 origin/main...HEAD -- clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp |
python3 clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py   -path build -p1 -quiet

</details>

<details>
<summary>
View the output from clang-tidy here.
</summary>

clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp:102:66: error: expected ';' after expression [clang-diagnostic-error]
  102 |                                        StringRef("[super init]"))
      |                                                                  ^
      |                                                                  ;

</details>

comment update:

:white_check_mark: With the latest revision this PR passed the RST documentation linter.

working as expected

---

case 4:

:warning: RST documentation linter, doc8 found issues in your code. :warning:

<details>
<summary>
You can test this locally with the following command:
</summary>

doc8 -q clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst

</details>

<details>
<summary>
View the output from doc8 here.
</summary>

clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst:6: D001 Line too long

<details>
<summary>
You can test this locally with the following command:
</summary>

git diff -U0 origin/main...HEAD -- clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp |
python3 clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py   -path build -p1 -quiet

</details>

<details>
<summary>
View the output from clang-tidy here.
</summary>

clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp:102:66: error: expected ';' after expression [clang-diagnostic-error]
  102 |                                        StringRef("[super init]"))
      |                                                                  ^
      |                                                                  ;

</details>

Updates are also working as expected.

[vbvictor]

Code looks good, I'll wait for test before final approve

[github-actions]

✅ With the latest revision this PR passed the RST documentation linter.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code linter.

[zeyi2]

I've finished testing the script modifications and I think it is working as expected.

The full log: #172123 (comment)

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[vbvictor]

One issue that I found with doc8 is it says "line too long", but doesn't say how long it should be.
If doc8 fails, can we give additional info to user that lines in documentation should be no more than 80 chars.

Also, can we add here https://clang.llvm.org/extra/clang-tidy/Contributing.html#documenting-your-check a couple of lines about doc linting with doc8.

[zeyi2]

can we add here https://clang.llvm.org/extra/clang-tidy/Contributing.html#documenting-your-check a couple of lines about doc linting with doc8.

It may require a lot of formatting that should not be a part of this PR. I'm planning to add it in #167269 then fix all the documentations in #170004

[vbvictor]

LGTM, but please wait for boomanaiden154's review.