"Fixnum is deprecated" // warning while converting scss to css

[sinkaszab]

I tried to create a flex based css when I encountered this message:

$ sass bootstrap-flex.scss bootstrap-flex.css
/usr/local/lib/ruby/gems/2.4.0/gems/sass-3.4.23/lib/sass/util.rb:1109: warning: constant ::Fixnum is deprecated

Setup

bootstrap-4.0.0-alpha.5

Sass version:
Sass 3.4.23 (Selective Steve)

Ruby version:
ruby 2.4.0p0 (2016-12-24 revision 57164) [x86_64-darwin16]

[richiethomas]

It appears Fixnum and Bignum are being deprecated starting with Ruby 2.4.0:

[https://github.com/rails/rails/pull/26732/files/cb0452e9a50e97f8ab2100f6226fbdd47a970a34]

I'd like to claim this issue. My first thought is to add a new switch statement referencing the "Integer" class, with the same implementation as the "Fixnum" switch statement, just before said "Fixnum" statement. @chriseppstein @nex3 - thoughts?

[richiethomas]

I was going to propose the following for the fix for this issue:

def json_value_of(v)
      case v
      when Integer, Fixnum
        v.to_s
      when String
        "\"" + json_escape_string(v) + "\""
      when Array
        "[" + v.map {|x| json_value_of(x)}.join(",") + "]"
      when NilClass
        "null"
      when TrueClass
        "true"
      when FalseClass
        "false"
      else
        raise ArgumentError.new("Unknown type: #{v.class.name}")
      end

But I just tried this on my local and it doesn't eliminate the warning. I also tried splitting the "Integer" and "Fixnum" references into separate 'when' clauses (with "Integer" first), and that didn't eliminate the warning either.

The only thing that worked was to delete the "Fixnum" clause altogether, and replace it with an "Integer" clause. I tried this both with Ruby 2.4.0 and Ruby 2.3.1, and it worked in both cases. I don't want to dig any deeper before getting the core team's thoughts on the above. If it sounds good to y'all, I can submit a PR ASAP. Cheers @chriseppstein and @nex3. :-)

I see the 'json_value_of' method, and specifically its handling of the Fixnum class, is already covered by tests in 'util_test.rb'. Looks like the same case would cover Integers as well for Ruby 2.4.0. I'd like to be able to verify that before submitting a PR by running the same command that @sinkaszab mentioned (sass filenameA.scss filenameA.css), but am unsure how to

[sinkaszab]

I am just learning Ruby, so this might just be nonsense.

Maybe what I would do is eliminate the reference for a deprecated class type. Could it be possible that you converted the class type to string for the type to be checked and validate it as a string? That way there would be no reference to a deprecated class. And would also be backward compatible.

[richiethomas]

@sinkaszab I'm wondering if removing the reference to the deprecated class would be a breaking change for users of older Ruby versions. I tested Ruby 2.3.1 yesterday (no problems there) and today I'll test 2.0.0 and 1.9.3, provided this project still supports those versions. If those versions behave the same with the Integer class, I can't think of anything else which would stop us from just switching Fixnum to Integer. Then again, I'm still learning as well. :-) Let's see what the core team has to say.

[nex3]

Sass 3.4 supports all the way back to Ruby 1.8.7, which I'm pretty sure doesn't have an Integer type. But soon we'll release Sass 3.5 (tracked on the next branch) in earnest, which only supports back to 2.0.0. If that has an Integer type, I'm in favor of landing your proposed fix on next.

[richiethomas]

I just tested Ruby 1.8.7 for presence of an Integer type, looks like it exists:

rvm rubies

   jruby-1.7.21 [ x86_64 ]
=> ruby-1.8.7-head [ i686 ]
   ruby-1.8.7-p374 [ i686 ]
   ruby-1.9.3-p551 [ x86_64 ]
   ruby-2.0.0-p648 [ x86_64 ]
   ruby-2.2.3 [ x86_64 ]
 * ruby-2.3.1 [ x86_64 ]
   ruby-2.4.0 [ x86_64 ]

# => - current
# =* - current && default
#  * - default

MacBook-Pro-5:hbase richiethomas$ irb
>> 5.is_a?(Integer)
=> true
>> 5.is_a?(Fixnum)
=> true

Same with 2.0.0:

MacBook-Pro-5:hbase richiethomas$ rvm use 2.0.0
Using /Users/richiethomas/.rvm/gems/ruby-2.0.0-p648
MacBook-Pro-5:hbase richiethomas$ irb
>> 5.is_a?(Fixnum)
=> true
>> 5.is_a?(Integer)
=> true

[nex3]

If replacing all instances of Fixnum with Integer works on all supported versions, I'm all for it!

[richiethomas]

I'll give it a shot on my local, if it checks out and the tests pass I'll submit a PR. :)

[richiethomas]

I'll go ahead and close #2222 as well.

[richiethomas]

@nex3 it looks like all but one of the Fixnum references are in comments which describe the type of data returned from a method. The one reference to Fixnum inside actual executable code, when changed to Integer, does not break any tests, and the binary continues to compile .scss files to .css for all the Ruby versions I tested (1.8.7, 1.9.3, 2.0.0, 2.3.1, and 2.4.0).

What do you think of having Fixnum-related comments which apply to versions earlier than 2.4, and Integer-related comments which apply to 2.4 and above?

[sinkaszab]

// Integer - if I know it well - is the superclass for Fixnum and Bignum all the way back. (But at least dating back to Ruby v2.2.)

[richiethomas]

@sinkaszab looks like you're right, going all the way back to at least 1.8.7.

[nex3]

@richiethomas I think it's fine to just update the comments to be Integer instead for all versions.

[richiethomas]

@nex3 OK will do. When I was familiarizing myself with the comments, I had questions about the following 2 items:

1. In error.rb, there's a "sass_mixin" method which is meant to return an integer. However, when I looked for this method in the MiniTest test suite (specifically, engine_test.rb line 403), I noticed an equality assertion comparing the return value of "sass_mixin" with a String intance. I ran that test assertion and logged the value of the sass_mixin method, and it was indeed a string. Does this mean the method should not return a Fixnum nor an Integer, but rather a string?

2. On line 75 of "staleness_checker.rb", the "mtime" param appears to be an instance of Time, not Fixnum nor Integer. I determined this by running the relevant MiniTest, similar to @charset in imported files #1 above. Should the documentation be changed accordingly?

I'll do a PR as soon as I have your feedback on the 2 questions above.

[richiethomas]

P.S.- I couldn't find any references to the MiniTest files in the README.md or CONTRIBUTING.md, so I wasn't sure if the MiniTest suite is still used or if it's just the "sass-spec" repo at this point. Clarification would be appreciated, thanks. :)

[nex3]

The MiniTests are definitely still being run. Any discrepancies between the actual return values and the comments are probably just outdated comments, though.

[dhh]

We're hoping to release Rails 5.1 RC1 this week. Would be lovely if there was a new gem release with this fix to coincide with that, if possible. Thanks!

[danielhaim1]

@dhh would this thread be updated once fixed? –dh

[funilrys]

Any news about this issues ❓

[richiethomas]

As I'm not on the core team that maintains this gem, I don't have context on its release schedule. @nex3 any thoughts on when the next release is?

[funilrys]

Hope this will be closed soon :)

Just did #2218 (comment) ==> Fixed 👍

[richiethomas]

Hi @chriseppstein, there's been a request for an update of this gem to support the recent release of Rails 5.1, of which this gem is a dependency. Are you the right person to ask about that? More context in the discussion thread above. Cheers!

[nex3]

Sorry for the long wait time! I've been on medical leave for the past couple months, but I'm back now and planning to cut a release today or tomorrow.

[mikekatip]

Any update on this? I see this is marked as closed, but I get this error unless I replace Fixnum with Integer.

sudo sed -i -e 's/Fixnum/Integer/g' /usr/lib/ruby/vendor_ruby/sass/util.rb.