[rb] Add missing RBS methods by aguspe · Pull Request #14621 · SeleniumHQ/selenium

aguspe · 2024-10-18T14:31:17Z

Description

This PR adds an array of RBS missing methods and reduces the RBS errors from 73 to 28

Motivation and Context

To add full RBS support for Selenium and allow us to eventually add an RBS check on the pipeline.
The reference feature is #10943

The goal is also to be able to have no steep errors to start adding the right type on the classes that have untyped to have the right type enforces

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist

I have read the contributing document.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing tests passed.

PR Type

enhancement

Description

Updated method signatures and return types across multiple modules to enhance functionality and type safety.
Added new methods and instance variables to improve class initialization and method operations.
Enhanced the handling of callbacks and message handlers in WebSocket and BiDi modules.
Improved the structure and capabilities of the DriverFinder, SearchContext, and Service modules.

Changes walkthrough 📝

Relevant files

Enhancement

11 files

bidi.rbs Update `remove_callback` method signature in BiDi module rb/sig/lib/selenium/webdriver/bidi.rbs Modified `remove_callback` method signature to accept parameters.	+1/-1
log_handler.rbs `Update return types in LogHandler module` rb/sig/lib/selenium/webdriver/bidi/log_handler.rbs Changed return type of `remove_message_handler` to `bool`. Changed return type of `subscribe_log_entry` to `bool`. Changed return type of `unsubscribe_log_entry` to `bool`.	+3/-3
driver_finder.rbs `Add initialization and path methods to DriverFinder` rb/sig/lib/selenium/webdriver/common/driver_finder.rbs Added instance variables `@options`, `@paths`, and `@service`. Added `initialize` method with parameters. Added new methods `browser_path`, `browser_path?`, and `driver_path`.	+17/-0
search_context.rbs `Add accessors and methods to SearchContext module` rb/sig/lib/selenium/webdriver/common/search_context.rbs Added `attr_accessor` for `self.extra_finders`. Added `self.finders` method.	+4/-0
service.rbs `Update Service module with new method and type change` rb/sig/lib/selenium/webdriver/common/service.rbs Changed return type of `env_path` to `String?`. Added `find_driver_path` method.	+3/-1
websocket_connection.rbs `Add and update callback methods in WebSocketConnection` rb/sig/lib/selenium/webdriver/common/websocket_connection.rbs Added `add_callback` method with parameters. Modified `remove_callback` method signature to accept parameters.	+4/-0
dialog.rbs `Add initialization method to FedCM Dialog class` rb/sig/lib/selenium/webdriver/fedcm/dialog.rbs Added `initialize` method with `Remote::Bridge` parameter.	+2/-0
options.rbs `Add options instance variable to Firefox Options` rb/sig/lib/selenium/webdriver/firefox/options.rbs Added instance variable `@options`.	+2/-0
common.rbs `Enhance HTTP Common with headers and user agent methods` rb/sig/lib/selenium/webdriver/remote/http/common.rbs Added `@common_headers` instance variable. Added `attr_accessor` for `self.extra_headers`. Added `attr_writer` and method for `self.user_agent`. Added `common_headers` method.	+10/-0
guard.rbs `Add tracker attribute reader to Guard class` rb/sig/lib/selenium/webdriver/support/guards/guard.rbs Added `attr_reader` for `tracker`.	+1/-0
script.rbs `Add initialization and update handler method in Script class` rb/sig/selenium/web_driver/script.rbs Added `initialize` method with `BiDi` parameter. Modified `remove_console_message_handler` to accept an `id` parameter.	+3/-1

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

qodo-code-review · 2024-10-24T08:24:17Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Method Signature Change The `remove_callback` method signature has been changed to accept parameters. Ensure this change is consistent with the implementation and doesn't break existing code. Return Type Changes The return types for `remove_message_handler`, `subscribe_log_entry`, and `unsubscribe_log_entry` have been changed from `false` to `bool`. Verify if this change is intentional and consistent with the actual implementation. New Methods Several new methods and instance variables have been added to the DriverFinder class. Ensure these additions are properly implemented and documented in the actual code. Method Signature Changes The `add_callback` and `remove_callback` methods have been modified to accept parameters. Verify that these changes are reflected in the implementation and don't break existing functionality.

qodo-code-review · 2024-10-24T08:24:40Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Enhancement	✅ Specify more precise types for method parameters to improve type safety and clarity Suggestion Impact: The commit implemented more precise types for the parameters in the remove_callback method, changing 'event' to 'String' and 'id' to 'Integer', which aligns with the suggestion's goal of improving type safety. code diff: - def remove_callback: (untyped event, untyped id) -> untyped + def remove_callback: (String event, Integer id) -> Error::WebDriverError? Consider specifying more precise types for the `event` and `id` parameters in the `remove_callback` method signature. This will improve type safety and make the method's usage clearer. rb/sig/lib/selenium/webdriver/bidi.rbs [16] -def remove_callback: (untyped event, untyped id) -> untyped +def remove_callback: (Symbol event, Integer id) -> untyped Apply this suggestion Suggestion importance[1-10]: 7 Why: The suggestion to specify more precise types for the `event` and `id` parameters in the `remove_callback` method enhances type safety and clarity, which is beneficial for maintainability and reducing potential runtime errors.	7
	Specify more precise types for method parameters to improve type information and clarity Consider specifying more precise types for the `options` and `service` parameters in the `initialize` method. This will provide better type information and improve code clarity. rb/sig/lib/selenium/webdriver/common/driver_finder.rbs [9] -def initialize: (untyped options,untyped service) -> void +def initialize: (Hash[Symbol, untyped] options, Service service) -> void Apply this suggestion Suggestion importance[1-10]: 7 Why: Providing more precise types for the `options` and `service` parameters in the `initialize` method improves type information and code clarity, which can help prevent type-related errors and make the code easier to understand.	7
	Specify a more precise type for a class attribute to improve type information and clarity Consider specifying a more precise type for `extra_finders` instead of using `untyped`. This will provide better type information and improve code clarity. rb/sig/lib/selenium/webdriver/common/search_context.rbs [8] -attr_accessor self.extra_finders: untyped +attr_accessor self.extra_finders: Hash[Symbol, Proc] Apply this suggestion Suggestion importance[1-10]: 7 Why: Using a more precise type for `extra_finders` instead of `untyped` enhances type information and clarity, which aids in understanding the code and catching type-related issues early.	7

💡 Need additional feedback ? start a PR chat

…bs_errors

aguspe and others added 3 commits October 18, 2024 16:31

Add missing methods

30dd316

Reduce errors to 28

04d3f5d

Merge branch 'trunk' into reduce_rbs_errors

2bed538

aguspe marked this pull request as ready for review October 24, 2024 08:23

qodo-code-review Bot added P-enhancement PR with a new feature Review effort [1-5]: 3 labels Oct 24, 2024

Merge branch 'trunk' into reduce_rbs_errors

636f5e6

aguspe changed the title ~~Add missing methods~~ [rb] Add missing RBS methods Oct 24, 2024

aguspe and others added 4 commits October 24, 2024 21:59

Update bidi type

634006c

Merge branch 'trunk' into reduce_rbs_errors

bd550ec

Update types

ecb25fb

Merge remote-tracking branch 'origin/reduce_rbs_errors' into reduce_r…

19fd7d9

…bs_errors

p0deje approved these changes Oct 25, 2024

View reviewed changes

p0deje merged commit 798f3f9 into SeleniumHQ:trunk Oct 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[rb] Add missing RBS methods#14621

[rb] Add missing RBS methods#14621
p0deje merged 8 commits intoSeleniumHQ:trunkfrom
aguspe:reduce_rbs_errors

aguspe commented Oct 18, 2024 •

edited

Loading

Uh oh!

qodo-code-review Bot commented Oct 24, 2024

Uh oh!

qodo-code-review Bot commented Oct 24, 2024 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

aguspe commented Oct 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

Types of changes

Checklist

PR Type

Description

Changes walkthrough 📝

Uh oh!

qodo-code-review Bot commented Oct 24, 2024

PR Reviewer Guide 🔍

Uh oh!

qodo-code-review Bot commented Oct 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aguspe commented Oct 18, 2024 •

edited

Loading

qodo-code-review Bot commented Oct 24, 2024 •

edited

Loading