Skip to content

[codex] extract Bazel build file parsing utility#128

Merged
hofbi merged 1 commit into
hofbi:masterfrom
lukasoyen:codex/add-forbidden-tag-hook
Apr 30, 2026
Merged

[codex] extract Bazel build file parsing utility#128
hofbi merged 1 commit into
hofbi:masterfrom
lukasoyen:codex/add-forbidden-tag-hook

Conversation

@lukasoyen
Copy link
Copy Markdown

@lukasoyen lukasoyen commented Apr 30, 2026

Summary

Extracts shared Bazel BUILD file parsing helpers into dev_tools.build_file_parsing_util.

This moves rule-call parsing and tag detection out of check_rule_has_tag.py, keeps the existing hook behavior unchanged, and adds dedicated utility tests.

Validation

  • uv run --extra dev pytest tests/test_build_file_parsing_util.py tests/test_check_rule_has_tag.py
  • uv run --with ruff ruff check dev_tools/build_file_parsing_util.py dev_tools/check_rule_has_tag.py tests/test_build_file_parsing_util.py tests/test_check_rule_has_tag.py
  • uv run --with ruff ruff format --check dev_tools/build_file_parsing_util.py dev_tools/check_rule_has_tag.py tests/test_build_file_parsing_util.py tests/test_check_rule_has_tag.py
  • uv run --with ty ty check dev_tools/build_file_parsing_util.py dev_tools/check_rule_has_tag.py tests/test_build_file_parsing_util.py tests/test_check_rule_has_tag.py

@lukasoyen lukasoyen changed the title [codex] add forbidden tag hook Add forbidden tag hook Apr 30, 2026
@lukasoyen lukasoyen marked this pull request as ready for review April 30, 2026 09:49
@lukasoyen lukasoyen force-pushed the codex/add-forbidden-tag-hook branch from 69d21f1 to 0a15164 Compare April 30, 2026 09:50
Comment thread dev_tools/forbidden_tag.py Outdated
Comment thread dev_tools/forbidden_tag.py Outdated
@hofbi
Copy link
Copy Markdown
Owner

hofbi commented Apr 30, 2026

Thanks for the cleanup. Could you split this up in 2 PRs. A first one where we extract the common logic + tests into the utils file and utils tests. We get that merged first. Then a second PR adding the new hook. This makes it easier to review.

@lukasoyen lukasoyen force-pushed the codex/add-forbidden-tag-hook branch 2 times, most recently from 8a81d91 to 3a15c77 Compare April 30, 2026 10:25
@lukasoyen lukasoyen changed the title Add forbidden tag hook [codex] extract Bazel build file parsing utility Apr 30, 2026
@lukasoyen
Copy link
Copy Markdown
Author

Done. I split this so PR #128 now only extracts the shared Bazel BUILD file parsing/tag helpers into dev_tools/build_file_parsing_util.py and adds dedicated utility tests. The new hook work is kept separately on a follow-up branch for a second PR after this one lands.

@lukasoyen
Copy link
Copy Markdown
Author

Created the stacked follow-up PR for the new hook here: lukasoyen#1

It is based on the utility branch from this PR and contains only the forbidden-tag hook commit.

Comment thread dev_tools/check_rule_has_tag.py Outdated
import sys
from typing import TYPE_CHECKING

from dev_tools.build_file_parsing_util import find_rule_calls as find_build_file_rule_calls
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
from dev_tools.build_file_parsing_util import find_rule_calls as find_build_file_rule_calls
from dev_tools.build_file_parsing_util import find_rule_calls

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Oh now I get it, the impl slighly changed and we have the function still here. Can you explain?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Previously the find_rule_calls() got the rule kind to inject into the regex. With the new helper function that is no longer the case as the forbidden tag use case needs to be able to glob on the rule kind.

So now the regex matches all targets and the function needs to filter down to the rule kinds it is interested in for check_rule_has_tag.py. Also the return type is enhanced because the forbidden tags use case needs to get the rule kind and the target body.

We want to have the filtering in the helper find_rule_calls() because we can do that before we try to do the expensive parenthesis matching.

Comment thread dev_tools/check_rule_has_tag.py Outdated
Comment on lines +6 to +7
from dev_tools.build_file_parsing_util import find_rule_calls as find_build_file_rule_calls
from dev_tools.build_file_parsing_util import rule_has_tag
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Since we are moving these two, we should also move their tests in https://github.com/hofbi/dev-tools/blob/master/tests/test_check_rule_has_tag.py instead of creating new ones.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done in 761899f: moved the parser/tag tests out of test_check_rule_has_tag.py so they now live in tests/test_build_file_parsing_util.py, and left test_check_rule_has_tag.py focused on the hook-level file checks.

@lukasoyen lukasoyen force-pushed the codex/add-forbidden-tag-hook branch from 3a15c77 to 761899f Compare April 30, 2026 11:32
@hofbi hofbi merged commit a3c5147 into hofbi:master Apr 30, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants