Skip to content

Commit

Permalink
Add Sorbet/FetchWhenMust cop
Browse files Browse the repository at this point in the history
Checks for `T.must(object[key])` and recommends `object.fetch(key)`
instead.

While usually safe, false positives are possible if `object` does not
respond to `fetch`, or if `Hash#default_proc` (or similar) is being
used.
  • Loading branch information
sambostock committed Apr 24, 2023
1 parent 166eb19 commit 7066e9d
Show file tree
Hide file tree
Showing 6 changed files with 135 additions and 0 deletions.
11 changes: 11 additions & 0 deletions config/default.yml
Expand Up @@ -66,6 +66,17 @@ Sorbet/FalseSigil:
- db/**/*.rb
- script/**/*

Sorbet/FetchWhenMust:
Description: >-
Checks for `T.must(object[key])` and recommends `object.fetch(key)` instead.
While usually safe, false positives are possible if `object` does not respond to `fetch`,
or if `Hash#default_proc` (or similar) is being used.
Enabled: pending
VersionAdded: '<<next>>'
Safe: false
SafeAutoCorrect: false

Sorbet/ForbidExtendTSigHelpersInShims:
Description: 'Forbid the use of `extend T::Sig` and `extend T::Helpers` in RBI shims'
Enabled: true
Expand Down
65 changes: 65 additions & 0 deletions lib/rubocop/cop/sorbet/fetch_when_must.rb
@@ -0,0 +1,65 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Sorbet
# rubocop:disable Lint/RedundantCopDisableDirective

# Checks for `T.must(object[key])` and recommends `object.fetch(key)` instead.
#
# @safety
# False positives are possible if `object` does not respond to `fetch`,
# or if `Hash#default_proc` (or similar) is being used.
#
# @example
# # bad
# T.must(object[key])
#
# # good
# object.fetch(key)
#
# # good
# object[key]
#
# # good
# # If `object` does not `respond_to? :fetch`, or if using `Hash` `default_proc`
# T.must(object[key]) # rubocop:disable Sorbet/FetchWhenMust
#
class FetchWhenMust < RuboCop::Cop::Base
# rubocop:enable Lint/RedundantCopDisableDirective

extend AutoCorrector

MSG = "Use `%<expected>s` instead of `%<actual>s` when value must always be found, and receiver supports it."
RESTRICT_ON_SEND = [:must].freeze

# @!method t_must_on_index_result(node)
def_node_matcher :t_must_on_index_result, <<~PATTERN
(send
(const { nil? cbase } :T) :must
{
(index $_ $_)
(send $_ :[] $_)
}
)
PATTERN

def on_send(node)
t_must_on_index_result(node) do |receiver, key|
message = format(MSG, expected: expected(receiver, key), actual: node.source)

add_offense(node, message: message) do |corrector|
corrector.replace(node, expected(receiver, key))
end
end
end

private

def expected(receiver, key)
"#{receiver.source}.fetch(#{key.source})"
end
end
end
end
end
1 change: 1 addition & 0 deletions lib/rubocop/cop/sorbet_cops.rb
@@ -1,6 +1,7 @@
# frozen_string_literal: true
require_relative "sorbet/binding_constant_without_type_alias"
require_relative "sorbet/constants_from_strings"
require_relative "sorbet/fetch_when_must"
require_relative "sorbet/forbid_superclass_const_literal"
require_relative "sorbet/forbid_include_const_literal"
require_relative "sorbet/forbid_untyped_struct_props"
Expand Down
1 change: 1 addition & 0 deletions manual/cops.md
Expand Up @@ -15,6 +15,7 @@ In the following section you find all available cops:
* [Sorbet/EnforceSignatures](cops_sorbet.md#sorbetenforcesignatures)
* [Sorbet/EnforceSingleSigil](cops_sorbet.md#sorbetenforcesinglesigil)
* [Sorbet/FalseSigil](cops_sorbet.md#sorbetfalsesigil)
* [Sorbet/FetchWhenMust](cops_sorbet.md#sorbetfetchwhenmust)
* [Sorbet/ForbidExtendTSigHelpersInShims](cops_sorbet.md#sorbetforbidextendtsighelpersinshims)
* [Sorbet/ForbidIncludeConstLiteral](cops_sorbet.md#sorbetforbidincludeconstliteral)
* [Sorbet/ForbidRBIOutsideOfAllowedPaths](cops_sorbet.md#sorbetforbidrbioutsideofallowedpaths)
Expand Down
25 changes: 25 additions & 0 deletions manual/cops_sorbet.md
Expand Up @@ -259,6 +259,31 @@ SuggestedStrictness | `false` | String
Include | `**/*.{rb,rbi,rake,ru}` | Array
Exclude | `bin/**/*`, `db/**/*.rb`, `script/**/*` | Array

## Sorbet/FetchWhenMust

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
--- | --- | --- | --- | ---
Enabled | No | Yes (Unsafe) | <<next>> | -

Checks for `T.must(object[key])` and recommends `object.fetch(key)` instead.

### Examples

```ruby
# bad
T.must(object[key])

# good
object.fetch(key)

# good
object[key]

# good
# If `object` does not `respond_to? :fetch`, or if using `Hash` `default_proc`
T.must(object[key]) # rubocop:disable Sorbet/FetchWhenMust
```

## Sorbet/ForbidExtendTSigHelpersInShims

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
Expand Down
32 changes: 32 additions & 0 deletions spec/rubocop/cop/sorbet/fetch_when_must_spec.rb
@@ -0,0 +1,32 @@
# frozen_string_literal: true

RSpec.describe(RuboCop::Cop::Sorbet::FetchWhenMust, :config) do
it "registers an offense when using `T.must(object[key])`" do
expect_offense(<<~RUBY)
T.must(object[key])
^^^^^^^^^^^^^^^^^^^ Use `object.fetch(key)` instead of `T.must(object[key])` when value must always be found, and receiver supports it.
RUBY

expect_correction(<<~RUBY)
object.fetch(key)
RUBY
end

it "does not register an offense when using `object[key]` without `T.must`" do
expect_no_offenses(<<~RUBY)
object[key]
RUBY
end

it "does not register an offense when using `object[key]` without `T.must`" do
expect_no_offenses(<<~RUBY)
T.must(object.fetch(key))
RUBY
end

it "does not register an offense when using `T.must(object[key1, key2])`" do
expect_no_offenses(<<~RUBY)
T.must(object[key1, key2])
RUBY
end
end

0 comments on commit 7066e9d

Please sign in to comment.