Skip to content

Commit 88fd457

Browse files
authored
[bc-linter] Add BC Linter configuration support (#7016)
This PR makes bc-linter configurable (via .bc-linter.yml, [see the spec](https://github.com/pytorch/test-infra/blob/e73b14f75422a0fb38212c2d465f9a86fb20a11c/tools/stronghold/docs/bc_linter_config.md)). #### Changes: - added api.config with yaml loader - config status is logged (and loaded config is printed with --verbose) - simplified default excludes to [".", "./", "/./", "/."] (removed pytorch-specific paths, added them back [via config file](pytorch/pytorch#161319)) - for globs used pathspec (gitwildmatch) #### Action + Dependencies - added runtime deps file tools/stronghold/requirements.runtime.txt (PyYAML, pathspec). - composite action now installs runtime deps before build/run. #### Tests - Added tests for glob semantics, loader behaviors, defaults, scan flags, annotations, and top‑level vs nested paths. Backtesting [new config](pytorch/pytorch#161319) against paths in PyTorch codebase: ``` - Total .py files: 13820 - Old allowed (a07e5d9 rules): 4926 - New allowed: 4926 - Diffs: 0 (perfect match) ``` Testing action end-to-end close to prod: pytorch/pytorch#161325 see [this run](https://github.com/pytorch/pytorch/actions/runs/17168177594/job/48713225885?pr=161325#step:2:6155), correctly returns a single warning (job failure is expected).
1 parent 93dacae commit 88fd457

File tree

15 files changed

+1067
-80
lines changed

15 files changed

+1067
-80
lines changed

.github/actions/bc-lint/action.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ runs:
6161
shell: bash
6262
run: |
6363
set -eux
64+
python3 -m pip install -r ../_test-infra/tools/stronghold/requirements.runtime.txt
6465
../_test-infra/tools/stronghold/bin/build-check-api-compatibility
6566
../_test-infra/tools/stronghold/bin/check-api-compatibility \
6667
--base-commit=${{ inputs.base_sha }} \
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
name: BC-Linter Tests
2+
3+
on:
4+
pull_request:
5+
paths:
6+
- .github/workflows/bc-linter-tests.yml
7+
- tools/stronghold/**
8+
push:
9+
branches:
10+
- main
11+
paths:
12+
- .github/workflows/bc-linter-tests.yml
13+
- tools/stronghold/**
14+
15+
defaults:
16+
run:
17+
working-directory: tools/stronghold/
18+
19+
jobs:
20+
bc-linter-tests:
21+
name: BC-Linter Tests
22+
runs-on: ubuntu-latest
23+
steps:
24+
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
25+
- uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0
26+
with:
27+
python-version: '3.11'
28+
cache: pip
29+
- name: Install dependencies
30+
run: pip install -r requirements.txt
31+
- name: Run BC-Linter tests
32+
run: python -m pytest tests/ -v
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
# BC Linter Configuration (beta)
2+
3+
This document describes the configuration format for the Stronghold BC linter.
4+
The config enables repo‑specific path selection, rule suppression, and custom
5+
annotations to include/exclude specific APIs.
6+
7+
### Config file location
8+
- Place a YAML file named `.bc-linter.yml` at the repository root being linted
9+
(the target repo).
10+
- If the file is missing or empty, defaults are applied (see below).
11+
12+
### Schema (YAML)
13+
```yml
14+
version: 1
15+
16+
paths:
17+
include:
18+
- "**/*.py" # globs of files to consider (default)
19+
exclude:
20+
- "**/.*/**" # exclude hidden directories by default
21+
- "**/.*" # exclude hidden files by default
22+
23+
scan:
24+
functions: true # check free functions and methods
25+
classes: true # check classes/dataclasses
26+
public_only: true # ignore names starting with "_" at any level
27+
28+
annotations:
29+
include: # decorators that force‑include a symbol
30+
- name: "bc_linter_include" # matched by simple name or dotted suffix
31+
propagate_to_members: false # for classes, include methods/inner classes
32+
exclude: # decorators that force‑exclude a symbol
33+
- name: "bc_linter_skip" # matched by simple name or dotted suffix
34+
propagate_to_members: true # for classes, exclude methods/inner classes
35+
36+
excluded_violations: [] # e.g. ["ParameterRenamed", "FieldTypeChanged"]
37+
```
38+
39+
### Behavior notes
40+
- Regardless of the config, ONLY `.py` files are considered.
41+
- Paths precedence: `annotations.exclude` > `annotations.include` > `paths`.
42+
Annotations can override file include/exclude rules.
43+
- Name matching for annotations: A decorator matches if either its simple name
44+
equals the configured `name` (e.g., `@bc_linter_skip`) or if its dotted
45+
attribute ends with the configured `name` (e.g., `@proj.bc_linter_skip`).
46+
- `public_only`: When true, any symbol whose qualified name contains a component
47+
that starts with `_` is ignored (e.g., `module._Internal.func`, `Class._m`).
48+
- Rule suppression: `excluded_violations` contains class names from
49+
`api.violations` to omit from output (e.g., `FieldTypeChanged`).
50+
- Invariants not affected by config:
51+
- Deleted methods of a deleted class are not double‑reported (only the class).
52+
- Nested class deletions collapse to the outermost deleted class.
53+
- Dataclass detection and field inference are unchanged.
54+
55+
### Defaults
56+
If `.bc-linter.yml` is missing or empty, the following defaults apply:
57+
58+
```
59+
version: 1
60+
paths:
61+
include: ["**/*.py"]
62+
exclude: [".*", ".*/**", ".*/**/*", "**/.*/**", "**/.*"]
63+
scan:
64+
functions: true
65+
classes: true
66+
public_only: true
67+
annotations:
68+
include: []
69+
exclude: []
70+
excluded_violations: []
71+
```
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
PyYAML==6.0.2
2+
pathspec==0.12.1

tools/stronghold/requirements.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,5 @@ flake8==5.0.4
33
mypy==0.990
44
pip==23.3
55
pytest==7.2.0
6+
PyYAML==6.0.2
7+
pathspec==0.12.1

tools/stronghold/src/api/__init__.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ class Parameters:
2222
variadic_kwargs: bool
2323
# The line where the function is defined.
2424
line: int
25+
# Decorator names applied to this function/method (simple or dotted form).
26+
decorators: Sequence[str] = ()
2527

2628

2729
@dataclasses.dataclass
@@ -62,6 +64,8 @@ class Class:
6264
fields: Sequence[Field]
6365
line: int
6466
dataclass: bool = False
67+
# Decorator names applied to the class (simple or dotted form).
68+
decorators: Sequence[str] = ()
6569

6670

6771
@dataclasses.dataclass

tools/stronghold/src/api/ast.py

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,11 +80,15 @@ def _function_def_to_parameters(node: ast.FunctionDef) -> api.Parameters:
8080
)
8181
for i, arg in enumerate(args.kwonlyargs)
8282
]
83+
dec_names = tuple(
84+
n for n in (_decorator_to_name(d) for d in node.decorator_list) if n
85+
)
8386
return api.Parameters(
8487
parameters=params,
8588
variadic_args=args.vararg is not None,
8689
variadic_kwargs=args.kwarg is not None,
8790
line=node.lineno,
91+
decorators=dec_names,
8892
)
8993

9094

@@ -106,10 +110,11 @@ def visit_ClassDef(self, node: ast.ClassDef) -> None:
106110
# class name pushed onto a new context.
107111
if self._classes is not None:
108112
name = ".".join(self._context + [node.name])
113+
dec_names = [
114+
n for n in (_decorator_to_name(d) for d in node.decorator_list) if n
115+
]
109116
is_dataclass = any(
110-
(isinstance(dec, ast.Name) and dec.id == "dataclass")
111-
or (isinstance(dec, ast.Attribute) and dec.attr == "dataclass")
112-
for dec in node.decorator_list
117+
(n == "dataclass" or n.endswith(".dataclass")) for n in dec_names
113118
)
114119
fields: list[api.Field] = []
115120
for stmt in node.body:
@@ -180,7 +185,10 @@ def _is_field_func(f: ast.AST) -> bool:
180185
)
181186
)
182187
self._classes[name] = api.Class(
183-
fields=fields, line=node.lineno, dataclass=is_dataclass
188+
fields=fields,
189+
line=node.lineno,
190+
dataclass=is_dataclass,
191+
decorators=tuple(dec_names),
184192
)
185193

186194
_ContextualNodeVisitor(
@@ -191,3 +199,39 @@ def visit_FunctionDef(self, node: ast.FunctionDef) -> None:
191199
# Records this function.
192200
name = ".".join(self._context + [node.name])
193201
self._functions[name] = node
202+
203+
def visit_AsyncFunctionDef(self, node: ast.AsyncFunctionDef) -> None:
204+
# Treat async functions similarly by normalizing to FunctionDef shape.
205+
name = ".".join(self._context + [node.name])
206+
fnode = ast.FunctionDef(
207+
name=node.name,
208+
args=node.args,
209+
body=node.body,
210+
decorator_list=node.decorator_list,
211+
returns=node.returns,
212+
type_comment=node.type_comment,
213+
)
214+
self._functions[name] = fnode
215+
216+
217+
def _decorator_to_name(expr: ast.AST) -> str:
218+
"""Extracts a dotted name for a decorator expression.
219+
For calls like @dec(...), returns the callee name "dec".
220+
For attributes like @pkg.mod.dec, returns "pkg.mod.dec".
221+
For names like @dec, returns "dec".
222+
"""
223+
224+
def _attr_chain(a: ast.AST) -> str | None:
225+
if isinstance(a, ast.Name):
226+
return a.id
227+
if isinstance(a, ast.Attribute):
228+
left = _attr_chain(a.value)
229+
if left is None:
230+
return a.attr
231+
return f"{left}.{a.attr}"
232+
return None
233+
234+
if isinstance(expr, ast.Call):
235+
expr = expr.func
236+
name = _attr_chain(expr)
237+
return name or ""

tools/stronghold/src/api/checker.py

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
"""Runs the API backward compatibility on the head commit."""
22

33
import argparse
4+
import dataclasses
5+
import json
46
import pathlib
57
import subprocess
68
import sys
79

810
import api.compatibility
11+
import api.config
912
import api.git
1013
import api.github
1114

@@ -22,6 +25,13 @@ def run() -> None:
2225
help="Failures are suppressed"
2326
"(alternative to #suppress-api-compatibility-check commit message tag).",
2427
)
28+
parser.add_argument(
29+
"--verbose",
30+
default=False,
31+
required=False,
32+
action="store_true",
33+
help="Enable verbose output",
34+
)
2535
args = parser.parse_args(sys.argv[1:])
2636

2737
repo = api.git.Repository(pathlib.Path("."))
@@ -35,8 +45,21 @@ def run() -> None:
3545
repo.run(["fetch", "origin", args.base_commit], check=True)
3646
print("::endgroup::")
3747

48+
# Load config and optionally print when detected
49+
cfg, cfg_status = api.config.load_config_with_status(repo.dir)
50+
if cfg_status == "parsed":
51+
# Explicitly log successful config discovery and parsing
52+
print("BC-linter: Using .bc-linter.yml (parsed successfully)")
53+
if args.verbose:
54+
print("BC-linter: Parsed config:")
55+
print(json.dumps(dataclasses.asdict(cfg), indent=2, sort_keys=True))
56+
elif args.verbose:
57+
# In verbose mode, also indicate fallback to defaults
58+
reason = "missing" if cfg_status == "default_missing" else "invalid/malformed"
59+
print(f"BC-linter: No usable config ({reason}); using defaults")
60+
3861
violations = api.compatibility.check_range(
39-
repo, head=args.head_commit, base=args.base_commit
62+
repo, head=args.head_commit, base=args.base_commit, config=cfg
4063
)
4164
if len(violations) == 0:
4265
return

0 commit comments

Comments
 (0)