Fix support to rubocop autocorrect (#346)

* Fix support to rubocop autocorrect RuboCop introduced some breaking changes on v1, and this addresses the changes to fix autocorrect. https://docs.rubocop.org/rubocop/v1_upgrade_notes.html * Make code compatible with rubocop-shopify >=2.7.1 rubocop 1.30 is not compatible with rubocop-shopify < 2.7.1 and rubocop-shopify >= 2.7.1 requires ruby 2.7. So it was required to adapt code style to rubocop-shopify version 2.7.1 and change git workflow lint step to run on ruby 2.7
Shopify · Jun 5, 2023 · 88d15b5 · 88d15b5
1 parent acdffda
commit 88d15b5
Show file tree

Hide file tree

Showing 27 changed files with 278 additions and 178 deletions.
diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
@@ -13,7 +13,7 @@ jobs:
     - name: Set up Ruby
       uses: ruby/setup-ruby@v1
       with:
-        ruby-version: 2.6
+        ruby-version: 2.7
         bundler-cache: true
 
     - name: Run Rubocop

diff --git a/Gemfile b/Gemfile
@@ -8,6 +8,6 @@ gem "minitest"
 gem "rspec"
 gem "mocha"
 gem "yard"
-gem "rubocop", [">= 1.0", "< 1.30"] # TODO: Our cops are broken by rubocop 1.30, we need to figure out why
+gem "rubocop", ">= 1.0"
 gem "rubocop-shopify", require: false
 gem "benchmark-ips"
diff --git a/bin/rake b/bin/rake
@@ -9,8 +9,10 @@
 #
 
 require "pathname"
-ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile",
-  Pathname.new(__FILE__).realpath)
+ENV["BUNDLE_GEMFILE"] ||= File.expand_path(
+  "../../Gemfile",
+  Pathname.new(__FILE__).realpath,
+)
 
 bundle_binstub = File.expand_path("../bundle", __FILE__)
 

diff --git a/bin/rubocop b/bin/rubocop
@@ -9,8 +9,10 @@
 #
 
 require "pathname"
-ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile",
-  Pathname.new(__FILE__).realpath)
+ENV["BUNDLE_GEMFILE"] ||= File.expand_path(
+  "../../Gemfile",
+  Pathname.new(__FILE__).realpath,
+)
 
 bundle_binstub = File.expand_path("../bundle", __FILE__)
 

diff --git a/lib/statsd/instrument.rb b/lib/statsd/instrument.rb
@@ -27,42 +27,44 @@ def statsd_instrumentations
       end
     end
 
-    # Generates a metric name for an instrumented method.
-    # @private
-    # @return [String]
-    def self.generate_metric_name(name, callee, *args)
-      name.respond_to?(:call) ? name.call(callee, args).gsub("::", ".") : name.gsub("::", ".")
-    end
+    class << self
+      # Generates a metric name for an instrumented method.
+      # @private
+      # @return [String]
+      def generate_metric_name(name, callee, *args)
+        name.respond_to?(:call) ? name.call(callee, args).gsub("::", ".") : name.gsub("::", ".")
+      end
 
-    # Generates the tags for an instrumented method.
-    # @private
-    # @return [Array[String]]
-    def self.generate_tags(tags, callee, *args)
-      return if tags.nil?
+      # Generates the tags for an instrumented method.
+      # @private
+      # @return [Array[String]]
+      def generate_tags(tags, callee, *args)
+        return if tags.nil?
 
-      tags.respond_to?(:call) ? tags.call(callee, args) : tags
-    end
+        tags.respond_to?(:call) ? tags.call(callee, args) : tags
+      end
 
-    # Even though this method is considered private, and is no longer used internally,
-    # applications in the wild rely on it. As a result, we cannot remove this method
-    # until the next major version.
-    #
-    # @deprecated Use Process.clock_gettime(Process::CLOCK_MONOTONIC) instead.
-    def self.current_timestamp
-      Process.clock_gettime(Process::CLOCK_MONOTONIC)
-    end
+      # Even though this method is considered private, and is no longer used internally,
+      # applications in the wild rely on it. As a result, we cannot remove this method
+      # until the next major version.
+      #
+      # @deprecated Use Process.clock_gettime(Process::CLOCK_MONOTONIC) instead.
+      def current_timestamp
+        Process.clock_gettime(Process::CLOCK_MONOTONIC)
+      end
 
-    # Even though this method is considered private, and is no longer used internally,
-    # applications in the wild rely on it. As a result, we cannot remove this method
-    # until the next major version.
-    #
-    # @deprecated You can implement similar functionality yourself using
-    #   `Process.clock_gettime(Process::CLOCK_MONOTONIC)`. Think about what will
-    #   happen if an exception happens during the block execution though.
-    def self.duration
-      start = Process.clock_gettime(Process::CLOCK_MONOTONIC)
-      yield
-      Process.clock_gettime(Process::CLOCK_MONOTONIC) - start
+      # Even though this method is considered private, and is no longer used internally,
+      # applications in the wild rely on it. As a result, we cannot remove this method
+      # until the next major version.
+      #
+      # @deprecated You can implement similar functionality yourself using
+      #   `Process.clock_gettime(Process::CLOCK_MONOTONIC)`. Think about what will
+      #   happen if an exception happens during the block execution though.
+      def duration
+        start = Process.clock_gettime(Process::CLOCK_MONOTONIC)
+        yield
+        Process.clock_gettime(Process::CLOCK_MONOTONIC) - start
+      end
     end
 
     # Adds execution duration instrumentation to a method as a timing.
@@ -294,7 +296,12 @@ def add_to_method(method, name, action, &block)
       if instrumentation_module.respond_to?(:ruby2_keywords, true)
         instrumentation_module.send(:ruby2_keywords, method)
       end
-      prepend(instrumentation_module) unless self < instrumentation_module
+
+      if self < instrumentation_module
+        return
+      end
+
+      prepend(instrumentation_module)
     end
 
     def remove_from_method(method, name, action)
@@ -358,8 +365,15 @@ def singleton_client
     # @!method service_check(name, status, tags: nil, hostname: nil, timestamp: nil, message: nil)
     #   (see StatsD::Instrument::Client#service_check)
 
-    def_delegators :singleton_client, :increment, :gauge, :set, :measure,
-      :histogram, :distribution, :event, :service_check
+    def_delegators :singleton_client,
+      :increment,
+      :gauge,
+      :set,
+      :measure,
+      :histogram,
+      :distribution,
+      :event,
+      :service_check
 
     private
 
@@ -385,6 +399,6 @@ def extended(klass)
 require "statsd/instrument/helpers"
 require "statsd/instrument/assertions"
 require "statsd/instrument/expectation"
-require "statsd/instrument/matchers" if defined?(::RSpec)
-require "statsd/instrument/railtie" if defined?(::Rails::Railtie)
+require "statsd/instrument/matchers" if defined?(RSpec)
+require "statsd/instrument/railtie" if defined?(Rails::Railtie)
 require "statsd/instrument/strict" if ENV["STATSD_STRICT_MODE"]
diff --git a/lib/statsd/instrument/batched_udp_sink.rb b/lib/statsd/instrument/batched_udp_sink.rb
@@ -10,14 +10,14 @@ class BatchedUDPSink
       # https://docs.datadoghq.com/developers/dogstatsd/high_throughput/?code-lang=ruby#ensure-proper-packet-sizes
       DEFAULT_MAX_PACKET_SIZE = 1472
 
-      def self.for_addr(addr, **kwargs)
-        host, port_as_string = addr.split(":", 2)
-        new(host, Integer(port_as_string), **kwargs)
-      end
-
       attr_reader :host, :port
 
       class << self
+        def for_addr(addr, **kwargs)
+          host, port_as_string = addr.split(":", 2)
+          new(host, Integer(port_as_string), **kwargs)
+        end
+
         def finalize(dispatcher)
           proc { dispatcher.shutdown }
         end
@@ -122,7 +122,7 @@ def flush(blocking:)
 
             packet << next_datagram
             next_datagram = nil
-            unless packet.bytesize > @max_packet_size
+            if packet.bytesize <= @max_packet_size
               while (next_datagram = @buffer.pop_nonblock)
                 if @max_packet_size - packet.bytesize - 1 > next_datagram.bytesize
                   packet << NEWLINE << next_datagram

diff --git a/lib/statsd/instrument/client.rb b/lib/statsd/instrument/client.rb
@@ -338,8 +338,14 @@ def latency(name, sample_rate: nil, tags: nil, metric_type: nil, no_prefix: fals
       #
       # @note Supported by the Datadog implementation only.
       def service_check(name, status, timestamp: nil, hostname: nil, tags: nil, message: nil, no_prefix: false)
-        emit(datagram_builder(no_prefix: no_prefix)._sc(name, status,
-          timestamp: timestamp, hostname: hostname, tags: tags, message: message))
+        emit(datagram_builder(no_prefix: no_prefix)._sc(
+          name,
+          status,
+          timestamp: timestamp,
+          hostname: hostname,
+          tags: tags,
+          message: message,
+        ))
       end
 
       # Emits an event. An event represents any record of activity noteworthy for engineers.
@@ -359,9 +365,17 @@ def service_check(name, status, timestamp: nil, hostname: nil, tags: nil, messag
       def event(title, text, timestamp: nil, hostname: nil, aggregation_key: nil, priority: nil,
         source_type_name: nil, alert_type: nil, tags: nil, no_prefix: false)
 
-        emit(datagram_builder(no_prefix: no_prefix)._e(title, text, timestamp: timestamp,
-          hostname: hostname, tags: tags, aggregation_key: aggregation_key, priority: priority,
-          source_type_name: source_type_name, alert_type: alert_type))
+        emit(datagram_builder(no_prefix: no_prefix)._e(
+          title,
+          text,
+          timestamp: timestamp,
+          hostname: hostname,
+          tags: tags,
+          aggregation_key: aggregation_key,
+          priority: priority,
+          source_type_name: source_type_name,
+          alert_type: alert_type,
+        ))
       end
 
       # Instantiates a new StatsD client that uses the settings of the current client,
@@ -378,9 +392,13 @@ def with_options(
         default_tags: nil,
         datagram_builder_class: nil
       )
-        client = clone_with_options(sink: sink, prefix: prefix,
-          default_sample_rate: default_sample_rate, default_tags: default_tags,
-          datagram_builder_class: datagram_builder_class)
+        client = clone_with_options(
+          sink: sink,
+          prefix: prefix,
+          default_sample_rate: default_sample_rate,
+          default_tags: default_tags,
+          datagram_builder_class: datagram_builder_class,
+        )
 
         yield(client)
       end

diff --git a/lib/statsd/instrument/datagram.rb b/lib/statsd/instrument/datagram.rb
@@ -40,7 +40,7 @@ def value
       end
 
       def tags
-        @tags ||= parsed_datagram[:tags] ? parsed_datagram[:tags].split(",") : nil
+        @tags ||= parsed_datagram[:tags]&.split(",")
       end
 
       def inspect

diff --git a/lib/statsd/instrument/datagram_builder.rb b/lib/statsd/instrument/datagram_builder.rb
@@ -5,16 +5,18 @@ module Instrument
     # @note This class is part of the new Client implementation that is intended
     #   to become the new default in the next major release of this library.
     class DatagramBuilder
-      def self.unsupported_datagram_types(*types)
-        types.each do |type|
-          define_method(type) do |_, _, _, _|
-            raise NotImplementedError, "Type #{type} metrics are not supported by #{self.class.name}."
+      class << self
+        def unsupported_datagram_types(*types)
+          types.each do |type|
+            define_method(type) do |_, _, _, _|
+              raise NotImplementedError, "Type #{type} metrics are not supported by #{self.class.name}."
+            end
           end
         end
-      end
 
-      def self.datagram_class
-        StatsD::Instrument::Datagram
+        def datagram_class
+          StatsD::Instrument::Datagram
+        end
       end
 
       def initialize(prefix: nil, default_tags: nil)

diff --git a/lib/statsd/instrument/dogstatsd_datagram_builder.rb b/lib/statsd/instrument/dogstatsd_datagram_builder.rb
@@ -7,10 +7,11 @@ module Instrument
     class DogStatsDDatagramBuilder < StatsD::Instrument::DatagramBuilder
       unsupported_datagram_types :kv
 
-      def self.datagram_class
-        StatsD::Instrument::DogStatsDDatagram
+      class << self
+        def datagram_class
+          StatsD::Instrument::DogStatsDDatagram
+        end
       end
-
       def latency_metric_type
         :d
       end

diff --git a/lib/statsd/instrument/helpers.rb b/lib/statsd/instrument/helpers.rb
@@ -11,32 +11,34 @@ def capture_statsd_datagrams(client: nil, &block)
       # For backwards compatibility
       alias_method :capture_statsd_calls, :capture_statsd_datagrams
 
-      def self.add_tag(tags, key, value)
-        tags = tags.dup || {}
-
-        if tags.is_a?(String)
-          tags = tags.empty? ? "#{key}:#{value}" : "#{tags},#{key}:#{value}"
-        elsif tags.is_a?(Array)
-          tags << "#{key}:#{value}"
-        elsif tags.is_a?(Hash)
-          tags[key] = value
-        else
-          raise ArgumentError, "add_tag only supports string, array or hash, #{tags.class} provided"
+      class << self
+        def add_tag(tags, key, value)
+          tags = tags.dup || {}
+
+          if tags.is_a?(String)
+            tags = tags.empty? ? "#{key}:#{value}" : "#{tags},#{key}:#{value}"
+          elsif tags.is_a?(Array)
+            tags << "#{key}:#{value}"
+          elsif tags.is_a?(Hash)
+            tags[key] = value
+          else
+            raise ArgumentError, "add_tag only supports string, array or hash, #{tags.class} provided"
+          end
+
+          tags
         end
 
-        tags
-      end
-
-      def self.prefix_metric(metric_name, client: nil)
-        client ||= StatsD.singleton_client
-        client&.prefix ? "#{client.prefix}.#{metric_name}" : metric_name
-      end
+        def prefix_metric(metric_name, client: nil)
+          client ||= StatsD.singleton_client
+          client&.prefix ? "#{client.prefix}.#{metric_name}" : metric_name
+        end
 
-      def self.prefixed_metric?(metric_name, client: nil)
-        client ||= StatsD.singleton_client
-        return false unless client&.prefix
+        def prefixed_metric?(metric_name, client: nil)
+          client ||= StatsD.singleton_client
+          return false unless client&.prefix
 
-        metric_name =~ /\A#{Regexp.escape(client.prefix)}\./
+          metric_name =~ /\A#{Regexp.escape(client.prefix)}\./
+        end
       end
     end
   end

diff --git a/lib/statsd/instrument/rubocop.rb b/lib/statsd/instrument/rubocop.rb
@@ -5,11 +5,24 @@ module Cop
     module StatsD
       METRIC_METHODS = [:increment, :gauge, :measure, :set, :histogram, :distribution, :key_value]
 
-      METAPROGRAMMING_METHODS = [:statsd_measure, :statsd_distribution, :statsd_count_success, :statsd_count_if,
-                                 :statsd_count,]
+      METAPROGRAMMING_METHODS = [
+        :statsd_measure,
+        :statsd_distribution,
+        :statsd_count_success,
+        :statsd_count_if,
+        :statsd_count,
+      ]
 
-      SINGLETON_CONFIGURATION_METHODS = [:backend, :"backend=", :prefix, :"prefix=", :default_tags, :"default_tags=",
-                                         :default_sample_rate, :"default_sample_rate=",]
+      SINGLETON_CONFIGURATION_METHODS = [
+        :backend,
+        :"backend=",
+        :prefix,
+        :"prefix=",
+        :default_tags,
+        :"default_tags=",
+        :default_sample_rate,
+        :"default_sample_rate=",
+      ]
 
       private
 

diff --git a/lib/statsd/instrument/rubocop/measure_as_dist_argument.rb b/lib/statsd/instrument/rubocop/measure_as_dist_argument.rb
@@ -1,4 +1,4 @@
-# frozen-string-literal: true
+# frozen_string_literal: true
 
 require_relative "../rubocop" unless defined?(RuboCop::Cop::StatsD)
 

diff --git a/lib/statsd/instrument/rubocop/metaprogramming_positional_arguments.rb b/lib/statsd/instrument/rubocop/metaprogramming_positional_arguments.rb
@@ -1,4 +1,4 @@
-# frozen-string-literal: true
+# frozen_string_literal: true
 
 require_relative "../rubocop" unless defined?(RuboCop::Cop::StatsD)
 

diff --git a/lib/statsd/instrument/rubocop/metric_prefix_argument.rb b/lib/statsd/instrument/rubocop/metric_prefix_argument.rb
@@ -1,4 +1,4 @@
-# frozen-string-literal: true
+# frozen_string_literal: true
 
 require_relative "../rubocop" unless defined?(RuboCop::Cop::StatsD)