Skip to content

Conversation

@RenderMichael
Copy link
Contributor

@RenderMichael RenderMichael commented Nov 20, 2025

User description

This will allow JavaScript syntax highlighting for users that do something like:

Driver.ExecuteScript("performance.now()");

Visual Studio does not have great support for this, but Rider does (works the same as its language injection).


PR Type

Enhancement


Description

  • Add [StringSyntax("javascript")] attribute to JavaScript string parameters

  • Enable IDE syntax highlighting for JavaScript code in string parameters

  • Create polyfill StringSyntaxAttribute for .NET versions before 8.0

  • Apply attribute across multiple JavaScript execution methods and interfaces


Diagram Walkthrough

flowchart LR
  A["JavaScript String Parameters"] -->|Apply StringSyntax Attribute| B["IDE Syntax Highlighting"]
  C["StringSyntaxAttribute Polyfill"] -->|NET8_0_OR_GREATER conditional| D["Native or Custom Implementation"]
  B --> E["Enhanced Developer Experience"]
  D --> E
Loading

File Walkthrough

Relevant files
Enhancement
13 files
StringSyntaxAttribute.cs
Create StringSyntaxAttribute polyfill for older .NET versions
+89/-0   
StringSyntaxAttribute.cs
Create StringSyntaxAttribute polyfill for older .NET versions
+89/-0   
EventFiringWebDriver.cs
Add StringSyntax attribute to ExecuteScript methods           
+3/-2     
WebDriverScriptEventArgs.cs
Add StringSyntax attribute to script parameter                     
+2/-1     
WebDriverExtensions.cs
Add StringSyntax attribute to ExecuteJavaScript extension methods
+3/-2     
JavaScript.cs
Add StringSyntax attribute to JavaScript evaluation methods
+3/-2     
IJavaScriptEngine.cs
Add StringSyntax attribute to interface method signatures
+3/-2     
IJavascriptExecutor.cs
Add StringSyntax attribute to interface method signatures
+3/-2     
InitializationScript.cs
Add StringSyntax attribute to ScriptSource property           
+2/-0     
FileUtilities.cs
Simplify StringSyntax attribute usage with polyfill           
+2/-5     
JavaScriptEngine.cs
Add StringSyntax attribute to AddInitializationScript and PinScript
+2/-2     
PinnedScript.cs
Add StringSyntax attribute to Source property                       
+2/-0     
WebDriver.cs
Add StringSyntax attribute to ExecuteScript methods           
+3/-3     

@selenium-ci selenium-ci added C-dotnet .NET Bindings B-support Issue or PR related to support classes labels Nov 20, 2025
@selenium-ci
Copy link
Member

Thank you, @RenderMichael for this code suggestion.

The support packages contain example code that many users find helpful, but they do not necessarily represent
the best practices for using Selenium, and the Selenium team is not currently merging changes to them.

After reviewing the change, unless it is a critical fix or a feature that is needed for Selenium
to work, we will likely close the PR.

We actively encourage people to add the wrapper and helper code that makes sense for them to their own frameworks.
If you have any questions, please contact us

@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Nov 20, 2025

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: 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: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
No auditing: The new annotations and related method signatures add JavaScript execution capabilities
but do not include any auditing or logging of critical actions, which may be acceptable
for a non-security change but cannot be confirmed from the diff.

Referred Code
public async Task<InitializationScript> AddInitializationScript(string scriptName, [StringSyntax("javascript")] string script)
{
    if (scriptName is null)
    {
        throw new ArgumentNullException(nameof(scriptName));

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:
Input annotation: The change adds StringSyntax annotations to script parameters without altering validation
logic; while this improves IDE hints, it does not itself validate or sanitize inputs,
which may be acceptable given existing downstream validation not visible in the diff.

Referred Code
public object? ExecuteAsyncScript([StringSyntax("javascript")] string script, params object?[]? args)
{
    return this.ExecuteScriptCommand(script, DriverCommand.ExecuteAsyncScript, args);
}

/// <summary>
/// Executes JavaScript in the context of the currently selected frame or window
/// </summary>
/// <param name="script">The JavaScript code to execute.</param>
/// <param name="args">The arguments to the script.</param>
/// <returns>The value returned by the script.</returns>
public object? ExecuteScript([StringSyntax("javascript")] string script, params object?[]? args)
{

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-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Nov 20, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Avoid code duplication by linking files
Suggestion Impact:The commit modified the duplicated StringSyntaxAttribute.cs by removing the #if !NET8_0_OR_GREATER guards, indicating changes to how the file is included across target frameworks. This aligns with addressing duplication/conditional inclusion concerns raised by the suggestion.

code diff:

-
-#if !NET8_0_OR_GREATER
 
 namespace System.Diagnostics.CodeAnalysis;
 
@@ -84,5 +82,3 @@
     /// <summary>The syntax identifier for strings containing XML.</summary>
     public const string Xml = nameof(Xml);
 }
-
-#endif

Remove the duplicated StringSyntaxAttribute.cs file and instead link to the
single source file from the project file to avoid code duplication.

dotnet/src/support/Internal/StringSyntaxAttribute.cs [1-88]

-// <copyright file="StringSyntaxAttribute.cs" company="Selenium Committers">
-// Licensed to the Software Freedom Conservancy (SFC) under one
-// or more contributor license agreements.  See the NOTICE file
-// distributed with this work for additional information
-// regarding copyright ownership.  The SFC licenses this file
-// to you under the Apache License, Version 2.0 (the
-// "License"); you may not use this file except in compliance
-// with the License.  You may obtain a copy of the License at
-//
-//   http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing,
-// software distributed under the License is distributed on an
-// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-// KIND, either express or implied.  See the License for the
-// specific language governing permissions and limitations
-// under the License.
-// </copyright>
+// This file should be deleted.
 
-#if !NET8_0_OR_GREATER
-
-namespace System.Diagnostics.CodeAnalysis;
-
-/// <summary>Specifies the syntax used in a string.</summary>
-[AttributeUsage(AttributeTargets.Parameter | AttributeTargets.Field | AttributeTargets.Property, AllowMultiple = false, Inherited = false)]
-internal sealed class StringSyntaxAttribute : Attribute
-{
-    /// <summary>Initializes the <see cref="StringSyntaxAttribute"/> with the identifier of the syntax used.</summary>
-    /// <param name="syntax">The syntax identifier.</param>
-    public StringSyntaxAttribute(string syntax)
-    {
-        Syntax = syntax;
-        Arguments = [];
-    }
-
-    /// <summary>Initializes the <see cref="StringSyntaxAttribute"/> with the identifier of the syntax used.</summary>
-    /// <param name="syntax">The syntax identifier.</param>
-    /// <param name="arguments">Optional arguments associated with the specific syntax employed.</param>
-    public StringSyntaxAttribute(string syntax, params object?[] arguments)
-    {
-        Syntax = syntax;
-        Arguments = arguments;
-    }
-
-    /// <summary>Gets the identifier of the syntax used.</summary>
-    public string Syntax { get; }
-
-    /// <summary>Optional arguments associated with the specific syntax employed.</summary>
-    public object?[] Arguments { get; }
-
-    /// <summary>The syntax identifier for strings containing composite formats for string formatting.</summary>
-    public const string CompositeFormat = nameof(CompositeFormat);
-
-    /// <summary>The syntax identifier for strings containing date format specifiers.</summary>
-    public const string DateOnlyFormat = nameof(DateOnlyFormat);
-
-    /// <summary>The syntax identifier for strings containing date and time format specifiers.</summary>
-    public const string DateTimeFormat = nameof(DateTimeFormat);
-
-    /// <summary>The syntax identifier for strings containing <see cref="Enum"/> format specifiers.</summary>
-    public const string EnumFormat = nameof(EnumFormat);
-
-    /// <summary>The syntax identifier for strings containing <see cref="Guid"/> format specifiers.</summary>
-    public const string GuidFormat = nameof(GuidFormat);
-
-    /// <summary>The syntax identifier for strings containing JavaScript Object Notation (JSON).</summary>
-    public const string Json = nameof(Json);
-
-    /// <summary>The syntax identifier for strings containing numeric format specifiers.</summary>
-    public const string NumericFormat = nameof(NumericFormat);
-
-    /// <summary>The syntax identifier for strings containing regular expressions.</summary>
-    public const string Regex = nameof(Regex);
-
-    /// <summary>The syntax identifier for strings containing time format specifiers.</summary>
-    public const string TimeOnlyFormat = nameof(TimeOnlyFormat);
-
-    /// <summary>The syntax identifier for strings containing <see cref="TimeSpan"/> format specifiers.</summary>
-    public const string TimeSpanFormat = nameof(TimeSpanFormat);
-
-    /// <summary>The syntax identifier for strings containing URIs.</summary>
-    public const string Uri = nameof(Uri);
-
-    /// <summary>The syntax identifier for strings containing XML.</summary>
-    public const string Xml = nameof(Xml);
-}
-
-#endif
-

[Suggestion processed]

Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that the PR introduces a duplicated file and proposes a standard .NET practice to resolve it, which improves code maintainability.

Low
  • Update

@nvborisenko
Copy link
Member

I am 50/50. From one point of view it is nice feature for end users. From another hand it is expensive to support "old" users (net462, netstandard2.0). We are trying to bring good compiler/tools features even if "old" users are unintended to support it. Hacks like defining missing attributes in our codebase. I remember how we introduced nullability and NRT - magic unnecessary internal code for analyzer/compiler.

I would support this proposal only for modern users (net8+).

@RenderMichael
Copy link
Contributor Author

I would support this proposal only for modern users (net8+).

That is actually harder to implement than a #if NET8_OR_GREATER directive. You can see the current implementation of GenerateRandomTempDirectoryName to see how much simpler this is.

image

Additionally, this isn't just for old users, it is for anyone using our .NET Standard package. Not necessarily ancient users!

@nvborisenko
Copy link
Member

Good point, keeping one extra internal class is better than #if directives everywhere.

@RenderMichael RenderMichael merged commit a2b4269 into SeleniumHQ:trunk Nov 22, 2025
10 checks passed
@RenderMichael RenderMichael deleted the dotnet-string-syntax-js branch November 22, 2025 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-support Issue or PR related to support classes C-dotnet .NET Bindings Review effort 2/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants