[build] run ruff on python files outside py directory

[titusfortner]

User description

💥 What does this PR do?

Adds a root-level ruff.toml to lint Python files outside the py/ directory (scripts, build tools, codegen).
Applies the resulting linting fixes to those files.

🔧 Implementation Notes

The config extends py/pyproject.toml to maintain consistent Python style across the entire repo.
Including common directories and excluding known vendor directories

💡 Additional Considerations

This works with our existing format.sh execution

🔄 Types of changes

• Cleanup (formatting, renaming)

---

PR Type

Enhancement, Tests

---

Description

• Add root-level ruff.toml to lint Python files outside py/ directory

• Apply ruff formatting fixes across scripts and build tools

• Modernize string formatting to f-strings throughout codebase

• Remove deprecated imports and update type hints to modern syntax

• Simplify file operations by removing redundant mode parameters

---

Diagram Walkthrough

flowchart LR
  A["Root ruff.toml"] -->|"extends"| B["py/pyproject.toml"]
  A -->|"lints"| C["scripts/*.py"]
  A -->|"lints"| D["common/*.py"]
  A -->|"lints"| E["dotnet/*.py"]
  A -->|"lints"| F["java/*.py"]
  A -->|"lints"| G["javascript/*.py"]
  C -->|"formatted"| H["Modernized code"]
  D -->|"formatted"| H
  E -->|"formatted"| H
  F -->|"formatted"| H
  G -->|"formatted"| H

Loading

File Walkthrough

	Relevant files
Configuration changes	1 files ruff.toml Add root-level ruff configuration for Python linting          +18/-0
Formatting	9 files convert_protocol_to_json.py Simplify file operations and formatting                                    +2/-4      pdl.py Modernize imports, type hints, and string formatting          +7/-18    generate_resources_tool.py Update type hints to modern syntax and simplify file operations +9/-10    gen_file.py Convert string formatting to f-strings                                      +38/-45  pinned_browsers.py Convert string formatting to f-strings and simplify list operations +36/-59  selenium_manager.py Convert string formatting to .format() method                        +9/-9      update_cdp.py Improve docstrings and convert string formatting                  +23/-41  update_copyright.py Simplify file operations and list comprehensions                  +5/-14    update_multitool_binaries.py Improve docstring formatting                                                          +1/-1

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Generated code injection Description: The generated Bazel/repository-rule snippets interpolate externally sourced fields (e.g., browser_url, browser_version, driver_url) directly into quoted strings via f-strings without escaping, so a malicious or compromised upstream API response containing quotes/newlines could inject unintended Starlark content into the generated output. pinned_browsers.py [259-367] Referred Code def mac_edge_browser_content(browser_url, browser_hash, browser_version): """Generate Bazel content for Mac Edge browser.""" return f""" pkg_archive( name = "mac_edge", url = "{browser_url}", sha256 = "{browser_hash.lower()}", move = {{ "MicrosoftEdge-{browser_version}.pkg/Payload/Microsoft Edge.app": "Edge.app", }}, build_file_content = \"\"\" load("@aspect_rules_js//js:defs.bzl", "js_library") package(default_visibility = ["//visibility:public"]) exports_files(["Edge.app"]) js_library( name = "edge-js", data = glob(["Edge.app/**/*"]), ) \"\"\", ... (clipped 88 lines)
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: 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: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Unhandled empty matches: New uses of next(...) and min(...) can raise StopIteration/ValueError when expected JSON fields or platform entries are missing, without graceful handling or actionable context. Referred Code milestone = min([version["milestone"] for version in all_versions if version["milestone"]]) r = http.request( "GET", "https://googlechromelabs.github.io/chrome-for-testing/known-good-versions-with-downloads.json", ) versions = json.loads(r.data)["versions"] return sorted( filter(lambda v: v["version"].split(".")[0] == str(milestone), versions), key=lambda v: parse(v["version"]), )[-1] def chromedriver(selected_version, workspace_prefix=""): content = "" drivers = selected_version["downloads"]["chromedriver"] url = next(d["url"] for d in drivers if d["platform"] == "linux64") sha = calculate_hash(url) ... (clipped 51 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: Unvalidated file reads: New direct open(file).read() usage reads paths from js_map without visible validation/sanitization in the diff, which may be safe but cannot be confirmed from the provided hunks. Referred Code contents = open(file).read() write_atom_literal(out, name, contents, "hh", utf8) out.write( f""" static inline {string_type} asString(const {char_type}* const atom[]) {{ {string_type} source; for (int i = 0; atom[i] != NULL; i++) {{ source += atom[i]; }} return source; }} }} // namespace atoms }} // namespace webdriver #endif // {define_guard} """ ) ... (clipped 18 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]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Preserve file encoding when writing updates In write_update_notice, specify encoding="utf-8-sig" when opening the file for writing to match the encoding used for reading, ensuring consistent behavior and preventing potential encoding errors. scripts/update_copyright.py [26-85] class Copyright: ... def update(self, files): for file in files: with open(file, encoding="utf-8-sig") as f: lines = f.readlines() ... def write_update_notice(self, file, lines): ... # Only write if different with open(file, encoding="utf-8-sig") as f: old_content = f.read() if new_content == old_content: return print(f"Adding notice to {file}") - with open(file, "w") as f: + with open(file, "w", encoding="utf-8-sig") as f: f.write(new_content) [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out an inconsistency in file handling where a file is read with a specific encoding (utf-8-sig) but written with the system's default encoding. Applying the same encoding for writing makes the code more robust and predictable across different platforms.	Medium
	Specify file encoding for cross-platform compatibility Explicitly specify encoding="utf-8" and use with statements when opening files in generate_header, generate_cc_source, and generate_java_source to ensure cross-platform compatibility. javascript/private/gen_file.py [78-170] def generate_header(file_name, out, js_map, just_declare, utf8): ... for name, file in js_map.items(): if just_declare: out.write(f"extern const {char_type}* const {name.upper()}[];\n") else: - contents = open(file).read() + with open(file, encoding="utf-8") as f: + contents = f.read() write_atom_literal(out, name, contents, "hh", utf8) ... def generate_cc_source(out, js_map, utf8): ... for name, file in js_map.items(): - contents = open(file).read() + with open(file, encoding="utf-8") as f: + contents = f.read() write_atom_literal(out, name, contents, "cc", utf8) ... def generate_java_source(file_name, out, preamble, js_map): ... for name, file in js_map.items(): - contents = open(file).read() + with open(file, encoding="utf-8") as f: + contents = f.read() write_atom_literal(out, name, contents, "java", True) ... [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that not specifying an encoding for open() can lead to platform-dependent issues. Explicitly adding encoding="utf-8" and using a with statement improves the code's robustness and is a good practice.	Low
Learned best practice	Always close streamed responses Ensure the streamed urllib3 response is always closed/released (even on exceptions) by using try/finally and calling r.release_conn()/r.close() after reading. scripts/pinned_browsers.py [19-25] def calculate_hash(url): print(f"Calculate hash for {url}", file=sys.stderr) h = hashlib.sha256() r = http.request("GET", url, preload_content=False) - for b in iter(lambda: r.read(4096), b""): - h.update(b) + try: + for b in iter(lambda: r.read(4096), b""): + h.update(b) + finally: + r.release_conn() return h.hexdigest() Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - In code that creates external resources (HTTP responses, temporary resources, file handles), wrap usage in try/finally (or context managers) and always perform cleanup/close in the finally block so failures cannot leak resources.	Low
	✅ Validate external data before use Suggestion Impact: The commit replaced `next(...)` lookups with `next((...), None)` and added explicit `ValueError` checks when no download URL is found for a given platform (including the suggested linux64 chromedriver case). It also extended the same validation pattern to other platforms and to milestone extraction. code diff: - url = next(d["url"] for d in drivers if d["platform"] == "linux64") + url = next((d["url"] for d in drivers if d["platform"] == "linux64"), None) + if url is None: + raise ValueError("No chromedriver download found for platform 'linux64'") sha = calculate_hash(url) content += f""" http_archive( @@ -72,7 +77,9 @@ ) """ - url = next(d["url"] for d in drivers if d["platform"] == "mac-arm64") + url = next((d["url"] for d in drivers if d["platform"] == "mac-arm64"), None) + if url is None: + raise ValueError("No chromedriver download found for platform 'mac-arm64'") sha = calculate_hash(url) content += f""" @@ -102,7 +109,9 @@ content = "" chrome_downloads = selected_version["downloads"]["chrome"] - url = next(d["url"] for d in chrome_downloads if d["platform"] == "linux64") + url = next((d["url"] for d in chrome_downloads if d["platform"] == "linux64"), None) + if url is None: + raise ValueError("No Chrome download found for platform 'linux64'") sha = calculate_hash(url) content += f""" @@ -129,7 +138,9 @@ ) """ - url = next(d["url"] for d in chrome_downloads if d["platform"] == "mac-arm64") + url = next((d["url"] for d in chrome_downloads if d["platform"] == "mac-arm64"), None) + if url is None: + raise ValueError("No Chrome download found for platform 'mac-arm64'") Guard against missing platform entries from the external API by providing a default and raising a descriptive error instead of letting StopIteration propagate. scripts/pinned_browsers.py [53] -url = next(d["url"] for d in drivers if d["platform"] == "linux64") +url = next((d["url"] for d in drivers if d.get("platform") == "linux64"), None) +if not url: + raise ValueError("Missing chromedriver download URL for platform 'linux64'") [Suggestion processed] Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Add explicit validation and availability guards at integration boundaries by checking presence/type and failing with clear errors before using external inputs.	Low
Update

[Copilot]

Pull request overview

This PR adds a root-level ruff.toml configuration file to enable consistent Python linting across the entire repository, not just the py/ directory. It extends the existing py/pyproject.toml configuration and applies automated formatting fixes to scripts and build tools throughout the codebase. The changes include modernizing string formatting to f-strings, removing deprecated Python 2 compatibility imports, updating type hints to modern Python 3.10+ syntax, and simplifying file operations.

Changes:

• Added root-level ruff.toml that extends py/pyproject.toml to lint Python files in scripts, common, dotnet, java, javascript, and rb directories
• Modernized string formatting from % operator and .format() to f-strings throughout scripts
• Removed deprecated from __future__ import print_function import (only needed for Python 2)
• Updated type hints from List[T] and Tuple[T] to modern list[T] and tuple[T] syntax
• Removed redundant "r" mode parameter from open() calls (default mode)
• Refactored list comprehensions with indexing to use next() for finding first matching elements

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
ruff.toml	Added root-level ruff configuration extending py/pyproject.toml to include scripts and build tools across the repository
scripts/update_multitool_binaries.py	Fixed docstring formatting to comply with style guidelines
scripts/update_copyright.py	Removed redundant file mode parameters and simplified multi-line expressions
scripts/update_cdp.py	Improved docstring formatting, removed redundant parameters, simplified expressions, and converted to f-strings and next()
scripts/selenium_manager.py	Converted string formatting from % operator to .format() method
scripts/pinned_browsers.py	Converted to f-strings, replaced list comprehensions with next(), simplified expressions
javascript/private/gen_file.py	Converted to f-strings throughout, removed redundant file mode parameters
dotnet/private/generate_resources_tool.py	Updated type hints to modern syntax (list/tuple instead of List/Tuple), removed redundant file mode parameter
common/devtools/pdl.py	Removed deprecated future import, reordered imports, simplified expressions, converted to f-strings
common/devtools/convert_protocol_to_json.py	Simplified argument parser description and removed redundant file mode parameter

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

[titusfortner]

that's just funny. Sure.