steep icon indicating copy to clipboard operation
steep copied to clipboard

optional types should not be optional anymore after nil/empty check

Open HoneyryderChuck opened this issue 4 years ago • 3 comments

Consider the following code:

# @type var x: Integer?
x = nil # or 1
return unless x
x + 1

steep will complain about this, because + can't be used on nil. However, the nil check has already been done.

The static analyzer should therefore "transform" the check on x to perceive it as an Integer after the nil check.

A variation of the same (that also produces an error is):

# @type var x: Array[untyped]?
x = [] # or [1]
return unless x.empty?
x.first

Although first can be untyped or nil, after the empty check, it can only be untyped.

HoneyryderChuck avatar Jul 20 '21 14:07 HoneyryderChuck

@soutaro any progress on this? i'm still seeing with 0.49.1, which makes it very awkward when empty returns work after #500 was merged

clinejj avatar Mar 17 '22 22:03 clinejj

@HoneyryderChuck @clinejj I will take a look at the problem if we can make flow sensitive typing work here.

In fact, it is intentional, that the type annotation wins over flow sensitive typing.

The code below works because the type of a is inferred.

a = [1].sample()
return unless a
a + 1

soutaro avatar Mar 18 '22 02:03 soutaro

@soutaro awesome, thank you. The issue I'm running into is something like this:

class OrdersController < ApplicationController
  before_action :set_order

  def clone
    new_order = clone_order
    
    if new_order&.save
      redirect_to new_order
    else
      redirect_to @order
    end
  end

  private

  def set_order
    @order = Orders.first
  end

  def clone_order
    return unless @order
    new_order = @order.dup
    new_order.author = current_user
    # Steep errors on this line about @order.attachments in both calls
    new_order.attachments.attach(@order.attachments.blobs) if @order.attachments.exists?
    new_order
  end
end

with the type definition below:

class OrdersController < ApplicationController
  @order: Order?

  def clone: -> void

  private

  def set_order: -> Order?
  def clone_order: -> Order?
end

This makes it difficult to code in such a way that steep passes, since there effectively isn't a way to code the clone_order method such that Steep doesn't have an issue, particularly when you start to call into third party library methods where you can't modify the type definition. In this case, we could use the safe navigation operator on @order.attachments, however the #attach method comes from ActiveStorage and cannot accept a nil parameter.

clinejj avatar Mar 18 '22 15:03 clinejj