Repository navigation
feat(lint): add buildifier linting for Starlark files - #1255
Closed
LittleHuba wants to merge 11 commits into
Closed
LittleHuba wants to merge 11 commits into
LittleHuba wants to merge 11 commits into
Conversation
Add //:buildifier.check and //:buildifier.fix, which accept Bazel-style package patterns (including `-` exclusions) like the other linters, and a gating //:buildifier_test. A dedicated CI job converts the findings to SARIF (//tools/lint:buildifier_sarif), uploads them to Code Scanning and fails on findings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
|
||
| import io | ||
| import json | ||
| import unittest |
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
With --format=json buildifier exits 0 and flags syntax errors (valid=false) and unformatted files (formatted=false) without any warning entry, so they were missing from the SARIF output and could not fail the job. Report them as syntax-error/reformat results and treat all findings as errors, as buildifier has no severity levels. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Result of `bazel run //:buildifier.fix`: formatting and auto-fixable lint findings (explicit loads of native rules, unused and unsorted loads). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ing) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…cstring) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…n-docstring-args) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Prefix the required but unused parameters of the transition and module extension implementations with an underscore. rust_unit_test accepted target_compatible_with but silently ignored it; forward it to the public test like cc_unit_test does. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… external-path) The coverage aspect deliberately filters out files from external repositories by their path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…nonical-repository) - coverage_scope: detect main-repository labels via Label.repo_name instead of matching the canonical string prefix. - message_passing and score_baselibs_stub: derive the canonical unit scope with package_relative_label instead of hardcoding "@@..." names. The resulting scope strings are unchanged. - rustfmt wrapper: rename the template placeholders so they no longer look like canonical labels. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
LittleHuba
force-pushed
the
spike/buildifier-rules-lint
branch
from
October 2, 2026 18:00
b3bac50 to
d5ae1b8
Compare
LittleHuba
commented
Oct 2, 2026
Contributor
Author
There was a problem hiding this comment.
@hoe-jo this should probably be done by rules_score itself.
I would consider this only a temporary fix here until this is fixed upstream.
LittleHuba
marked this pull request as ready for review
October 2, 2026 18:03
LittleHuba
requested review from
bemerybmw,
castler,
crimson11,
hoe-jo and
limdor
as code owners
October 2, 2026 18:03
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
limdor
requested changes
Oct 5, 2026
limdor
left a comment
limdor
left a comment
Contributor
There was a problem hiding this comment.
This is quite a lot of different changes in different parts of the code. To get the attention that it deserve, I would like we split at least the PR in 2, one for fixing the issues found by buildifier, and one for adding the job and the scripts for the run.
This was referenced Oct 5, 2026
Contributor
Author
NemanjaTrifunovicRTRK
pushed a commit
to NemanjaTrifunovicRTRK/communication
that referenced
this pull request
Oct 5, 2026
Mechanical buildifier autofixes (formatting, unused/sorted loads, including unused rules_score loads). No functional change. Split out of eclipse-score#1255.
etas-contrib-automation Bot
pushed a commit
to etas-contrib/score_communication
that referenced
this pull request
Oct 5, 2026
…ipse-score#1273) Adds missing module/function/argument docstrings and moves load statements below the license header. Stacked on the autofix PR (base: split/2-buildifier-autofixes); retarget to main after it merges. Split out of eclipse-score#1255.
etas-contrib-automation Bot
pushed a commit
to etas-contrib/score_communication
that referenced
this pull request
Oct 5, 2026
…score#1274) Replaces hardcoded canonical labels (`@@//...`, `@@score_baselibs+//...`) in rules_score unit scopes with apparent names (buildifier `canonical-repository`). Split out of eclipse-score#1255.
prabakaranklst
pushed a commit
to qorix-group/communication
that referenced
this pull request
Oct 21, 2026
Resolves unused variables, documents intentional external-path findings, and removes hardcoded canonical names in coverage_scope and the rustfmt wrapper. Split out of eclipse-score#1255.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds buildifier linting for all Starlark files without any per-BUILD wiring.
bazel run //:buildifier.check|fix -- [patterns]: same entry points asruff/clang-tidy; accepts//pkg,//pkg:target,//pkg/...and-exclusions (default//...). Resolved bytools/lint/buildifier_files.py(unit-tested).//:buildifier_test: gating test, part ofbazel test //....//tools/lint:buildifier_sarif: converts findings to SARIF and exits non-zero on findings.buildifierCI job: uploads SARIF to Code Scanning (categorybuildifier) and as an artifact, then fails on findings.quality/quality.md,CI.md, release template.Known limitations
No
--config=buildifierand no hold-the-line (buildifier is not run through a rules_lint aspect, which cannot cover BUILD files).Note
Existing findings (~124) are intentionally not fixed in this PR, so the failing path of the CI job (SARIF upload + job failure) can be verified. Fixes will follow in a separate change before this is enforced.