ruby-core@ruby-lang.org archive (unofficial mirror)
 help / color / mirror / Atom feed
From: yasuo.honda@gmail.com
To: ruby-core@ruby-lang.org
Subject: [ruby-core:99192] [Ruby master Bug#17017] Range#max & Range#minmax incorrectly use Float end as max
Date: Thu, 16 Jul 2020 09:31:57 +0000 (UTC)	[thread overview]
Message-ID: <redmine.journal-86571.20200716093157.41304@ruby-lang.org> (raw)
In-Reply-To: redmine.issue-17017.20200707201640.41304@ruby-lang.org

Issue #17017 has been updated by yahonda (Yasuo Honda).


Let me provide another incompatibility/breaking change since this commit.

One of the Rails validation, `validates_length_of(:title, within: 5..Float::INFINITY)` raises `Infinity (FloatDomainError)` with Ruby 2.8.0dev.
This behavior of `validates_length_of` implemented since Rails 4.0.0 by https://github.com/rails/rails/pull/4905

I think there is no "Integer::INFINITY" in Ruby, "<integer object>..Float::INFINITY" may be used by other applications/frameworks. 
It would be appreciated that this Rails behavior kept with Ruby 2.8.0-dev.


* Script to reproduce

```ruby
# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem "rails", github: "rails/rails"
  gem "sqlite3"
end

require "active_record"
require "minitest/autorun"
require "logger"

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :posts, force: true do |t|
    t.string :title
  end
end

class Post < ActiveRecord::Base
  validates_length_of(:title, within: 5..Float::INFINITY)
end

class ValidationTest < Minitest::Test
  def test_validates_length_of
    post = Post.create!(title: "first post")
    assert post.title, "first post"
  end
end
```


* Result with ruby 2.8.0dev raises "Infinity (FloatDomainError)"

```
% ruby -v ; ruby diag.rb
ruby 2.8.0dev (2020-07-16T02:49:09Z master 1fb4e28002) [x86_64-darwin20]
Fetching https://github.com/rails/rails.git
Fetching gem metadata from https://rubygems.org/......
Fetching gem metadata from https://rubygems.org/............
Fetching gem metadata from https://rubygems.org/............
Resolving dependencies...
Using rake 13.0.1
Using bundler 2.2.0.dev
Using method_source 1.0.0
Using minitest 5.14.1
Using zeitwerk 2.4.0
Using builder 3.2.4
Using erubi 1.9.0
Using mini_portile2 2.4.0
Using crass 1.0.6
Using rack 2.2.3
Using nio4r 2.5.2
Using websocket-extensions 0.1.5
Using mimemagic 0.3.5
Using mini_mime 1.0.2
Using marcel 0.3.3
Using sqlite3 1.4.2
Using concurrent-ruby 1.1.6
Using nokogiri 1.10.10
Using rack-test 1.1.0
Using websocket-driver 0.7.3
Using thor 1.0.1
Using mail 2.7.1
Using i18n 1.8.3
Using tzinfo 2.0.2
Using sprockets 4.0.2
Using loofah 2.6.0
Using activesupport 6.1.0.alpha from https://github.com/rails/rails.git (at master@bcfa51f)
Using rails-html-sanitizer 1.3.0
Using rails-dom-testing 2.0.3
Using globalid 0.4.2
Using activemodel 6.1.0.alpha from https://github.com/rails/rails.git (at master@bcfa51f)
Using actionview 6.1.0.alpha from https://github.com/rails/rails.git (at master@bcfa51f)
Using activejob 6.1.0.alpha from https://github.com/rails/rails.git (at master@bcfa51f)
Using actionpack 6.1.0.alpha from https://github.com/rails/rails.git (at master@bcfa51f)
Using activerecord 6.1.0.alpha from https://github.com/rails/rails.git (at master@bcfa51f)
Using actioncable 6.1.0.alpha from https://github.com/rails/rails.git (at master@bcfa51f)
Using activestorage 6.1.0.alpha from https://github.com/rails/rails.git (at master@bcfa51f)
Using railties 6.1.0.alpha from https://github.com/rails/rails.git (at master@bcfa51f)
Using sprockets-rails 3.2.1
Using actionmailer 6.1.0.alpha from https://github.com/rails/rails.git (at master@bcfa51f)
Using actionmailbox 6.1.0.alpha from https://github.com/rails/rails.git (at master@bcfa51f)
Using actiontext 6.1.0.alpha from https://github.com/rails/rails.git (at master@bcfa51f)
Using rails 6.1.0.alpha from https://github.com/rails/rails.git (at master@bcfa51f)
-- create_table(:posts, {:force=>true})
D, [2020-07-16T18:20:56.790796 #66220] DEBUG -- :    (0.7ms)  SELECT sqlite_version(*)
D, [2020-07-16T18:20:56.791115 #66220] DEBUG -- :    (0.0ms)  DROP TABLE IF EXISTS "posts"
D, [2020-07-16T18:20:56.791443 #66220] DEBUG -- :    (0.2ms)  CREATE TABLE "posts" ("id" integer PRIMARY KEY AUTOINCREMENT NOT NULL, "title" varchar)
   -> 0.0038s
D, [2020-07-16T18:20:56.814223 #66220] DEBUG -- :    (0.1ms)  CREATE TABLE "ar_internal_metadata" ("key" varchar NOT NULL PRIMARY KEY, "value" varchar, "created_at" datetime(6) NOT NULL, "updated_at" datetime(6) NOT NULL)
D, [2020-07-16T18:20:56.824002 #66220] DEBUG -- :   ActiveRecord::InternalMetadata Load (0.1ms)  SELECT "ar_internal_metadata".* FROM "ar_internal_metadata" WHERE "ar_internal_metadata"."key" = ? LIMIT ?  [["key", "environment"], ["LIMIT", 1]]
D, [2020-07-16T18:20:56.827703 #66220] DEBUG -- :   TRANSACTION (0.0ms)  begin transaction
D, [2020-07-16T18:20:56.827989 #66220] DEBUG -- :   ActiveRecord::InternalMetadata Create (0.1ms)  INSERT INTO "ar_internal_metadata" ("key", "value", "created_at", "updated_at") VALUES (?, ?, ?, ?)  [["key", "environment"], ["value", "development"], ["created_at", "2020-07-16 09:20:56.827304"], ["updated_at", "2020-07-16 09:20:56.827304"]]
D, [2020-07-16T18:20:56.828184 #66220] DEBUG -- :   TRANSACTION (0.0ms)  commit transaction
/Users/yahonda/.rbenv/versions/2.8.0-dev/lib/ruby/gems/2.8.0/bundler/gems/rails-bcfa51fba6ee/activemodel/lib/active_model/validations/length.rb:14:in `floor': Infinity (FloatDomainError)
	from /Users/yahonda/.rbenv/versions/2.8.0-dev/lib/ruby/gems/2.8.0/bundler/gems/rails-bcfa51fba6ee/activemodel/lib/active_model/validations/length.rb:14:in `max'
	from /Users/yahonda/.rbenv/versions/2.8.0-dev/lib/ruby/gems/2.8.0/bundler/gems/rails-bcfa51fba6ee/activemodel/lib/active_model/validations/length.rb:14:in `initialize'
	from /Users/yahonda/.rbenv/versions/2.8.0-dev/lib/ruby/gems/2.8.0/bundler/gems/rails-bcfa51fba6ee/activemodel/lib/active_model/validations/with.rb:86:in `new'
	from /Users/yahonda/.rbenv/versions/2.8.0-dev/lib/ruby/gems/2.8.0/bundler/gems/rails-bcfa51fba6ee/activemodel/lib/active_model/validations/with.rb:86:in `block in validates_with'
	from /Users/yahonda/.rbenv/versions/2.8.0-dev/lib/ruby/gems/2.8.0/bundler/gems/rails-bcfa51fba6ee/activemodel/lib/active_model/validations/with.rb:85:in `each'
	from /Users/yahonda/.rbenv/versions/2.8.0-dev/lib/ruby/gems/2.8.0/bundler/gems/rails-bcfa51fba6ee/activemodel/lib/active_model/validations/with.rb:85:in `validates_with'
	from /Users/yahonda/.rbenv/versions/2.8.0-dev/lib/ruby/gems/2.8.0/bundler/gems/rails-bcfa51fba6ee/activerecord/lib/active_record/validations/length.rb:20:in `validates_length_of'
	from diag.rb:29:in `<class:Post>'
	from diag.rb:28:in `<main>'
%
```


----------------------------------------
Bug #17017: Range#max & Range#minmax incorrectly use Float end as max
https://bugs.ruby-lang.org/issues/17017#change-86571

* Author: sambostock (Sam Bostock)
* Status: Open
* Priority: Normal
* ruby -v: ruby 2.8.0dev (2020-07-14T04:19:55Z master e60cd14d85) [x86_64-darwin17]
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
While continuing to add edge cases to [`Range#minmax` specs](https://github.com/ruby/spec/pull/777), I discovered the following bug:

```ruby
(1..3.1).to_a        == [1, 2, 3] # As expected

(1..3.1).to_a.max    == 3         # As expected
(1..3.1).to_a.minmax == [1, 3]    # As expected

(1..3.1).max    == 3.1            # Should be 3, as above
(1..3.1).minmax == [1, 3.1]       # Should be [1, 3], as above
```

One way to detect this scenario might be to do (whatever the C equivalent is of)

```ruby
range_end.is_a?(Numeric)                      // Is this a numeric range?
  && (range_end - range_begin).modulo(1) == 0 // Can we reach the range_end using the standard step size (1)
```

As for how to handle it, a couple options come to mind:

- We could error out and do something similar to what we do for exclusive ranges

```ruby
raise TypeError, 'cannot exclude non Integer end value'
```

- We might be able to calculate the range end by doing something like

```ruby
num_steps = (range_end / range_beg).to_i - 1 # one fewer steps than would exceed the range_end
max = range_beg + num_steps                  # take that many steps all at once
```

- We could delegate to `super` and enumerate the range to find the max

```ruby
super
```

- We could update the documentation to define the max for this case as the `range_end`, similarly to how the documentation for `include?` says it behaves like `cover?` for numeric ranges.



-- 
https://bugs.ruby-lang.org/

  parent reply	other threads:[~2020-07-16  9:32 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-07 20:16 [ruby-core:99079] [Ruby master Bug#17017] Range#max & Range#minmax incorrectly use Float end as max bosticko
2020-07-07 20:41 ` [ruby-core:99080] " bosticko
2020-07-07 20:43 ` [ruby-core:99081] " bosticko
2020-07-07 20:56 ` [ruby-core:99082] " bosticko
2020-07-10 16:29 ` [ruby-core:99113] " merch-redmine
2020-07-15  0:47 ` [ruby-core:99171] " koic.ito
2020-07-15  5:24 ` [ruby-core:99174] " merch-redmine
2020-07-15  7:19 ` [ruby-core:99175] " koic.ito
2020-07-15 15:37 ` [ruby-core:99178] " marcandre-ruby-core
2020-07-15 15:58 ` [ruby-core:99179] " merch-redmine
2020-07-15 16:40 ` [ruby-core:99181] " marcandre-ruby-core
2020-07-16  9:31 ` yasuo.honda [this message]
2020-07-16 17:17 ` [ruby-core:99194] " merch-redmine
2020-07-19  1:17 ` [ruby-core:99216] " yasuo.honda
2020-07-19  6:41 ` [ruby-core:99222] " marcandre-ruby-core
2020-07-19  9:59 ` [ruby-core:99223] " eregontp
2020-07-19 10:02 ` [ruby-core:99224] " eregontp
2020-07-19 12:38 ` [ruby-core:99225] " merch-redmine
2020-07-19 14:12 ` [ruby-core:99226] " marcandre-ruby-core
2020-07-19 14:19 ` [ruby-core:99227] " marcandre-ruby-core
2020-08-14 16:09 ` [ruby-core:99589] " daniel
2020-08-14 17:54 ` [ruby-core:99590] " merch-redmine
2020-09-01  5:13 ` [ruby-core:99813] " matz
2020-09-01 14:49 ` [ruby-core:99816] " merch-redmine
2020-09-01 17:54 ` [ruby-core:99821] " merch-redmine

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.ruby-lang.org/en/community/mailing-lists/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=redmine.journal-86571.20200716093157.41304@ruby-lang.org \
    --to=ruby-core@ruby-lang.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).