Merge pull request #1045 from Shopify/uk-fix-method-name-validation

Fix method name validation edge-case

Author: paracycle

lib/tapioca/helpers/rbi_helper.rb

@@ -93,7 +93,10 @@ def self.serialize_type_variable(type, variance, fixed, upper, lower)
 
     sig { params(name: String).returns(T::Boolean) }
     def valid_method_name?(name)
-      !name.to_sym.inspect.start_with?(':"', ":@", ":$")
+      name == "==" || !(
+        name.to_sym.inspect.start_with?(':"', ":@", ":$") ||
+        name.delete_suffix("=").to_sym.inspect.start_with?(':"', ":@", ":$")
+      )
     end
 
     sig { params(name: String).returns(T::Boolean) }

spec/tapioca/dsl/compilers/active_record_columns_spec.rb

@@ -211,6 +211,35 @@ def title?; end
                 assert_includes(output, expected)
               end
 
+              it "skips columns with names that can't be Ruby method names" do
+                add_ruby_file("schema.rb", <<~RUBY)
+                  ActiveRecord::Migration.suppress_messages do
+                    ActiveRecord::Schema.define do
+                      create_table :posts do |t|
+                        t.string :"4_to_5"
+                        t.string :"foo-bar"
+                        t.string :"@foo"
+                      end
+                    end
+                  end
+                RUBY
+
+                add_ruby_file("post.rb", <<~RUBY)
+                  class Post < ActiveRecord::Base
+                  end
+                RUBY
+
+                output = rbi_for(:Post)
+
+                # "4_to_5" should never be the start of a method name,
+                # but method names like "saved_change_to_4_to_5" are fine.
+                refute_includes(output, "def 4_to_5")
+                # no method name should include "foo-bar"
+                refute_includes(output, "foo-bar")
+                # no method name should include "@foo"
+                refute_includes(output, "@foo")
+              end
+
               it "generates a proper type for every ActiveRecord column type" do
                 add_ruby_file("schema.rb", <<~RUBY)
                   ActiveRecord::Migration.suppress_messages do

spec/tapioca/helpers/rbi_helper_spec.rb

@@ -13,19 +13,19 @@ class Tapioca::RBIHelperSpec < Minitest::Spec
         "!", "~", "+@", "**", "-@", "*", "/", "%", "+", "-", "<<", ">>", "&", "|", "^",
         "<", "<=", ">", ">=", "==", "===", "!=", "=~", "!~", "<=>", "[]", "[]=", "`",
       ].each do |name|
-        assert(valid_method_name?(name))
+        assert(valid_method_name?(name), "Expected `#{name}` to be a valid method name")
       end
     end
 
     it "rejects invalid method names" do
-      ["", "42", "42foo", "!foo", "-@foo", "foo-", "foo-bar", "foo.bar", "=>"].each do |name|
-        refute(valid_method_name?(name))
+      ["", "42", "42foo", "42foo=", "!foo", "-@foo", "foo-", "foo-bar", "foo.bar", "=>"].each do |name|
+        refute(valid_method_name?(name), "Expected `#{name}` to be an invalid method name")
       end
     end
 
     it "accepts valid parameter names" do
       ["f", "foo", "_foo", "foo_", "fOO", "f00", "éxéctôr", "❨╯°□°❩╯︵┻━┻"].each do |name|
-        assert(valid_parameter_name?(name))
+        assert(valid_parameter_name?(name), "Expected `#{name}` to be a valid parameter name")
       end
     end