Skip to content

feat(pre-commit): add internal-import alias check with auto-fix #49

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dengqiaoyu wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(pre-commit): add internal-import alias check with auto-fix #49

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,20 @@ repos:
types: [python]
exclude: ^tests/

- repo: local
hooks:
- id: check-internal-import-aliases
name: Check `_`-prefix aliasing of internal imports
description: |
Enforce code_style_guide.md §3.3: public modules must alias
symbols imported from private modules with a `_` prefix; private
modules must not. Auto-fixes the import statement and renames
references in the same file.
entry: python scripts/pre_commit/check_internal_import_aliases.py --fix

@crowbat crowbat Jul 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general it seems like there are a lot of potential corner cases associated with automatically fixing the code. What do you think about making --fix opt-in instead where the user manually runs it if they want to have things be automatically fixed?

We could have the pre-commit hook only do the detection part of it to fail the commit if it finds any transgressions. I'm thinking that if any automatic fixes are made which break the code, other pre-commit hooks may also have run (linting, line too long, etc.) which would conflate the changes if the user needs to revert the code back to before this hook took action.

language: system
types: [python]
exclude: ^tests/

- repo: local
hooks:
- id: towncrier-check
Expand Down
12 changes: 12 additions & 0 deletions docs/contributing/code_style_guide.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ This guide documents coding conventions and best practices for the Core AI Optim
- [Why this matters](#why-this-matters)
- [Best practice](#best-practice)
- [When to use this pattern](#when-to-use-this-pattern)
- [Enforcement](#enforcement)
- [3.4 Import Examples](#34-import-examples)
- [4. Type Annotations](#4-type-annotations)
- [4.1 When to Annotate](#41-when-to-annotate)
Expand Down Expand Up @@ -330,6 +331,17 @@ from proj_pkg.api import private_helper # ✗ AttributeError - not accessible

- When importing project-internal symbols into public modules to prevent unintended re-export

#### Enforcement

The pre-commit hook `check-internal-import-aliases` (defined in `.pre-commit-config.yaml`, implemented at `scripts/pre_commit/check_internal_import_aliases.py`) enforces this rule. It runs `--fix` on every commit and:

- Adds the missing `_` alias in public modules.
- Removes the unnecessary `_` alias in private modules.
- Renames every reference to the bound name in the same file so the rewrite leaves the module compiling.
- Reports a violation and asks for a manual fix when the bound name is shadowed inside a function scope (rare).

You can run it manually with `pre-commit run check-internal-import-aliases --all-files` or directly via `python scripts/pre_commit/check_internal_import_aliases.py --fix`.

### 3.4 Import Examples

**Importing from Public API:**
Expand Down
Loading