[GR-45844] Add Primitive.true? and Primitive.false?

andrykonchin · andrykonchin · commit c759edea1d31 · 2023-04-26T20:38:11.000Z
PullRequest: truffleruby/3791
diff --git a/.rubocop.yml b/.rubocop.yml
@@ -1,8 +1,9 @@
 require:
   - ./tool/rubocop-truffleruby/cop/replace_with_primitive_nil.rb
-  - ./tool/rubocop-truffleruby/cop/replace_with_primitive_object_class.rb
-  - ./tool/rubocop-truffleruby/cop/replace_with_primitive_object_equal.rb
-  - ./tool/rubocop-truffleruby/cop/replace_with_primitive_object_kind_of.rb
+  - ./tool/rubocop-truffleruby/cop/replace_with_primitive_class.rb
+  - ./tool/rubocop-truffleruby/cop/replace_with_primitive_equal.rb
+  - ./tool/rubocop-truffleruby/cop/replace_with_primitive_is_a.rb
+  - ./tool/rubocop-truffleruby/cop/replace_with_primitive_true_and_false_predicates.rb
 
 AllCops:
   TargetRubyVersion: 3.1
@@ -406,21 +407,28 @@ TruffleRuby/ReplaceWithPrimitiveNil:
     - src/main/ruby/**/*.rb
 
 # Supports --auto-correct
-TruffleRuby/ReplaceWithPrimitiveObjectClass:
+TruffleRuby/ReplaceWithPrimitiveClass:
   Enabled: true
   Include: # inspect *only* these files
     - lib/truffle/**/*.rb
     - src/main/ruby/**/*.rb
 
 # Supports --auto-correct
-TruffleRuby/ReplaceWithPrimitiveObjectEqual:
+TruffleRuby/ReplaceWithPrimitiveEqual:
   Enabled: true
   Include: # inspect *only* these files
     - lib/truffle/**/*.rb
     - src/main/ruby/**/*.rb
 
 # Supports --auto-correct
-TruffleRuby/ReplaceWithPrimitiveObjectKindOf:
+TruffleRuby/ReplaceWithPrimitiveIsA:
+  Enabled: true
+  Include: # inspect *only* these files
+    - lib/truffle/**/*.rb
+    - src/main/ruby/**/*.rb
+
+# Supports --auto-correct
+TruffleRuby/ReplaceWithPrimitiveTrueAndFalsePredicates:
   Enabled: true
   Include: # inspect *only* these files
     - lib/truffle/**/*.rb
diff --git a/lib/truffle/socket/truffle.rb b/lib/truffle/socket/truffle.rb
@@ -333,7 +333,7 @@ def self.convert_reverse_lookup(socket = nil, reverse_lookup = nil)
       elsif reverse_lookup == :numeric
         reverse_lookup = false
 
-      elsif reverse_lookup != true and reverse_lookup != false
+      elsif !Primitive.true?(reverse_lookup) and !Primitive.false?(reverse_lookup)
         raise ArgumentError,
           "invalid reverse_lookup flag: #{reverse_lookup.inspect}"
       end
diff --git a/lib/truffle/truffle/cext.rb b/lib/truffle/truffle/cext.rb
@@ -633,7 +633,7 @@ def rb_filesystem_encoding
 
   def rb_to_encoding_index(enc)
     enc = Truffle::Type.coerce_to_encoding(enc)
-    return -1 if enc == false
+    return -1 if Primitive.false?(enc)
     rb_enc_to_index(enc)
   end
 
@@ -1186,7 +1186,7 @@ def rb_class_new(superclass)
 
   def rb_define_class_under(mod, name, superclass)
     # nil is TypeError (checked below), false is ArgumentError
-    if Primitive.equal?(false, superclass)
+    if Primitive.false?(superclass)
       raise ArgumentError, "no super class for `#{name}'"
     end
 
diff --git a/src/main/java/org/truffleruby/core/support/TypeNodes.java b/src/main/java/org/truffleruby/core/support/TypeNodes.java
@@ -224,6 +224,32 @@ protected boolean other(Object value) {
         }
     }
 
+    @Primitive(name = "true?")
+    public abstract static class IsTrue extends PrimitiveArrayArgumentsNode {
+        @Specialization
+        protected boolean bool(boolean value) {
+            return value;
+        }
+
+        @Fallback
+        protected boolean other(Object value) {
+            return false;
+        }
+    }
+
+    @Primitive(name = "false?")
+    public abstract static class IsFalse extends PrimitiveArrayArgumentsNode {
+        @Specialization
+        protected boolean bool(boolean value) {
+            return !value;
+        }
+
+        @Fallback
+        protected boolean other(Object value) {
+            return false;
+        }
+    }
+
     @Primitive(name = "object_ivars")
     public abstract static class ObjectInstanceVariablesNode extends PrimitiveArrayArgumentsNode {
 
diff --git a/src/main/ruby/truffleruby/core/dir.rb b/src/main/ruby/truffleruby/core/dir.rb
@@ -266,7 +266,7 @@ def glob(pattern, flags = 0, base: nil, sort: true, &block)
 
       matches = []
       index = 0
-      flags |= File::FNM_GLOB_NOSORT if Primitive.equal?(sort, false)
+      flags |= File::FNM_GLOB_NOSORT if Primitive.false?(sort)
 
       normalized_base = if Primitive.nil? base
                           nil
diff --git a/src/main/ruby/truffleruby/core/io.rb b/src/main/ruby/truffleruby/core/io.rb
@@ -1461,7 +1461,7 @@ def fcntl(command, arg = 0)
 
     if !arg
       arg = 0
-    elsif arg == true
+    elsif Primitive.true?(arg)
       arg = 1
     elsif Primitive.is_a?(arg, String)
       raise NotImplementedError, 'cannot handle String'
@@ -1491,7 +1491,7 @@ def ioctl(command, arg = 0)
 
     if !arg
       real_arg = 0
-    elsif arg == true
+    elsif Primitive.true?(arg)
       real_arg = 1
     elsif Primitive.is_a?(arg, String)
       # This could be faster.
diff --git a/src/main/ruby/truffleruby/core/kernel.rb b/src/main/ruby/truffleruby/core/kernel.rb
@@ -54,7 +54,7 @@ def Complex(real, imag = undefined, exception: true)
   module_function :Complex
 
   def Float(obj, exception: true)
-    raise_exception = !Primitive.equal?(exception, false)
+    raise_exception = !Primitive.false?(exception)
     obj = Truffle::Interop.unbox_if_needed(obj)
 
     case obj
@@ -104,7 +104,7 @@ def Integer(obj, base = 0, exception: true)
     obj = Truffle::Interop.unbox_if_needed(obj)
     converted_base = Truffle::Type.rb_check_to_integer(base, :to_int)
     base = Primitive.nil?(converted_base) ? 0 : converted_base
-    raise_exception = !Primitive.equal?(exception, false)
+    raise_exception = !Primitive.false?(exception)
 
     if Primitive.is_a?(obj, String)
       Primitive.string_to_inum(obj, base, true, raise_exception)
diff --git a/src/main/ruby/truffleruby/core/numeric.rb b/src/main/ruby/truffleruby/core/numeric.rb
@@ -30,7 +30,7 @@ class Numeric
   include Comparable
 
   def clone(freeze: nil)
-    if Primitive.equal?(freeze, false)
+    if Primitive.false?(freeze)
       raise ArgumentError, "can't unfreeze #{Primitive.class(self).name}"
     end
     self
diff --git a/src/main/ruby/truffleruby/core/pre.rb b/src/main/ruby/truffleruby/core/pre.rb
@@ -58,7 +58,7 @@ def include(mod)
       result = Primitive.unbound_method_ruby2_keywords(method)
       if Primitive.nil?(result)
         warn "Skipping set of ruby2_keywords flag for #{name} (method accepts keywords or method does not accept argument splat)", uplevel: 1
-      elsif result == false
+      elsif Primitive.false?(result)
         warn "Skipping set of ruby2_keywords flag for #{name} (unknown reason)", uplevel: 1
       end
     end
@@ -71,7 +71,7 @@ def ruby2_keywords
     result = Primitive.proc_ruby2_keywords(self)
     if Primitive.nil?(result)
       warn 'Skipping set of ruby2_keywords flag for proc (proc accepts keywords or proc does not accept argument splat)', uplevel: 1
-    elsif result == false
+    elsif Primitive.false?(result)
       warn 'Skipping set of ruby2_keywords flag for proc (unknown reason)', uplevel: 1
     end
     self
diff --git a/src/main/ruby/truffleruby/core/range.rb b/src/main/ruby/truffleruby/core/range.rb
@@ -112,7 +112,7 @@ def bsearch(&block)
     return nil if max < min || max == min && self.exclude_end? #-inf.prev_float == -inf!
     if max == min
       result = yield max
-      return result == true || result == 0 ? max : nil
+      return Primitive.true?(result) || result == 0 ? max : nil
     end
 
     # max == -Float::INFINITY will already be covered by the comparisons above.
@@ -136,7 +136,7 @@ def bsearch(&block)
     if min == -Float::INFINITY
       result = yield min
       # Find-minimum mode: It's not going to get any smaller than negative infinity.
-      return min if result == true || result == 0
+      return min if Primitive.true?(result) || result == 0
       return nil if Primitive.is_a?(result, Numeric) && result < 0
       min = normalized_begin = -Float::MAX # guaranteed to be <= max
     end
@@ -175,12 +175,12 @@ def bsearch(&block)
       # max is untested only if it's the end of the range.
       # It can only change the result of the search if we haven't found an admissible value yet (+ infinity edge case).
       result = yield max
-      return max if result == true || result == 0
+      return max if Primitive.true?(result) || result == 0
     elsif mid == max && min == normalized_begin
       # min is untested only if it's the begin of the range.
       result = yield min
       # Favor smallest value in case we're in find-minimum mode.
-      return min if result == true || result == 0
+      return min if Primitive.true?(result) || result == 0
     end
 
     last_admissible
diff --git a/src/main/ruby/truffleruby/core/truffle/complex_operations.rb b/src/main/ruby/truffleruby/core/truffle/complex_operations.rb
@@ -11,7 +11,7 @@
 module Truffle
   module ComplexOperations
     def self.convert_not_real_arguments(real, imag, exception)
-      raise_exception = !Primitive.equal?(exception, false)
+      raise_exception = !Primitive.false?(exception)
       if Primitive.nil?(real) || Primitive.nil?(imag)
         return nil unless raise_exception
         raise TypeError, "can't convert nil into Complex"
diff --git a/src/main/ruby/truffleruby/core/truffle/exception_operations.rb b/src/main/ruby/truffleruby/core/truffle/exception_operations.rb
@@ -130,8 +130,8 @@ def self.full_message(exception, highlight, order)
       highlight = if Primitive.nil?(highlight)
                     Exception.to_tty?
                   else
-                    raise ArgumentError, "expected true of false as highlight: #{highlight}" unless Primitive.equal?(highlight, true) || Primitive.equal?(highlight, false)
-                    !Primitive.equal?(highlight, false)
+                    raise ArgumentError, "expected true of false as highlight: #{highlight}" unless Primitive.true?(highlight) || Primitive.false?(highlight)
+                    !Primitive.false?(highlight)
                   end
 
       raise ArgumentError, "expected :top or :bottom as order: #{order}" unless Primitive.equal?(order, :top) || Primitive.equal?(order, :bottom)
diff --git a/src/main/ruby/truffleruby/core/truffle/io_operations.rb b/src/main/ruby/truffleruby/core/truffle/io_operations.rb
@@ -225,7 +225,7 @@ def self.poll(io, event_mask, timeout)
         while timeout_ms > 2147483647 # INT_MAX
           timeout_ms -= 2147483000
           ret = poll(io, event_mask, 2147483)
-          return ret unless ret == false
+          return ret unless Primitive.false?(ret)
         end
 
         remaining_timeout = timeout_ms
diff --git a/src/main/ruby/truffleruby/core/truffle/process_operations.rb b/src/main/ruby/truffleruby/core/truffle/process_operations.rb
@@ -286,7 +286,7 @@ def parse_options(options)
               @options[:unsetenv_others] = true
             end
           when :pgroup
-            if value == true
+            if Primitive.true?(value)
               value = 0
             elsif value
               value = Truffle::Type.coerce_to value, Integer, :to_int
diff --git a/tool/import-rubocop-truffleruby.sh b/tool/import-rubocop-truffleruby.sh
@@ -20,4 +20,9 @@ if [ ! "$(ls $SOURCE_DIR)" ]; then
     echo "FAILURE: git clone failed"
   fi
 fi
+
+# handle a case when a source cop file was renamed/deleted
+rm -r $TARGET_DIR/cop
+mkdir $TARGET_DIR/cop
+
 cp $SOURCE_DIR/$SOURCE_PATH/* $TARGET_DIR/cop/
diff --git a/tool/rubocop-truffleruby/cop/replace_with_primitive_class.rb b/tool/rubocop-truffleruby/cop/replace_with_primitive_class.rb
@@ -13,7 +13,7 @@ module TruffleRuby
       #   # good
       #   Primitive.class(object)
       #
-      class ReplaceWithPrimitiveObjectClass < Base
+      class ReplaceWithPrimitiveClass < Base
         extend AutoCorrector
 
         MSG = 'Use `Primitive.class` instead of `Object#class`'
diff --git a/tool/rubocop-truffleruby/cop/replace_with_primitive_equal.rb b/tool/rubocop-truffleruby/cop/replace_with_primitive_equal.rb
@@ -14,7 +14,7 @@ module TruffleRuby
       #   # good
       #   Primitive.equal?(foo, bar)
       #
-      class ReplaceWithPrimitiveObjectEqual < Base
+      class ReplaceWithPrimitiveEqual < Base
         extend AutoCorrector
 
         MSG = 'Use `Primitive.equal?` instead of `#equal?`'
diff --git a/tool/rubocop-truffleruby/cop/replace_with_primitive_is_a.rb b/tool/rubocop-truffleruby/cop/replace_with_primitive_is_a.rb
@@ -23,7 +23,7 @@ module TruffleRuby
       #   # good
       #   Primitive.is_a?(a, String)
       #
-      class ReplaceWithPrimitiveObjectKindOf < Base
+      class ReplaceWithPrimitiveIsA < Base
         extend AutoCorrector
 
         MSG = 'Use `Primitive.is_a?` instead of `#kind_of?` or `#is_a?`'
diff --git a/tool/rubocop-truffleruby/cop/replace_with_primitive_true_and_false_predicates.rb b/tool/rubocop-truffleruby/cop/replace_with_primitive_true_and_false_predicates.rb
@@ -0,0 +1,88 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module TruffleRuby
+      # Prefer `Primitive.true?` and `Primitive.false?` to check whether object
+      # is `true` or `false`.
+      #
+      # @example
+      #
+      #   # bad
+      #   foo == true
+      #   foo == false
+      #
+      #   # bad
+      #   foo.equal?(true)
+      #   foo.equal?(false)
+      #
+      #   # bad
+      #   Primitive.equal?(foo, true)
+      #
+      #   # good
+      #   Primitive.true?(foo)
+      #   Primitive.false?(foo)
+      #
+      class ReplaceWithPrimitiveTrueAndFalsePredicates < Base
+        extend AutoCorrector
+
+        MSG = 'Use `Primitive.true?` and `Primitive.false?` instead of `==` or `#equal?`'
+
+        RESTRICT_ON_SEND = %i[== != equal?].freeze
+
+        # @!method bad_core_method?(node)
+        def_node_matcher :bad_core_method?, <<~PATTERN
+          (send $_ ${ :== :!= :equal? } ${ true false })
+        PATTERN
+
+        # @!method bad_primitive_method?(node)
+        def_node_matcher :bad_primitive_method?, <<~PATTERN
+          (send
+            (const {nil? cbase} :Primitive)
+            :equal?
+            {
+              $_ ${ true false }
+              |
+              ${ true false } $_
+            }
+          )
+        PATTERN
+
+        def on_send(node)
+          on_bad_core_method(node) or on_bad_primitive_method(node)
+        end
+
+        private
+
+        def on_bad_core_method(node)
+          captures = bad_core_method?(node)
+          return unless captures
+
+          add_offense(node) do |corrector|
+            receiver, method, true_or_false = captures
+
+            receiver_source = receiver&.source || 'self'
+            optional_negation = method == :!= ? '!' : ''
+            primitive_name = true_or_false.true_type? ? :true? : :false?
+
+            source_string = "#{optional_negation}Primitive.#{primitive_name}(#{receiver_source})"
+            corrector.replace(node.loc.expression, source_string)
+          end
+        end
+
+        def on_bad_primitive_method(node)
+          captures = bad_primitive_method?(node)
+          return unless captures
+
+          add_offense(node) do |corrector|
+            true_or_false, object = captures[0].boolean_type? ? captures : captures.reverse
+            primitive_name = true_or_false.true_type? ? :true? : :false?
+
+            source_string = "Primitive.#{primitive_name}(#{object.source})"
+            corrector.replace(node.loc.expression, source_string)
+          end
+        end
+      end
+    end
+  end
+end
