Skip to content

Conversation

@cgoldberg
Copy link
Member

@cgoldberg cgoldberg commented Nov 30, 2025

User description

💥 What does this PR do?

This PR updates the target-version for ruff linting/formatting to Python 3.10, since that is now the minimum supported version.

It also removes the exclusion of generate.py from linting and fixes docstrings and import ordering in that file so no violations exist.

🔄 Types of changes

  • Cleanup (formatting, renaming)
  • Linting

PR Type

Enhancement


Description

  • Bump ruff target-version from Python 3.9 to 3.10

  • Remove generate.py from ruff exclusion list

  • Fix docstring formatting to comply with ruff standards

  • Reorganize imports in alphabetical order per ruff rules


Diagram Walkthrough

flowchart LR
  A["Ruff Configuration"] -->|target-version| B["Python 3.10"]
  A -->|remove exclusion| C["generate.py"]
  C -->|fix docstrings| D["Single-line format"]
  C -->|organize imports| E["Alphabetical order"]
Loading

File Walkthrough

Relevant files
Formatting
generate.py
Fix docstrings and import ordering for ruff compliance     

py/generate.py

  • Reorganized imports in alphabetical order with proper grouping
    (stdlib, third-party)
  • Fixed docstring formatting: converted multi-line docstrings to
    single-line format where appropriate
  • Updated docstring backtick escaping from double backticks to single
    backticks in code references
  • Improved variable naming (e.g., l to line) and removed commented-out
    code
+36/-46 
Configuration changes
pyproject.toml
Update ruff configuration for Python 3.10                               

py/pyproject.toml

  • Removed generate.py from ruff's exclude list
  • Updated ruff target-version from py39 to py310
+1/-2     

@cgoldberg cgoldberg self-assigned this Nov 30, 2025
@cgoldberg cgoldberg added the C-py Python Bindings label Nov 30, 2025
@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Nov 30, 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: 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 logging added: The changes focus on linting, import reordering, and docstring edits without adding or
modifying any critical action logging, which neither proves nor disproves audit trail
compliance in the affected generator utilities.

Referred Code
def parse_json_event(json: T_JSON_DICT) -> typing.Any:
    ''' Parse a JSON dictionary into a CDP event. '''
    return _event_parsers[json['method']].from_json(json['params'])
"""


def indent(s, n):
    """A shortcut for `textwrap.indent` that always uses spaces."""
    return tw_indent(s, n * " ")


BACKTICK_RE = re.compile(r"`([^`]+)`(\w+)?")


def escape_backticks(docstr):
    """Escape backticks in a docstring by doubling them up.

    This is a little tricky because RST requires a non-letter character after
    the closing backticks, but some CDPs docs have things like "`AxNodeId`s".
    If we double the backticks in that string, then it won't be valid RST. The
    fix is to insert an apostrophe if an "s" trails the backticks.


 ... (clipped 861 lines)

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:
No new handling: The PR updates style and docstrings but does not modify existing error handling or add
edge-case checks, leaving robustness unchanged and unverifiable from this diff alone.

Referred Code
def parse_json_event(json: T_JSON_DICT) -> typing.Any:
    ''' Parse a JSON dictionary into a CDP event. '''
    return _event_parsers[json['method']].from_json(json['params'])
"""


def indent(s, n):
    """A shortcut for `textwrap.indent` that always uses spaces."""
    return tw_indent(s, n * " ")


BACKTICK_RE = re.compile(r"`([^`]+)`(\w+)?")


def escape_backticks(docstr):
    """Escape backticks in a docstring by doubling them up.

    This is a little tricky because RST requires a non-letter character after
    the closing backticks, but some CDPs docs have things like "`AxNodeId`s".
    If we double the backticks in that string, then it won't be valid RST. The
    fix is to insert an apostrophe if an "s" trails the backticks.


 ... (clipped 861 lines)

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:
Validation unchanged: The changes are non-functional (lint/docstrings) and do not introduce or alter input
validation or data handling, so security posture cannot be assessed solely from this diff.

Referred Code
def parse_json_event(json: T_JSON_DICT) -> typing.Any:
    ''' Parse a JSON dictionary into a CDP event. '''
    return _event_parsers[json['method']].from_json(json['params'])
"""


def indent(s, n):
    """A shortcut for `textwrap.indent` that always uses spaces."""
    return tw_indent(s, n * " ")


BACKTICK_RE = re.compile(r"`([^`]+)`(\w+)?")


def escape_backticks(docstr):
    """Escape backticks in a docstring by doubling them up.

    This is a little tricky because RST requires a non-letter character after
    the closing backticks, but some CDPs docs have things like "`AxNodeId`s".
    If we double the backticks in that string, then it won't be valid RST. The
    fix is to insert an apostrophe if an "s" trails the backticks.


 ... (clipped 861 lines)

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 Nov 30, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Learned
best practice
Validate file input and JSON

Add basic existence/type checks and handle JSON parsing errors to fail fast with
a clear message. Consider a small timeout or size guard if inputs can be
untrusted.

py/generate.py [930-931]

-with open(json_path, encoding="utf-8") as json_file:
-    schema = json.load(json_file)
+if not isinstance(json_path, (str, os.PathLike)) or not Path(json_path).exists():
+    raise FileNotFoundError(f"Invalid schema path: {json_path}")
+try:
+    with open(json_path, encoding="utf-8") as json_file:
+        schema = json.load(json_file)
+except json.JSONDecodeError as e:
+    raise ValueError(f"Invalid JSON schema at {json_path}: {e}") from e

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Validate and sanitize external inputs before use, especially when reading files and parsing JSON.

Low
General
Remove statement with no effect
Suggestion Impact:The commit removed the useless standalone 'expr' statement from the code.

code diff:

@@ -297,7 +297,6 @@
             if self.items.ref:
                 py_ref = ref_to_python(self.items.ref)
                 expr = f"[{py_ref}.from_json(i) for i in {dict_}['{self.name}']]"
-                expr
             else:
                 cons = CdpPrimitiveType.get_constructor(self.items.type, "i")
                 expr = f"[{cons} for i in {dict_}['{self.name}']]"

In the generate_from_json method, remove the line expr which is a statement with
no effect and likely a debugging leftover.

py/generate.py [294-312]

 def generate_from_json(self, dict_):
     """Generate the code that creates an instance from a JSON dict named `dict_`."""
     if self.items:
         if self.items.ref:
             py_ref = ref_to_python(self.items.ref)
             expr = f"[{py_ref}.from_json(i) for i in {dict_}['{self.name}']]"
-            expr
         else:
             cons = CdpPrimitiveType.get_constructor(self.items.type, "i")
             expr = f"[{cons} for i in {dict_}['{self.name}']]"
     else:
         if self.ref:
             py_ref = ref_to_python(self.ref)
             expr = f"{py_ref}.from_json({dict_}['{self.name}'])"
         else:
             expr = CdpPrimitiveType.get_constructor(self.type, f"{dict_}['{self.name}']")
     if self.optional:
         expr = f"{expr} if '{self.name}' in {dict_} else None"
     return expr

[Suggestion processed]

Suggestion importance[1-10]: 3

__

Why: The suggestion correctly identifies a useless statement on line 300 that has no effect on the program's logic, and removing it improves code clarity.

Low
  • Update

@cgoldberg cgoldberg merged commit 96a4e90 into SeleniumHQ:trunk Dec 1, 2025
4 checks passed
@cgoldberg cgoldberg deleted the py-ruff-py310-generate branch December 1, 2025 00:15
@SeleniumHQ SeleniumHQ deleted a comment from harshraj2171 Dec 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant