[dotnet] Add bazel rules for dotnet format and paket deps

[titusfortner]

User description

🔗 Related Issues

Part of lint/format tooling improvements

💥 What does this PR do?

Adds hermetic Bazel rules that use the rules_dotnet toolchain:

• dotnet_format: runs dotnet format on src projects via bazel run //dotnet:format
• paket_deps: runs paket update or paket install via bazel run //dotnet:paket-update or bazel run //dotnet:paket-install
• adds //dotnet:format to format.sh file

Both rules generate cross-platform scripts (Unix/Windows) and use the Bazel-managed dotnet SDK rather than requiring a local installation.

🔧 Implementation Notes

• Uses @rules_dotnet//dotnet:toolchain_type to get the hermetic dotnet executable
• Platform detection via ctx.target_platform_has_constraint for Windows vs Unix script generation
• Scripts locate the workspace root via BUILD_WORKSPACE_DIRECTORY when run with bazel run
• The paket_deps rule includes helpful output suggesting the follow-up paket2bazel command

💡 Additional Considerations

• The format target currently hardcodes the two src project paths; could be parameterized if needed
  went ahead and fixed it

🔄 Types of changes

• New feature (non-breaking change which adds functionality)

---

PR Type

Enhancement

---

Description

• Add hermetic Bazel rules for dotnet format and paket dependencies

• Create cross-platform scripts (Unix/Windows) using rules_dotnet toolchain

• Integrate dotnet format into format.sh and Rake build tasks

• Enable paket update/install via bazel run with helpful follow-up guidance

---

Diagram Walkthrough

flowchart LR
  A["dotnet_format.bzl<br/>Unix/Windows scripts"] --> B["dotnet/BUILD.bazel<br/>format target"]
  C["paket_deps.bzl<br/>update/install modes"] --> B
  B --> D["format.sh<br/>integrated"]
  B --> E["Rakefile<br/>rake task"]

Loading

File Walkthrough

	Relevant files
Configuration changes	defs.bzl Export new dotnet format and paket rules                                  dotnet/defs.bzl Import dotnet_format rule from dotnet/private/dotnet_format.bzl Import paket_deps rule from dotnet/private/paket_deps.bzl Export both rules as public API +4/-0      format.sh Integrate dotnet format into format script                              scripts/format.sh Add new "Dotnet" section to format script Execute bazel run //dotnet:format for dotnet source formatting +4/-0      Rakefile Add Rake task for dotnet formatting                                            Rakefile Add new dotnet:format Rake task Execute bazel run //dotnet:format via Bazel integration +5/-0      BUILD.bazel Define format and paket targets                                                    dotnet/BUILD.bazel Import dotnet_format and paket_deps rules Create paket-update target with mode="update" Create paket-install target with mode="install" Create format target for dotnet formatting +15/-1
Enhancement	dotnet_format.bzl New rule for hermetic dotnet format execution                        dotnet/private/dotnet_format.bzl Implement dotnet_format rule using rules_dotnet toolchain Generate platform-specific scripts (bash for Unix, batch for Windows) Locate workspace root via BUILD_WORKSPACE_DIRECTORY environment variable Format two hardcoded src projects: Selenium.WebDriver.csproj and Selenium.WebDriver.Support.csproj +121/-0  paket_deps.bzl New rule for paket dependency management                                  dotnet/private/paket_deps.bzl Implement paket_deps rule with configurable mode attribute (update/install) Generate cross-platform scripts (bash for Unix, batch for Windows) Restore dotnet tools and run paket command with workspace root detection Include helpful output suggesting follow-up paket2bazel command +142/-0

---

[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 f4e21ac

Category	Suggestion	Impact
Possible issue	Support manifest-based runfiles on Windows In the Windows script within dotnet/private/paket_deps.bzl, add logic to resolve the dotnet executable using %RUNFILES_MANIFEST_FILE% as a fallback to support manifest-based runfiles. dotnet/private/paket_deps.bzl [87-91] +dotnet_manifest_key = dotnet_runfiles_path.replace("\\", "/") + script_content = """@echo off setlocal set RUNFILES_DIR=%~dp0%~n0.runfiles -set DOTNET=%RUNFILES_DIR%\\{dotnet_path} +if exist "%RUNFILES_DIR%\\{dotnet_path}" ( + set DOTNET=%RUNFILES_DIR%\\{dotnet_path} +) else if defined RUNFILES_MANIFEST_FILE ( + for /f "usebackq tokens=1,2 delims= " %%A in ("%RUNFILES_MANIFEST_FILE%") do ( + if "%%A"=="{dotnet_manifest_key}" set DOTNET=%%B + ) +) + +if not defined DOTNET ( + echo ERROR: Could not resolve dotnet from runfiles >&2 + exit /b 1 +) + Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion improves the robustness of the generated Windows batch script by adding support for manifest-based runfiles, which makes it compatible with more Bazel configurations.	Medium
	Fix shell safety flags Change set -eufo pipefail to set -euo pipefail in scripts/format.sh to correctly enable shell safety features and prevent script errors. scripts/format.sh [1-3] #!/usr/bin/env bash # Code formatter. -set -eufo pipefail +set -euo pipefail [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that set -eufo pipefail is an invalid combination of shell options and proposes the correct and standard set -euo pipefail to ensure script robustness.	Low
Learned best practice	Deduplicate shared runfiles helpers dotnet_format.bzl and paket_deps.bzl duplicate the same runfiles-path and script boilerplate; extract the shared helpers into a single dotnet/private/runfiles_utils.bzl and load it from both rules to keep behavior consistent. dotnet/private/dotnet_format.bzl [24-28] -def _to_runfiles_path(short_path): - """Convert a short_path to a runfiles path.""" - if short_path.startswith("../"): - return short_path[3:] - return "_main/" + short_path +load("//dotnet/private:runfiles_utils.bzl", "to_runfiles_path") Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Reduce duplication by centralizing shared behavior instead of copying the same helper and script boilerplate across multiple rules.	Low
	Validate external tool availability early Fail fast with a clear error if the resolved dotnet runfiles path doesn’t exist/executable, rather than proceeding and failing later with a less actionable message. dotnet/private/paket_deps.bzl [48] DOTNET="$RUNFILES_DIR/{dotnet}" +if [[ ! -x "$DOTNET" ]]; then + echo "ERROR: dotnet executable not found at $DOTNET" >&2 + exit 1 +fi Apply / Chat Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Add explicit validation/availability guards at integration boundaries (runfiles paths, environment-dependent locations) before invoking external tools.	Low
General	Validate workspace directory layout In dotnet/private/paket_deps.bzl, add a check to ensure the dotnet directory exists before proceeding, providing a more accurate error message if it is not found. dotnet/private/paket_deps.bzl [51-60] WORKSPACE_ROOT="${{BUILD_WORKSPACE_DIRECTORY:-$RUNFILES_DIR/_main}}" DOTNET_DIR="$WORKSPACE_ROOT/dotnet" + +if [[ ! -d "$DOTNET_DIR" ]]; then + echo "ERROR: Could not find dotnet directory at '$DOTNET_DIR'" >&2 + exit 1 +fi if [[ ! -f "$DOTNET_DIR/.config/dotnet-tools.json" ]]; then echo "ERROR: Could not find dotnet/.config/dotnet-tools.json" >&2 echo "Make sure you're running from the workspace root" >&2 exit 1 fi cd "$DOTNET_DIR" Apply / Chat Suggestion importance[1-10]: 5 __ Why: This suggestion improves error handling by adding a check for the dotnet directory, providing a clearer error message if it's missing, which aids in debugging.	Low
More

---

Previous suggestions

✅ Suggestions up to commit 3c535fc

Category	Suggestion	Impact
Possible issue	Support Windows runfiles manifest mode Improve the Windows script's robustness by adding a fallback to resolve the dotnet executable path using the RUNFILES_MANIFEST_FILE, ensuring it works in runfiles manifest mode. dotnet/private/dotnet_format.bzl [78-106] +dotnet_manifest_key = dotnet_runfiles_path.replace("\\", "/") + script_content = """@echo off setlocal set RUNFILES_DIR=%~dp0%~n0.runfiles -set DOTNET=%RUNFILES_DIR%\\{dotnet_path} + +if exist "%RUNFILES_DIR%\\{dotnet_path}" ( + set DOTNET=%RUNFILES_DIR%\\{dotnet_path} +) else if defined RUNFILES_MANIFEST_FILE ( + for /f "usebackq tokens=1,2 delims= " %%A in ("%RUNFILES_MANIFEST_FILE%") do ( + if "%%A"=="{dotnet_manifest_key}" set DOTNET=%%B + ) +) + +if not defined DOTNET ( + echo ERROR: Could not resolve dotnet from runfiles >&2 + exit /b 1 +) if defined BUILD_WORKSPACE_DIRECTORY ( set WORKSPACE_ROOT=%BUILD_WORKSPACE_DIRECTORY% ) else ( set WORKSPACE_ROOT=%RUNFILES_DIR%\\_main ) set DOTNET_DIR=%WORKSPACE_ROOT%\\dotnet cd /d "%DOTNET_DIR%" echo Running dotnet format on all projects... for /r "%DOTNET_DIR%\\src" %%%%p in (*.csproj) do ( echo Formatting %%%%p... "%DOTNET%" format "%%%%p" || exit /b 1 ) for /r "%DOTNET_DIR%\\test" %%%%p in (*.csproj) do ( echo Formatting %%%%p... "%DOTNET%" format "%%%%p" || exit /b 1 ) echo Done. """.format( dotnet_path = dotnet_runfiles_path, + dotnet_manifest_key = dotnet_manifest_key, ) Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that the Windows script only handles runfiles directories and will fail in environments using runfiles manifests, proposing a robust fix that improves portability.	Medium
Learned best practice	✅ Fail fast on missing inputs Suggestion Impact: The commit updated the WORKSPACE_ROOT fallback to use $RUNFILES_DIR/_main, matching the workspace-root approach shown in the suggestion context, but it did not implement the suggested fail-fast checks for $DOTNET_DIR existence or the presence of .csproj files. code diff: -WORKSPACE_ROOT="${{BUILD_WORKSPACE_DIRECTORY:-$(cd "$(dirname "$0")/../.." && pwd)}}" +WORKSPACE_ROOT="${{BUILD_WORKSPACE_DIRECTORY:-$RUNFILES_DIR/_main}}" DOTNET_DIR="$WORKSPACE_ROOT/dotnet" Validate that $DOTNET_DIR exists and that at least one .csproj was found before running, so mis-invocations fail fast with a clear message. dotnet/private/dotnet_format.bzl [49-59] # Find the workspace root WORKSPACE_ROOT="${{BUILD_WORKSPACE_DIRECTORY:-$RUNFILES_DIR/_main}}" DOTNET_DIR="$WORKSPACE_ROOT/dotnet" +if [[ ! -d "$DOTNET_DIR" ]]; then + echo "ERROR: Could not find dotnet directory at $DOTNET_DIR" >&2 + exit 1 +fi + cd "$DOTNET_DIR" echo "Running dotnet format on all projects..." -find "$DOTNET_DIR/src" "$DOTNET_DIR/test" -name "*.csproj" 2>/dev/null | while read -r proj; do +mapfile -t projects < <(find "$DOTNET_DIR/src" "$DOTNET_DIR/test" -name "*.csproj" 2>/dev/null) +if [[ "${#projects[@]}" -eq 0 ]]; then + echo "ERROR: No .csproj files found under $DOTNET_DIR/src or $DOTNET_DIR/test" >&2 + exit 1 +fi +for proj in "${projects[@]}"; do echo " Formatting $proj..." "$DOTNET" format "$proj" || exit 1 -done || exit 1 +done [Suggestion processed] Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Add explicit validation/guards at integration boundaries (derived paths/env vars) before use, and fail with clear errors.	Low
	Validate derived directories exist Also validate %DOTNET_DIR% exists before checking for tool config, so the error message is accurate when the workspace layout is unexpected. dotnet/private/paket_deps.bzl [98-105] set DOTNET_DIR=%WORKSPACE_ROOT%\\dotnet + +if not exist "%DOTNET_DIR%" ( + echo ERROR: Could not find dotnet directory at "%DOTNET_DIR%" >&2 + exit /b 1 +) if not exist "%DOTNET_DIR%\\.config\\dotnet-tools.json" ( echo ERROR: Could not find dotnet\\.config\\dotnet-tools.json >&2 exit /b 1 ) cd /d "%DOTNET_DIR%" Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Add explicit validation/guards at integration boundaries (derived paths/env vars) before use, and fail with clear errors.	Low

Suggestions up to commit b818b18

Category	Suggestion	Impact
Possible issue	Restore tools before formatting Improve the dotnet format script's reliability by first restoring local tools, then attempting to use dotnet tool run dotnet-format, and falling back to the built-in dotnet format command. dotnet/private/dotnet_format.bzl [55-59] +echo "Restoring dotnet tools (if configured)..." +if [[ -f "$DOTNET_DIR/.config/dotnet-tools.json" ]]; then + "$DOTNET" tool restore +fi + +if "$DOTNET" tool run dotnet-format --version >/dev/null 2>&1; then + FORMAT_CMD=( "$DOTNET" tool run dotnet-format ) +else + FORMAT_CMD=( "$DOTNET" format ) +fi + echo "Running dotnet format on all projects..." find "$DOTNET_DIR/src" "$DOTNET_DIR/test" -name "*.csproj" 2>/dev/null | while read -r proj; do echo " Formatting $proj..." - "$DOTNET" format "$proj" || exit 1 + "${FORMAT_CMD[@]}" "$proj" || exit 1 done || exit 1 Suggestion importance[1-10]: 8 __ Why: This suggestion significantly improves the robustness of the formatting script by handling different ways dotnet format can be available, making the rule work reliably across various .NET SDK versions and environments.	Medium
	Support manifest-based runfiles resolution Update the Windows batch script to support runfile manifest files by adding a fallback to resolve the dotnet executable path from %RUNFILES_MANIFEST_FILE% if the runfiles directory does not exist. dotnet/private/paket_deps.bzl [90-91] set RUNFILES_DIR=%~dp0%~n0.runfiles -set DOTNET=%RUNFILES_DIR%\\{dotnet_path} +if exist "%RUNFILES_DIR%\\{dotnet_path}" ( + set DOTNET=%RUNFILES_DIR%\\{dotnet_path} +) else if defined RUNFILES_MANIFEST_FILE ( + for /f "usebackq tokens=1,2 delims= " %%A in ("%RUNFILES_MANIFEST_FILE%") do ( + if "%%A"=="{dotnet_manifest_key}" set DOTNET=%%B + ) +) + +if not defined DOTNET ( + echo ERROR: Could not resolve dotnet from runfiles >&2 + exit /b 1 +) + Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that runfiles on Windows can be manifest-based and adds a fallback mechanism, which improves the robustness of the generated Windows batch script.	Medium
	Remove hardcoded runfiles prefix Replace the hardcoded _main/ runfiles prefix with a dynamic path using ctx.workspace_name to improve the rule's robustness across different workspace configurations. dotnet/private/dotnet_format.bzl [24-28] -def _to_runfiles_path(short_path): +def _to_runfiles_path(workspace_name, short_path): """Convert a short_path to a runfiles path.""" if short_path.startswith("../"): return short_path[3:] - return "_main/" + short_path + return workspace_name + "/" + short_path Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that hardcoding the _main workspace name is not robust and proposes using ctx.workspace_name instead, which is a good practice for creating reusable Bazel rules.	Low
Learned best practice	Centralize duplicated helper logic Both dotnet_format.bzl and paket_deps.bzl duplicate runfiles-path logic and script boilerplate; extract shared helpers into a small runfiles_utils.bzl and load them from both rules to keep behavior consistent. dotnet/private/paket_deps.bzl [24-28] -def _to_runfiles_path(short_path): - """Convert a short_path to a runfiles path.""" - if short_path.startswith("../"): - return short_path[3:] - return "_main/" + short_path +load("//dotnet/private:runfiles_utils.bzl", "to_runfiles_path") +# ... + +dotnet_runfiles_path = to_runfiles_path(ctx.workspace_name, dotnet.short_path) + Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Reduce duplication by centralizing shared behavior (avoid copy-pasted helpers across Bazel rules).	Low
	Validate external paths before use Validate that the computed DOTNET path exists and is executable (and fail with a clear error) before invoking it, to avoid confusing failures when runfiles are incomplete/mislocated. dotnet/private/dotnet_format.bzl [47-51] DOTNET="$RUNFILES_DIR/{dotnet}" +if [[ ! -x "$DOTNET" ]]; then + echo "ERROR: dotnet executable not found or not executable at $DOTNET" >&2 + exit 1 +fi # Find the workspace root WORKSPACE_ROOT="${{BUILD_WORKSPACE_DIRECTORY:-$(cd "$(dirname "$0")/../.." && pwd)}}" DOTNET_DIR="$WORKSPACE_ROOT/dotnet" Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Add explicit validation/guards at integration boundaries (paths/env/runfiles) before use.	Low

✅ Suggestions up to commit b70828e

Category	Suggestion	Impact
Possible issue	✅ Fix workspace root resolution Suggestion Impact: The commit replaced the previous relative-path-based workspace root discovery with a RUNFILES_DIR-based path ($RUNFILES_DIR/_main), aligning with the suggestion’s intent to use the Bazel runfiles tree for more robust workspace root resolution. It did not add the suggested existence check for the dotnet/ directory. code diff: # Find the workspace root -WORKSPACE_ROOT="${{BUILD_WORKSPACE_DIRECTORY:-$(cd "$(dirname "$0")/../.." && pwd)}}" +WORKSPACE_ROOT="${{BUILD_WORKSPACE_DIRECTORY:-$RUNFILES_DIR/_main}}" DOTNET_DIR="$WORKSPACE_ROOT/dotnet" Replace the brittle workspace root discovery logic with a more robust method using the Bazel runfiles tree to prevent unexpected failures. dotnet/private/dotnet_format.bzl [49-51] # Find the workspace root -WORKSPACE_ROOT="${{BUILD_WORKSPACE_DIRECTORY:-$(cd "$(dirname "$0")/../.." && pwd)}}" +WORKSPACE_ROOT="${BUILD_WORKSPACE_DIRECTORY:-$RUNFILES_DIR/{workspace}}" DOTNET_DIR="$WORKSPACE_ROOT/dotnet" +if [[ ! -d "$DOTNET_DIR" ]]; then + echo "ERROR: Could not find dotnet/ directory at $DOTNET_DIR" >&2 + exit 1 +fi + Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a brittle implementation for finding the workspace root, which can fail in certain Bazel execution environments, and proposes a robust, standard fix using the runfiles tree.	Medium
	✅ Resolve repo root via runfiles Suggestion Impact: The commit updated WORKSPACE_ROOT to derive from RUNFILES_DIR (using _main), aligning with the suggestion to resolve the repo root via Bazel runfiles rather than a relative path. It did not add the suggested dotnet/ directory existence check. code diff: # Find the workspace root (where dotnet/.config/dotnet-tools.json lives) -WORKSPACE_ROOT="${{BUILD_WORKSPACE_DIRECTORY:-$(cd "$(dirname "$0")/../.." && pwd)}}" +WORKSPACE_ROOT="${{BUILD_WORKSPACE_DIRECTORY:-$RUNFILES_DIR/_main}}" DOTNET_DIR="$WORKSPACE_ROOT/dotnet" Replace the brittle workspace root discovery logic with a more robust method using the Bazel runfiles tree to prevent unexpected failures. dotnet/private/paket_deps.bzl [50-52] # Find the workspace root (where dotnet/.config/dotnet-tools.json lives) -WORKSPACE_ROOT="${{BUILD_WORKSPACE_DIRECTORY:-$(cd "$(dirname "$0")/../.." && pwd)}}" +WORKSPACE_ROOT="${BUILD_WORKSPACE_DIRECTORY:-$RUNFILES_DIR/{workspace}}" DOTNET_DIR="$WORKSPACE_ROOT/dotnet" +if [[ ! -d "$DOTNET_DIR" ]]; then + echo "ERROR: Could not find dotnet/ directory at $DOTNET_DIR" >&2 + exit 1 +fi + Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a brittle implementation for finding the workspace root, which can fail in certain Bazel execution environments, and proposes a robust, standard fix using the runfiles tree.	Medium
Learned best practice	Remove hardcoded runfiles workspace Remove the hardcoded "_main/" and build the runfiles path using ctx.workspace_name so the rule works with non-default workspace names. dotnet/private/paket_deps.bzl [24-28] -def _to_runfiles_path(short_path): +def _to_runfiles_path(workspace_name, short_path): """Convert a short_path to a runfiles path.""" if short_path.startswith("../"): return short_path[3:] - return "_main/" + short_path + return workspace_name + "/" + short_path Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Avoid hardcoded Bazel runfiles workspace names; resolve runfiles paths using the actual workspace name for robustness across Bazel configurations.	Low

Suggestions up to commit 084192d

Category	Suggestion	Impact
Possible issue	Fix Windows runfiles tool path Correct the DOTNET tool path in the Windows batch script by using %~dp0%~n0.runfiles\{dotnet_path} to properly locate the runfiles directory. dotnet/private/paket_deps.bzl [90] -set DOTNET=%~dp0{dotnet_path} +set DOTNET=%~dp0%~n0.runfiles\{dotnet_path} Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a bug in the Windows batch script where the DOTNET tool path is constructed incorrectly, which would cause the script to fail.	Medium
	Stop hardcoding runfiles workspace Replace the hardcoded _main in _to_runfiles_path with ctx.workspace_name to make runfile path resolution more robust across different Bazel configurations. dotnet/private/dotnet_format.bzl [24-28] -def _to_runfiles_path(short_path): +def _to_runfiles_path(workspace_name, short_path): """Convert a short_path to a runfiles path.""" if short_path.startswith("../"): return short_path[3:] - return "_main/" + short_path + return workspace_name + "/" + short_path Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that hardcoding _main for the runfiles path is not robust and proposes using ctx.workspace_name for a more reliable implementation.	Low
	Fix runfiles directory detection Modify the runfiles detection logic in the generated bash script to check for the existence of $0.runfiles instead of $0.runfiles/_main for better compatibility. dotnet/private/dotnet_format.bzl [34-47] script_content = """#!/usr/bin/env bash set -euo pipefail # Locate runfiles directory -if [[ -d "$0.runfiles/_main" ]]; then +if [[ -d "$0.runfiles" ]]; then RUNFILES_DIR="$0.runfiles" elif [[ -n "${{RUNFILES_DIR:-}}" ]]; then RUNFILES_DIR="$RUNFILES_DIR" else echo "ERROR: Could not locate runfiles directory" >&2 exit 1 fi DOTNET="$RUNFILES_DIR/{dotnet}" Suggestion importance[1-10]: 6 __ Why: The suggestion correctly points out a brittle runfiles detection logic in the bash script that relies on a hardcoded _main directory, improving robustness.	Low
Learned best practice	Make path iteration whitespace-safe Avoid piping find into read without null delimiting, since project paths with spaces/newlines can break; use -print0 and a NUL-safe read loop. dotnet/private/dotnet_format.bzl [56-59] -find "$DOTNET_DIR/src" "$DOTNET_DIR/test" -name "*.csproj" 2>/dev/null | while read -r proj; do +find "$DOTNET_DIR/src" "$DOTNET_DIR/test" -name "*.csproj" -print0 2>/dev/null | while IFS= read -r -d '' proj; do echo " Formatting $proj..." "$DOTNET" format "$proj" done Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Add validation/robust input handling at integration boundaries (e.g., safely iterate filesystem paths and avoid whitespace-sensitive parsing).	Low
	Fix Windows error handling in loops %ERRORLEVEL% inside a parenthesized block can be expanded once at parse time; use if errorlevel 1 or delayed expansion (or || exit /b) so failures actually stop the loop. dotnet/private/dotnet_format.bzl [93-102] +setlocal EnableDelayedExpansion +... for /r "%DOTNET_DIR%\\src" %%%%p in (*.csproj) do ( echo Formatting %%%%p... "%DOTNET%" format "%%%%p" - if %ERRORLEVEL% neq 0 exit /b %ERRORLEVEL% + if errorlevel 1 exit /b !ERRORLEVEL! ) for /r "%DOTNET_DIR%\\test" %%%%p in (*.csproj) do ( echo Formatting %%%%p... "%DOTNET%" format "%%%%p" - if %ERRORLEVEL% neq 0 exit /b %ERRORLEVEL% + if errorlevel 1 exit /b !ERRORLEVEL! ) Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Make platform/lifecycle-sensitive scripting robust by avoiding batch-variable expansion pitfalls inside parentheses.	Low

✅ Suggestions up to commit 4cd3ab0

Category	Suggestion	Impact
High-level	Abstract common Bazel rule logic Abstract the shared boilerplate logic for platform detection, script generation, and workspace discovery from dotnet_format.bzl and paket_deps.bzl into a common helper function or generic rule to reduce duplication. Examples: dotnet/private/dotnet_format.bzl [3-121] def _dotnet_format_impl(ctx): toolchain = ctx.toolchains["@rules_dotnet//dotnet:toolchain_type"] dotnet = toolchain.runtime.files_to_run.executable is_windows = ctx.target_platform_has_constraint(ctx.attr._windows_constraint[platform_common.ConstraintValueInfo]) if is_windows: script = _create_windows_script(ctx, dotnet) else: script = _create_unix_script(ctx, dotnet) ... (clipped 109 lines) dotnet/private/paket_deps.bzl [3-142] def _paket_deps_impl(ctx): toolchain = ctx.toolchains["@rules_dotnet//dotnet:toolchain_type"] dotnet = toolchain.runtime.files_to_run.executable is_windows = ctx.target_platform_has_constraint(ctx.attr._windows_constraint[platform_common.ConstraintValueInfo]) if is_windows: script = _create_windows_script(ctx, dotnet) else: script = _create_unix_script(ctx, dotnet) ... (clipped 130 lines) Solution Walkthrough: Before: # In dotnet/private/dotnet_format.bzl def _dotnet_format_impl(ctx): # ... platform detection, toolchain setup ... if is_windows: script = _create_windows_script(ctx, dotnet) else: script = _create_unix_script(ctx, dotnet) # ... return DefaultInfo ... def _create_unix_script(ctx, dotnet): script_content = """ # ... boilerplate for runfiles and workspace root ... cd "$DOTNET_DIR" for proj in ...; do "$DOTNET" format "$DOTNET_DIR/$proj" done """ # ... write script file ... # In dotnet/private/paket_deps.bzl def _paket_deps_impl(ctx): # ... (Identical) platform detection, toolchain setup ... if is_windows: script = _create_windows_script(ctx, dotnet) else: script = _create_unix_script(ctx, dotnet) # ... (Identical) return DefaultInfo ... def _create_unix_script(ctx, dotnet): script_content = """ # ... (Identical) boilerplate for runfiles and workspace root ... cd "$DOTNET_DIR" "$DOTNET" tool restore "$DOTNET" tool run paket {mode} """ # ... (Identical) write script file ... After: # In a new shared file, e.g., dotnet/private/hermetic_script.bzl def _create_hermetic_script(ctx, unix_commands, windows_commands): # ... platform detection, toolchain setup ... if is_windows: script_content = """ @echo off # ... boilerplate for workspace root ... cd /d "%DOTNET_DIR%" {commands} """.format(commands=windows_commands) else: script_content = """ #!/usr/bin/env bash # ... boilerplate for runfiles and workspace root ... cd "$DOTNET_DIR" {commands} """.format(commands=unix_commands) # ... write script file and return it ... # In dotnet/private/dotnet_format.bzl def _dotnet_format_impl(ctx): unix_cmds = "for proj in ...; do ... done" win_cmds = "for %%p in (...) do (...)" script = _create_hermetic_script(ctx, unix_cmds, win_cmds) # ... return DefaultInfo ... # In dotnet/private/paket_deps.bzl def _paket_deps_impl(ctx): unix_cmds = '"$DOTNET" tool restore\n"$DOTNET" tool run paket {mode}' win_cmds = '"%DOTNET%" tool restore\n"%DOTNET%" tool run paket {mode}' script = _create_hermetic_script(ctx, unix_cmds, win_cmds) # ... return DefaultInfo ... Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies significant code duplication across the two new Bazel rule files, dotnet_format.bzl and paket_deps.bzl, and abstracting this logic would substantially improve code quality and maintainability.	Medium
General	Format solution instead of hardcoded projects Instead of formatting hardcoded project files, format the solution file (Selenium.sln) to automatically include all projects and improve maintainability. dotnet/private/dotnet_format.bzl [55-59] -echo "Running dotnet format on src projects..." -for proj in src/webdriver/Selenium.WebDriver.csproj src/support/Selenium.WebDriver.Support.csproj; do - echo " Formatting $proj..." - "$DOTNET" format "$DOTNET_DIR/$proj" -done +echo "Running dotnet format on solution..." +"$DOTNET" format Selenium.sln Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that hardcoding project paths is not scalable and proposes a more maintainable approach by using the solution file, which improves the rule's robustness.	Medium
Learned best practice	Centralize duplicated helper code The identical _to_runfiles_path helper is duplicated across multiple rule files; move it into a shared .bzl utility module and load() it from both places to keep behavior consistent and reduce maintenance. dotnet/private/paket_deps.bzl [24-28] -def _to_runfiles_path(short_path): - """Convert a short_path to a runfiles path.""" - if short_path.startswith("../"): - return short_path[3:] - return "_main/" + short_path +load("//dotnet/private:runfiles_utils.bzl", "to_runfiles_path") Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Reduce duplication by centralizing shared utilities/helpers.	Low
	✅ Add path existence validations Suggestion Impact: The commit does not add the requested existence checks, but it does adjust how WORKSPACE_ROOT is derived (using RUNFILES_DIR/_main instead of computing via script path). This change appears to address the same underlying concern about fragile env/runfiles assumptions, reducing the chance of confusing runtime path failures, albeit without the explicit validation/errors requested. code diff: # Find the workspace root -WORKSPACE_ROOT="${{BUILD_WORKSPACE_DIRECTORY:-$(cd "$(dirname "$0")/../.." && pwd)}}" +WORKSPACE_ROOT="${{BUILD_WORKSPACE_DIRECTORY:-$RUNFILES_DIR/_main}}" DOTNET_DIR="$WORKSPACE_ROOT/dotnet" Validate that DOTNET_DIR exists and each project file is present before cd/formatting, and fail with a clear error message to avoid confusing runtime failures when env/runfiles assumptions don't hold. dotnet/private/dotnet_format.bzl [50-59] WORKSPACE_ROOT="${{BUILD_WORKSPACE_DIRECTORY:-$(cd "$(dirname "$0")/../.." && pwd)}}" DOTNET_DIR="$WORKSPACE_ROOT/dotnet" + +if [[ ! -d "$DOTNET_DIR" ]]; then + echo "ERROR: Could not find dotnet directory at: $DOTNET_DIR" >&2 + exit 1 +fi cd "$DOTNET_DIR" echo "Running dotnet format on src projects..." for proj in src/webdriver/Selenium.WebDriver.csproj src/support/Selenium.WebDriver.Support.csproj; do + if [[ ! -f "$DOTNET_DIR/$proj" ]]; then + echo "ERROR: Missing project file: $DOTNET_DIR/$proj" >&2 + exit 1 + fi echo " Formatting $proj..." "$DOTNET" format "$DOTNET_DIR/$proj" done Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Add explicit validation/guards at integration boundaries (e.g., environment variables, filesystem paths) before use.	Low

[Copilot]

Pull request overview

This PR adds hermetic Bazel rules for .NET development tooling, enabling automated formatting and dependency management without requiring local .NET SDK installations.

Changes:

• Adds dotnet_format rule for running dotnet format on Selenium WebDriver source projects
• Adds paket_deps rule for managing .NET dependencies via paket (update/install modes)
• Integrates dotnet formatting into the repository's formatting workflow via format.sh and Rakefile

Reviewed changes

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

Show a summary per file

File	Description
dotnet/private/dotnet_format.bzl	New Bazel rule that generates cross-platform scripts to run dotnet format on WebDriver and Support projects
dotnet/private/paket_deps.bzl	New Bazel rule that generates cross-platform scripts to run paket update/install commands
dotnet/defs.bzl	Exports the new dotnet_format and paket_deps rules for use in BUILD files
dotnet/BUILD.bazel	Defines targets for format, paket-update, and paket-install using the new rules
scripts/format.sh	Adds dotnet format to the repository-wide formatting script
Rakefile	Adds rake task for running dotnet format via Bazel

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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