AaronC81/sord

Sord generates an invalid rbi file for haml

connorshea opened this issue · 7 comments

Describe the bug
haml.rbi has an invalid method in it for some reason:

sig { params(index: T.untyped).returns(T.untyped) }
def rstrip_buffer!(index = -1)); end

To Reproduce
Run sord on haml and then run srb tc on the resulting rbi.

Expected behavior
It'd output a valid rbi file.

Actual behavior
It outputs this method with an extra closing parenthesis:

sig { params(index: T.untyped).returns(T.untyped) }
def rstrip_buffer!(index = -1)); end

Additional information
The problem isn't with Haml's method, as far as I can tell it's valid:

# Get rid of and whitespace at the end of the buffer
# or the merged text
def rstrip_buffer!(index = -1)
  last = @to_merge[index]
  if last.nil?
    push_silent("_hamlout.rstrip!", false)
    return
  end

  case last.first
  when :text
    last[1] = last[1].rstrip
    if last[1].empty?
      @to_merge.slice! index
      rstrip_buffer! index
    end
  when :script
    last[1].gsub!(/\(haml_temp, (.*?)\);$/, '(haml_temp.rstrip, \1);')
    rstrip_buffer! index - 1
  else
    raise SyntaxError.new("[HAML BUG] Undefined entry in Haml::Compiler@to_merge.")
  end
end

https://github.com/haml/haml/blob/a054e2ad7964f894fce032737f9a4f0e146700a5/lib/haml/compiler.rb#L308

Here's a spec:

it 'generates valid rbi' do
  YARD.parse_string(<<-RUBY)
    module A
      def x(a = -1)
        # code
      end
    end
  RUBY

  expect(subject.generate.strip).to eq fix_heredoc(<<-RUBY)
    # typed: strong
    module A
      # sord omit - no YARD type given for "a", using T.untyped
      # sord omit - no YARD return type given, using T.untyped
      sig { params(a: T.untyped).returns(T.untyped) }
      def x(a = -1); end
    end
  RUBY
end

It seems like it doesn't like negative integers?

Actually, it seems like this might be a YARD bug!

If you run that test and add puts "name: #{name}, default: #{default}" to the meth.parameters loop on line 138 of rbi_generator.rb, the output includes this:

name: a, default: -1)

Yep, that's a YARD bug:

image

Good find!

Looks like we were bitten by lsegal/yard#894

I just got this same error again with Faker, this time it creates an extra comma as well (I guess it just duplicates the character that follows it?).

Input:

module Faker
  class Number < Base
    class << self
      def negative(legacy_from = NOT_GIVEN, legacy_to = NOT_GIVEN, from: -5000.00, to: -1.00)
        warn_for_deprecated_arguments do |keywords|
          keywords << :from if legacy_from != NOT_GIVEN
          keywords << :to if legacy_to != NOT_GIVEN
        end

        random_number = between(from: from, to: to)

        less_than_zero(random_number)
      end
    end
  end
end

Output:

sig do
  params(
    legacy_from: T.untyped,
    legacy_to: T.untyped,
    from: T.untyped,
    to: T.untyped
  ).returns(T.untyped)
end
def self.negative(legacy_from = NOT_GIVEN, legacy_to = NOT_GIVEN, from: -5000.00,, to: -1.00)); end

@AaronC81 I've been thinking about this since it still bothers me a bit, would it be insane to write a regex for detecting and fixing these cases?

@connorshea That sounds like a workaround worth looking into!

It'd certainly be good to have some kind of fix for this, and I guess fixing it within YARD is pretty tricky if the issue has been around since 2015.