Improve $INPUT_RECORD_SEPARATOR deprecation check

casperisfine commented 2 years ago

Ref: https://github.com/casperisfine/csv/commit/1f9cbc170ea4f3d9a9db48d12bcf9895bfe86c25

Setting $VERBOSE = false is not quite enough because some applications define Warning.warn to collect and process warnings beyond just priting them on STDERR.

For instance some apps might translate warnings into errors:

def Warning.warn(message)
  raise Deprecated, message
end

So hooking into Warning.warn to catch the deprecation warning is much less likely to cause issues.

cc @kou

etiennebarrie commented 2 years ago

I've had the same issue and worked around it like this: https://github.com/Shopify/maintenance_tasks/commit/734e949bce13f0ef0a2f4648df0bb1f810cd1d9d

I had started working on a fix, this was my test harness, which all pass with this branch:

if ARGV.delete("singleton")
  module Warning
    def self.warn(message)
      raise message.to_s
    end
  end
else
  module RaiseWarning
    def warn(message)
      raise message.to_s
    end
  end
  if ARGV.delete("prepend")
    Warning.prepend(RaiseWarning)
  else
    Warning.extend(RaiseWarning)
  end
end
Warning[:deprecated] = true
$VERBOSE = true

begin
  require "csv"
rescue
  p $!
  exit(1)
end

begin
  warn("should raise")
rescue
  exit(0)
end

puts "expected to raise"
exit(1)

$ ruby -Ilib test.rb || echo failed                                                                                                                                                                                                                                               
$ ruby -Ilib test.rb singleton || echo failed                                                                                                                                                                                                                                     
$ ruby -Ilib test.rb prepend || echo failed

kou commented 2 years ago

Good catch! But I feel that this approach is overkill... I pushed simple RUBY_VERSION based check approach. Sorry.

casperisfine commented 2 years ago

Haha, yeah that makes sense :)

ruby / csv

Improve $INPUT_RECORD_SEPARATOR deprecation check #233