diff --git a/config/default.yml b/config/default.yml index 63fdd04..22bd7c3 100644 --- a/config/default.yml +++ b/config/default.yml @@ -2,3 +2,8 @@ TypeToolkit/DontExpectUnexpectedNil: Description: "Detects misuse of UnexpectedNilError (rescuing, raising, or asserting it)." Enabled: true VersionAdded: "0.1.0" + +TypeToolkit/PreferNotNil: + Description: "Replaces T.must assertions with Type Toolkit's not_nil! assertion." + Enabled: true + VersionAdded: "0.1.0" diff --git a/lib/rubocop-type_toolkit.rb b/lib/rubocop-type_toolkit.rb index d55e476..e716759 100644 --- a/lib/rubocop-type_toolkit.rb +++ b/lib/rubocop-type_toolkit.rb @@ -3,3 +3,4 @@ require "rubocop" require_relative "rubocop/cop/type_toolkit/plugin" require_relative "rubocop/cop/type_toolkit/dont_expect_unexpected_nil" +require_relative "rubocop/cop/type_toolkit/prefer_not_nil" diff --git a/lib/rubocop/cop/type_toolkit/plugin.rb b/lib/rubocop/cop/type_toolkit/plugin.rb index a46c367..7bdcd89 100644 --- a/lib/rubocop/cop/type_toolkit/plugin.rb +++ b/lib/rubocop/cop/type_toolkit/plugin.rb @@ -13,7 +13,7 @@ def about name: "rubocop-type_toolkit", version: ::TypeToolkit::VERSION, homepage: "https://github.com/Shopify/type_toolkit", - description: "Detects misuse of UnexpectedNilError.", + description: "RuboCop rules for Type Toolkit.", ) end diff --git a/lib/rubocop/cop/type_toolkit/prefer_not_nil.rb b/lib/rubocop/cop/type_toolkit/prefer_not_nil.rb new file mode 100644 index 0000000..ff3968b --- /dev/null +++ b/lib/rubocop/cop/type_toolkit/prefer_not_nil.rb @@ -0,0 +1,96 @@ +# typed: true +# frozen_string_literal: true + +module RuboCop + module Cop + module TypeToolkit + # Replaces Sorbet's `T.must(value)` assertion with Type Toolkit's `value.not_nil!` assertion. + class PreferNotNil < Base + extend AutoCorrector + + MSG = "Use `.not_nil!` instead of `T.must()`." + RESTRICT_ON_SEND = [:must].freeze + + COMMA_BYTE = ",".ord + private_constant :COMMA_BYTE + + KEYWORD_EXPRESSION_TYPES = [:defined?, :super, :yield, :zsuper].freeze + private_constant :KEYWORD_EXPRESSION_TYPES + + #: (RuboCop::AST::SendNode) -> void + def on_send(node) + return unless (argument = extract_t_must_argument(node)) + + if nested_t_must?(node) + add_offense(node, message: MSG) + else + replacement = replacement_for(argument) + correction = correction_for(node, argument, replacement) + + add_offense(node, message: MSG) do |corrector| + corrector.replace(node, correction) + end + end + end + + private + + #: (RuboCop::AST::SendNode) -> RuboCop::AST::Node? + def extract_t_must_argument(node) + receiver = node.receiver + return unless receiver.is_a?(RuboCop::AST::ConstNode) + return unless receiver.short_name == :T && node.method?(:must) && node.arguments.one? + + namespace = receiver.namespace + return unless namespace.nil? || namespace.cbase_type? + + argument = node.first_argument + return unless argument + return if argument.splat_type? || argument.kwsplat_type? + + argument + end + + #: (RuboCop::AST::Node) -> String + def replacement_for(argument) + source = argument.source + source = "(#{source})" if requires_parentheses?(argument) + "#{source}.not_nil!" + end + + #: (RuboCop::AST::SendNode, RuboCop::AST::Node, String) -> String + def correction_for(node, argument, replacement) + return replacement unless node.multiline? + + grouped_range = node.source_range.with(begin_pos: node.loc.begin.begin_pos, end_pos: node.loc.end.end_pos) + grouped_source = grouped_range.source + comma_offset = argument.source_range.end_pos - grouped_range.begin_pos + grouped_source.slice!(comma_offset) if grouped_source.getbyte(comma_offset) == COMMA_BYTE + "#{grouped_source}.not_nil!" + end + + #: (RuboCop::AST::SendNode) -> bool + def nested_t_must?(node) + node.each_ancestor(:send).any? do |ancestor| + ancestor.is_a?(RuboCop::AST::SendNode) && extract_t_must_argument(ancestor) + end + end + + #: (RuboCop::AST::Node) -> bool + def requires_parentheses?(argument) + return false if argument.begin_type? + + if argument.is_a?(RuboCop::AST::SendNode) + return true if argument.operator_method? + return true if argument.arguments? && !argument.parenthesized_call? + end + return true if argument.range_type? || argument.operator_keyword? + return true if argument.if_type? || argument.assignment? + return true if argument.any_block_type? + + KEYWORD_EXPRESSION_TYPES.include?(argument.type) + end + end + end + end +end diff --git a/spec/rubocop/cop/type_toolkit/prefer_not_nil_spec.rb b/spec/rubocop/cop/type_toolkit/prefer_not_nil_spec.rb new file mode 100644 index 0000000..eb1cfce --- /dev/null +++ b/spec/rubocop/cop/type_toolkit/prefer_not_nil_spec.rb @@ -0,0 +1,229 @@ +# typed: true +# frozen_string_literal: true + +require "spec_helper" +require "rubocop" +require "rubocop/minitest/assert_offense" +require "rubocop-type_toolkit" + +module RuboCop + module Cop + module TypeToolkit + class PreferNotNilSpec < ::Minitest::Spec + include RuboCop::Minitest::AssertOffense + + MSG = "TypeToolkit/PreferNotNil: Use `.not_nil!` instead of `T.must()`." + + before do + @cop = PreferNotNil.new + end + + it "autocorrects T.must" do + assert_offense(<<~RUBY) + value = T.must(foo) + ^^^^^^^^^^^ #{MSG} + RUBY + + assert_correction(<<~RUBY) + value = foo.not_nil! + RUBY + end + + it "autocorrects ::T.must" do + assert_offense(<<~RUBY) + value = ::T.must(foo) + ^^^^^^^^^^^^^ #{MSG} + RUBY + + assert_correction(<<~RUBY) + value = foo.not_nil! + RUBY + end + + it "autocorrects inside string interpolation" do + assert_offense(<<~RUBY) + string = "\#{T.must(foo)}" + ^^^^^^^^^^^ #{MSG} + RUBY + + assert_correction(<<~RUBY) + string = "\#{foo.not_nil!}" + RUBY + end + + it "preserves the precedence of conditional and range expressions" do + assert_offense(<<~RUBY) + conditional = T.must(condition ? foo : bar) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{MSG} + range = T.must(foo..bar) + ^^^^^^^^^^^^^^^^ #{MSG} + RUBY + + assert_correction(<<~RUBY) + conditional = (condition ? foo : bar).not_nil! + range = (foo..bar).not_nil! + RUBY + end + + it "parenthesizes every expression that requires it" do + assert_offense(<<~RUBY) + operator = T.must(foo + bar) + ^^^^^^^^^^^^^^^^^ #{MSG} + logical = T.must(foo || bar) + ^^^^^^^^^^^^^^^^^^ #{MSG} + grouped = T.must((foo || bar)) + ^^^^^^^^^^^^^^^^^^^^ #{MSG} + assignment = T.must(foo = bar) + ^^^^^^^^^^^^^^^^^ #{MSG} + block_value = T.must(foo { bar }) + ^^^^^^^^^^^^^^^^^^^ #{MSG} + defined_value = T.must(defined?(foo)) + ^^^^^^^^^^^^^^^^^^^^^ #{MSG} + def example + T.must(yield foo) + ^^^^^^^^^^^^^^^^^ #{MSG} + end + def implicit_super + T.must(super) + ^^^^^^^^^^^^^ #{MSG} + end + def explicit_super + T.must(super(foo)) + ^^^^^^^^^^^^^^^^^^ #{MSG} + end + RUBY + + assert_correction(<<~RUBY) + operator = (foo + bar).not_nil! + logical = (foo || bar).not_nil! + grouped = (foo || bar).not_nil! + assignment = (foo = bar).not_nil! + block_value = (foo { bar }).not_nil! + defined_value = (defined?(foo)).not_nil! + def example + (yield foo).not_nil! + end + def implicit_super + (super).not_nil! + end + def explicit_super + (super(foo)).not_nil! + end + RUBY + end + + it "preserves the precedence of command calls" do + assert_offense(<<~RUBY) + value = T.must(fetch value) + ^^^^^^^^^^^^^^^^^^^ #{MSG} + RUBY + + assert_correction(<<~RUBY) + value = (fetch value).not_nil! + RUBY + end + + it "does not add unnecessary parentheses to parenthesized calls" do + assert_offense(<<~RUBY) + value = T.must(fetch(value)) + ^^^^^^^^^^^^^^^^^^^^ #{MSG} + RUBY + + assert_correction(<<~RUBY) + value = fetch(value).not_nil! + RUBY + end + + it "preserves comments in multiline calls" do + assert_offense(<<~RUBY) + value = T.must( + ^^^^^^^ #{MSG} + # Proven non-nil by validation. + foo, + ) + RUBY + + assert_correction(<<~RUBY) + value = ( + # Proven non-nil by validation. + foo + ).not_nil! + RUBY + end + + it "autocorrects multiline calls with whitespace before the method" do + assert_offense(<<~RUBY) + first = T .must( + ^^^^^^^^ #{MSG} + foo, + ) + second = T + ^ #{MSG} + .must( + bar, + ) + RUBY + + assert_correction(<<~RUBY) + first = ( + foo + ).not_nil! + second = ( + bar + ).not_nil! + RUBY + end + + it "removes a trailing comma before an inline comment" do + assert_offense(<<~RUBY) + value = T.must( + ^^^^^^^ #{MSG} + foo, # Proven non-nil. + ) + RUBY + + assert_correction(<<~RUBY) + value = ( + foo # Proven non-nil. + ).not_nil! + RUBY + end + + it "autocorrects nested T.must calls" do + assert_offense(<<~RUBY) + value = T.must(T.must(foo).bar) + ^^^^^^^^^^^^^^^^^^^^^^^ #{MSG} + ^^^^^^^^^^^ #{MSG} + RUBY + + assert_correction(<<~RUBY) + value = foo.not_nil!.bar.not_nil! + RUBY + end + + it "autocorrects three nested T.must calls" do + assert_offense(<<~RUBY) + value = T.must(T.must(T.must(foo).bar).baz) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{MSG} + ^^^^^^^^^^^^^^^^^^^^^^^ #{MSG} + ^^^^^^^^^^^ #{MSG} + RUBY + + assert_correction(<<~RUBY) + value = foo.not_nil!.bar.not_nil!.baz.not_nil! + RUBY + end + + it "ignores other receivers, methods, and argument counts" do + assert_no_offenses(<<~RUBY) + Other::T.must(foo) + object.must(foo) + T.let(foo, String) + T.must(foo, bar) + T.must(*values) + RUBY + end + end + end + end +end