Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sorbet generates unreachable assignment for AR default attributes #1370

Open
lake-effect opened this issue Jan 23, 2023 · 1 comment
Open

Comments

@lake-effect
Copy link

Referred from sorbet/sorbet#6621

Input

Model:

# typed: strict

class Shortcode < ApplicationRecord
  after_initialize :set_defaults, if: :new_record?

  private

  def set_defaults
    self.code ||= SecureRandom.urlsafe_base64(10)
  end
end

# == Schema Information
#
# Table name: shortcodes
#
#  id            :bigint           not null, primary key
#  code          :string           not null
#  created_at    :datetime         not null
#  updated_at    :datetime         not null
#
# Indexes
#
#  index_shortcodes_on_code  (code) UNIQUE

Migration:

class CreateShortcodes < ActiveRecord::Migration[7.0]
  def change
    create_table :shortcodes do |t|
      t.string :code, null: false
      t.index :code, unique: true
      t.timestamps
    end
  end
end

Annotation RBI (generated by tapioca):

class Shortcode
  include GeneratedAttributeMethods
  extend GeneratedRelationMethods

  private

  # cut for length

  module GeneratedAttributeMethods

  # cut for length

    sig { returns(::String) }
    def code; end

    sig { params(value: ::String).returns(::String) }
    def code=(value); end

    sig { returns(T::Boolean) }
    def code?; end

    sig { returns(T.nilable(::String)) }
    def code_before_last_save; end

    sig { returns(T.untyped) }
    def code_before_type_cast; end

    sig { returns(T::Boolean) }
    def code_came_from_user?; end

    sig { returns(T.nilable([::String, ::String])) }
    def code_change; end

    sig { returns(T.nilable([::String, ::String])) }
    def code_change_to_be_saved; end

    sig { returns(T::Boolean) }
    def code_changed?; end

    sig { returns(T.nilable(::String)) }
    def code_in_database; end

    sig { returns(T.nilable([::String, ::String])) }
    def code_previous_change; end

    sig { returns(T::Boolean) }
    def code_previously_changed?; end

    sig { returns(T.nilable(::String)) }
    def code_previously_was; end

    sig { returns(T.nilable(::String)) }
    def code_was; end

    sig { void }
    def code_will_change!; end
  end
end

Observed output

$ bundle exec srb tc
Indexing |=======================================================| ETA: 0h00m00s
Resolving |======================================================| ETA: 0h00m01s
app/models/shortcode.rb:7: This code is unreachable https://srb.help/700600m00s
    7 |    self.code ||= SecureRandom.urlsafe_base64(10)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    app/models/micropage.rb:7: This condition was always truthy (String)
    7 |    self.code ||= SecureRandom.urlsafe_base64(10)
            ^^^^^^^^^^^^^^^
  Got String originating from:
    app/models/shortcode.rb:7:
    7 |    self.short_code ||= SecureRandom.urlsafe_base64(10)
            ^^^^^^^^^^^^^^^
CFG+Inference |==================================================| ETA: 0h00m00s
Errors: 1
error Command failed with exit code 1.

Expected behavior

The check should have passed, because it is possible for self.short_code to be either nil or defined depending on how Shortcode.create! is called. You can call Shortcode.create!(code: "my_code") and create a record with code defined, and you can also call Shortcode.create! at which point line 7 above will assign a random code. The code is not unreachable.


Sorbet version: 0.5.10160
Tapioca version: 0.10.3

@Morriar
Copy link
Collaborator

Morriar commented Jan 24, 2023

@paracycle, regarding your comment in sorbet/sorbet#6621 (comment):

I am surprised that Tapioca is generating a non-nilable column type here.

It seems like we're generating this as a non-type on purpose: https://github.com/Shopify/tapioca/blob/main/spec/tapioca/dsl/compilers/active_record_columns_spec.rb#L169-L212. If the code presented here makes sense, we should change the DSL compiler to produce a nilable type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants