Add RBI shims for API client wrappers, remove ~110 T.unsafe calls#14615
Open
JamieMagee wants to merge 7 commits intomainfrom
Open
Add RBI shims for API client wrappers, remove ~110 T.unsafe calls#14615JamieMagee wants to merge 7 commits intomainfrom
JamieMagee wants to merge 7 commits intomainfrom
Conversation
…tokit, GitLab, and Bitbucket client wrappers use method_missing to delegate calls, which means Sorbet cant see the delegated methods. Every call to these clients required T.unsafe to suppress errors. This adds RBI shim files that declare the delegated methods on GithubWithRetries, GitlabWithRetries, and BitbucketWithRetries, plus a Sawyer::Response shim for pagination and a Terminal::Table shim for the table builder. With these declarations in place, the T.unsafe wrappers are no longer needed at call sites. This commit removes about 90 of the original 163 T.unsafe uses across the codebase. Where Sawyer::Resource dynamic attribute access was involved (e.g. response.type, response.sha), switched to bracket notation (response[:type]) which Sawyer::Resource already supports with a typed [] method.
…zed variables and parsed data structures were typed as T.untyped when their actual types are knowable from context. Replaced with the correct types: parsed TOML results use T::Hash[String, T.untyped], memoized strings use T.nilable(String), DependencySet and LockfileParser instances use their actual class types, etc. This removes 16 of the 22 T.let(x, T.untyped) occurrences in production code.
…ategories of T.unsafe fixed: Kwarg splatting: Dependency.new(**T.unsafe(args)) and similar patterns were needed because Sorbet cannot verify that a hash built at runtime matches the expected keyword arguments. Replaced with explicit keyword argument construction in 5 files (compile_file_updater, pip_compile_file_updater, pub/helpers, artifact_updater, vendor_updater). Duck-typed sentry_context: Several error handling paths called error.sentry_context after a respond_to? check, which Sorbet cannot track. Added a SentryContext interface module to errors.rb and included it in the error classes that implement sentry_context. Call sites now use T.cast(error, SentryContext) instead of T.unsafe(error).
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves Sorbet typing across Dependabot Core by introducing hand-written RBI shim files for GithubWithRetries, GitlabWithRetries, and BitbucketWithRetries (which delegate via method_missing) and then removing many call-site T.unsafe usages that were only needed for Sorbet visibility. It also tightens a number of previously-untyped memoized values and introduces a Dependabot::SentryContext interface to replace some T.unsafe calls with T.cast.
Changes:
- Add Sorbet RBI shims for retrying API client wrappers (GitHub/GitLab/Bitbucket) and for a couple of dynamic third-party types used in pagination/table rendering.
- Replace many
T.unsafe(...)call sites with typed calls (plus some conversions from dynamic Sawyer attribute access to[]where applicable). - Replace some
T.let(..., T.untyped)/ unsafe kwargs splats with concrete types and explicit keyword arguments, plus addDependabot::SentryContextfor typed#sentry_context.
Reviewed changes
Copilot reviewed 43 out of 48 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| uv/lib/dependabot/uv/file_updater/compile_file_updater.rb | Removes unsafe kwargs splat when creating a Dependency for requirement updates. |
| uv/lib/dependabot/uv/file_parser/pyproject_files_parser.rb | Tightens types for parsed TOML + memoized dependency sets. |
| updater/lib/dependabot/updater/error_handler.rb | Uses Dependabot::SentryContext + T.cast for fingerprint extraction. |
| updater/lib/dependabot/update_graph_command.rb | Uses Dependabot::SentryContext + T.cast for fingerprint extraction. |
| updater/lib/dependabot/update_files_command.rb | Uses Dependabot::SentryContext + T.cast for fingerprint extraction. |
| updater/lib/dependabot/service.rb | Removes T.unsafe around Terminal::Table construction; relies on RBI typing. |
| updater/lib/dependabot/file_fetcher_command.rb | Uses Dependabot::SentryContext + T.cast for fingerprint extraction. |
| updater/lib/dependabot/base_command.rb | Uses Dependabot::SentryContext + T.cast for fingerprint extraction. |
| sorbet/rbi/shims/terminal-table.rbi | Adds shim typing for Terminal::Table used in updater output. |
| sorbet/rbi/shims/sawyer.rbi | Adds shim typing for Sawyer pagination relations/response fields used by Octokit pagination. |
| sorbet/rbi/shims/gitlab_with_retries.rbi | Declares delegated Gitlab::Client methods on GitlabWithRetries. |
| sorbet/rbi/shims/github_with_retries.rbi | Declares delegated Octokit::Client methods on GithubWithRetries. |
| sorbet/rbi/shims/bitbucket_with_retries.rbi | Declares delegated Bitbucket client methods on BitbucketWithRetries. |
| python/lib/dependabot/python/language_version_manager.rb | Tightens typing for derived Python version string. |
| python/lib/dependabot/python/file_updater/pipfile_file_updater.rb | Tightens type for sanitized setup content cache. |
| python/lib/dependabot/python/file_updater/pip_compile_file_updater.rb | Removes unsafe kwargs splat when creating a Dependency for requirement updates. |
| python/lib/dependabot/python/file_parser/setup_file_parser.rb | Tightens type for CLOSING_BRACKET constant. |
| python/lib/dependabot/python/file_parser/pyproject_files_parser.rb | Tightens types for parsed TOML + memoized dependency sets. |
| python/lib/dependabot/python/file_parser.rb | Tightens memoized type for .in file selection. |
| pub/lib/dependabot/pub/helpers.rb | Replaces unsafe kwargs splat with explicit keyword args for Dependency.new. |
| npm_and_yarn/lib/dependabot/npm_and_yarn/file_updater.rb | Adds SentryContext to error class and marks sentry_context as override. |
| npm_and_yarn/lib/dependabot/npm_and_yarn/file_parser/json_lock.rb | Tightens memoized parsed JSON type. |
| git_submodules/lib/dependabot/git_submodules/file_fetcher.rb | Removes unsafe calls now covered by retry-client shims. |
| docker/lib/dependabot/shared/shared_file_updater.rb | Tightens type of updated_content memoized variable. |
| composer/lib/dependabot/composer/file_fetcher/path_dependency_builder.rb | Tightens lockfile type from untyped to T.nilable(DependencyFile). |
| common/lib/dependabot/shared_helpers.rb | Adds SentryContext to an error type and marks sentry_context as override. |
| common/lib/dependabot/pull_request_updater/gitlab.rb | Removes unsafe GitLab client calls now covered by retry-client shims. |
| common/lib/dependabot/pull_request_updater/github.rb | Removes unsafe GitHub client calls now covered by retry-client shims. |
| common/lib/dependabot/pull_request_creator/pr_name_prefixer.rb | Removes unsafe GitHub/GitLab client calls now covered by retry-client shims. |
| common/lib/dependabot/pull_request_creator/labeler.rb | Removes unsafe GitHub/GitLab client calls and types Octokit pagination access. |
| common/lib/dependabot/pull_request_creator/gitlab.rb | Removes unsafe GitLab client calls now covered by retry-client shims. |
| common/lib/dependabot/pull_request_creator/github.rb | Removes unsafe GitHub client calls now covered by retry-client shims. |
| common/lib/dependabot/metadata_finders/base/release_finder.rb | Removes unsafe client calls now covered by retry-client shims. |
| common/lib/dependabot/metadata_finders/base/commits_finder.rb | Removes unsafe client calls now covered by retry-client shims. |
| common/lib/dependabot/metadata_finders/base/changelog_finder.rb | Removes unsafe client calls now covered by retry-client shims. |
| common/lib/dependabot/git_commit_checker.rb | Removes unsafe client calls now covered by retry-client shims. |
| common/lib/dependabot/file_updaters/vendor_updater.rb | Replaces unsafe kwargs splat with explicit keyword args for DependencyFile.new. |
| common/lib/dependabot/file_updaters/artifact_updater.rb | Replaces unsafe kwargs splat with explicit keyword args for DependencyFile.new. |
| common/lib/dependabot/file_fetchers/base.rb | Removes unsafe client calls and uses Sawyer::Resource#[] for typed access. |
| common/lib/dependabot/errors.rb | Adds Dependabot::SentryContext Sorbet interface. |
| common/lib/dependabot/command_helpers.rb | Tightens PID type from untyped to T.nilable(Integer). |
| common/lib/dependabot/clients/gitlab_with_retries.rb | Removes internal T.unsafe(self) in VCS helper methods. |
| common/lib/dependabot/clients/github_with_retries.rb | Removes internal T.unsafe(self) in VCS helper methods. |
| cargo/lib/dependabot/cargo/file_parser.rb | Tightens parsed TOML cache type. |
| bundler/lib/dependabot/bundler/file_parser.rb | Tightens parsed lockfile type. |
| bun/lib/dependabot/bun/file_updater.rb | Adds SentryContext to error class and marks sentry_context as override. |
| bun/lib/dependabot/bun/file_parser/bun_lock.rb | Tightens parsed lockfile type. |
| bazel/lib/dependabot/bazel/update_checker/registry_client.rb | Removes unsafe GitHub client calls now covered by retry-client shims. |
Files not reviewed (5)
- sorbet/rbi/shims/bitbucket_with_retries.rbi: Language not supported
- sorbet/rbi/shims/github_with_retries.rbi: Language not supported
- sorbet/rbi/shims/gitlab_with_retries.rbi: Language not supported
- sorbet/rbi/shims/sawyer.rbi: Language not supported
- sorbet/rbi/shims/terminal-table.rbi: Language not supported
…cess on Sawyer::Resource and Gitlab::ObjectifiedHash (which use method_missing). Re-added T.unsafe at those specific dynamic attribute sites while keeping the client method calls properly typed. Other fixes: - Renamed Dependabot::SentryContext to HasSentryContext to avoid collision with the existing SentryContext processor class - Removed Sawyer::Relation from the shim (conflicted with the auto-generated RBI parameter count) - Added T.must for nilable arguments at call sites - Fixed unreachable code from python_major_minor returning String - Typed next_link as nilable in the GitHub label pagination loop
…b and base_command.rb now pass at the strong sigil level, moving the strong file count from 319 to 321.
…p autofix for line length, indentation, and style issues introduced by the type annotation changes. Reverted pub/helpers.rb to keep the original T.unsafe kwarg splatting since the explicit kwargs version pushed the module over the line limit.
…nes sentry_context but was missing the interface include, causing a T.cast failure at runtime.
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
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.
What are you trying to accomplish?
Most of our
T.unsafeusage (101 of 163 calls) existed becauseGithubWithRetries,GitlabWithRetries, andBitbucketWithRetriesdelegate to their underlying clients viamethod_missing. Sorbet can't see those delegated methods, so every call site neededT.unsafe.This PR adds RBI shim files that declare the delegated methods directly on the wrapper classes, then removes the
T.unsafewrappers at call sites. It also fixes a few other categories:T.let(x, T.untyped)with their actual types (parsed TOML hashes, memoized strings, DependencySet, etc.)Dependency.new(**T.unsafe(args))kwarg splatting with explicit keyword arguments in 5 filesSentryContextinterface module soerror.sentry_contextcalls can useT.castinstead ofT.unsafeAnything you want to highlight for special attention from reviewers?
The shim files in shims are hand-written, not Tapioca-generated. They supplement the auto-generated gem RBIs by adding signatures for methods that Tapioca can't infer (because the gems use
method_missing).tapioca gem --verifyis unaffected.The
Sawyer::Resourceattribute access pattern (.type,.sha, etc.) was converted to bracket notation ([:type],[:sha]) where possible, sinceSawyer::Resource#[]already has a typed signature.How will you know you've accomplished your goal?
Spoom coverage before and after: