Add 'Style/UnpackFirst' cop

This is a shorter way of getting the data you will need most of the time when unpacking a string.

While unpack1 is also faster (it skips creating the intermediate array object), I consider it to be more style related like Style/SymbolProc. The bad form runs 3+ million iterations per second, which is not a performance bottleneck for most Ruby apps.

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### New features
 
 * [#5597](https://github.com/bbatsov/rubocop/pull/5597): Add new `Rails/HttpStatus` cop. ([@anthony-robin][])
+* [#5643](https://github.com/bbatsov/rubocop/pull/5643): Add new `Style/UnpackFirst` cop. ([@bdewater][])
 
 ### Bug fixes
 

config/enabled.yml

@@ -2010,6 +2010,12 @@ Style/UnneededPercentQ:
   StyleGuide: '#percent-q'
   Enabled: true
 
+Style/UnpackFirst:
+  Description: >-
+                 Checks for accessing the first element of `String#unpack`
+                 instead of using `unpack1`
+  Enabled: true
+
 Style/VariableInterpolation:
   Description: >-
                  Don't interpolate global, instance and class variables

lib/rubocop.rb

@@ -522,6 +522,7 @@
 require_relative 'rubocop/cop/style/unneeded_capital_w'
 require_relative 'rubocop/cop/style/unneeded_interpolation'
 require_relative 'rubocop/cop/style/unneeded_percent_q'
+require_relative 'rubocop/cop/style/unpack_first'
 require_relative 'rubocop/cop/style/variable_interpolation'
 require_relative 'rubocop/cop/style/when_then'
 require_relative 'rubocop/cop/style/while_until_do'

lib/rubocop/cop/style/unpack_first.rb

@@ -0,0 +1,67 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Style
+      # This cop checks for accessing the first element of `String#unpack`
+      # which can be replaced with the shorter method `unpack1`.
+      #
+      # @example
+      #
+      #   # bad
+      #   'foo'.unpack('h*').first
+      #   'foo'.unpack('h*')[0]
+      #   'foo'.unpack('h*').slice(0)
+      #   'foo'.unpack('h*').at(0)
+      #   'foo'.unpack('h*').take(1)
+      #
+      #   # good
+      #   'foo'.unpack1('h*')
+      #
+      class UnpackFirst < Cop
+        extend TargetRubyVersion
+
+        minimum_target_ruby_version 2.4
+
+        MSG = 'Use `%<receiver>s.unpack1(%<format>s)` instead of '\
+          '`%<receiver>s.unpack(%<format>s)%<method>s`.'.freeze
+
+        def_node_matcher :unpack_and_first_element?, <<-PATTERN
+          {
+            (send $(send (...) :unpack $(...)) :first)
+            (send $(send (...) :unpack $(...)) {:[] :slice :at} (int 0))
+            (send $(send (...) :unpack $(...)) :take (int 1))
+          }
+        PATTERN
+
+        def on_send(node)
+          unpack_and_first_element?(node) do |unpack_call, unpack_arg|
+            range = first_element_range(node, unpack_call)
+            message = format(MSG,
+                             receiver:  unpack_call.receiver.source,
+                             format: unpack_arg.source,
+                             method: range.source)
+            add_offense(node, message: message)
+          end
+        end
+
+        def autocorrect(node)
+          unpack_and_first_element?(node) do |unpack_call, _unpack_arg|
+            lambda do |corrector|
+              corrector.remove(first_element_range(node, unpack_call))
+              corrector.replace(unpack_call.loc.selector, 'unpack1')
+            end
+          end
+        end
+
+        private
+
+        def first_element_range(node, unpack_call)
+          Parser::Source::Range.new(node.loc.expression.source_buffer,
+                                    unpack_call.loc.expression.end_pos,
+                                    node.loc.expression.end_pos)
+        end
+      end
+    end
+  end
+end

manual/cops.md

@@ -523,6 +523,7 @@ In the following section you find all available cops:
 * [Style/UnneededCapitalW](cops_style.md#styleunneededcapitalw)
 * [Style/UnneededInterpolation](cops_style.md#styleunneededinterpolation)
 * [Style/UnneededPercentQ](cops_style.md#styleunneededpercentq)
+* [Style/UnpackFirst](cops_style.md#styleunpackfirst)
 * [Style/VariableInterpolation](cops_style.md#stylevariableinterpolation)
 * [Style/WhenThen](cops_style.md#stylewhenthen)
 * [Style/WhileUntilDo](cops_style.md#stylewhileuntildo)

manual/cops_style.md

@@ -5954,6 +5954,29 @@ This cop checks for usage of the %q/%Q syntax when '' or "" would do.
 
 * [https://github.com/bbatsov/ruby-style-guide#percent-q](https://github.com/bbatsov/ruby-style-guide#percent-q)
 
+## Style/UnpackFirst
+
+Enabled by default | Supports autocorrection
+--- | ---
+Enabled | Yes
+
+This cop checks for accessing the first element of `String#unpack`
+which can be replaced with the shorter method `unpack1`.
+
+### Examples
+
+```ruby
+# bad
+'foo'.unpack('h*').first
+'foo'.unpack('h*')[0]
+'foo'.unpack('h*').slice(0)
+'foo'.unpack('h*').at(0)
+'foo'.unpack('h*').take(1)
+
+# good
+'foo'.unpack1('h*')
+```
+
 ## Style/VariableInterpolation
 
 Enabled by default | Supports autocorrection

spec/rubocop/cop/style/unpack_first_spec.rb

@@ -0,0 +1,97 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Style::UnpackFirst, :config do
+  subject(:cop) { described_class.new(config) }
+
+  context 'ruby version >= 2.4', :ruby24 do
+    context 'registers offense' do
+      it 'when using `#unpack` with `#first`' do
+        expect_offense(<<-RUBY.strip_indent)
+        x.unpack('h*').first
+        ^^^^^^^^^^^^^^^^^^^^ Use `x.unpack1('h*')` instead of `x.unpack('h*').first`.
+        RUBY
+      end
+
+      it 'when using `#unpack` with square brackets' do
+        expect_offense(<<-RUBY.strip_indent)
+        ''.unpack(y)[0]
+        ^^^^^^^^^^^^^^^ Use `''.unpack1(y)` instead of `''.unpack(y)[0]`.
+        RUBY
+      end
+
+      it 'when using `#unpack` with dot and square brackets' do
+        expect_offense(<<-RUBY.strip_indent)
+        ''.unpack(y).[](0)
+        ^^^^^^^^^^^^^^^^^^ Use `''.unpack1(y)` instead of `''.unpack(y).[](0)`.
+        RUBY
+      end
+
+      it 'when using `#unpack` with `#slice`' do
+        expect_offense(<<-RUBY.strip_indent)
+        ''.unpack(y).slice(0)
+        ^^^^^^^^^^^^^^^^^^^^^ Use `''.unpack1(y)` instead of `''.unpack(y).slice(0)`.
+        RUBY
+      end
+
+      it 'when using `#unpack` with `#at`' do
+        expect_offense(<<-RUBY.strip_indent)
+        ''.unpack(y).at(0)
+        ^^^^^^^^^^^^^^^^^^ Use `''.unpack1(y)` instead of `''.unpack(y).at(0)`.
+        RUBY
+      end
+
+      it 'when using `#unpack` with `#take`' do
+        expect_offense(<<-RUBY.strip_indent)
+        x.unpack('h*').take(1)
+        ^^^^^^^^^^^^^^^^^^^^^^ Use `x.unpack1('h*')` instead of `x.unpack('h*').take(1)`.
+        RUBY
+      end
+    end
+
+    context 'does not register offense' do
+      it 'when using `#unpack1`' do
+        expect_no_offenses(<<-RUBY.strip_indent)
+          x.unpack1(y)
+        RUBY
+      end
+
+      it 'when using `#unpack` accessing second element' do
+        expect_no_offenses(<<-RUBY.strip_indent)
+          ''.unpack('h*')[1]
+        RUBY
+      end
+    end
+
+    context 'autocorrects' do
+      it '`#unpack` with `#first to `#unpack1`' do
+        expect(autocorrect_source("x.unpack('h*').first"))
+          .to eq("x.unpack1('h*')")
+      end
+
+      it 'autocorrects `#unpack` with square brackets' do
+        expect(autocorrect_source("x.unpack('h*')[0]"))
+          .to eq("x.unpack1('h*')")
+      end
+
+      it 'autocorrects `#unpack` with dot and square brackets' do
+        expect(autocorrect_source("x.unpack('h*').[](0)"))
+          .to eq("x.unpack1('h*')")
+      end
+
+      it 'autocorrects `#unpack` with `#slice`' do
+        expect(autocorrect_source("x.unpack('h*').slice(0)"))
+          .to eq("x.unpack1('h*')")
+      end
+
+      it 'autocorrects `#unpack` with `#at`' do
+        expect(autocorrect_source("x.unpack('h*').at(0)"))
+          .to eq("x.unpack1('h*')")
+      end
+
+      it 'autocorrects `#unpack` with `#take`' do
+        expect(autocorrect_source("x.unpack('h*').take(1)"))
+          .to eq("x.unpack1('h*')")
+      end
+    end
+  end
+end