[Lint]Add lint hooks for clang-format, shellcheck, forbidden imports, and boolean context manager checks (#7511)
### What this PR does / why we need it?
This PR introduces several upstream `vllm`-aligned lint hooks into
`vllm-ascend` and makes them part of the actual `pre-commit` flow.
Main changes in this PR:
- add `check-boolean-context-manager` to catch boolean expressions in
`with` statements
- add `check-forbidden-imports` to forbid direct `re` imports and
disallowed direct `triton` imports
- enable shell script linting through `tools/shellcheck.sh`
- add root `.clang-format` aligned with upstream `vllm`, enable
`clang-format` in `pre-commit`, temporarily **exclude all `csrc/**`**
from `clang-format` to avoid bringing a large native code reformat into
this PR
This PR focuses on landing the smaller and immediately useful lint
alignment first, without mixing in the larger requirements-management
migration.
### Does this PR introduce _any_ user-facing change?
No.
This PR only updates repository lint configuration, static checks, and
internal import/style enforcement. It does not change runtime behavior
or public interfaces.
### How was this patch tested?
Tested locally in the project virtual environment.
Commands used:
```bash
bash format.sh
```
Verified checks passed:
``` bash
ruff check...............................................................Passed
ruff format..............................................................Passed
codespell................................................................Passed
typos....................................................................Passed
clang-format.............................................................Passed
Lint GitHub Actions workflow files.......................................Passed
Lint shell scripts.......................................................Passed
Lint PNG exports from excalidraw.........................................Passed
Check for spaces in all filenames........................................Passed
Enforce __init__.py in Python packages...................................Passed
Check for forbidden imports..............................................Passed
Check for boolean ops in with-statements.................................Passed
Suggestion...............................................................Passed
- hook id: suggestion
- duration: 0s
To bypass pre-commit hooks, add --no-verify to git commit.
```
**note:**
clang-format is enabled but currently excludes all csrc/**
- vLLM version: v0.17.0
- vLLM main:
8b6325758c
---------
Signed-off-by: MrZ20 <2609716663@qq.com>
This commit is contained in:
SILONG ZENG
@@ -18,7 +18,6 @@ import hashlib
|
||||
import json
|
||||
import logging
|
||||
import os
|
||||
import re
|
||||
import subprocess
|
||||
import tempfile
|
||||
from pathlib import Path
|
||||
@@ -26,6 +25,7 @@ from pathlib import Path
|
||||
import filelock
|
||||
import huggingface_hub
|
||||
import pandas as pd
|
||||
import regex as re
|
||||
from modelscope import snapshot_download # type: ignore
|
||||
|
||||
BENCHMARK_HOME = os.getenv("BENCHMARK_HOME", os.path.abspath("./benchmark"))
|
||||
|
||||
87
tools/check_boolean_context_manager.py
Normal file
87
tools/check_boolean_context_manager.py
Normal file
@@ -0,0 +1,87 @@
|
||||
#!/usr/bin/env python3
|
||||
#
|
||||
# Copyright (c) 2026 Huawei Technologies Co., Ltd. All Rights Reserved.
|
||||
# Copyright 2023 The vLLM team.
|
||||
#
|
||||
# Licensed under the Apache License, Version 2.0 (the "License");
|
||||
# you may not use this file except in compliance with the License.
|
||||
# You may obtain a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS,
|
||||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
# See the License for the specific language governing permissions and
|
||||
# limitations under the License.
|
||||
# This file is a part of the vllm-ascend project.
|
||||
# Adapted from https://github.com/vllm-project/vllm/tree/main/tools
|
||||
#
|
||||
"""Lint: detect `with a() and b():` (boolean op in with-statement context).
|
||||
|
||||
Using `and`/`or` to combine context managers is almost always a bug:
|
||||
|
||||
with ctx_a() and ctx_b(): # BUG: only ctx_b is entered
|
||||
with ctx_a() or ctx_b(): # BUG: only ctx_a is entered
|
||||
|
||||
The correct way to combine context managers is:
|
||||
|
||||
with ctx_a(), ctx_b(): # comma-separated
|
||||
with (ctx_a(), ctx_b()): # parenthesized (Python 3.10+)
|
||||
with contextlib.ExitStack() ... # ExitStack
|
||||
"""
|
||||
|
||||
import ast
|
||||
import sys
|
||||
|
||||
|
||||
def check_file(filepath: str) -> list[str]:
|
||||
try:
|
||||
with open(filepath, encoding="utf-8") as f:
|
||||
source = f.read()
|
||||
except (OSError, UnicodeDecodeError):
|
||||
return []
|
||||
|
||||
try:
|
||||
tree = ast.parse(source, filename=filepath)
|
||||
except SyntaxError:
|
||||
return []
|
||||
|
||||
violations = []
|
||||
for node in ast.walk(tree):
|
||||
if isinstance(node, (ast.With, ast.AsyncWith)):
|
||||
for item in node.items:
|
||||
if isinstance(item.context_expr, ast.BoolOp):
|
||||
op = "and" if isinstance(item.context_expr.op, ast.And) else "or"
|
||||
violations.append(
|
||||
f"{filepath}:{item.context_expr.lineno}: "
|
||||
f"boolean `{op}` used to combine context managers "
|
||||
"in `with` statement; use a comma instead"
|
||||
)
|
||||
return violations
|
||||
|
||||
|
||||
def main() -> int:
|
||||
if len(sys.argv) < 2:
|
||||
print("Usage: check_boolean_context_manager.py <file> ...", file=sys.stderr)
|
||||
return 1
|
||||
|
||||
all_violations = []
|
||||
for filepath in sys.argv[1:]:
|
||||
all_violations.extend(check_file(filepath))
|
||||
|
||||
if all_violations:
|
||||
print(
|
||||
"Boolean operator used to combine context managers in a `with` "
|
||||
"statement.\n"
|
||||
"Use `with a(), b():` or `with (a(), b()):` instead.\n"
|
||||
)
|
||||
for violation in all_violations:
|
||||
print(f" {violation}")
|
||||
return 1
|
||||
|
||||
return 0
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main())
|
||||
96
tools/check_forbidden_imports.py
Normal file
96
tools/check_forbidden_imports.py
Normal file
@@ -0,0 +1,96 @@
|
||||
#!/usr/bin/env python3
|
||||
#
|
||||
# Copyright (c) 2026 Huawei Technologies Co., Ltd. All Rights Reserved.
|
||||
# Copyright 2023 The vLLM team.
|
||||
#
|
||||
# Licensed under the Apache License, Version 2.0 (the "License");
|
||||
# you may not use this file except in compliance with the License.
|
||||
# You may obtain a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS,
|
||||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
# See the License for the specific language governing permissions and
|
||||
# limitations under the License.
|
||||
# This file is a part of the vllm-ascend project.
|
||||
# Adapted from https://github.com/vllm-project/vllm/tree/main/tools
|
||||
#
|
||||
|
||||
import sys
|
||||
from dataclasses import dataclass, field
|
||||
|
||||
import regex as re
|
||||
|
||||
|
||||
@dataclass
|
||||
class ForbiddenImport:
|
||||
pattern: str
|
||||
tip: str
|
||||
allowed_pattern: re.Pattern = re.compile(r"^$")
|
||||
allowed_files: set[str] = field(default_factory=set)
|
||||
|
||||
|
||||
CHECK_IMPORTS = {
|
||||
"pickle/cloudpickle": ForbiddenImport(
|
||||
pattern=(
|
||||
r"^\s*(import\s+(pickle|cloudpickle)(\s|$|\sas)"
|
||||
r"|from\s+(pickle|cloudpickle)\s+import\b)"
|
||||
),
|
||||
tip=("Avoid using pickle or cloudpickle or add this file to tools/check_forbidden_imports.py."),
|
||||
allowed_files={
|
||||
"vllm_ascend/distributed/kv_transfer/kv_pool/cpu_offload/metadata.py",
|
||||
},
|
||||
),
|
||||
"re": ForbiddenImport(
|
||||
pattern=r"^\s*(?:import\s+re(?:$|\s|,)|from\s+re\s+import)",
|
||||
tip="Replace 'import re' with 'import regex as re' or 'import regex'.",
|
||||
allowed_pattern=re.compile(r"^\s*import\s+regex(\s*|\s+as\s+re\s*)$"),
|
||||
),
|
||||
"triton": ForbiddenImport(
|
||||
pattern=r"^(from|import)\s+triton(\s|\.|$)",
|
||||
tip=("Use 'from vllm.triton_utils import triton'/'tl'."),
|
||||
allowed_pattern=re.compile(
|
||||
r"^\s*import\s+triton\.language\.extra\.cann\.extension\s+as\s+_extension_module(\s+#.*)?$"
|
||||
),
|
||||
),
|
||||
}
|
||||
|
||||
|
||||
def check_file(path: str) -> int:
|
||||
try:
|
||||
with open(path, encoding="utf-8") as f:
|
||||
content = f.read()
|
||||
except (OSError, UnicodeDecodeError):
|
||||
return []
|
||||
|
||||
return_code = 0
|
||||
for import_name, forbidden_import in CHECK_IMPORTS.items():
|
||||
if path in forbidden_import.allowed_files:
|
||||
continue
|
||||
|
||||
for match in re.finditer(forbidden_import.pattern, content, re.MULTILINE):
|
||||
if forbidden_import.allowed_pattern.match(match.group()):
|
||||
continue
|
||||
|
||||
line_num = content[: match.start() + 1].count("\n") + 1
|
||||
print(
|
||||
f"{path}:{line_num}: "
|
||||
"\033[91merror:\033[0m "
|
||||
f"Found forbidden import: {import_name}. {forbidden_import.tip}"
|
||||
)
|
||||
return_code = 1
|
||||
|
||||
return return_code
|
||||
|
||||
|
||||
def main() -> int:
|
||||
return_code = 0
|
||||
for path in sys.argv[1:]:
|
||||
return_code |= check_file(path)
|
||||
return return_code
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main())
|
||||
@@ -1,96 +0,0 @@
|
||||
#
|
||||
# Copyright (c) 2025 Huawei Technologies Co., Ltd. All Rights Reserved.
|
||||
# Copyright 2023 The vLLM team.
|
||||
#
|
||||
# Licensed under the Apache License, Version 2.0 (the "License");
|
||||
# you may not use this file except in compliance with the License.
|
||||
# You may obtain a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS,
|
||||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
# See the License for the specific language governing permissions and
|
||||
# limitations under the License.
|
||||
# This file is a part of the vllm-ascend project.
|
||||
# Adapted from https://github.com/vllm-project/vllm/tree/main/tools
|
||||
#
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
import regex as re
|
||||
|
||||
FORBIDDEN_PATTERNS = re.compile(r"^\s*(?:import\s+re(?:$|\s|,)|from\s+re\s+import)")
|
||||
ALLOWED_PATTERNS = [
|
||||
re.compile(r"^\s*import\s+regex\s+as\s+re\s*$"),
|
||||
re.compile(r"^\s*import\s+regex\s*$"),
|
||||
]
|
||||
|
||||
|
||||
def get_staged_python_files() -> list[str]:
|
||||
try:
|
||||
result = subprocess.run(
|
||||
["git", "diff", "--cached", "--name-only", "--diff-filter=AM"], capture_output=True, text=True, check=True
|
||||
)
|
||||
files = result.stdout.strip().split("\n") if result.stdout.strip() else []
|
||||
return [f for f in files if f.endswith(".py")]
|
||||
except subprocess.CalledProcessError:
|
||||
return []
|
||||
|
||||
|
||||
def is_forbidden_import(line: str) -> bool:
|
||||
line = line.strip()
|
||||
return bool(FORBIDDEN_PATTERNS.match(line) and not any(pattern.match(line) for pattern in ALLOWED_PATTERNS))
|
||||
|
||||
|
||||
def check_file(filepath: str) -> list[tuple[int, str]]:
|
||||
violations = []
|
||||
try:
|
||||
with open(filepath, encoding="utf-8") as f:
|
||||
for line_num, line in enumerate(f, 1):
|
||||
if is_forbidden_import(line):
|
||||
violations.append((line_num, line.strip()))
|
||||
except (OSError, UnicodeDecodeError):
|
||||
pass
|
||||
return violations
|
||||
|
||||
|
||||
def main() -> int:
|
||||
files = get_staged_python_files()
|
||||
if not files:
|
||||
return 0
|
||||
|
||||
total_violations = 0
|
||||
|
||||
for filepath in files:
|
||||
if not Path(filepath).exists():
|
||||
continue
|
||||
|
||||
if filepath == "setup.py":
|
||||
continue
|
||||
|
||||
violations = check_file(filepath)
|
||||
if violations:
|
||||
print(f"\n❌ {filepath}:")
|
||||
for line_num, line in violations:
|
||||
print(f" Line {line_num}: {line}")
|
||||
total_violations += 1
|
||||
|
||||
if total_violations > 0:
|
||||
print(f"\n💡 Found {total_violations} violation(s).")
|
||||
print("❌ Please replace 'import re' with 'import regex as re'")
|
||||
print(" Also replace 'from re import ...' with 'from regex import ...'") # noqa: E501
|
||||
print("✅ Allowed imports:")
|
||||
print(" - import regex as re")
|
||||
print(" - import regex") # noqa: E501
|
||||
return 1
|
||||
|
||||
return 0
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
raise SystemExit(main())
|
||||
@@ -16,10 +16,11 @@
|
||||
# Adapted from https://github.com/vllm-project/vllm/tree/main/tools
|
||||
#
|
||||
import argparse
|
||||
import re
|
||||
import sys
|
||||
from datetime import datetime
|
||||
|
||||
import regex as re
|
||||
|
||||
p = re.compile(r"@(?P<user>[A-Za-z0-9-_]+)[^\`]*\`(?P<sha>[0-9a-fA-F]+)\`\s*[-–—]\s*(?P<date>.+)$")
|
||||
|
||||
|
||||
|
||||
@@ -19,13 +19,12 @@
|
||||
# Adapted from https://github.com/vllm-project/vllm/tree/main/tools
|
||||
#
|
||||
|
||||
set -e
|
||||
set -euo pipefail
|
||||
|
||||
scversion="stable"
|
||||
|
||||
if [ -d "shellcheck-${scversion}" ]; then
|
||||
PATH="$PATH:$(pwd)/shellcheck-${scversion}"
|
||||
export PATH
|
||||
export PATH="$PATH:$(pwd)/shellcheck-${scversion}"
|
||||
fi
|
||||
|
||||
if ! [ -x "$(command -v shellcheck)" ]; then
|
||||
@@ -34,12 +33,9 @@ if ! [ -x "$(command -v shellcheck)" ]; then
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# automatic local install if linux x86_64
|
||||
wget -qO- "https://github.com/koalaman/shellcheck/releases/download/${scversion?}/shellcheck-${scversion?}.linux.x86_64.tar.xz" | tar -xJv
|
||||
PATH="$PATH:$(pwd)/shellcheck-${scversion}"
|
||||
export PATH
|
||||
export PATH="$PATH:$(pwd)/shellcheck-${scversion}"
|
||||
fi
|
||||
|
||||
# should enable this
|
||||
# find . -path ./.git -prune -o -name "*.sh" -print0 \
|
||||
# | xargs -0 -I {} sh -c 'git check-ignore -q "{}" || shellcheck -s bash "{}"'
|
||||
find . -path ./.git -prune -o -name "*.sh" -print0 | \
|
||||
xargs -0 sh -c "for f in \"\$@\"; do git check-ignore -q \"\$f\" || shellcheck -s bash \"\$f\"; done" --
|
||||
|
||||
Reference in New Issue
Block a user