-
-
Notifications
You must be signed in to change notification settings - Fork 8.6k
[build] add smart targeting and --lint flag to format.sh #17035
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.
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
Merged
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
ecc4373
[build] add smart targeting and --lint flag to format.sh
titusfortner b6a7e87
[build] include staged changes for pre-commit support
titusfortner ebcc309
[build] add --pre-commit and --pre-push flags to format.sh
titusfortner f3101b3
[build] remove outdated note from format.sh
titusfortner 8d49eb5
[build] include untracked files in default mode, clarify actionlint m…
titusfortner fe08cc2
[build] enforce mutually exclusive modes, use portable find -exec, fi…
titusfortner 7b984c1
[build] use baseline comparison to allow pre-existing uncommitted work
titusfortner 4318d7e
[build] simplify trunk detection to local branch only
titusfortner b34d038
[build] run all formatters before failing, simplify arg parsing, upda…
titusfortner cffd3a4
[build] revert to fail-fast, fix lint order and ruff-check target
titusfortner 62d6f2e
[build] add --all flag to skip change detection
titusfortner cb468d4
[build] add --all flag, update format.ps1 to match format.sh
titusfortner File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,41 +1,142 @@ | ||
| # Code formatter. | ||
| # Code formatter - runs targeted formatters based on what changed from trunk. | ||
| # Usage: format.ps1 [-All] [-PreCommit] [-PrePush] [-Lint] | ||
| # (default) Check all changes relative to trunk including uncommitted work | ||
| # -All Format everything, skip change detection | ||
| # -PreCommit Only check staged changes | ||
| # -PrePush Only check committed changes relative to trunk | ||
| # -Lint Also run linters before formatting | ||
|
|
||
| param( | ||
| [switch]$All, | ||
| [switch]$PreCommit, | ||
| [switch]$PrePush, | ||
| [switch]$Lint | ||
| ) | ||
|
|
||
| Set-StrictMode -Version 'Latest' | ||
| $ErrorActionPreference = 'Stop' | ||
|
|
||
| # Validate mutually exclusive flags | ||
| if ($PreCommit -and $PrePush) { | ||
| Write-Error "Cannot use both -PreCommit and -PrePush" | ||
| exit 1 | ||
| } | ||
|
|
||
| function section($message) { | ||
| Write-Host "- $message" -ForegroundColor Green | ||
| } | ||
|
|
||
| # Find what's changed compared to trunk (skip if -All) | ||
| $formatAll = $All | ||
| $trunkRef = git rev-parse --verify trunk 2>$null | ||
|
|
||
| if (-not $formatAll -and $trunkRef) { | ||
| $base = git merge-base HEAD $trunkRef 2>$null | ||
| if ($base) { | ||
titusfortner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if ($PreCommit) { | ||
| $changed = git diff --name-only --cached | ||
| } elseif ($PrePush) { | ||
| $changed = git diff --name-only $base HEAD | ||
| } else { | ||
| $committed = git diff --name-only $base HEAD | ||
| $staged = git diff --name-only --cached | ||
| $unstaged = git diff --name-only | ||
| $untracked = git ls-files --others --exclude-standard | ||
| $changed = ($committed + $staged + $unstaged + $untracked) | Sort-Object -Unique | ||
| } | ||
| } else { | ||
| $formatAll = $true | ||
| } | ||
| } elseif (-not $formatAll) { | ||
| # No trunk ref found, format everything | ||
| $formatAll = $true | ||
| } | ||
|
|
||
| # Helper to check if a pattern matches changed files | ||
| function changedMatches($pattern) { | ||
| if ($formatAll) { return $true } | ||
| return ($changed | Where-Object { $_ -match $pattern }).Count -gt 0 | ||
| } | ||
|
|
||
| $WORKSPACE_ROOT = (bazel info workspace) | ||
| $GOOGLE_JAVA_FORMAT = (bazel run --run_under=echo //scripts:google-java-format) | ||
|
|
||
| # Capture baseline to detect formatter-introduced changes | ||
| $baseline = git status --porcelain | ||
|
|
||
| # Always run buildifier and copyright | ||
| section "Buildifier" | ||
| Write-Host " buildifier" -ForegroundColor Green | ||
| Write-Host " buildifier" | ||
| bazel run //:buildifier | ||
|
|
||
| section "Java" | ||
| Write-Host " google-java-format" -ForegroundColor Green | ||
| Get-ChildItem -Path "$PWD/java" -Include "*.java" -Recurse | ForEach-Object { | ||
| &"$GOOGLE_JAVA_FORMAT" --replace $_.FullName | ||
| section "Copyright" | ||
| Write-Host " update_copyright" | ||
| bazel run //scripts:update_copyright | ||
|
|
||
| # Run language formatters only if those files changed | ||
| if (changedMatches '^java/') { | ||
| section "Java" | ||
| Write-Host " google-java-format" | ||
| $GOOGLE_JAVA_FORMAT = (bazel run --run_under=echo //scripts:google-java-format) | ||
| Get-ChildItem -Path "$WORKSPACE_ROOT/java" -Include "*.java" -Recurse | ForEach-Object { | ||
| & "$GOOGLE_JAVA_FORMAT" --replace $_.FullName | ||
| } | ||
| } | ||
|
|
||
| section "Javascript" | ||
| Write-Host " javascript/selenium-webdriver - prettier" -ForegroundColor Green | ||
| $NODE_WEBDRIVER = "$WORKSPACE_ROOT/javascript/selenium-webdriver" | ||
| bazel run //javascript:prettier -- "$NODE_WEBDRIVER" --write "$NODE_WEBDRIVER/.prettierrc" --log-level=warn | ||
| if (changedMatches '^javascript/selenium-webdriver/') { | ||
| section "JavaScript" | ||
| Write-Host " prettier" | ||
| $NODE_WEBDRIVER = "$WORKSPACE_ROOT/javascript/selenium-webdriver" | ||
| bazel run //javascript:prettier -- "$NODE_WEBDRIVER" --write "$NODE_WEBDRIVER/.prettierrc" --log-level=warn | ||
| } | ||
|
|
||
| section "Ruby" | ||
| Write-Host " rubocop" -ForegroundColor Green | ||
| bazel run //rb:lint | ||
| if (changedMatches '^rb/|^rake_tasks/|^Rakefile') { | ||
| section "Ruby" | ||
| Write-Host " rubocop -a" | ||
| if ($Lint) { | ||
| bazel run //rb:rubocop -- -a | ||
| } else { | ||
| bazel run //rb:rubocop -- -a --fail-level F | ||
| } | ||
| } | ||
|
|
||
| section "Rust" | ||
| Write-Host " rustfmt" -ForegroundColor Green | ||
| bazel run @rules_rust//:rustfmt | ||
| if (changedMatches '^rust/') { | ||
| section "Rust" | ||
| Write-Host " rustfmt" | ||
| bazel run @rules_rust//:rustfmt | ||
| } | ||
|
|
||
| section "Python" | ||
| Write-Host " python - ruff" -ForegroundColor Green | ||
| bazel run //py:ruff-format | ||
| if (changedMatches '^py/') { | ||
| section "Python" | ||
| if ($Lint) { | ||
| Write-Host " ruff check" | ||
| bazel run //py:ruff-check | ||
| } | ||
| Write-Host " ruff format" | ||
| bazel run //py:ruff-format | ||
| } | ||
titusfortner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| section "Copyright" | ||
| bazel run //scripts:update_copyright | ||
| if (changedMatches '^dotnet/') { | ||
| section ".NET" | ||
| Write-Host " dotnet format" | ||
| bazel run //dotnet:format -- style --severity warn | ||
| bazel run //dotnet:format -- whitespace | ||
| } | ||
|
|
||
| # Run shellcheck and actionlint when -Lint is passed | ||
| if ($Lint) { | ||
| section "Shell/Actions" | ||
| Write-Host " actionlint (with shellcheck)" | ||
| $SHELLCHECK = (bazel run --run_under=echo @multitool//tools/shellcheck) | ||
| bazel run @multitool//tools/actionlint:cwd -- -shellcheck "$SHELLCHECK" | ||
| } | ||
|
|
||
| # Check if formatting introduced new changes (comparing to baseline) | ||
| $after = git status --porcelain | ||
| if ($after -ne $baseline) { | ||
| Write-Host "" | ||
| Write-Host "Formatters modified files:" -ForegroundColor Red | ||
| git diff --name-only | ||
titusfortner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| exit 1 | ||
| } | ||
|
|
||
| Write-Host "Format check passed." -ForegroundColor Green | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,39 +1,147 @@ | ||
| #!/usr/bin/env bash | ||
| # Code formatter. | ||
| # Code formatter - runs targeted formatters based on what changed from trunk. | ||
| # Usage: format.sh [--all] [--pre-commit] [--pre-push] [--lint] | ||
| # (default) Check all changes relative to trunk including uncommitted work | ||
| # --all Format everything, skip change detection (previous behavior) | ||
| # --pre-commit Only check staged changes | ||
| # --pre-push Only check committed changes relative to trunk | ||
| # --lint Also run linters before formatting | ||
| set -eufo pipefail | ||
|
|
||
| echo "Note: for more flexibility, use './go format' or './go dotnet:format' or './go format -dotnet', etc" >&2 | ||
| echo "" >&2 | ||
| run_lint=false | ||
| format_all=false | ||
| mode="default" | ||
| for arg in "$@"; do | ||
| case "$arg" in | ||
| --lint) run_lint=true ;; | ||
titusfortner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| --all) format_all=true ;; | ||
|
|
||
| --pre-commit|--pre-push) | ||
| [[ "$mode" == "default" ]] || { echo "Cannot use both --pre-commit and --pre-push" >&2; exit 1; } | ||
| mode="${arg#--}" | ||
| ;; | ||
| *) | ||
| echo "Unknown option: $arg" >&2 | ||
| echo "Usage: $0 [--all] [--pre-commit] [--pre-push] [--lint]" >&2 | ||
| exit 1 | ||
| ;; | ||
| esac | ||
| done | ||
|
|
||
| section() { | ||
| echo "- $*" >&2 | ||
| } | ||
|
|
||
| # Find what's changed compared to trunk (skip if --all) | ||
| trunk_ref="$(git rev-parse --verify trunk 2>/dev/null || echo "")" | ||
|
|
||
| if [[ "$format_all" == "false" && -n "$trunk_ref" ]]; then | ||
| base="$(git merge-base HEAD "$trunk_ref" 2>/dev/null || echo "")" | ||
| if [[ -n "$base" ]]; then | ||
| case "$mode" in | ||
| pre-commit) | ||
| changed="$(git diff --name-only --cached)" | ||
| ;; | ||
| pre-push) | ||
| changed="$(git diff --name-only "$base" HEAD)" | ||
| ;; | ||
| default) | ||
| committed="$(git diff --name-only "$base" HEAD)" | ||
| staged="$(git diff --name-only --cached)" | ||
| unstaged="$(git diff --name-only)" | ||
| untracked="$(git ls-files --others --exclude-standard)" | ||
| changed="$(printf '%s\n%s\n%s\n%s' "$committed" "$staged" "$unstaged" "$untracked" | sort -u)" | ||
titusfortner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ;; | ||
| esac | ||
| else | ||
| format_all=true | ||
| fi | ||
| elif [[ "$format_all" == "false" ]]; then | ||
| # No trunk ref found, format everything | ||
| format_all=true | ||
| fi | ||
|
|
||
| # Helper to check if a pattern matches changed files | ||
| changed_matches() { | ||
| [[ "$format_all" == "true" ]] || echo "$changed" | grep -qE "$1" | ||
titusfortner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| WORKSPACE_ROOT="$(bazel info workspace)" | ||
|
|
||
| GOOGLE_JAVA_FORMAT="$(bazel run --run_under=echo //scripts:google-java-format)" | ||
| # Capture baseline to detect formatter-introduced changes (allows pre-existing uncommitted work) | ||
| baseline="$(git status --porcelain)" | ||
titusfortner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| # Always run buildifier and copyright | ||
| section "Buildifier" | ||
| echo " buildifier" >&2 | ||
| bazel run //:buildifier | ||
|
|
||
| section "Java" | ||
| echo " google-java-format" >&2 | ||
| find "$PWD/java" -type f -name '*.java' | xargs "$GOOGLE_JAVA_FORMAT" --replace | ||
| section "Copyright" | ||
| echo " update_copyright" >&2 | ||
| bazel run //scripts:update_copyright | ||
|
|
||
| # Run language formatters only if those files changed | ||
| if changed_matches '^java/'; then | ||
| section "Java" | ||
| echo " google-java-format" >&2 | ||
| GOOGLE_JAVA_FORMAT="$(bazel run --run_under=echo //scripts:google-java-format)" | ||
| find "${WORKSPACE_ROOT}/java" -type f -name '*.java' -exec "$GOOGLE_JAVA_FORMAT" --replace {} + | ||
titusfortner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| fi | ||
|
|
||
| if changed_matches '^javascript/selenium-webdriver/'; then | ||
| section "JavaScript" | ||
| echo " prettier" >&2 | ||
| NODE_WEBDRIVER="${WORKSPACE_ROOT}/javascript/selenium-webdriver" | ||
| bazel run //javascript:prettier -- "${NODE_WEBDRIVER}" --write "${NODE_WEBDRIVER}/.prettierrc" --log-level=warn | ||
| fi | ||
|
|
||
| if changed_matches '^rb/|^rake_tasks/|^Rakefile'; then | ||
| section "Ruby" | ||
| echo " rubocop -a" >&2 | ||
| if [[ "$run_lint" == "true" ]]; then | ||
| bazel run //rb:rubocop -- -a | ||
| else | ||
| bazel run //rb:rubocop -- -a --fail-level F | ||
| fi | ||
| fi | ||
|
|
||
| if changed_matches '^rust/'; then | ||
| section "Rust" | ||
| echo " rustfmt" >&2 | ||
| bazel run @rules_rust//:rustfmt | ||
| fi | ||
|
|
||
| if changed_matches '^py/'; then | ||
| section "Python" | ||
| if [[ "$run_lint" == "true" ]]; then | ||
| echo " ruff check" >&2 | ||
| bazel run //py:ruff-check | ||
| fi | ||
| echo " ruff format" >&2 | ||
| bazel run //py:ruff-format | ||
titusfortner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| fi | ||
titusfortner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| section "Javascript" | ||
| echo " javascript/selenium-webdriver - prettier" >&2 | ||
| NODE_WEBDRIVER="${WORKSPACE_ROOT}/javascript/selenium-webdriver" | ||
| bazel run //javascript:prettier -- "${NODE_WEBDRIVER}" --write "${NODE_WEBDRIVER}/.prettierrc" --log-level=warn | ||
| if changed_matches '^dotnet/'; then | ||
| section ".NET" | ||
| echo " dotnet format" >&2 | ||
| bazel run //dotnet:format -- style --severity warn | ||
| bazel run //dotnet:format -- whitespace | ||
| fi | ||
|
|
||
| section "Ruby" | ||
| echo " rubocop" >&2 | ||
| bazel run //rb:rubocop -- -a --fail-level F | ||
| # Run shellcheck and actionlint when --lint is passed | ||
| if [[ "$run_lint" == "true" ]]; then | ||
| section "Shell/Actions" | ||
| echo " actionlint (with shellcheck)" >&2 | ||
| SHELLCHECK="$(bazel run --run_under=echo @multitool//tools/shellcheck)" | ||
| bazel run @multitool//tools/actionlint:cwd -- -shellcheck "$SHELLCHECK" | ||
| fi | ||
titusfortner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| section "Rust" | ||
| echo " rustfmt" >&2 | ||
| bazel run @rules_rust//:rustfmt | ||
| # Check if formatting introduced new changes (comparing to baseline) | ||
| if [[ "$(git status --porcelain)" != "$baseline" ]]; then | ||
| echo "" >&2 | ||
| echo "Formatters modified files:" >&2 | ||
| git diff --name-only >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| section "Python" | ||
| echo " python - ruff" >&2 | ||
| bazel run //py:ruff-format | ||
| echo "Format check passed." >&2 | ||
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.
Uh oh!
There was an error while loading. Please reload this page.