Skip to content

[dotnet] Add linting support with configurable dotnet format#16999

Merged
titusfortner merged 5 commits intotrunkfrom
dotnet_lint
Jan 25, 2026
Merged

[dotnet] Add linting support with configurable dotnet format#16999
titusfortner merged 5 commits intotrunkfrom
dotnet_lint

Conversation

@titusfortner
Copy link
Member

@titusfortner titusfortner commented Jan 25, 2026

User description

Figuring out how to get linting and formatting working across project

💥 What does this PR do?

In #16986 I added support for dotnet's format feature, but it hard coded one setting.

This PR allows the user to pass in parameters just like to native format:

bazel run //dotnet:format                  # everything (format + style + analyzers)
bazel run //dotnet:format -- whitespace    # whitespace only
bazel run //dotnet:format -- style         # style rules only
bazel run //dotnet:format -- analyzers     # analyzers only

The previous also added dotnet to format.sh which adds about 100 seconds to execution.

This PR only runs: bazel run //dotnet:format -- whitespace which is about 30 seconds.

This PR then wraps the typical features with:

  • ./go dotnet:format - runs whitespace-only formatting
  • ./go dotnet:lint - runs full format + style + analyzers

Finally, @nvborisenko created dotnet/.editorconfig last year, I set it to error so it would fail the lint job.

🔧 Implementation Notes

Whitespace-only mode is ~3x faster than full linting, making it suitable for the general format.sh script while full linting can be run separately.

💡 Additional Considerations

This is a precursor to a final lint PR that makes sure everything is handled consistently across bindings and properly run by CI.

More things can be enforced by .editorconfig now if you want (@RenderMichael / @nvborisenko)

🔄 Types of changes

  • Cleanup (formatting, renaming)
  • New feature (non-breaking change which adds functionality)

PR Type

Enhancement, Tests


Description

  • Add configurable dotnet format with argument support

  • Implement separate format (whitespace) and lint (full) tasks

  • Apply linting results to codebase with namespace declarations

  • Enforce file-scoped namespace style via .editorconfig


Diagram Walkthrough

flowchart LR
  A["dotnet_format.bzl"] -- "accept arguments" --> B["format target"]
  B -- "whitespace mode" --> C["format.sh"]
  B -- "full mode" --> D["dotnet.rake"]
  D -- "format task" --> E["whitespace only"]
  D -- "lint task" --> F["format + style + analyzers"]
  G[".editorconfig"] -- "enforce error level" --> F
  H["WebElementWrapper.cs"] -- "apply linting" --> I["file-scoped namespace"]
Loading

File Walkthrough

Relevant files
Enhancement
dotnet_format.bzl
Add argument passing to format target                                       

dotnet/private/dotnet_format.bzl

  • Modified bash and batch scripts to accept and pass arguments to dotnet
    format
  • Updated echo messages to display passed arguments
  • Changed format command invocation to include $@ (bash) and %* (batch)
    parameters
+5/-5     
dotnet.rake
Implement format and lint rake tasks                                         

rake_tasks/dotnet.rake

  • Added format task that runs whitespace-only formatting via bazel
  • Added lint task that runs full format with style and analyzers
  • Both tasks use the dotnet:format bazel target with appropriate
    arguments
+14/-0   
format.sh
Limit format.sh to whitespace mode                                             

scripts/format.sh

  • Updated dotnet format invocation to use whitespace-only mode
  • Changed command from bazel run //dotnet:format to bazel run
    //dotnet:format -- whitespace
  • Updated echo message to reflect whitespace-only formatting
+2/-2     
Configuration changes
.editorconfig
Enforce file-scoped namespace as error                                     

dotnet/.editorconfig

  • Changed csharp_style_namespace_declarations severity from suggestion
    to error
  • Enforces file-scoped namespace declarations as a linting requirement
+1/-1     
Formatting
WebElementWrapper.cs
Apply file-scoped namespace formatting                                     

dotnet/test/common/WebElementWrapper.cs

  • Converted namespace declaration from block-scoped to file-scoped
    syntax
  • Adjusted indentation of class and all members to align with
    file-scoped namespace
  • No functional changes, purely formatting and style compliance
+54/-55 

@selenium-ci selenium-ci added C-dotnet .NET Bindings B-build Includes scripting, bazel and CI integrations labels Jan 25, 2026
@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Jan 25, 2026

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Command injection

Description: The Windows batch wrapper passes user-controlled arguments via %* into a cmd.exe command
line without robust escaping/quoting, which can enable command injection if an attacker
can influence the formatter arguments (e.g., passing & whoami or & del ... would be
interpreted as command chaining by cmd.exe).
dotnet_format.bzl [93-101]

Referred Code
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
)
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
Copy link
Contributor

qodo-code-review bot commented Jan 25, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Simplify and optimize the format script

Instead of iterating through each project file to run dotnet format, run the
command once on a solution (.sln) file. This simplifies the script and is more
efficient.

Examples:

dotnet/private/dotnet_format.bzl [56-59]
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
done || exit 1
dotnet/private/dotnet_format.bzl [94-101]
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
)

Solution Walkthrough:

Before:

# In dotnet_format.bzl (generated shell script)
find "$DOTNET_DIR/src" "$DOTNET_DIR/test" -name "*.csproj" | while read -r proj; do
    echo "  Formatting $proj..."
    "$DOTNET" format "$@" "$proj" || exit 1
done

# In dotnet_format.bzl (generated batch script)
for /r "%DOTNET_DIR%\\src" %%p in (*.csproj) do (
    "%DOTNET%" format %* "%%p" || exit /b 1
)
for /r "%DOTNET_DIR%\\test" %%p in (*.csproj) do (
    "%DOTNET%" format %* "%%p" || exit /b 1
)

After:

# In dotnet_format.bzl (generated shell script)
# Assumes a solution file like 'dotnet/Selenium.sln' exists
echo "Running dotnet format $@ on all projects..."
"$DOTNET" format "$@" "$DOTNET_DIR/Selenium.sln" || exit 1

# In dotnet_format.bzl (generated batch script)
# Assumes a solution file like 'dotnet/Selenium.sln' exists
echo Running dotnet format %* on all projects...
"%DOTNET%" format %* "%DOTNET_DIR%\\Selenium.sln" || exit /b 1
Suggestion importance[1-10]: 7

__

Why: The suggestion offers a valid performance and simplification improvement by recommending the use of a solution file for dotnet format, which is more idiomatic and efficient than iterating over individual project files.

Medium
General
Prevent forwarding arguments to the lint task

Disallow passing arguments to the lint task to ensure it consistently performs
its intended function without unexpected overrides.

rake_tasks/dotnet.rake [124-128]

 desc 'Run .NET linter (format + style + analyzers)'
 task :lint do |_task, arguments|
+  raise ArgumentError, 'arguments not supported for this task' unless arguments.to_a.empty?
+
   puts '  Running dotnet format...'
-  Bazel.execute('run', ['--'] + arguments.to_a, '//dotnet:format')
+  Bazel.execute('run', ['--'], '//dotnet:format')
 end
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion improves the robustness of the new lint task by preventing unintended behavior from arbitrary arguments, aligning it with the implementation of the new format task.

Low
Remove stderr redirection for better error visibility

Remove the 2>/dev/null redirection from the find command to ensure errors, such
as a missing directory, are visible.

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" | while read -r proj; do
     echo "  Formatting $proj..."
     "$DOTNET" format "$@" "$proj" || exit 1
 done || exit 1
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies that suppressing stderr can hide errors, and removing it improves script robustness and ease of debugging.

Low
Learned
best practice
Add null guard for inputs

Guard against null element in the constructor (or primary-constructor body) so
callers get a clear ArgumentNullException instead of later
NullReferenceExceptions.

dotnet/test/common/WebElementWrapper.cs [25-27]

 public class WebElementWrapper(IWebElement element) : IWebElement, IWrapsElement
 {
-    public IWebElement WrappedElement { get; } = element;
+    public IWebElement WrappedElement { get; } = element ?? throw new ArgumentNullException(nameof(element));
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Add explicit validation/null guards at integration boundaries (e.g., public APIs and wrapper types) before using inputs to prevent null-driven runtime failures.

Low
  • Update

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR enhances the .NET binding's linting and formatting capabilities by making the dotnet format tool configurable and separating fast formatting from comprehensive linting. Building on PR #16986, it introduces argument passing to allow different format modes (whitespace, style, analyzers) and optimizes the format script to run only whitespace formatting (~30s) instead of full linting (~100s). The PR also enforces file-scoped namespace declarations via .editorconfig and applies this formatting rule across the codebase.

Changes:

  • Added configurable argument passing to dotnet format bazel target for both Unix and Windows scripts
  • Optimized format.sh to run whitespace-only formatting for faster execution
  • Created separate rake tasks: dotnet:format (whitespace-only) and dotnet:lint (full linting)
  • Enforced file-scoped namespace style by changing .editorconfig severity from suggestion to error
  • Applied the file-scoped namespace formatting to WebElementWrapper.cs as an example

Reviewed changes

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

Show a summary per file
File Description
dotnet/private/dotnet_format.bzl Modified to accept and pass command-line arguments to dotnet format via $@ (bash) and %* (Windows)
scripts/format.sh Updated to invoke dotnet format with whitespace argument for performance optimization
rake_tasks/dotnet.rake Added format task (whitespace-only, no args) and lint task (full linting with optional args)
dotnet/.editorconfig Changed csharp_style_namespace_declarations from suggestion to error to enforce file-scoped namespaces
dotnet/test/common/WebElementWrapper.cs Converted from block-scoped to file-scoped namespace declaration with adjusted indentation (formatting-only change)

@titusfortner titusfortner merged commit c2a394c into trunk Jan 25, 2026
16 checks passed
@titusfortner titusfortner deleted the dotnet_lint branch January 25, 2026 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-build Includes scripting, bazel and CI integrations C-dotnet .NET Bindings Review effort 2/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments