Skip to content

Commit

Permalink
Add cop against .void.checked(:tests)
Browse files Browse the repository at this point in the history
  • Loading branch information
jez committed Dec 30, 2023
1 parent 5e5e119 commit 8be5132
Show file tree
Hide file tree
Showing 6 changed files with 191 additions and 0 deletions.
5 changes: 5 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -293,3 +293,8 @@ Sorbet/ValidSigil:
- bin/**/*
- db/**/*.rb
- script/**/*

Sorbet/VoidCheckedTests:
Description: 'Forbid `.void.checked(:tests)`'
Enabled: true
VersionAdded: 0.7.7
77 changes: 77 additions & 0 deletions lib/rubocop/cop/sorbet/signatures/void_checked_tests.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Sorbet
# Disallows the usage of `.void.checked(:tests)`.
#
# Using `.void` changes the value returned from the method, but only if
# runtime type checking is enabled for the method. Methods marked `.void`
# will return different values in tests compared with non-test
# environments. This is particularly troublesome if branching on the
# result of a `.void` method, because the returned value in test code
# will be the truthy `VOID` value, while the non-test return value may be
# falsy depending on the method's implementation.
#
# - Use `.returns(T.anything).checked(:tests)` to keep the runtime type
# checking for the rest of the parameters.
# - Use `.void.checked(:never)` if you are on an older version of Sorbet
# which does not have `T.anything` (meaning versions 0.5.10781 or
# earlier. Versions released after 2023-04-14 include `T.anything`.)
#
# @example
#
# # bad
# sig { void.checked(:tests) }
#
# # good
# sig { void }
# sig { returns(T.anything).checked(:tests) }
# sig { void.checked(:never) }
class VoidCheckedTests < ::RuboCop::Cop::Base
include(RuboCop::Cop::RangeHelp)
include(RuboCop::Cop::Sorbet::SignatureHelp)
extend AutoCorrector

# @!method checked_tests(node)
def_node_search(:checked_tests, <<~PATTERN)
(send _ :checked (sym :tests))
PATTERN

MESSAGE =
"The return value in a `.void.checked(:tests)` makes test behavior " \
"diffferent from non-test behavior. Either use " \
"`.returns(T.anything).checked(:tests)` to keep checking in tests, " \
"or `.void.checked(:never)` to leave the return completely untouched."
private_constant(:MESSAGE)

private def top_level_void(node)
return nil unless node.is_a?(RuboCop::AST::SendNode)

if node.method_name == :void
node
else
top_level_void(node.receiver)
end
end

def on_signature(node)
checked_send = checked_tests(node).first
return unless checked_send

_recv, _block_args, body = *node
void_send = top_level_void(body)

return unless void_send

add_offense(
void_send.selector,
message: MESSAGE,
) do |corrector|
corrector.replace(void_send.selector, "returns(T.anything)")
end
end
end
end
end
end
1 change: 1 addition & 0 deletions lib/rubocop/cop/sorbet_cops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

require_relative "sorbet/signatures/allow_incompatible_override"
require_relative "sorbet/signatures/checked_true_in_signature"
require_relative "sorbet/signatures/void_checked_tests"
require_relative "sorbet/signatures/keyword_argument_ordering"
require_relative "sorbet/signatures/signature_build_order"
require_relative "sorbet/signatures/enforce_signatures"
Expand Down
1 change: 1 addition & 0 deletions manual/cops.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ In the following section you find all available cops:
* [Sorbet/TrueSigil](cops_sorbet.md#sorbettruesigil)
* [Sorbet/TypeAliasName](cops_sorbet.md#sorbettypealiasname)
* [Sorbet/ValidSigil](cops_sorbet.md#sorbetvalidsigil)
* [Sorbet/VoidCheckedTests](cops_sorbet.md#sorbetvoidcheckedtests)

<!-- END_COP_LIST -->

Expand Down
34 changes: 34 additions & 0 deletions manual/cops_sorbet.md
Original file line number Diff line number Diff line change
Expand Up @@ -884,3 +884,37 @@ MinimumStrictness | `nil` | String
ExactStrictness | `nil` | String
Include | `**/*.{rb,rbi,rake,ru}` | Array
Exclude | `bin/**/*`, `db/**/*.rb`, `script/**/*` | Array
## Sorbet/VoidCheckedTests
Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
--- | --- | --- | --- | ---
Enabled | Yes | Yes | 0.7.7 | -
Disallows the usage of `.void.checked(:tests)`.
Using `.void` changes the value returned from the method, but only if
runtime type checking is enabled for the method. Methods marked `.void`
will return different values in tests compared with non-test
environments. This is particularly troublesome if branching on the
result of a `.void` method, because the returned value in test code
will be the truthy `VOID` value, while the non-test return value may be
falsy depending on the method's implementation.

- Use `.returns(T.anything).checked(:tests)` to keep the runtime type
checking for the rest of the parameters.
- Use `.void.checked(:never)` if you are on an older version of Sorbet
which does not have `T.anything` (meaning versions 0.5.10781 or
earlier. Versions released after 2023-04-14 include `T.anything`.)

### Examples

```ruby
# bad
sig { void.checked(:tests) }
# good
sig { void }
sig { returns(T.anything).checked(:tests) }
sig { void.checked(:never) }
```
73 changes: 73 additions & 0 deletions spec/rubocop/cop/sorbet/signatures/void_checked_tests_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
# frozen_string_literal: true

require "spec_helper"

RSpec.describe(RuboCop::Cop::Sorbet::VoidCheckedTests, :config) do
def message
"The return value in a `.void.checked(:tests)` makes test behavior " \
"diffferent from non-test behavior. Either use " \
"`.returns(T.anything).checked(:tests)` to keep checking in tests, " \
"or `.void.checked(:never)` to leave the return completely untouched."
end

describe("offenses") do
it("disallows using .void.checked(:tests)") do
expect_offense(<<~RUBY)
sig { void.checked(:tests) }
^^^^ #{message}
def foo; end
RUBY

expect_correction(<<~RUBY)
sig { returns(T.anything).checked(:tests) }
def foo; end
RUBY

expect_offense(<<~RUBY)
sig { void.params(x: Integer).override.checked(:tests) }
^^^^ #{message}
def foo(x); end
RUBY

expect_correction(<<~RUBY)
sig { returns(T.anything).params(x: Integer).override.checked(:tests) }
def foo(x); end
RUBY

expect_offense(<<~RUBY)
sig { params(x: Integer).void.checked(:tests) }
^^^^ #{message}
def foo(x); end
RUBY

expect_correction(<<~RUBY)
sig { params(x: Integer).returns(T.anything).checked(:tests) }
def foo(x); end
RUBY
end

it("allows using .returns(T.anything).checked(:tests)") do
expect_no_offenses(<<~RUBY)
sig { returns(T.anything).checked(:tests) }
def foo; end
RUBY

expect_no_offenses(<<~RUBY)
sig { returns(T.anything).params(x: Integer).override.checked(:tests) }
def foo(x); end
RUBY

expect_no_offenses(<<~RUBY)
sig { params(x: Integer).returns(T.anything).checked(:tests) }
def foo(x); end
RUBY
end

it("is not tripped up by the `.void` in `T.proc.void`") do
expect_no_offenses(<<~RUBY)
sig { params(blk: T.proc.void).returns(T.anything).checked(:tests) }
def foo(&blk); end
RUBY
end
end
end

0 comments on commit 8be5132

Please sign in to comment.