From f200f042674172dc096617cd57c85bf270ffe2ea Mon Sep 17 00:00:00 2001 From: Alexandre Terrasa Date: Tue, 19 Sep 2023 16:24:56 -0400 Subject: [PATCH] Add cop to forbid T::Struct uses and autocorrect them to bare classes Signed-off-by: Alexandre Terrasa --- config/default.yml | 7 + lib/rubocop/cop/sorbet/forbid_t_struct.rb | 223 +++++++++++ lib/rubocop/cop/sorbet_cops.rb | 1 + manual/cops.md | 1 + manual/cops_sorbet.md | 39 ++ .../cop/sorbet/forbid_t_struct_spec.rb | 378 ++++++++++++++++++ 6 files changed, 649 insertions(+) create mode 100644 lib/rubocop/cop/sorbet/forbid_t_struct.rb create mode 100644 spec/rubocop/cop/sorbet/forbid_t_struct_spec.rb diff --git a/config/default.yml b/config/default.yml index f943701f..35e50559 100644 --- a/config/default.yml +++ b/config/default.yml @@ -97,6 +97,13 @@ Sorbet/ForbidSuperclassConstLiteral: Exclude: - db/migrate/*.rb +Sorbet/ForbidTStruct: + Description: 'Forbid usage of T::Struct.' + Enabled: false + VersionAdded: <> + VersionChanged: <> + Safe: false + Sorbet/ForbidTUnsafe: Description: 'Forbid usage of T.unsafe.' Enabled: false diff --git a/lib/rubocop/cop/sorbet/forbid_t_struct.rb b/lib/rubocop/cop/sorbet/forbid_t_struct.rb new file mode 100644 index 00000000..a46b2fe2 --- /dev/null +++ b/lib/rubocop/cop/sorbet/forbid_t_struct.rb @@ -0,0 +1,223 @@ +# frozen_string_literal: true + +require "rubocop" + +module RuboCop + module Cop + module Sorbet + # Disallow using `T::Struct` and `T::Props`. + # + # @example + # + # # bad + # class MyStruct < T::Struct + # const :foo, String + # prop :bar, Integer, default: 0 + # + # def some_method; end + # end + # + # # good + # class MyStruct + # extend T::Sig + # + # sig { returns(String) } + # attr_reader :foo + # + # sig { returns(Integer) } + # attr_accessor :bar + # + # sig { params(foo: String, bar: Integer) } + # def initialize(foo:, bar: 0) + # @foo = foo + # @bar = bar + # end + # + # def some_method; end + # end + class ForbidTStruct < RuboCop::Cop::Base + include Alignment + include RangeHelp + include CommentsHelp + extend AutoCorrector + + RESTRICT_ON_SEND = [:include, :prepend, :extend].freeze + + MSG_STRUCT = "Using `T::Struct` or its variants is deprecated." + MSG_PROPS = "Using `T::Props` or its variants is deprecated." + + # This class walks down the class body of a T::Struct and collects all the properties that will need to be + # translated into `attr_reader` and `attr_accessor` methods. + class TStructWalker + include AST::Traversal + extend AST::NodePattern::Macros + + attr_reader :props, :has_extend_t_sig + + def initialize + @props = [] + @has_extend_t_sig = false + end + + # @!method extend_t_sig?(node) + def_node_matcher :extend_t_sig?, <<~PATTERN + (send _ :extend (const (const {nil? | cbase} :T) :Sig)) + PATTERN + + # @!method t_struct_prop?(node) + def_node_matcher(:t_struct_prop?, <<~PATTERN) + (send nil? {:const :prop} ...) + PATTERN + + def on_send(node) + if extend_t_sig?(node) + # So we know we won't need to generate again a `extend T::Sig` line in the new class body + @has_extend_t_sig = true + return + end + + return unless t_struct_prop?(node) + + kind = node.method?(:const) ? :attr_reader : :attr_accessor + name = node.arguments[0].source.delete_prefix(":") + type = node.arguments[1].source + default = nil + factory = nil + + node.arguments[2..-1].each do |arg| + next unless arg.hash_type? + + arg.each_pair do |key, value| + case key.source + when "default" + default = value.source + when "factory" + factory = value.source + end + end + end + + @props << Property.new(node, kind, name, type, default: default, factory: factory) + end + end + + class Property + attr_reader :node, :kind, :name, :type, :default, :factory + + def initialize(node, kind, name, type, default:, factory:) + @node = node + @kind = kind + @name = name + @type = type + @default = default + @factory = factory + + # A T::Struct should have both a default and a factory, if we find one let's raise an error + raise if @default && @factory + end + + def attr_sig + "sig { returns(#{type}) }" + end + + def attr_accessor + "#{kind} :#{name}" + end + + def initialize_sig_param + "#{name}: #{type}" + end + + def initialize_param + rb = String.new + rb << "#{name}:" + rb << " #{default}" if default + rb << " #{factory}" if factory + rb + end + + def initialize_assign + rb = String.new + rb << "@#{name} = #{name}" + rb << ".call" if factory + rb + end + end + + # @!method t_struct?(node) + def_node_matcher(:t_struct?, <<~PATTERN) + (const (const {nil? cbase} :T) {:Struct :ImmutableStruct :InexactStruct}) + PATTERN + + # @!method t_props?(node) + def_node_matcher(:t_props?, "(send nil? {:include :prepend :extend} `(const (const {nil? cbase} :T) :Props))") + + def on_class(node) + return unless t_struct?(node.parent_class) + + add_offense(node, message: MSG_STRUCT) do |corrector| + walker = TStructWalker.new + walker.walk(node.body) + + range = range_between(node.identifier.source_range.end_pos, node.parent_class.source_range.end_pos) + corrector.remove(range) + next if node.single_line? + + unless walker.has_extend_t_sig + indent = offset(node) + corrector.insert_after(node.identifier, "\n#{indent} extend T::Sig\n") + end + + first_prop = walker.props.first + walker.props.each do |prop| + node = prop.node + indent = offset(node) + line_range = range_by_whole_lines(prop.node.source_range) + new_line = prop != first_prop && !previous_line_blank?(node) + trailing_comments = processed_source.each_comment_in_lines(line_range.line..line_range.line) + + corrector.replace( + line_range, + "#{new_line ? "\n" : ""}" \ + "#{trailing_comments.map { |comment| "#{indent}#{comment.text}\n" }.join}" \ + "#{indent}#{prop.attr_sig}\n#{indent}#{prop.attr_accessor}", + ) + end + + last_prop = walker.props.last + if last_prop + indent = offset(last_prop.node) + line_range = range_by_whole_lines(last_prop.node.source_range, include_final_newline: true) + corrector.insert_after(line_range, initialize_method(indent, walker.props)) + end + end + end + + def on_send(node) + return unless t_props?(node) + + add_offense(node, message: MSG_PROPS) + end + + private + + def initialize_method(indent, props) + # We sort optional keyword arguments after required ones + props = props.sort_by { |prop| prop.default || prop.factory ? 1 : 0 } + + string = +"\n" + string << "#{indent}sig { params(#{props.map(&:initialize_sig_param).join(", ")}).void }\n" + string << "#{indent}def initialize(#{props.map(&:initialize_param).join(", ")})\n" + props.each do |prop| + string << "#{indent} #{prop.initialize_assign}\n" + end + string << "#{indent}end\n" + end + + def previous_line_blank?(node) + processed_source.buffer.source_line(node.source_range.line - 1).blank? + end + end + end + end +end diff --git a/lib/rubocop/cop/sorbet_cops.rb b/lib/rubocop/cop/sorbet_cops.rb index 83912772..93831a98 100644 --- a/lib/rubocop/cop/sorbet_cops.rb +++ b/lib/rubocop/cop/sorbet_cops.rb @@ -10,6 +10,7 @@ require_relative "sorbet/implicit_conversion_method" require_relative "sorbet/one_ancestor_per_line" require_relative "sorbet/callback_conditionals_binding" +require_relative "sorbet/forbid_t_struct" require_relative "sorbet/forbid_t_unsafe" require_relative "sorbet/forbid_t_untyped" require_relative "sorbet/redundant_extend_t_sig" diff --git a/manual/cops.md b/manual/cops.md index 6946c44e..8a05089b 100644 --- a/manual/cops.md +++ b/manual/cops.md @@ -20,6 +20,7 @@ In the following section you find all available cops: * [Sorbet/ForbidIncludeConstLiteral](cops_sorbet.md#sorbetforbidincludeconstliteral) * [Sorbet/ForbidRBIOutsideOfAllowedPaths](cops_sorbet.md#sorbetforbidrbioutsideofallowedpaths) * [Sorbet/ForbidSuperclassConstLiteral](cops_sorbet.md#sorbetforbidsuperclassconstliteral) +* [Sorbet/ForbidTStruct](cops_sorbet.md#sorbetforbidtstruct) * [Sorbet/ForbidTUnsafe](cops_sorbet.md#sorbetforbidtunsafe) * [Sorbet/ForbidTUntyped](cops_sorbet.md#sorbetforbidtuntyped) * [Sorbet/ForbidUntypedStructProps](cops_sorbet.md#sorbetforbiduntypedstructprops) diff --git a/manual/cops_sorbet.md b/manual/cops_sorbet.md index b2addd10..dea5c070 100644 --- a/manual/cops_sorbet.md +++ b/manual/cops_sorbet.md @@ -421,6 +421,45 @@ Name | Default value | Configurable values --- | --- | --- Exclude | `db/migrate/*.rb` | Array +## Sorbet/ForbidTStruct + +Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged +--- | --- | --- | --- | --- +Disabled | No | Yes | <> | <> + +Disallow using `T::Struct` and `T::Props`. + +### Examples + +```ruby +# bad +class MyStruct < T::Struct + const :foo, String + prop :bar, Integer, default: 0 + + def some_method; end +end + +# good +class MyStruct + extend T::Sig + + sig { returns(String) } + attr_reader :foo + + sig { returns(Integer) } + attr_accessor :bar + + sig { params(foo: String, bar: Integer) } + def initialize(foo:, bar: 0) + @foo = foo + @bar = bar + end + + def some_method; end +end +``` + ## Sorbet/ForbidTUnsafe Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged diff --git a/spec/rubocop/cop/sorbet/forbid_t_struct_spec.rb b/spec/rubocop/cop/sorbet/forbid_t_struct_spec.rb new file mode 100644 index 00000000..58be5d10 --- /dev/null +++ b/spec/rubocop/cop/sorbet/forbid_t_struct_spec.rb @@ -0,0 +1,378 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe(RuboCop::Cop::Sorbet::ForbidTStruct, :config) do + describe("Offenses") do + it "adds offense when inheriting T::Struct on a multiline class" do + expect_offense(<<~RUBY) + class Foo < T::Struct + ^^^^^^^^^^^^^^^^^^^^^ Using `T::Struct` or its variants is deprecated. + end + RUBY + end + + it "adds offense when inheriting T::Struct on a singleline class" do + expect_offense(<<~RUBY) + class Foo < T::Struct; end + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Using `T::Struct` or its variants is deprecated. + RUBY + end + + it "adds offense when inheriting ::T::Struct" do + expect_offense(<<~RUBY) + class Foo < ::T::Struct; end + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Using `T::Struct` or its variants is deprecated. + RUBY + end + + it "adds offense when inheriting T::ImmutableStruct" do + expect_offense(<<~RUBY) + class Foo < T::ImmutableStruct + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Using `T::Struct` or its variants is deprecated. + end + RUBY + end + + it "adds offense when inheriting T::InexactStruct" do + expect_offense(<<~RUBY) + class Foo < T::InexactStruct + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Using `T::Struct` or its variants is deprecated. + end + RUBY + end + + it "adds offense when including anything related to T::Props" do + expect_offense(<<~RUBY) + class Foo + include T::Props + ^^^^^^^^^^^^^^^^ Using `T::Props` or its variants is deprecated. + include T::Props::Constructor + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Using `T::Props` or its variants is deprecated. + include T::Props::WeakConstructor + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Using `T::Props` or its variants is deprecated. + prepend T::Props::Foo + ^^^^^^^^^^^^^^^^^^^^^ Using `T::Props` or its variants is deprecated. + extend T::Props::Bar + ^^^^^^^^^^^^^^^^^^^^ Using `T::Props` or its variants is deprecated. + extend ::T::Props + ^^^^^^^^^^^^^^^^^ Using `T::Props` or its variants is deprecated. + end + RUBY + end + + it "adds offense for nested structs" do + expect_offense(<<~RUBY) + class Foo < T::Struct + ^^^^^^^^^^^^^^^^^^^^^ Using `T::Struct` or its variants is deprecated. + class Bar < T::Struct + ^^^^^^^^^^^^^^^^^^^^^ Using `T::Struct` or its variants is deprecated. + end + end + RUBY + end + end + + describe("No offenses") do + it "does not add offense when not using T::Struct" do + expect_no_offenses(<<~RUBY) + class Foo + end + + class Bar < Baz; end + + class Baz + extend T::Struct + end + + class T::Struct; end + RUBY + end + end + + describe("Autocorrect") do + it "changes T::Struct to a bare class" do + source = <<~RUBY + class Foo < T::Struct; end + RUBY + + corrected = <<~RUBY + class Foo; end + RUBY + + expect(autocorrect_source(source)).to(eq(corrected)) + end + + it "generates the bare class body" do + source = <<~RUBY + class Foo < T::Struct + const :foo, Integer + prop :bar, String, default: "foo" + const :baz, T.nilable(Symbol), factory: ->{ nil } + end + RUBY + + corrected = <<~RUBY + class Foo + extend T::Sig + + sig { returns(Integer) } + attr_reader :foo + + sig { returns(String) } + attr_accessor :bar + + sig { returns(T.nilable(Symbol)) } + attr_reader :baz + + sig { params(foo: Integer, bar: String, baz: T.nilable(Symbol)).void } + def initialize(foo:, bar: "foo", baz: ->{ nil }) + @foo = foo + @bar = bar + @baz = baz.call + end + end + RUBY + + expect(autocorrect_source(source)).to(eq(corrected)) + end + + it "generates initialize parameters in the correct order" do + source = <<~RUBY + class Foo < T::Struct + const :foo, Integer + prop :bar, String, default: "foo" + const :baz, Symbol + end + RUBY + + corrected = <<~RUBY + class Foo + extend T::Sig + + sig { returns(Integer) } + attr_reader :foo + + sig { returns(String) } + attr_accessor :bar + + sig { returns(Symbol) } + attr_reader :baz + + sig { params(foo: Integer, baz: Symbol, bar: String).void } + def initialize(foo:, baz:, bar: "foo") + @foo = foo + @baz = baz + @bar = bar + end + end + RUBY + + expect(autocorrect_source(source)).to(eq(corrected)) + end + + it "keeps other nodes in the body" do + source = <<~RUBY + class Foo < T::Struct + CONST = 42 + + const :foo, Integer + + @foo = 42 + + # Some comment + sig { params(x: Integer).returns(String) } + def foo(x) + "foo" * x + end + + private + + sig do + void + end + def self.bar; end + + class << self + def bar; end + end + + # Another comment + class Bar + class Baz; end + end + end + RUBY + + corrected = <<~RUBY + class Foo + extend T::Sig + + CONST = 42 + + sig { returns(Integer) } + attr_reader :foo + + sig { params(foo: Integer).void } + def initialize(foo:) + @foo = foo + end + + @foo = 42 + + # Some comment + sig { params(x: Integer).returns(String) } + def foo(x) + "foo" * x + end + + private + + sig do + void + end + def self.bar; end + + class << self + def bar; end + end + + # Another comment + class Bar + class Baz; end + end + end + RUBY + + expect(autocorrect_source(source)).to(eq(corrected)) + end + + it "preserves the right indent" do + source = <<~RUBY + module Bar + class Foo < T::Struct + const :foo, Integer + + def bar; end + end + end + RUBY + + corrected = <<~RUBY + module Bar + class Foo + extend T::Sig + + sig { returns(Integer) } + attr_reader :foo + + sig { params(foo: Integer).void } + def initialize(foo:) + @foo = foo + end + + def bar; end + end + end + RUBY + + expect(autocorrect_source(source)).to(eq(corrected)) + end + + it "does not duplicate extend T::Sig" do + source = <<~RUBY + class Foo < T::Struct + extend T::Sig + + const :foo, Integer + + def bar; end + end + RUBY + + corrected = <<~RUBY + class Foo + extend T::Sig + + sig { returns(Integer) } + attr_reader :foo + + sig { params(foo: Integer).void } + def initialize(foo:) + @foo = foo + end + + def bar; end + end + RUBY + + expect(autocorrect_source(source)).to(eq(corrected)) + end + + it "preserves right blank lines between properties" do + source = <<~RUBY + class Foo < T::Struct + const :foo, Integer + + prop :bar, String + end + RUBY + + corrected = <<~RUBY + class Foo + extend T::Sig + + sig { returns(Integer) } + attr_reader :foo + + sig { returns(String) } + attr_accessor :bar + + sig { params(foo: Integer, bar: String).void } + def initialize(foo:, bar:) + @foo = foo + @bar = bar + end + end + RUBY + + expect(autocorrect_source(source)).to(eq(corrected)) + end + + it "preserves comments on properties" do + source = <<~RUBY + class Foo < T::Struct + # Some comment here + const :foo, Integer + prop :bar, String # Another comment here + + # Some loose comment after + end + RUBY + + corrected = <<~RUBY + class Foo + extend T::Sig + + # Some comment here + sig { returns(Integer) } + attr_reader :foo + + # Another comment here + sig { returns(String) } + attr_accessor :bar + + sig { params(foo: Integer, bar: String).void } + def initialize(foo:, bar:) + @foo = foo + @bar = bar + end + + # Some loose comment after + end + RUBY + + expect(autocorrect_source(source)).to(eq(corrected)) + end + end +end