Merge pull request #1937 from alex-tan/no-nilable-for-persisted

In ActiveRecordColumns persisted mode, remove T.nilable from reflected sigs

lib/tapioca/dsl/helpers/active_record_column_type_helper.rb

@@ -104,7 +104,7 @@ def column_type_for(column_name)
 
           column = @constant.columns_hash[column_name]
           column_type = @constant.attribute_types[column_name]
-          getter_type = type_for_activerecord_value(column_type)
+          getter_type = type_for_activerecord_value(column_type, column_nullability: !!column&.null)
           setter_type =
             case column_type
             when ActiveRecord::Enum::EnumType
@@ -121,8 +121,8 @@ def column_type_for(column_name)
           end
         end
 
-        sig { params(column_type: T.untyped).returns(String) }
-        def type_for_activerecord_value(column_type)
+        sig { params(column_type: T.untyped, column_nullability: T::Boolean).returns(String) }
+        def type_for_activerecord_value(column_type, column_nullability:)
           case column_type
           when ->(type) { defined?(MoneyColumn) && MoneyColumn::ActiveRecordType === type }
             "::Money"
@@ -133,11 +133,12 @@ def type_for_activerecord_value(column_type)
                }
             # Reflect to see if `ActiveModel::Type::Value` is being used first.
             getter_type = Tapioca::Dsl::Helpers::ActiveModelTypeHelper.type_for(column_type)
-            return getter_type unless getter_type == "T.untyped"
 
-            # Otherwise fallback to String as `ActiveRecord::Encryption::EncryptedAttributeType` inherits from
+            # Fallback to String as `ActiveRecord::Encryption::EncryptedAttributeType` inherits from
             # `ActiveRecord::Type::Text` which inherits from `ActiveModel::Type::String`.
-            "::String"
+            return "::String" if getter_type == "T.untyped"
+
+            as_non_nilable_if_persisted_and_not_nullable(getter_type, column_nullability:)
           when ActiveRecord::Type::String
             "::String"
           when ActiveRecord::Type::Date
@@ -160,7 +161,7 @@ def type_for_activerecord_value(column_type)
                  defined?(ActiveRecord::Normalization::NormalizedValueType) &&
                    ActiveRecord::Normalization::NormalizedValueType === type
                }
-            type_for_activerecord_value(column_type.cast_type)
+            type_for_activerecord_value(column_type.cast_type, column_nullability:)
           when ->(type) {
                  defined?(ActiveRecord::ConnectionAdapters::PostgreSQL::OID::Uuid) &&
                    ActiveRecord::ConnectionAdapters::PostgreSQL::OID::Uuid === type
@@ -180,12 +181,25 @@ def type_for_activerecord_value(column_type)
                  defined?(ActiveRecord::ConnectionAdapters::PostgreSQL::OID::Array) &&
                    ActiveRecord::ConnectionAdapters::PostgreSQL::OID::Array === type
                }
-            "T::Array[#{type_for_activerecord_value(column_type.subtype)}]"
+            "T::Array[#{type_for_activerecord_value(column_type.subtype, column_nullability:)}]"
           else
-            ActiveModelTypeHelper.type_for(column_type)
+            as_non_nilable_if_persisted_and_not_nullable(
+              ActiveModelTypeHelper.type_for(column_type),
+              column_nullability: column_nullability,
+            )
           end
         end
 
+        sig { params(base_type: String, column_nullability: T::Boolean).returns(String) }
+        def as_non_nilable_if_persisted_and_not_nullable(base_type, column_nullability:)
+          # It's possible that when ActiveModel::Type::Value is used, the signature being reflected on in
+          # ActiveModelTypeHelper.type_for(type_value) may say the type can be nilable. However, if the type is
+          # persisted and the column is not nullable, we can assume it's not nilable.
+          return as_non_nilable_type(base_type) if @column_type_option.persisted? && !column_nullability
+
+          base_type
+        end
+
         sig { params(column_type: ActiveRecord::Enum::EnumType).returns(String) }
         def enum_setter_type(column_type)
           # In Rails < 7 this method is private. When support for that is dropped we can call the method directly

spec/tapioca/dsl/compilers/active_record_columns_spec.rb

@@ -282,6 +282,49 @@ def custom; end
                 assert_includes(rbi_for(:Post), expected)
               end
 
+              it "strips T.nilable from sigs in persisted mode when using EncryptedAttributeType" do
+                add_ruby_file("schema.rb", <<~RUBY)
+                  ActiveRecord::Migration.suppress_messages do
+                    ActiveRecord::Schema.define do
+                      create_table :posts do |t|
+                        t.string :custom, null: false
+                      end
+                    end
+                  end
+                RUBY
+
+                add_ruby_file("custom_type.rb", <<~RUBY)
+                  class CustomType < ActiveRecord::Encryption::EncryptedAttributeType
+                    extend T::Sig
+
+                    sig { params(value: T.untyped).returns(T.nilable(CustomType)) }
+                    def deserialize(value)
+                      CustomType.new(value) unless value.nil?
+                    end
+
+                    def serialize(value)
+                      value
+                    end
+                  end
+                RUBY
+
+                add_ruby_file("post.rb", <<~RUBY)
+                  class Post < ActiveRecord::Base
+                    attribute :custom, CustomType.new(
+                      scheme: ActiveRecord::Encryption::Scheme.new,
+                    )
+                  end
+                RUBY
+
+                expected = indented(<<~RBI, 2)
+                  module GeneratedAttributeMethods
+                    sig { returns(::CustomType) }
+                    def custom; end
+                RBI
+
+                assert_includes(rbi_for(:Post), expected)
+              end
+
               it "respects nullability of attributes" do
                 add_ruby_file("schema.rb", <<~RUBY)
                   ActiveRecord::Migration.suppress_messages do
@@ -984,6 +1027,53 @@ def cost=(value); end
                 assert_includes(rbi_for(:Post), expected)
               end
 
+              it "strips T.nilable from reflected signatures method for non-nilable columns in persisted mode" do
+                add_ruby_file("schema.rb", <<~RUBY)
+                  ActiveRecord::Migration.suppress_messages do
+                    ActiveRecord::Schema.define do
+                      create_table :posts do |t|
+                        t.decimal :cost, null: false
+                      end
+                    end
+                  end
+                RUBY
+
+                add_ruby_file("custom_type.rb", <<~RUBY)
+                  class CustomType
+                    attr_accessor :value
+
+                    def initialize(number = 0.0)
+                      @value = number
+                    end
+
+                    class Type < ActiveRecord::Type::Value
+                      extend(T::Sig)
+
+                      sig { params(value: T.nilable(Numeric)).returns(T.nilable(::CustomType))}
+                      def deserialize(value)
+                        CustomType.new(value) if value
+                      end
+                    end
+                  end
+                RUBY
+
+                add_ruby_file("post.rb", <<~RUBY)
+                  class Post < ActiveRecord::Base
+                    attribute :cost, CustomType::Type.new
+                  end
+                RUBY
+
+                expected = indented(<<~RBI, 4)
+                  sig { returns(::CustomType) }
+                  def cost; end
+
+                  sig { params(value: ::CustomType).returns(::CustomType) }
+                  def cost=(value); end
+                RBI
+
+                assert_includes(rbi_for(:Post), expected)
+              end
+
               it "generates id accessors when primary key isn't id" do
                 add_ruby_file("schema.rb", <<~RUBY)
                   ActiveRecord::Migration.suppress_messages do