[build] Separate format and lint tasks, add per-language format commands

[titusfortner]

Finally consolidate all the lint work into one cohesive thing.

Both Ruby & .NET can take a long time to run format.sh (several minutes), and are unnecessary if you're doing Java or Python work, so helpful to filter
Right now we're mixing formatting and linting a little bit, and this clarifies the intent.

💥 What does this PR do?

Separates formatting and linting into distinct tasks with clear responsibilities:

Format tasks (explicit format/spacing things, auto-fix, no-fail, run in ci-lint.yml):

• ./go format - buildifier, copyright, rust, all language formatters
• ./go <lang>:format - language-specific formatting
• ./go format -<lang1> -<lang2> - skip one or more languages

Lint tasks (static checks, fix-if-can, run in language-specific CI):

• ./go lint - actionlint/shellcheck + all language linters
• ./go <lang>:lint - linters, type checkers, docs build verification
• Each lint task includes docs_generate to verify docs can build

Per-language lint includes:

Language	Format	Lint
Python	ruff format	ruff check, mypy, docs
Ruby	rubocop -a	rubocop, steep, docs
Java	google-java-format	docs (ErrorProne/SpotBugs run in build/test)
.NET	dotnet format whitespace+style	dotnet analyzers, docs
JavaScript	prettier	eslint, docs
Rust	rustfmt	(no clippy configured)

CI changes:

• ci-lint.yml: actionlint GitHub Action + ./go format
• ci-python.yml: ./go py:lint (removed separate docs/typing jobs)
• ci-ruby.yml: ./go rb:lint (removed separate docs job)
• ci-dotnet.yml: added ./go dotnet:lint

format.sh changes

• Removed dotnet from it because it takes too long to run, even after you've built dotnet
• removed copyright from it, removed

🔧 Implementation Notes

• I'm not sure if we should generate docs as part of "lint" task, but it's pretty quick (doesn't stage or release them)
• Ideally, all the lint tasks are straightforward bazel targets and are just included in bazel test //<lang>/...

💡 Additional Considerations

• Java lint is just docs generation since ErrorProne runs at build time and SpotBugs runs as test targets in RBE
• Rust lint is a no-op until clippy is configured (low priority)
• scripts/format.sh still works but suggests ./go format

🔄 Types of changes

• Cleanup (formatting, renaming)

[qodo-code-review]

PR Type

Enhancement, Tests

---

Description

• Separate format and lint tasks with distinct responsibilities

• Add per-language format commands for all language bindings

• Include docs generation as part of language-specific lint tasks

• Update CI workflows to use new format and lint task structure

• Remove dotnet from format.sh, suggest ./go format instead

---

File Walkthrough

Relevant files

Enhancement

12 files dotnet.rake Split docs task, add format and lint separation                    +22/-9    java.rake Add docs_generate task, rename lint to format                        +18/-5    node.rake Split docs task, add format and lint separation                    +16/-5    python.rake Split docs task, separate format and lint tasks                    +16/-12  ruby.rake Add format task, split docs and lint responsibilities        +21/-9    rust.rake Add format task, make lint a no-op placeholder                      +8/-4      format.sh Remove dotnet, add note about ./go format                                +4/-8      ci-dotnet.yml Add dotnet lint job to CI workflow                                              +8/-0      ci-lint.yml Replace check-format.sh with ./go format command                  +9/-2      ci-python.yml Consolidate docs and typing into py:lint task                        +1/-28    ci-ruby.yml Consolidate docs and steep into rb:lint task                          +4/-12    Rakefile Restructure format and lint tasks, add language aliases    +36/-45

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Latest suggestions up to b8eef8a

Category	Suggestion	Impact
Possible issue	Validate and parse skip args Improve the all:format rake task by only processing arguments that start with - as skip tokens and by adding validation to raise an error for unknown language arguments. Rakefile [245-252] task :format do |_task, arguments| all_langs = %w[java py rb dotnet node] - skip = arguments.to_a.map { |a| LANG_ALIASES.fetch(a.delete_prefix('-'), a.delete_prefix('-')) } + raw = arguments.to_a.select { |a| a.start_with?('-') } + skip = raw.map { |a| LANG_ALIASES.fetch(a.delete_prefix('-'), a.delete_prefix('-')) } + + invalid = skip - all_langs + raise "Unknown languages: #{invalid.join(', ')}. Valid: #{all_langs.join(', ')}" if invalid.any? + (all_langs - skip).each do |lang| puts "Formatting #{lang}..." Rake::Task["#{lang}:format"].invoke end end Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion improves the robustness of the new all:format rake task by adding validation for arguments, preventing incorrect usage and aligning its behavior with similar tasks in the codebase.	Medium
	Batch formatting to avoid limits Modify the java:format task to process Java files in batches of 200 to prevent potential failures caused by exceeding command-line argument length limits. rake_tasks/java.rake [394-401] task :format do puts ' Running google-java-format...' java_files = Dir.glob(File.join(Dir.pwd, 'java', '**', '*.java')) return if java_files.empty? - args = ['--', '--replace'] + java_files - Bazel.execute('run', args, '//scripts:google-java-format') + java_files.each_slice(200) do |batch| + args = ['--', '--replace'] + batch + Bazel.execute('run', args, '//scripts:google-java-format') + end end Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion proactively addresses a potential command-line length limit issue, especially on Windows, by processing files in batches, which improves the robustness of the script.	Medium
	Fix prettier config argument Fix the prettier command in scripts/format.sh by passing the configuration file path via the --config flag instead of as a positional argument to ensure it is used correctly. scripts/format.sh [24-27] 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 +bazel run //javascript:prettier -- "${NODE_WEBDRIVER}" --write --config="${NODE_WEBDRIVER}/.prettierrc" --log-level=warn [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies a bug where the prettier command-line arguments are incorrect, potentially causing the wrong configuration to be used for formatting.	Low
Possible issue	Prevent invalid early-exit jump In the .NET lint task, replace next with return to prevent a LocalJumpError since it is used outside an iterator. rake_tasks/dotnet.rake [137-141] enforced_diagnostics = [] -next if enforced_diagnostics.empty? +return if enforced_diagnostics.empty? arguments = %w[-- style --severity info --verify-no-changes --diagnostics] + enforced_diagnostics Bazel.execute('run', arguments, '//dotnet:format') Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that using next outside of an iterator will raise a LocalJumpError, which would break the rake task. Replacing it with return is the correct fix for this latent bug.	Medium
	Fail when auto-fixes applied In the Python lint task, add the --exit-non-zero-on-fix flag to the ruff check command to ensure the CI job fails if any auto-fixes are applied. rake_tasks/python.rake [179-180] puts ' Running ruff check...' -Bazel.execute('run', %w[-- --fix --show-fixes], '//py:ruff-check') +Bazel.execute('run', %w[-- --fix --exit-non-zero-on-fix --show-fixes], '//py:ruff-check') Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that the lint task auto-fixes issues without failing the build, which contradicts the purpose of a lint check in CI. Adding --exit-non-zero-on-fix aligns the behavior with CI best practices.	Medium
Learned best practice	Harden CI format step Add shell: bash plus set -euo pipefail, and use an explicit/robust dirty-check (e.g., git status --porcelain) so the step fails fast and predictably if formatting changed files. .github/workflows/ci-lint.yml [53-60] +shell: bash run: | + set -euo pipefail + # Run format - auto-fixes code style issues ./go format + # Fail if there were changes (triggers commit-fixes job) - if ! git diff --quiet; then - echo "Format made changes" + if [[ -n "$(git status --porcelain)" ]]; then + echo "ERROR: format made changes; run './go format' and commit results" >&2 + git diff --name-only >&2 || true exit 1 fi Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - In CI run steps, enable strict shell execution and fail fast with clear errors when conditions are not met.	Low
More

---

Previous suggestions

✅ Suggestions up to commit 43d2aaf

Category	Suggestion	Impact
Possible issue	Enforce nightly docs guard Move the nightly version safeguard from the dotnet:docs task to the dotnet:docs_generate task to ensure all call sites, including dotnet:lint, are protected from updating docs on nightly versions. rake_tasks/dotnet.rake [73-78] desc 'Generate .NET documentation without staging' -task :docs_generate do +task :docs_generate do |_task, arguments| + if dotnet_version.include?('nightly') && !arguments.to_a.include?('force') + abort('Aborting documentation update: nightly versions should not update docs.') + end + puts 'Generating .NET documentation' FileUtils.rm_rf('build/docs/api/dotnet/') Bazel.execute('run', [], '//dotnet:docs') end Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a missing safeguard in the docs_generate task, which could lead to unintended documentation updates for nightly builds, and proposes moving the check to ensure consistent behavior.	Medium
	Make lint non-mutating The rb:lint task should not auto-correct files. Remove the -a flag from the rubocop command to make it a check-only operation, consistent with a linting task. rake_tasks/ruby.rake [149-156] desc 'Run Ruby linters (rubocop, steep, docs)' task :lint do - puts ' Running rubocop...' - Bazel.execute('run', ['--', '-a'], '//rb:rubocop') + puts ' Running rubocop (check-only)...' + Bazel.execute('run', ['--', '--fail-level', 'F'], '//rb:rubocop') puts ' Running steep type checker...' Bazel.execute('run', [], '//rb:steep') Rake::Task['rb:docs_generate'].invoke end Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that the rb:lint task modifies files, which contradicts the PR's goal of separating linting from formatting, and proposes a fix to make it a check-only operation.	Medium
	Avoid auto-fixes during lint The py:lint task should not auto-correct files. Replace the --fix flag in the ruff check command with --exit-non-zero-on-fix to ensure the lint task is check-only. rake_tasks/python.rake [177-184] desc 'Run Python linters (ruff check, mypy, docs)' task :lint do puts ' Running ruff check...' - Bazel.execute('run', %w[-- --fix --show-fixes], '//py:ruff-check') + Bazel.execute('run', %w[-- --show-fixes --exit-non-zero-on-fix], '//py:ruff-check') puts ' Running mypy...' Bazel.execute('run', [], '//py:mypy') Rake::Task['py:docs_generate'].invoke end Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that the py:lint task modifies files due to the --fix flag, which contradicts the PR's goal of separating linting from formatting, and proposes a correct fix.	Medium
Learned best practice	✅ Make conditional enforcement explicit Suggestion Impact: The TODO comment was updated to match the suggested wording, but the core change (replacing the early-exit with an explicit conditional and optional logging) was not implemented; the `next if enforced_diagnostics.empty?` remains. code diff: - # TODO: Can also identify specific diagnostics to elevate and add to this list - # TODO: Add IDE0060 after merging #17019 + # TODO: Identify specific diagnostics that we want to enforce but can't be auto-corrected (e.g., 'IDE0060'): enforced_diagnostics = [] + next if enforced_diagnostics.empty? + arguments = %w[-- style --severity info --verify-no-changes --diagnostics] + enforced_diagnostics Bazel.execute('run', arguments, '//dotnet:format') Replace the next early-exit with an explicit conditional so the task’s behavior is clear (and optionally log that extra diagnostics enforcement is currently skipped). rake_tasks/dotnet.rake [136-141] # TODO: Identify specific diagnostics that we want to enforce but can't be auto-corrected (e.g., 'IDE0060'): enforced_diagnostics = [] -next if enforced_diagnostics.empty? +if enforced_diagnostics.any? + arguments = %w[-- style --severity info --verify-no-changes --diagnostics] + enforced_diagnostics + Bazel.execute('run', arguments, '//dotnet:format') +else + puts ' No additional enforced diagnostics configured' +end -arguments = %w[-- style --severity info --verify-no-changes --diagnostics] + enforced_diagnostics -Bazel.execute('run', arguments, '//dotnet:format') - [Suggestion processed] Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Avoid silent early exits in automation; gate optional work explicitly and keep lint tasks deterministic and rerunnable.	Low
	Enable strict shell execution Add set -euo pipefail at the start of the run: block so unexpected failures (including unset variables) stop the step immediately with a non-zero exit. .github/workflows/ci-lint.yml [53-60] run: | + set -euo pipefail # Run format - auto-fixes code style issues ./go format # Fail if there were changes (triggers commit-fixes job) if ! git diff --quiet; then echo "Format made changes" exit 1 fi Suggestion importance[1-10]: 5 __ Why: Relevant best practice - In CI run steps, enable strict shell execution (e.g., set -euo pipefail) and fail fast with clear errors.	Low

✅ Suggestions up to commit 6d3d245

Category	Suggestion	Impact
Incremental [*]	Skip docs generation on nightly In the node:lint task, add a check to skip documentation generation for nightly versions, similar to the safeguard in the node:docs task. rake_tasks/node.rake [169-170] # TODO: Add eslint once bazel target properly resolves workspace modules +if node_version.include?('nightly') + puts ' Skipping docs generation for nightly version...' + next +end Rake::Task['node:docs_generate'].invoke Suggestion importance[1-10]: 8 __ Why: The suggestion correctly points out that the node:lint task bypasses a version check, which could lead to incorrect documentation generation for nightly builds, thus preventing a potential bug.	Medium
	✅ Make formatter invocation robust Suggestion Impact: The formatter path is now captured via Bazel.execute with safe nil handling and an explicit failure check, and java file selection was refactored to use an absolute glob list before invoking the formatter (instead of a brittle shell pipeline). code diff: formatter = nil Bazel.execute('run', ['--run_under=echo'], '//scripts:google-java-format') do |output| - formatter = output.lines.last.strip - end - sh formatter, '--replace', *Dir.glob('java/**/*.java') + formatter = output.lines.last&.strip + end + raise 'Failed to get google-java-format path' if formatter.nil? || formatter.empty? + + java_files = Dir.glob(File.join(Dir.pwd, 'java', '**', '*.java')) + sh formatter, '--replace', *java_files unless java_files.empty? Refactor the java:format task to robustly handle file paths by using find -print0 | xargs -0 and a safer method to capture the formatter path. rake_tasks/java.rake [396-399] -formatter = `bazel run --run_under=echo //scripts:google-java-format 2>/dev/null`.strip -raise 'Failed to get google-java-format path' if formatter.empty? || !$CHILD_STATUS.success? +formatter = nil +Bazel.execute('run', ['--run_under=echo'], '//scripts:google-java-format') do |output| + formatter = output.lines.last&.strip +end +raise 'Failed to get google-java-format path' if formatter.to_s.empty? -sh "find #{Dir.pwd}/java -name '*.java' | xargs #{formatter} --replace" +sh %(find "#{File.join(Dir.pwd, 'java')}" -name '*.java' -print0 | xargs -0 "#{formatter}" --replace) [Suggestion processed] Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a brittle shell command and proposes a more robust implementation, improving the script's reliability against paths with spaces or special characters.	Medium
Possible issue	Fail on any working-tree changes Improve the format check by using git status --porcelain in addition to git diff --quiet to detect untracked files, ensuring the job fails if any files are created by the formatter. .github/workflows/ci-lint.yml [53-60] run: | # Run format - auto-fixes code style issues ./go format - # Fail if there were changes (triggers commit-fixes job) - if ! git diff --quiet; then + # Fail if there were changes (tracked or untracked) + if ! git diff --quiet || [ -n "$(git status --porcelain)" ]; then echo "Format made changes" + git status --porcelain exit 1 fi Suggestion importance[1-10]: 8 __ Why: The suggestion correctly points out that git diff --quiet misses untracked files, and the proposed change to use git status --porcelain makes the format check more robust by catching all working tree changes.	Medium
	Re-enable tasks inside loops In the all:format Rake task, re-enable each language-specific format task before invoking it within the loop to ensure it runs every time, even if called elsewhere in the same process. Rakefile [245-252] task :format do |_task, arguments| all_langs = %w[java py rb dotnet node] skip = arguments.to_a.map { |a| LANG_ALIASES.fetch(a.delete_prefix('-'), a.delete_prefix('-')) } (all_langs - skip).each do |lang| puts "Formatting #{lang}..." - Rake::Task["#{lang}:format"].invoke + t = Rake::Task["#{lang}:format"] + t.reenable + t.invoke end end Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential issue with Rake where tasks only run once, and adding reenable makes the task execution more robust and predictable, preventing silent failures.	Medium
	Ensure all lint tasks run In the all:lint Rake task, re-enable each language-specific lint task before invoking it within the loop to ensure it runs every time, even if called elsewhere in the same process. Rakefile [255-263] task :lint do all_langs = %w[java py rb dotnet node] failures = [] all_langs.each do |lang| puts "Linting #{lang}..." - Rake::Task["#{lang}:lint"].invoke + t = Rake::Task["#{lang}:lint"] + t.reenable + t.invoke rescue StandardError => e failures << "#{lang}: #{e.message}" end Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential issue with Rake where tasks only run once, and adding reenable makes the task execution more robust and predictable, preventing silent failures.	Medium

✅ Suggestions up to commit 9692ebe

Category	Suggestion	Impact
Incremental [*]	✅ Fix invalid control flow Suggestion Impact: The commit adopted the suggested updated TODO comment wording, but it did not implement the core fix: it kept/added `next if enforced_diagnostics.empty?` instead of replacing it with an `unless` guard. code diff: - # TODO: Can also identify specific diagnostics to elevate and add to this list - # TODO: Add IDE0060 after merging #17019 + # TODO: Identify specific diagnostics that we want to enforce but can't be auto-corrected (e.g., 'IDE0060'): enforced_diagnostics = [] + next if enforced_diagnostics.empty? + Replace next with a conditional guard (unless) to prevent a LocalJumpError in the Rake task, as next is only valid within an iterator. rake_tasks/dotnet.rake [136-138] # TODO: Identify specific diagnostics that we want to enforce but can't be auto-corrected (e.g., 'IDE0060'): enforced_diagnostics = [] -next if enforced_diagnostics.empty? +unless enforced_diagnostics.empty? + arguments = %w[-- style --severity info --verify-no-changes --diagnostics] + enforced_diagnostics + Bazel.execute('run', arguments, '//dotnet:format') +end Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies that using next outside of a loop will cause a LocalJumpError, which is a bug that would break the dotnet:lint task.	High
	✅ Avoid command line length overflow Suggestion Impact: Replaced passing a large Ruby-collected file list to Bazel with a find|xargs pipeline that feeds files to the formatter, avoiding command-line length overflow in a different way than batching. code diff: - files = Dir.glob("#{Dir.pwd}/java/**/*.java") - Bazel.execute('run', ['--', '--replace'] + files, '//scripts:google-java-format') + formatter = `bazel run --run_under=echo //scripts:google-java-format 2>/dev/null`.strip + raise 'Failed to get google-java-format path' if formatter.empty? || !$CHILD_STATUS.success? + + sh "find #{Dir.pwd}/java -name '*.java' | xargs #{formatter} --replace" Batch the list of Java files passed to Bazel.execute to avoid exceeding command-line length limits, which can cause the format task to fail. rake_tasks/java.rake [396-397] files = Dir.glob("#{Dir.pwd}/java/**/*.java") -Bazel.execute('run', ['--', '--replace'] + files, '//scripts:google-java-format') +files.each_slice(200) do |batch| + Bazel.execute('run', ['--', '--replace'] + batch, '//scripts:google-java-format') +end [Suggestion processed] Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential command-line length limit issue and provides a robust solution by batching file arguments, improving script reliability.	Medium
	Prevent lint from modifying files Remove the auto-correct flag (-a) from the rubocop command in the lint task to prevent it from modifying files, ensuring it only reports issues. rake_tasks/ruby.rake [152] -Bazel.execute('run', ['--', '-a'], '//rb:rubocop') +Bazel.execute('run', ['--', '--fail-level', 'F'], '//rb:rubocop') Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that the lint task should not modify files, which is a good practice to ensure CI jobs fail clearly instead of creating uncommitted changes.	Medium
Possible issue	Make lint non-mutating Remove the --fix flag from the ruff check command in the py:lint task to ensure it only checks for issues without modifying files. rake_tasks/python.rake [177-184] desc 'Run Python linters (ruff check, mypy, docs)' task :lint do puts ' Running ruff check...' - Bazel.execute('run', %w[-- --fix --show-fixes], '//py:ruff-check') + Bazel.execute('run', %w[-- --show-fixes], '//py:ruff-check') puts ' Running mypy...' Bazel.execute('run', [], '//py:mypy') Rake::Task['py:docs_generate'].invoke end Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that the py:lint task uses --fix, which modifies files and contradicts the PR's goal of separating check-only lint tasks from auto-fixing format tasks.	Medium
	✅ Prevent lint from auto-fixing Suggestion Impact: The eslint invocation (including the --fix flag) was removed from the node:lint task entirely, preventing auto-fixing/mutating behavior, though it did not add --max-warnings 0. code diff: -desc 'Run JavaScript linter (eslint, docs)' +desc 'Run JavaScript linter (docs only for now, eslint needs bazel integration work)' task :lint do - puts ' Running eslint...' - Bazel.execute('run', ['--', '--fix', '.'], '//javascript/selenium-webdriver:eslint') + # TODO: Add eslint once bazel target properly resolves workspace modules Rake::Task['node:docs_generate'].invoke Remove the --fix flag from the eslint command in the node:lint task and add --max-warnings 0 to make it a strict, non-mutating check. rake_tasks/node.rake [167-172] desc 'Run JavaScript linter (eslint, docs)' task :lint do puts ' Running eslint...' - Bazel.execute('run', ['--', '--fix', '.'], '//javascript/selenium-webdriver:eslint') + Bazel.execute('run', ['--', '--max-warnings', '0', '.'], '//javascript/selenium-webdriver:eslint') Rake::Task['node:docs_generate'].invoke end [Suggestion processed] Suggestion importance[1-10]: 8 __ Why: The suggestion correctly points out that the node:lint task modifies files using --fix, which is inconsistent with the PR's goal of separating lint and format tasks.	Medium

✅ Suggestions up to commit f2b4a25

Category	Suggestion	Impact
Possible issue	Add aggregated failure raise In the all:lint task, add a final check to raise an error if the failures array is not empty, ensuring the task fails correctly. Rakefile [258-263] all_langs.each do |lang| puts "Linting #{lang}..." Rake::Task["#{lang}:lint"].invoke rescue StandardError => e failures << "#{lang}: #{e.message}" end +raise "Lint failed:\n#{failures.join("\n")}" unless failures.empty? + Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that the all:lint task collects failures but does not raise an exception at the end, which would cause the CI job to pass silently despite linting errors. This is a significant correctness issue.	Medium
	Aggregate shellcheck failures In the top-level :lint task, aggregate failures from shellcheck/actionlint and the all:lint sub-task, then raise a single exception with all collected errors at the end. Rakefile [144-151] desc 'Run linters (non-auto-fixable checks)' task :lint do + failures = [] + puts 'Linting shell scripts and GitHub Actions...' - shellcheck = Bazel.execute('build', [], '@multitool//tools/shellcheck') - Bazel.execute('run', ['--', '-shellcheck', shellcheck], '@multitool//tools/actionlint:cwd') + begin + shellcheck = Bazel.execute('build', [], '@multitool//tools/shellcheck') + Bazel.execute('run', ['--', '-shellcheck', shellcheck], '@multitool//tools/actionlint:cwd') + rescue StandardError => e + failures << "shellcheck/actionlint: #{e.message}" + end - Rake::Task['all:lint'].invoke + begin + Rake::Task['all:lint'].invoke + rescue StandardError => e + failures << e.message + end + + raise "Lint failed:\n#{failures.join("\n")}" unless failures.empty? end Suggestion importance[1-10]: 6 __ Why: The suggestion correctly points out that the refactored top-level :lint task lost its error aggregation logic, causing it to fail on the first error. Reintroducing this logic makes the linting process more robust by reporting all failures at once.	Low
	✅ Fix ineffective linter command arguments Suggestion Impact: The commit addresses the same underlying issue (an empty `enforced_diagnostics` making `--diagnostics` ineffective) by short-circuiting the task with `next if enforced_diagnostics.empty?` before building arguments, rather than conditionally appending `--diagnostics`. code diff: - # TODO: Can also identify specific diagnostics to elevate and add to this list - # TODO: Add IDE0060 after merging #17019 + # TODO: Identify specific diagnostics that we want to enforce but can't be auto-corrected (e.g., 'IDE0060'): enforced_diagnostics = [] + next if enforced_diagnostics.empty? + arguments = %w[-- style --severity info --verify-no-changes --diagnostics] + enforced_diagnostics Bazel.execute('run', arguments, '//dotnet:format') Conditionally add the --diagnostics argument to the Bazel.execute call in the .NET :lint task only if the enforced_diagnostics array is not empty. rake_tasks/dotnet.rake [130-141] desc 'Run .NET linter (dotnet format analyzers, docs)' task :lint do puts ' Running dotnet format analyzers...' Bazel.execute('run', ['--', 'analyzers', '--verify-no-changes'], '//dotnet:format') Rake::Task['dotnet:docs_generate'].invoke # TODO: Can also identify specific diagnostics to elevate and add to this list # TODO: Add IDE0060 after merging #17019 enforced_diagnostics = [] - arguments = %w[-- style --severity info --verify-no-changes --diagnostics] + enforced_diagnostics + arguments = %w[-- style --severity info --verify-no-changes] + arguments += %w[--diagnostics] + enforced_diagnostics if enforced_diagnostics.any? Bazel.execute('run', arguments, '//dotnet:format') end [Suggestion processed] Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies that the enforced_diagnostics array is currently empty, making the --diagnostics argument ineffective. The proposed change improves code clarity and robustness by making the argument conditional.	Low
Learned best practice	Validate task flags and languages Only accept skip args that start with - and are in the allowed language list (after alias mapping), otherwise raise an explicit error listing valid options. Rakefile [245-252] task :format do |_task, arguments| all_langs = %w[java py rb dotnet node] - skip = arguments.to_a.map { |a| LANG_ALIASES.fetch(a.delete_prefix('-'), a.delete_prefix('-')) } + + raw = arguments.to_a + invalid_flags = raw.reject { |a| a.start_with?('-') } + raise "Invalid args: #{invalid_flags.join(', ')}. Use -<lang> to skip." if invalid_flags.any? + + skip = raw.map { |a| LANG_ALIASES.fetch(a.delete_prefix('-'), a.delete_prefix('-')) } + invalid = skip - all_langs + raise "Unknown languages: #{invalid.join(', ')}. Valid: #{all_langs.join(', ')}" if invalid.any? + (all_langs - skip).each do |lang| puts "Formatting #{lang}..." Rake::Task["#{lang}:format"].invoke end end Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Validate and sanitize CLI/task inputs before use and fail fast with a clear, actionable error when invalid.	Low
	Make workflow step fail fast Add explicit shell: bash plus set -euo pipefail, and fail fast with a clearer error message when formatting produces changes (and when git/repo state is unexpected). .github/workflows/ci-lint.yml [53-60] +shell: bash run: | - # Run format - auto-fixes code style issues + set -euo pipefail + ./go format - # Fail if there were changes (triggers commit-fixes job) - if ! git diff --quiet; then - echo "Format made changes" + + if ! command -v git >/dev/null 2>&1; then + echo "ERROR: git is required to detect formatting changes" >&2 exit 1 fi + if ! git diff --quiet; then + echo "ERROR: formatting changed files; please run './go format' locally and commit the results" >&2 + exit 1 + fi + Suggestion importance[1-10]: 5 __ Why: Relevant best practice - In CI/workflow scripts, enable strict execution (e.g., set -euo pipefail) and validate required assumptions before use, failing fast with a clear error.	Low

[Copilot]

Pull request overview

This PR refactors the build tooling to separate formatting (auto-fix, no-fail) from linting (static checks that can fail). The changes introduce per-language format and lint commands accessible via ./go <lang>:format and ./go <lang>:lint, allowing developers to run only the checks relevant to their work.

Changes:

• Separated format and lint tasks with distinct responsibilities and CI placement
• Added language-specific format/lint commands with optional language skipping via -<lang> flags
• Consolidated CI lint workflows to use new unified task structure

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
scripts/format.sh	Added deprecation notice, removed dotnet and copyright sections, fixed Ruby formatting to use new lint target with fail-level flag
rake_tasks/rust.rake	Split lint task into separate format (rustfmt) and lint (no-op) tasks
rake_tasks/ruby.rake	Split into format (rubocop -a) and lint (rubocop + steep + docs) tasks, separated docs generation from staging
rake_tasks/python.rake	Split into format (ruff format) and lint (ruff check + mypy + docs) tasks, separated docs generation from staging
rake_tasks/node.rake	Split into format (prettier) and lint (eslint + docs) tasks, separated docs generation from staging
rake_tasks/java.rake	Renamed lint to format, added new lint task that only runs docs generation
rake_tasks/dotnet.rake	Enhanced format to include style, added comprehensive lint task with analyzers and style checks
Rakefile	Created top-level format task with copyright/buildifier/rust, refactored lint task, added language aliases and skip logic
.github/workflows/ci-ruby.yml	Consolidated separate docs and type-check jobs into single lint job
.github/workflows/ci-python.yml	Removed separate docs and typing jobs, consolidated into lint job
.github/workflows/ci-lint.yml	Replaced check-format.sh with inline ./go format command
.github/workflows/ci-dotnet.yml	Added new lint job for .NET

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

[titusfortner]

Honestly the filtering piece is much less useful if we have #17035 but I'll merge it anyway