[dotnet] run dotnet-format adopting all suggestions#17011
Closed
titusfortner wants to merge 13 commits intotrunkfrom
Closed
[dotnet] run dotnet-format adopting all suggestions#17011titusfortner wants to merge 13 commits intotrunkfrom
titusfortner wants to merge 13 commits intotrunkfrom
Conversation
selenium-ci
added
C-dotnet
Member
|
Thank you, @titusfortner for this code suggestion. The support packages contain example code that many users find helpful, but they do not necessarily represent After reviewing the change, unless it is a critical fix or a feature that is needed for Selenium We actively encourage people to add the wrapper and helper code that makes sense for them to their own frameworks. |
Contributor
User descriptionI did not understand what I was doing in #16999 This turns on formatting for suggestion/info level, and this is what it spits out. I have no idea if we actually want this, but I'd really like to have some kind of enforceable style for dotnet. If this is not the one you want, I think we can tweak .editor.config values to get what we want? I think? Run this from trunk to see what the rules are it is flagging on: @nvborisenko & @RenderMichael which of these do we want to enforce vs ignore? PR TypeEnhancement DescriptionComprehensive modernization of .NET codebase to adopt modern C# syntax and patterns:
Diagram Walkthroughflowchart LR
A["Legacy C# Syntax<br/>new List<T>()<br/>new Type()<br/>!(x is T)<br/>using statements"] -->|Primary Constructors| B["Modern C# Syntax<br/>Primary constructors<br/>Collection expressions<br/>Pattern matching<br/>Using declarations"]
A -->|Collection Init| C["Modern Collections<br/>[] syntax<br/>new() expressions<br/>Initializer syntax"]
A -->|Event Handlers| D["Simplified Events<br/>?.Invoke()<br/>Local functions<br/>Null-coalescing"]
B --> E["Cleaner, More<br/>Maintainable Code"]
C --> E
D --> E
|
| Relevant files | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Formatting | 14 files
| |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Enhancement | 39 files
|
Contributor
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||||||||||||
Contributor
There was a problem hiding this comment.
Pull request overview
This PR applies dotnet-format suggestions at info/suggestion level across the .NET bindings and tests, modernizing syntax (primary constructors, target-typed new, collection expressions, pattern matching, null-coalescing, etc.) and wiring the .NET Rake format task to also run style rules at --severity info. Most changes are mechanical style updates intended to match .editorconfig preferences, with a few small cleanups (event invocation null-propagation, unused members removal, simplified using/StringBuilder patterns).
Changes:
- Extend the
dotnetRake:formattask to run both whitespace-only and style formatting (dotnet format --severity info), so style suggestions become enforceable. - Apply analyzer-driven style changes throughout
dotnet/src/webdriver(primary constructors, target-typednew, object/collection initializers, null-conditional/event invocation, use of spans/slices, expression-bodied and pattern-based updates). - Apply the same style/simplification pass across a large set of .NET tests (
dotnet/test/...), including using discards for unused values, collection expressions, target-typednew, simplified arithmetic, and more idiomatic C# constructs.
Reviewed changes
Copilot reviewed 239 out of 239 changed files in this pull request and generated 31 comments.
Show a summary per file
| File | Description |
|---|---|
rake_tasks/dotnet.rake |
Extends :format to run bazel run //dotnet:format -- whitespace and then -- style --severity info, enabling style-level enforcement for .NET code. |
dotnet/test/support/UI/WebDriverWaitTest.cs |
Refactors a local Func to a static local method and converts TickingClock to a primary-constructor-based implementation with expression/auto-property simplifications. |
dotnet/test/support/UI/SlowLoadableComponentTest.cs |
Uses primary constructors and target-typed new for nested test helper types, and simplifies assignments and constructor wiring. |
dotnet/test/support/UI/PopupWindowFinderTest.cs |
Uses target-typed new for PopupWindowFinder instances. |
dotnet/test/support/UI/LoadableComponentTests.cs |
Converts nested LoadsOk helper to use a primary constructor and target-typed new. |
dotnet/test/support/UI/HandCrankClock.cs |
Uses target-typed new and compound assignment for the fake clock. |
dotnet/test/support/Extensions/ExecuteJavaScriptTest.cs |
Converts a subclass of ReadOnlyCollection<object> to a primary constructor. |
dotnet/test/remote/RemoteWebDriverSpecificTests.cs |
Uses an object initializer with target-typed new to set Url on RemoteWebDriver. |
dotnet/test/remote/RemoteSessionCreationTests.cs |
Uses object initializers/target-typed new for remote driver instances and dictionary creation. |
dotnet/test/ie/IeSpecificTests.cs |
Replaces new List<T>() { ... } with C# collection expressions and target-typed new for Actions. |
dotnet/test/firefox/FirefoxProfileTests.cs |
Uses the [] collection expression to initialize a list. |
dotnet/test/firefox/FirefoxDriverServiceTest.cs |
Uses target-typed new and object initializer for FirefoxOptions. |
dotnet/test/common/WindowSwitchingTest.cs |
Uses [] collection expression for tracking seen window handles. |
dotnet/test/common/VisibilityTest.cs |
Converts multiple string[] initializations to collection expressions. |
dotnet/test/common/UnexpectedAlertBehaviorTest.cs |
Uses target-typed new for UnhandledPromptBehaviorOptions. |
dotnet/test/common/TypingTest.cs |
Replaces unused locals with discards to satisfy analyzers. |
dotnet/test/common/TextPagesTest.cs |
Uses target-typed new for Cookie construction. |
dotnet/test/common/TestUtilities.cs |
Uses range slicing ([..]) and inline out variable declaration in double.TryParse. |
dotnet/test/common/TakesScreenshotTest.cs |
Uses collection expressions for HashSet<string> creation. |
dotnet/test/common/PrintTest.cs |
Uses object initializer for PrintOptions. |
dotnet/test/common/PositionAndSizeTest.cs |
Replaces unused coordinate local with a discard. |
dotnet/test/common/PageLoadingTest.cs |
Collapses null-check and Quit into null-conditional, uses object initializer for PageLoadStrategyOptions. |
dotnet/test/common/ObjectStateAssumptionsTest.cs |
Uses discards for values retrieved solely for side-effects/assumption checks. |
dotnet/test/common/NetworkInterceptionTests.cs |
Uses target-typed new with object/initializer syntax for network handler types. |
dotnet/test/common/NavigationTest.cs |
Uses target-typed new for Uri instances. |
dotnet/test/common/MiscTest.cs |
Uses collection expression for a list of string values. |
dotnet/test/common/JavascriptEnabledBrowserTest.cs |
Simplifies EqualConstraint creation and modernizes is not patterns. |
dotnet/test/common/Internal/Logging/FileLogHandlerTest.cs |
Refactors the exception-asserting lambda into a named local function and passes it to Assert.That with a Throws.ArgumentException constraint. |
dotnet/test/common/Interactions/DragAndDropTest.cs |
Uses target-typed new for Actions, Point, and Size values. |
dotnet/test/common/Interactions/CombinedInputActionsTest.cs |
Uses target-typed new for Actions instances. |
dotnet/test/common/Interactions/BasicWheelInterfaceTest.cs |
Uses target-typed new and object initializer for WheelInputDevice.ScrollOrigin. |
dotnet/test/common/Interactions/BasicKeyboardInterfaceTest.cs |
Uses target-typed new for Actions, replaces unused locals with discards, and removes an unused helper. |
dotnet/test/common/Interactions/ActionBuilderTest.cs |
Uses target-typed new for ActionBuilder. |
dotnet/test/common/ImplicitWaitTest.cs |
Uses target-typed new for the list of window handles. |
dotnet/test/common/GetLogsTest.cs |
Uses collection expression for the log dictionary and target-typed new for ChromeOptions. |
dotnet/test/common/FrameSwitchingTest.cs |
Uses range slicing ([..^1]) instead of Substring. |
dotnet/test/common/FormHandlingTests.cs |
Uses target-typed new for FileInfo objects and drops an unused emptyTextArea local. |
dotnet/test/common/Environment/UrlBuilder.cs |
Uses target-typed new for Uri construction. |
dotnet/test/common/Environment/InlinePage.cs |
Uses collection expressions for internal lists and target-typed new for StringBuilder. |
dotnet/test/common/Environment/EnvironmentManager.cs |
Uses target-typed new for DirectoryInfo and null-conditional invocation for DriverStarting. |
dotnet/test/common/Environment/DriverFactory.cs |
Uses collection expressions for dictionaries and lists, and simplifies constructor reflection and options/service creation. |
dotnet/test/common/ElementSelectingTest.cs |
Removes unused constants/properties to satisfy analyzers. |
dotnet/test/common/ElementAttributeTest.cs |
Drops an unused pElement local and uses collection expression for acceptable onclick values. |
dotnet/test/common/DownloadsTest.cs |
Uses target-typed new for WebDriverWait and DownloadableFilesOptions (with object initializer). |
dotnet/test/common/DevTools/DevToolsTargetTest.cs |
Uses target-typed new delegates and ManualResetEventSlim, converting inline lambdas to named local functions. |
dotnet/test/common/DevTools/DevToolsSecurityTest.cs |
Same pattern: target-typed new for ManualResetEventSlim and named local handler function. |
dotnet/test/common/DevTools/DevToolsProfilerTest.cs |
Same as above for profiler console profile events. |
dotnet/test/common/DevTools/DevToolsLogTest.cs |
Same pattern for log entry-added events. |
dotnet/test/common/DevTools/DevToolsConsoleTest.cs |
Same pattern for console message-added events. |
dotnet/test/common/CustomDriverConfigs/* |
Uses target-typed new in static DefaultOptions properties and object initializers where appropriate. |
dotnet/test/common/CorrectEventFiringTest.cs |
Uses list collection expressions and target-typed new for StringBuilder in event ordering tests. |
dotnet/test/common/CookieTest.cs |
Uses target-typed new for Cookie construction and discards unused ReturnedCookie instance. |
dotnet/test/common/ClickScrollingTest.cs |
Replaces an unused size local with a discard. |
dotnet/test/common/BiDi/Network/NetworkTest.cs |
Changes several async exception assertions to use Func<Task<NavigateResult>> + Assert.That(..., Throws...) instead of an async lambda; this potentially alters how NUnit awaits and evaluates exceptions (see comments). |
dotnet/test/common/BiDi/BrowsingContext/BrowsingContextTest.cs |
Uses discards for unused created windows to satisfy analyzers. |
dotnet/test/common/AlertsTest.cs |
Removes unused helper methods related to onbeforeunload alerts. |
dotnet/src/webdriver/Window.cs |
Converts to a primary-constructor class with a readonly driver field and uses object-initializer syntax for the window rect parameter dictionaries. |
dotnet/src/webdriver/WebElementFactory.cs |
Uses primary constructor and property initializer for ParentDriver. |
dotnet/src/webdriver/WebDriverError.cs |
Uses target-typed new for the resultMap dictionary. |
dotnet/src/webdriver/VirtualAuth/VirtualAuthenticatorOptions.cs |
Uses dictionary index initializers for ToDictionary(). |
dotnet/src/webdriver/VirtualAuth/Credential.cs |
Uses dictionary index initializers for ToDictionary(). |
dotnet/src/webdriver/UserAgent.cs |
Uses primary constructor and property initializer for UserAgentString. |
dotnet/src/webdriver/Timeouts.cs |
Converts to a primary-constructor-based type, removes an unused legacy constant, and simplifies timeout parameter dictionary creation. |
dotnet/src/webdriver/Support/DefaultWait{T}.cs |
Uses collection expression for ignoredExceptions. |
dotnet/src/webdriver/StaleElementReferenceException.cs |
Makes supportUrl readonly. |
dotnet/src/webdriver/ShadowRoot.cs |
Uses a primary constructor and object-initializer syntax for command parameter dictionaries. |
dotnet/src/webdriver/SessionId.cs |
Uses primary constructor with readonly backing field. |
dotnet/src/webdriver/SeleniumManager.cs |
Uses target-typed new for StringBuilder, Process, and event handlers. |
dotnet/src/webdriver/Screenshot.cs |
Converts to a primary-constructor-derived class from EncodedFile with richer XML docs. |
dotnet/src/webdriver/Safari/SafariDriverService.cs |
Uses target-typed new for StringBuilder. |
dotnet/src/webdriver/Safari/SafariDriver.cs |
Uses target-typed new for DriverFinder, request dictionaries, and capability dictionaries. |
dotnet/src/webdriver/Response.cs |
Uses primary constructor and property initializers for SessionId, Value, and Status. |
dotnet/src/webdriver/Remote/SendingRemoteHttpRequestEventArgs.cs |
Converts to primary-constructor event args, uses dictionary field initialization, and inlines header null checks. |
dotnet/src/webdriver/Remote/RemoteSessionSettings.cs |
Uses collection and dictionary expressions; builds capability dictionaries with collection expressions. |
dotnet/src/webdriver/Remote/ReadOnlyDesiredCapabilities.cs |
Uses collection expression for the capabilities dictionary. |
dotnet/src/webdriver/Remote/DesiredCapabilities.cs |
Same as above. |
dotnet/src/webdriver/RelativeBy.cs |
Uses collection/dictionary expressions for filters and parameters, and simplifies dictionary creation. |
dotnet/src/webdriver/Proxy.cs |
Uses collection expressions for proxy bypass lists and capability serialization. |
dotnet/src/webdriver/PrintDocument.cs |
Converts to primary-constructor-based class and modern using declarations. |
dotnet/src/webdriver/PasswordCredentials.cs |
Uses a primary constructor to set UserName and Password, leaving the parameterless ctor available. |
dotnet/src/webdriver/OptionsManager.cs |
Converts to primary constructor and readonly driver field. |
dotnet/src/webdriver/NoSuchElementException.cs |
Makes supportUrl readonly. |
dotnet/src/webdriver/NoSuchDriverException.cs |
Makes supportUrl readonly. |
dotnet/src/webdriver/NetworkResponseReceivedEventArgs.cs |
Uses collection expression for headers dictionary. |
dotnet/src/webdriver/NetworkRequestSentEventArgs.cs |
Same for request headers. |
dotnet/src/webdriver/Navigator.cs |
Uses primary constructor for driver and target-typed new for navigation parameters. |
dotnet/src/webdriver/Logs.cs |
Uses primary constructor plus collection expressions for available log types and log entries. |
dotnet/src/webdriver/LogEntry.cs |
Uses target-typed new in FromDictionary. |
dotnet/src/webdriver/JavaScriptExceptionThrownEventArgs.cs |
Uses primary constructor and a property initializer. |
dotnet/src/webdriver/JavaScriptConsoleApiCalledEventArgs.cs |
Same pattern for console API event args. |
dotnet/src/webdriver/JavaScriptCallbackExecutedEventArgs.cs |
Same pattern for callback executed event args. |
dotnet/src/webdriver/InvalidSelectorException.cs |
Makes supportUrl readonly. |
dotnet/src/webdriver/Internal/ReturnedCookie.cs |
Converts to primary-constructor-based derived class with expanded XML documentation. |
dotnet/src/webdriver/Internal/ReturnedCapabilities.cs |
Uses collection expression for capabilities dictionary. |
dotnet/src/webdriver/Internal/ResourceUtilities.cs |
Uses modern using-declaration style while preserving platform detection semantics. |
dotnet/src/webdriver/Internal/Logging/TextWriterHandler.cs |
Uses collection expression for _levels. |
dotnet/src/webdriver/Internal/Logging/Logger.cs |
Uses primary constructor and auto-properties. |
dotnet/src/webdriver/Internal/Logging/LogEvent.cs |
Uses primary constructor and property initializers. |
dotnet/src/webdriver/Internal/Logging/Log.cs |
Uses target-typed new for LogContextManager. |
dotnet/src/webdriver/Internal/Logging/FileLogHandler.cs |
Uses collection expression for _levels and target-typed new for _lockObj. |
dotnet/src/webdriver/Internal/FileUtilities.cs |
Uses target-typed new for DirectoryInfo. |
dotnet/src/webdriver/Internal/Base64UrlEncoder.cs |
Uses collection expression syntax for the base64 character table. |
dotnet/src/webdriver/Interactions/WheelInputDevice.cs |
Converts to primary constructor and uses dictionary initializer in ToDictionary. |
dotnet/src/webdriver/Interactions/PauseInteraction.cs |
Uses dictionary initializer in ToDictionary. |
dotnet/src/webdriver/Interactions/Actions.cs |
Uses target-typed new for ActionBuilder and null-coalescing assignment (target ??=). |
dotnet/src/webdriver/Interactions/ActionSequence.cs |
Uses collection expressions. |
dotnet/src/webdriver/Interactions/ActionBuilder.cs |
Uses collection/target-typed new for dictionaries, lists, and sequences. |
dotnet/src/webdriver/IE/InternetExplorerOptions.cs |
Uses collection expression for IE options and dictionary initializers in capability building. |
dotnet/src/webdriver/IE/InternetExplorerDriverService.cs |
Uses target-typed new for StringBuilder. |
dotnet/src/webdriver/IE/InternetExplorerDriver.cs |
Converts one constructor to a primary constructor and uses target-typed new for DriverFinder. |
dotnet/src/webdriver/HttpResponseData.cs |
Uses collection expressions for headers and cookie headers. |
dotnet/src/webdriver/Firefox/Preferences.cs |
Uses collection expressions for dictionaries/sets and modern using-declaration in WriteToFile. |
dotnet/src/webdriver/Firefox/Internal/IniFileReader.cs |
Uses collection expressions, range slicing, and target-typed new. |
dotnet/src/webdriver/Firefox/FirefoxProfileManager.cs |
Uses collection expression and target-typed new for IniFileReader. |
dotnet/src/webdriver/Firefox/FirefoxProfile.cs |
Uses collection expressions, target-typed new, range slicing, and modern using patterns for zips/memory streams. |
dotnet/src/webdriver/Firefox/FirefoxOptions.cs |
Uses collection expressions and dictionary initializers for nested log option dictionaries. |
dotnet/src/webdriver/Firefox/FirefoxDriverService.cs |
Uses target-typed new for StringBuilder and null-conditional logging. |
dotnet/src/webdriver/Firefox/FirefoxDriver.cs |
Uses collection expressions for custom command registry and target-typed new for DriverFinder and parameter dictionaries. |
dotnet/src/webdriver/Firefox/FirefoxAndroidOptions.cs |
Converts to primary-constructor Android options type and uses collection expression for intent arguments. |
dotnet/src/webdriver/ErrorResponse.cs |
Uses list collection expressions and spread syntax to convert stack traces into arrays. |
dotnet/src/webdriver/ElementCoordinates.cs |
Uses primary constructor and readonly element field. |
dotnet/src/webdriver/Edge/EdgeDriver.cs |
Uses collection expression for custom command map and for composing combined custom commands. |
dotnet/src/webdriver/DriverProcessStartingEventArgs.cs |
Uses primary constructor and property initializer. |
dotnet/src/webdriver/DriverOptions.cs |
Uses collection expressions for internal capability/logging dictionaries, and target-typed new for merged options and logging preference capabilities. |
dotnet/src/webdriver/DriverFinder.cs |
Uses primary constructor, readonly options/paths, collection expression for paths, and target-typed new for StringBuilder. |
dotnet/src/webdriver/DevTools/v144/* |
Uses readonly fields, collection/array expressions, list initializers, and target-typed new for DevTools v144 target/network/log/javascript bindings. |
dotnet/src/webdriver/DevTools/v143/* |
Same set of changes for DevTools v143 classes. |
dotnet/src/webdriver/DevTools/v142/* |
Same set of changes for DevTools v142 classes. |
dotnet/src/webdriver/DevTools/WebSocketConnectionDataReceivedEventArgs.cs |
Uses primary constructor and property initializer. |
dotnet/src/webdriver/DevTools/TargetInfo.cs |
Uses primary constructor and property initializers. |
dotnet/src/webdriver/DevTools/TargetDetachedEventArgs.cs |
Uses primary constructor. |
dotnet/src/webdriver/DevTools/TargetAttachedEventArgs.cs |
Uses primary constructor. |
dotnet/src/webdriver/DevTools/Target.cs |
Uses null-conditional event invocation. |
dotnet/src/webdriver/DevTools/ResponsePausedEventArgs.cs |
Uses primary constructor and property initializer. |
dotnet/src/webdriver/DevTools/RequestPausedEventArgs.cs |
Uses primary constructor and property initializers. |
dotnet/src/webdriver/DevTools/LogEntry.cs |
Uses primary constructor and property initializers. |
dotnet/src/webdriver/DevTools/Log.cs |
Uses null-conditional event invocation. |
dotnet/src/webdriver/DevTools/Json/JsonEnumMemberConverter.cs |
Uses collection expressions for the mapping dictionaries. |
dotnet/src/webdriver/DevTools/JavaScript.cs |
Uses null-conditional event invocation for script events. |
dotnet/src/webdriver/DevTools/ExceptionThrownEventArgs.cs |
Uses primary constructor. |
dotnet/src/webdriver/DevTools/EntryAddedEventArgs.cs |
Uses primary constructor. |
dotnet/src/webdriver/DevTools/DevToolsSessionLogMessageEventArgs.cs |
Uses primary constructor and formatted-message initializer. |
dotnet/src/webdriver/DevTools/DevToolsSessionEventReceivedEventArgs.cs |
Uses primary constructor and property initializers. |
dotnet/src/webdriver/DevTools/DevToolsSession.cs |
Uses readonly concurrent dictionary and blocking collection, modern using for HttpClient, and null-conditional event logging invocations. |
dotnet/src/webdriver/DevTools/DevToolsEventData.cs |
Uses primary constructor and property initializers. |
dotnet/src/webdriver/DevTools/DevToolsDomains.cs |
Uses collection expression for supported versions list. |
dotnet/src/webdriver/DevTools/DevToolsCommandData.cs |
Uses primary constructor, property initializers, and inline ManualResetEventSlim initialization. |
dotnet/src/webdriver/DevTools/ConsoleApiCalledEventArgs.cs |
Uses primary constructor and property initializers. |
dotnet/src/webdriver/DevTools/ConsoleApiArgument.cs |
Uses primary constructor and property initializers. |
dotnet/src/webdriver/DevTools/BindingCalledEventArgs.cs |
Uses primary constructor and property initializers. |
dotnet/src/webdriver/DevTools/AuthRequiredEventArgs.cs |
Uses primary constructor and property initializers. |
dotnet/src/webdriver/CookieJar.cs |
Uses collection expression for parameters and collection expression for result list. |
dotnet/src/webdriver/Cookie.cs |
Uses collection expression for the SameSite set and target-typed new for epoch base date. |
dotnet/src/webdriver/CommandInfoRepository.cs |
Uses collection expression for the internal command dictionary. |
dotnet/src/webdriver/Command.cs |
Uses primary constructor, property initializers, and collection expression fallback for parameters. |
dotnet/src/webdriver/Chromium/ChromiumPerformanceLoggingPreferences.cs |
Uses collection expression for tracing categories. |
dotnet/src/webdriver/Chromium/ChromiumNetworkConditions.cs |
Uses target-typed new in FromDictionary. |
dotnet/src/webdriver/Chromium/ChromiumDriverService.cs |
Uses target-typed new for StringBuilder. |
dotnet/src/webdriver/Chromium/ChromiumAndroidOptions.cs |
Uses primary constructor to pass package to base. |
dotnet/src/webdriver/Chrome/ChromeDriver.cs |
Uses collection expression for custom command map. |
dotnet/src/webdriver/CapabilityType.cs |
Uses collection expression for known spec-compliant capability names set. |
dotnet/src/webdriver/By.cs |
Uses target-typed new and object initializer for By instances, modern slicing, and target-typed Regex construction. |
dotnet/src/webdriver/BiDi/WebExtension/Extension.cs |
Removes a custom PrintMembers to use default record printing. |
dotnet/src/webdriver/BiDi/Script/* |
Various small updates: fix an always-true pattern, use compound assignment, remove redundant PrintMembers, target-typed new for conversions. |
dotnet/src/webdriver/BiDi/Network/* |
Removes custom PrintMembers overrides and relies on record base implementation. |
dotnet/src/webdriver/BiDi/Json/Converters/BrowsingContextConverter.cs |
Uses primary-constructor style for converter and stores BiDi in a readonly field. |
dotnet/src/webdriver/BiDi/BrowsingContext/BrowsingContext.cs |
Removes custom PrintMembers for record. |
dotnet/src/webdriver/BiDi/Browser/UserContext.cs |
Same. |
dotnet/src/webdriver/Alert.cs |
Converts to primary-constructor class with driver field and uses dictionary initializer for alert text. |
dotnet/src/support/UI/SelectElement.cs |
Uses collection expressions, object initializers for XPath StringBuilders, and collection operations. |
dotnet/src/support/UI/PopupWindowFinder.cs |
Converts to a primary-constructor-based class and simplifies the WebDriverWait instantiation. |
dotnet/src/support/Events/WebElementValueEventArgs.cs |
Uses primary constructor and property initializer. |
dotnet/src/support/Events/WebElementEventArgs.cs |
Uses primary constructor and property initializers. |
dotnet/src/support/Events/WebDriverScriptEventArgs.cs |
Uses primary constructor and property initializers. |
dotnet/src/support/Events/WebDriverNavigationEventArgs.cs |
Uses primary constructor and property initializers, keeping the parameterless overload. |
dotnet/src/support/Events/WebDriverExceptionEventArgs.cs |
Uses primary constructor and property initializers. |
dotnet/src/support/Events/GetShadowRootEventArgs.cs |
Uses primary constructor and property initializers. |
dotnet/src/support/Events/FindElementEventArgs.cs |
Uses primary constructor and property initializers, keeping the existing constructor overload. |
There was a problem hiding this comment.
Disposable 'CancellationTokenSource' is created but not disposed.
There was a problem hiding this comment.
Disposable 'Process' is created but not disposed.
There was a problem hiding this comment.
Disposable 'ManualResetEventSlim' is created but not disposed.
There was a problem hiding this comment.
Disposable 'ManualResetEventSlim' is created but not disposed.
There was a problem hiding this comment.
Disposable 'ManualResetEventSlim' is created but not disposed.
Comment on lines
383
to
385
There was a problem hiding this comment.
Poor error handling: empty catch block.
There was a problem hiding this comment.
This assignment to parsedLine is useless, since its value is never read.
There was a problem hiding this comment.
This assignment to originalLocation is useless, since its value is never read.
Contributor
RenderMichael
left a comment
RenderMichael
left a comment
There was a problem hiding this comment.
Fully in support of doing a formatting pass, including through the older code!
Left some style comments, the only blockers to address is the bool PrintMembers issue and the merge issues in HttpCommandInfo.
| @@ -27,64 +27,52 @@ | |||
| // "In Project Suppression File". | |||
| // You do not need to add suppressions to this file manually. | |||
| [assembly: System.CLSCompliant(true)] | |||
Contributor
There was a problem hiding this comment.
Likely as a follow-up given the mechanical and titanic nature of this PR:
This is the only useful attribute in this file; the various old-school global suppressions should be addressed or folded into the editorconfig.
| } | ||
|
|
||
| // Includes Id only for brevity | ||
| private bool PrintMembers(StringBuilder builder) |
Contributor
There was a problem hiding this comment.
This is a magic method that records use
@nvborisenko I think you made these. Maybe we should fix BiDi.ToString() or override these methods' various ToString()s instead of this magic here?
Contributor
There was a problem hiding this comment.
Does this pass all format checks?
Contributor
There was a problem hiding this comment.
Contributor
There was a problem hiding this comment.
Ditto https://github.com/SeleniumHQ/selenium/pull/17011/changes#r2729548469
Same across all 4 DevTools versions
Contributor
There was a problem hiding this comment.
👍 for future-roofing, this will become free perf as we upgrade .NET.
Point for this PR conceptually!
Contributor
There was a problem hiding this comment.
Same object sender -> object? sender in this file
Contributor
There was a problem hiding this comment.
Inline is clearer here
Suggested change
| Assert.That((Func<FileLogHandler>)act, Throws.ArgumentException); | |
| Assert.That(() => new FileLogHandler(path), Throws.ArgumentException); |
Contributor
There was a problem hiding this comment.
Suggested change
| Assert.That((Func<Cookie>)getCookieAction, Throws.ArgumentException); | |
| Assert.That(() => driver.Manage().Cookies.GetCookieNamed(cookieName), Throws.ArgumentException); |
Contributor
There was a problem hiding this comment.
Suggested change
| Assert.That((Action)deleteCookieAction, Throws.ArgumentException); | |
| Assert.That(() => driver.Manage().Cookies.DeleteCookieNamed(cookieName), Throws.ArgumentException); |
| EqualConstraint fourthConstraint = new EqualConstraint("focus change blur focus change blur"); //What Chrome does | ||
| EqualConstraint thirdConstraint = new("focus blur change focus blur change"); | ||
| EqualConstraint fourthConstraint = new("focus change blur focus change blur"); //What Chrome does | ||
| // I weep. |
| internal static readonly char[] s_base64Table = | ||
| { | ||
| [ | ||
| 'A','B','C','D','E','F','G','H','I','J','K','L','M','N','O','P','Q','R','S','T','U','V','W','X','Y','Z', |
Contributor
There was a problem hiding this comment.
CI linting says these should all be one-character-per-line: https://github.com/SeleniumHQ/selenium/actions/runs/21374865626/job/61528435134?pr=17011#step:21:485
I don't agree, but I have no strong opinion. This type is a polyfill anyways.
| new List<string>() { Keys.Control, Keys.Shift }, | ||
| new List<string>() { Keys.Control, Keys.Alt }, | ||
| new List<string>() { Keys.Shift, Keys.Alt }, | ||
| new List<string>() { Keys.Control, Keys.Shift, Keys.Alt} |
Contributor
There was a problem hiding this comment.
Not sure how this didn't catch before this change, but CI is right
Suggested change
| new List<string>() { Keys.Control, Keys.Shift, Keys.Alt} | |
| new List<string>() { Keys.Control, Keys.Shift, Keys.Alt } |
Member
Author
|
@RenderMichael thank you for going through all of these! I think we need to figure out which style settings to apply and how to enforce them, then add them one at a time until we have the consistent code base we want. I'll run some more analysis on my side, but maybe there is an obvious one or two IDE numbers of the above that we should start with? |
Member
Author
|
Looks like Visual Studio is supposed to be able to auto-generate an editorconfig file that matches existing repo style. I'll load up a VM and see what that gives us. |
This reverts commit 7f1c4f9.
titusfortner
force-pushed
the
dotnet_format_severity
branch
from
b9ac03e to
4802b05
Compare
Contributor
|
All of the fixes here are valid code style issues; since this PR is already put together, I don’t see a problem taking it as-is modulo the various feedback. This PR is large but it’s mostly in the less-active Support and Test projects. |
Member
Author
|
closing in favor of #17019 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Update: In draft until we figure out the right approach. See comments below.
I did not understand what I was doing in #16999
This turns on formatting for suggestion/info level, and this is what it spits out.
I have no idea if we actually want this, but I'd really like to have some kind of enforceable style for dotnet. If this is not the one you want, I think we can tweak .editor.config values to get what we want? I think?
Run this from trunk to see what the rules are it is flagging on:
@nvborisenko & @RenderMichael which of these do we want to enforce vs ignore?