rubocop update

.rubocop.yml

@@ -24,6 +24,10 @@ Metrics/BlockNesting:
 Metrics/ClassLength:
   Max: 107
 
+# Legacy Ops module is long
+Metrics/ModuleLength:
+  Max: 553
+
 # Offense count: 20
 Metrics/CyclomaticComplexity:
   Max: 17
@@ -68,7 +72,12 @@ Style/SingleLineBlockParams:
 Style/TrivialAccessors:
   Enabled: false
 
+
 # the debugger invocation is deliberate here, it's not a forgotten leftover...
 Lint/Debugger:
   Exclude:
     - src/ruby/yast/debugger.rb
+
+# alias method is more convenient method for method aliasing
+Style/Alias:
+  EnforcedStyle: prefer_alias_method

.travis.yml

@@ -5,7 +5,7 @@ before_install:
     # disable rvm, use system Ruby
     - rvm reset
     - wget https://raw.githubusercontent.com/yast/yast-devtools/master/travis-tools/travis_setup.sh
-    - sh ./travis_setup.sh -p "docbook-xsl xsltproc yast2-core-dev yast2-devtools libxcrypt-dev cmake yast2-ycp-ui-bindings-dev ruby2.1 ruby2.1-dev rake ruby-fast-gettext language-pack-en language-pack-cs screen" -g "yast-rake rspec:3.3.0 rubocop:0.29.1"
+    - sh ./travis_setup.sh -p "docbook-xsl xsltproc yast2-core-dev yast2-devtools libxcrypt-dev cmake yast2-ycp-ui-bindings-dev ruby2.1 ruby2.1-dev rake ruby-fast-gettext language-pack-en language-pack-cs screen" -g "yast-rake rspec:3.3.0 rubocop:0.42.1"
 script:
     - rake check:syntax
     - rubocop

src/ruby/yast/builtins.rb

@@ -50,11 +50,7 @@ def self.change(object, *params)
     # @deprecated use ruby native select method
     def self.filter(object, &block)
       # TODO: investigate break and continue with filter as traverse workflow is different for ruby
-      if object.is_a?(::Array) || object.is_a?(::Hash)
-        Yast.deep_copy(object).select(&block)
-      else
-        return nil
-      end
+      (object.is_a?(::Array) || object.is_a?(::Hash)) ? Yast.deep_copy(object).select(&block) : nil
     end
 
     # find() Yast built-in
@@ -350,7 +346,7 @@ def self.flatten(value)
 
       value.reduce([]) do |acc, i|
         return nil if i.nil?
-        acc.push(*Yast.deep_copy(i))
+        acc.concat(Yast.deep_copy(i))
       end
     end
 
@@ -361,11 +357,11 @@ module List
       def self.reduce(*params, &block)
         return nil if params.first.nil?
         list = if params.size == 2 # so first is default and second is list
-                 return nil if params[1].nil?
-                 [params.first].concat(Yast.deep_copy(params[1]))
-               else
-                 params.first
-               end
+          return nil if params[1].nil?
+          [params.first].concat(Yast.deep_copy(params[1]))
+        else
+          params.first
+        end
         Yast.deep_copy(list).reduce(&block)
       end
 
@@ -426,7 +422,7 @@ def self.merge(a1, a2)
     def self.prepend(list, element)
       return nil if list.nil?
 
-      [Yast.deep_copy(element)].push(*Yast.deep_copy(list))
+      [Yast.deep_copy(element)].concat(Yast.deep_copy(list))
     end
 
     # setcontains() Yast built-in
@@ -445,10 +441,10 @@ def self.sort(array, &block)
       return nil if array.nil?
 
       res = if block_given?
-              array.sort { |x, y| block.call(x, y) ? -1 : 1 }
-            else
-              array.sort { |x, y| Yast::Ops.comparable_object(x) <=> y }
-            end
+        array.sort { |x, y| block.call(x, y) ? -1 : 1 }
+      else
+        array.sort { |x, y| Yast::Ops.comparable_object(x) <=> y }
+      end
 
       Yast.deep_copy(res)
     end
@@ -465,7 +461,7 @@ def self.splitstring(string, sep)
     end
 
     # @private we must mark somehow default value for length
-    DEF_LENGHT = "default"
+    DEF_LENGHT = "default".freeze
     # Extracts a sublist
     # - sublist(<list>, <offset>)
     # - sublist(<list>, <offset>, <length>)
@@ -547,11 +543,7 @@ def self.tomap(object)
     # Evaluate a Yast value.
     # @deprecated for lazy evaluation use builtin lambda or block calls
     def self.eval(object)
-      if object.respond_to? :call
-        return object.call
-      else
-        return Yast.deep_copy(object)
-      end
+      object.respond_to?(:call) ? object.call : Yast.deep_copy(object)
     end
 
     # Change or add an environment variable
@@ -717,10 +709,10 @@ def self.time_to_tm(time)
       # tm_isdst > 0 -> in effect
       # tm_isdst = 0 -> not in effect
       # tm_isdst < 0 -> unknown
-      if time.respond_to?(:isdst)
-        tm[:tm_isdst] = time.isdst ? 1 : 0
+      tm[:tm_isdst] = if time.respond_to?(:isdst)
+        time.isdst ? 1 : 0
       else
-        tm[:tm_isdst] = -1
+        -1
       end
       tm
     end
@@ -977,7 +969,7 @@ def self.tostring(val, width = nil)
         # There is also extra "any" in lists/maps:
         #   Yast:    <YCPRef:list <map> bar (list <map> a)>
         #   Ruby:    <YCPRef:list <map<any,any>> bar (list <map<any,any>>)>
-        val.signature.match(/(.*)\((.*)\)/)
+        val.signature =~ /(.*)\((.*)\)/
         "<YCPRef:#{Regexp.last_match(1)}#{val.remote_method.name} (#{Regexp.last_match(2)})>"
       else
         y2warning 1, "tostring builtin called on wrong type #{val.class}"
@@ -987,11 +979,7 @@ def self.tostring(val, width = nil)
 
     # @private string is handled diffent if string is inside other structure
     def self.inside_tostring(val)
-      if val.is_a? ::String
-        return val.inspect
-      else
-        tostring val
-      end
+      val.is_a?(::String) ? val.inspect : tostring(val)
     end
 
     # toupper() Yast built-in
@@ -1090,15 +1078,11 @@ def self.toterm(symbol, list = DEF_LENGHT)
 
       case symbol
       when ::String
-        return Yast::Term.new(symbol.to_sym)
+        Yast::Term.new(symbol.to_sym)
       when ::Symbol
-        if list == DEF_LENGHT
-          return Yast::Term.new(symbol)
-        else
-          return Yast::Term.new(symbol, *list)
-        end
+        list == DEF_LENGHT ? Yast::Term.new(symbol) : Yast::Term.new(symbol, *list)
       when Yast::Term
-        return symbol
+        symbol
       end
     end
 

src/ruby/yast/convert.rb

@@ -64,23 +64,21 @@ def self.convert(object, options)
       return nil if object.nil?
       return object if from == to
 
-      if from == "any" && allowed_type(object, to)
-        return object
-      elsif to == "float"
+      return object if from == "any" && allowed_type(object, to)
+      if to == "float"
         return nil unless (object.is_a? Fixnum) || (object.is_a? Bignum)
         return object.to_f
       elsif to == "integer"
         return nil unless object.is_a? Float
         Yast.y2warning "Conversion from integer to float lead to loose precision."
         return object.to_i
-      elsif to == "locale" && from == "string"
-        return object
-      elsif to == "string" && from == "locale"
-        return object
-      else
-        Yast.y2warning(-1, "Cannot convert #{object.class} from '#{from}' to '#{to}'")
-        return nil
       end
+
+      return object if to == "locale" && from == "string"
+      return object if to == "string" && from == "locale"
+
+      Yast.y2warning(-1, "Cannot convert #{object.class} from '#{from}' to '#{to}'")
+      nil
     end
 
     # @private
@@ -90,7 +88,7 @@ def self.allowed_type(object, to)
 
       types = [types] unless types.is_a? Array
 
-      types.any? { |t|  object.is_a? t }
+      types.any? { |t| object.is_a? t }
     end
   end
 end

src/ruby/yast/debugger.rb

@@ -147,7 +147,7 @@ def start_gui_session(port)
           end
 
           # start the debugger client in an xterm session
-          exec "xterm", "-e", "byebug", "-R", "#{port}"
+          exec "xterm", "-e", "byebug", "-R", port.to_s
         end
 
         # detach the process, we do not wait for it so avoid zombies
@@ -167,10 +167,10 @@ def debugger_message(remote, port)
           remote_ips = Socket.ip_address_list.select { |a| a.ipv4? && !a.ipv4_loopback? }
           cmd = remote_ips.map { |a| debugger_cmd(a.ip_address, port) }.join("\n")
 
-          if remote_ips.size > 1
-            prefix = "To connect to the debugger from a remote machine use one of these commands:"
+          prefix = if remote_ips.size > 1
+            "To connect to the debugger from a remote machine use one of these commands:"
           else
-            prefix = "To connect to the debugger from a remote machine use this command:"
+            "To connect to the debugger from a remote machine use this command:"
           end
         else
           prefix = "To start the debugger switch to another console and run:"

src/ruby/yast/i18n.rb

@@ -5,9 +5,9 @@ module Yast
   module I18n
     # @private
     # TODO: load alternative in development recent translation
-    LOCALE_DIR = "/usr/share/YaST2/locale"
+    LOCALE_DIR = "/usr/share/YaST2/locale".freeze
     # if every heuristic fails then use the default for locale
-    DEFAULT_LOCALE = "en_US"
+    DEFAULT_LOCALE = "en_US".freeze
 
     # sets new text domain
     def textdomain(domain)

src/ruby/yast/ops.rb

@@ -27,7 +27,7 @@ module Ops
       "locale"    => ::String,
       "function"  => [Yast::FunRef, Yast::YReference],
       "byteblock" => Yast::Byteblock
-    }
+    }.freeze
 
     # Types for which we generate shortcut methods,
     # e.g. {Yast::Ops.get_string}
@@ -43,7 +43,7 @@ module Ops
       "term",
       "path",
       "locale"
-    ]
+    ].freeze
 
     # @!method                    self.get_boolean(       obj, idx, def )
     #   @return [Boolean, nil] {Convert.to_boolean}({get}(obj, idx, def))
@@ -121,7 +121,7 @@ def self.get(object, indexes, default = nil, skip_frames = 0)
         case res
         when ::Array, Yast::Term
           if i.is_a? Fixnum
-            if (0..res.size - 1).include? i
+            if (0..res.size - 1).cover? i
               res = res[i]
             else
               Yast.y2milestone skip_frames, "Index #{i} is out of array size"
@@ -132,11 +132,9 @@ def self.get(object, indexes, default = nil, skip_frames = 0)
             return block_given? ? yield : default
           end
         when ::Hash
-          if res.key? i
-            res = res[i]
-          else
-            return block_given? ? yield : default
-          end
+          return block_given? ? yield : default unless res.key?(i)
+
+          res = res[i]
         when ::NilClass
           Yast.y2milestone skip_frames, "Ops.get called on nil."
           return block_given? ? yield : default
@@ -192,31 +190,34 @@ def self.set(object, indexes, value)
       last = indexes.pop
       res = object
 
+      # return here is needed for quick exit of method and workaround with any or
+      # all is nasty and decrease readability
+      # rubocop:disable Lint/NonLocalExitFromIterator
       indexes.each do |i|
         case res
         when ::Array, Yast::Term
-          if i.is_a? Fixnum
-            if (0..res.size - 1).include? i
-              res = res[i]
-            else
-              Yast.y2warning OUTER_LOOP_FRAME, "Index #{i} is out of array size"
-              return
-            end
-          else
+          if !i.is_a?(Fixnum)
             Yast.y2warning OUTER_LOOP_FRAME, "Passed #{i.inspect} as index key for array."
             return
           end
-        when ::Hash
-          if res.key? i
-            res = res[i]
-          else
+
+          if !(0..res.size - 1).cover?(i)
+            Yast.y2warning OUTER_LOOP_FRAME, "Index #{i} is out of array size"
             return
           end
+
+          res = res[i]
+        when ::Hash
+          return unless res.key? i
+
+          res = res[i]
         else
           Yast.y2warning OUTER_LOOP_FRAME, "Builtin assign called on wrong type #{res.class}"
           return
         end
       end
+      # rubocop:enable Lint/NonLocalExitFromIterator
+
       case res
       when ::Array, Yast::Term, ::Hash
         res[last] = Yast.deep_copy(value)
@@ -233,17 +234,15 @@ def self.add(first, second)
 
       case first
       when ::Array
-        if second.is_a? ::Array
-          return Yast.deep_copy(first + second)
-        else
-          return Yast.deep_copy(first).push(Yast.deep_copy(second))
-        end
+        return Yast.deep_copy(first + second) if second.is_a?(::Array)
+
+        Yast.deep_copy(first).push(Yast.deep_copy(second))
       when ::Hash
-        return Yast.deep_copy(first).merge Yast.deep_copy(second)
+        Yast.deep_copy(first).merge(Yast.deep_copy(second))
       when ::String
-        return first + second.to_s
+        first + second.to_s
       else
-        return first + second
+        first + second
       end
     end
 
@@ -495,29 +494,23 @@ def initialize(value, localized = false)
       # ordered classes from low priority to high
       # Only tricky part is Fixnum/Bignum, which is in fact same, so it has special handling in code
       CLASS_ORDER = [::NilClass, ::FalseClass, ::TrueClass, ::Fixnum, ::Bignum, ::Float,
-                     ::String, Yast::Path, ::Symbol, ::Array, Yast::Term, ::Hash]
+                     ::String, Yast::Path, ::Symbol, ::Array, Yast::Term, ::Hash].freeze
       def <=>(other)
         if @value.class == other.class
           case @value
           when ::Array
-            return ListComparator.new(@value, @localized) <=> other
+            ListComparator.new(@value, @localized) <=> other
           when ::NilClass
-            return 0 # comparison of two nils is equality
+            0 # comparison of two nils is equality
           when ::Hash
-            return HashComparator.new(@value, @localized) <=> other
+            HashComparator.new(@value, @localized) <=> other
           when ::String
-            if @localized
-              return Yast.strcoll(@value, other)
-            else
-              return @value <=> other
-            end
+            @localized ? Yast.strcoll(@value, other) : (@value <=> other)
           else
             @value <=> other
           end
         else
-          if @value.is_a?(::Numeric) && other.is_a?(::Numeric)
-            return @value <=> other
-          end
+          return @value <=> other if @value.is_a?(::Numeric) && other.is_a?(::Numeric)
 
           CLASS_ORDER.index(@value.class) <=> CLASS_ORDER.index(other.class)
         end