chore: run rubocop on .toys directory (#22533)

.github/workflows/ci.yml

@@ -50,6 +50,9 @@ jobs:
           - os: windows-latest
             ruby: "3.2"
             task: "--test"
+          - os: ubuntu-latest
+            ruby: "3.2"
+            task: "--rubocop-toplevel"
       fail-fast: false
     runs-on: ${{ matrix.os }}
     steps:

.rubocop_root.yml

@@ -2,10 +2,36 @@ inherit_gem:
   google-style: google-style.yml
 
 AllCops:
+  Exclude:
+    - "integration/**/*"
   Include:
-    - "NOTHING.rb"
-#   - ".toys/**/*.rb"
+    - ".toys/**/*.rb"
 
+Lint/NonLocalExitFromIterator:
+  Enabled: false
+Metrics/AbcSize:
+  Max: 40
 Metrics/BlockLength:
   Exclude:
     - ".toys/**/*.rb"
+Metrics/CyclomaticComplexity:
+  Max: 20
+Metrics/MethodLength:
+  Max: 50
+Metrics/PerceivedComplexity:
+  Max: 20
+Naming/AccessorMethodName:
+  Enabled: false
+Naming/FileName:
+  Exclude:
+    - ".toys/**/*.rb"
+Style/GuardClause:
+  AllowConsecutiveConditionals: true
+Style/MultilineBlockChain:
+  Enabled: false
+Style/StderrPuts:
+  Enabled: false
+Style/TrailingCommaInArrayLiteral:
+  EnforcedStyleForMultiline: comma
+Style/TrailingCommaInHashLiteral:
+  EnforcedStyleForMultiline: comma

.toys/analyze-clients.rb

@@ -24,8 +24,8 @@
   outdated_wrappers: "List wrapper gems prioritizing an outdated gapic",
   incomplete_bazel: "List incomplete Ruby bazel configs",
   wrapper_bazel: "List missing Ruby wrapper bazel configs",
-  gapic_ready: "List complete Ruby bazel configs that haven't yet been generated"
-}
+  gapic_ready: "List complete Ruby bazel configs that haven't yet been generated",
+}.freeze
 
 at_least_one desc: "Analyses" do
   flag :all, desc: "Run all analyses except those explicitly disabled"
@@ -91,7 +91,7 @@ def gem_version gem_name
     func = proc do
       Dir.chdir gem_name do
         spec = Gem::Specification.load "#{gem_name}.gemspec"
-        puts spec.version.to_s
+        puts spec.version
       end
     end
     capture_proc(func).strip
@@ -164,7 +164,7 @@ def outdated_wrappers_analysis
       match = /^#{gem_name}-(v\d+)$/.match versioned_name
       ga_versions << match[1] if match
     end
-    expected_version = (ga_versions.empty? ? pre_versions : ga_versions).sort.last
+    expected_version = (ga_versions.empty? ? pre_versions : ga_versions).max
     unless expected_version
       puts "#{gem_name}: No expected version"
       next
@@ -189,7 +189,7 @@ def incomplete_bazel_analysis
   count = 0
   puts "Results:", :cyan
   Dir.chdir googleapis_path do
-    Dir.glob("**/BUILD.bazel") do |build_file|
+    Dir.glob "**/BUILD.bazel" do |build_file|
       content = File.read build_file
       next unless content.include? "ruby_cloud_gapic_library"
       unless content.include?("ruby-cloud-api-id=") &&
@@ -208,7 +208,7 @@ def wrapper_bazel_analysis
   count = 0
   puts "Results:", :cyan
   Dir.chdir googleapis_path do
-    Dir.glob("**/BUILD.bazel") do |build_file|
+    Dir.glob "**/BUILD.bazel" do |build_file|
       dir = File.dirname build_file
       if dir =~ /v\d\w+$/
         wrapper_bazel_path = File.join File.dirname(dir), "BUILD.bazel"
@@ -226,7 +226,7 @@ def gapic_ready_analysis
   count = 0
   puts "Results:", :cyan
   Dir.chdir googleapis_path do
-    Dir.glob("**/BUILD.bazel") do |build_file|
+    Dir.glob "**/BUILD.bazel" do |build_file|
       content = File.read build_file
       next unless content.include?("ruby_cloud_gapic_library") &&
                   content.include?("ruby-cloud-api-id=") &&

.toys/batch-review.rb

@@ -41,7 +41,7 @@
       /\.release-please-manifest\.json$/,
       /\/CHANGELOG\.md$/,
       /\/version\.rb$/,
-      /\/snippets\/snippet_metadata_[\w\.]+\.json$/
+      /\/snippets\/snippet_metadata_[\w\.]+\.json$/,
     ],
   },
   "releases-wrappers" => {
@@ -62,18 +62,18 @@
       /\.release-please-manifest\.json$/,
       /\/CHANGELOG\.md$/,
       /\/version\.rb$/,
-      /\/snippets\/snippet_metadata_[\w\.]+\.json$/
+      /\/snippets\/snippet_metadata_[\w\.]+\.json$/,
     ],
   },
-}
+}.freeze
 
 REPO = "googleapis/google-cloud-ruby"
 BOT_USERS = [
   "yoshi-code-bot",
   "yoshi-automation",
   "gcf-owl-bot[bot]",
-  "release-please[bot]"
-]
+  "release-please[bot]",
+].freeze
 
 desc "Interactive mass code review"
 
@@ -153,8 +153,8 @@ def find_prs
 
 def check_automerge pr_data
   if !@automerge_enabled ||
-      @automerge_count_max.positive? && @automerge_count >= @automerge_count_max ||
-      !pr_data.diff_files.all? { |file| @omits.any? { |omit| omit.call file } }
+     (@automerge_count_max.positive? && @automerge_count >= @automerge_count_max) ||
+     !pr_data.diff_files.all? { |file| @omits.any? { |omit| omit.call file } }
     @automerge_count = 1
     return false
   end
@@ -208,7 +208,7 @@ def handle_input pr_data
 
 def handle_merge pr_data
   get_commit_message pr_data
-  get_commit_detail pr_data if detail_type
+  get_commit_detail if detail_type
   do_approve pr_data
   do_merge pr_data
 end
@@ -259,7 +259,7 @@ def handle_display arg, pr_data
   end
 end
 
-def handle_omit arg, *extra_args
+def handle_omit arg, *extra_args # rubocop:disable Metrics/AbcSize
   case arg
   when /^x/
     @omits = []
@@ -301,9 +301,7 @@ def handle_omit arg, *extra_args
       puts "... added omit for deleted file paths: #{regexp}"
     end
   when /^i/
-    omit = Omit.new "All changes affect indentation only" do |file|
-      file.only_indentation
-    end
+    omit = Omit.new "All changes affect indentation only", &:only_indentation
     @omits << omit
     puts "... added omit for indentation-only diffs"
   else
@@ -324,7 +322,7 @@ def handle_suppress *args
   omit = Omit.new "All changes match given regexes" do |file|
     file.reduce_hunks true do |val, hunk|
       val && hunk.all? do |line|
-        line_without_mark = line[1..-1]
+        line_without_mark = line[1..]
         if line.start_with? "+"
           adds.any? { |regex| regex.match? line_without_mark }
         elsif line.start_with? "-"
@@ -360,7 +358,7 @@ def display_pr_title pr_data
 
 def display_filenames pr_data
   pr_data.diff_files.each_with_index do |file, index|
-    write "%3d " % index
+    write format("%3d ", index)
     write file.type, :bold, :yellow
     write " "
     puts file.path
@@ -370,7 +368,7 @@ def display_filenames pr_data
 def display_all_diffs pr_data, force_all: false
   files = pr_data.diff_files
   unless force_all
-    disp_files, omit_files = files.partition { |file| !@omits.any? { |omit| omit.call file } }
+    disp_files, omit_files = files.partition { |file| @omits.none? { |omit| omit.call file } }
   end
   omit_files.each do |file|
     puts "Omitting display of #{file.path}"
@@ -403,11 +401,11 @@ def get_commit_message pr_data
   @commit_message = ask("Message: ", default: @commit_message) if @edit_enabled
 end
 
-def get_commit_detail pr_data
+def get_commit_detail
   return unless @edit_enabled
   case detail_type
   when :shared
-    file = Tempfile.new("commit-default")
+    file = Tempfile.new "commit-default"
     begin
       file.write @commit_detail
       file.rewind
@@ -478,10 +476,11 @@ def paged_api path
 end
 
 def error *messages
-  messages.each { |msg| STDERR.puts msg }
+  messages.each { |msg| $stderr.puts msg }
   exit 1
 end
 
+# Represents a diff output
 class DiffFile
   def initialize text
     @text = text
@@ -528,7 +527,6 @@ def initial_analysis
 
   def analyze_changes
     hunk = nil
-    from_path = to_path = nil
     @lines.each do |line|
       if line.start_with? "@@"
         yield hunk if hunk && !hunk.empty?
@@ -551,7 +549,7 @@ def analyze_only_indentation hunk
           if pos == minuses.length
             minuses = [line]
             pos = 0
-          elsif pos == 0
+          elsif pos.zero?
             minuses << line
           else
             throw :fail
@@ -570,6 +568,7 @@ def analyze_only_indentation hunk
   end
 end
 
+# Represents an omit rule
 class Omit
   def initialize desc, &block
     raise "Block required" unless block
@@ -584,6 +583,7 @@ def call pr_file
   end
 end
 
+# Represents information about a pull request
 class PrData
   def initialize context, pr_resource
     @context = context
@@ -595,30 +595,33 @@ def initialize context, pr_resource
   attr_reader :title
 
   def raw_diff_data
-    @raw_diff_data ||= @context.capture(["curl", "-s", "https://patch-diff.githubusercontent.com/raw/#{REPO}/pull/#{id}.diff"], e: true)
+    @raw_diff_data ||= begin
+      cmd = ["curl", "-s", "https://patch-diff.githubusercontent.com/raw/#{REPO}/pull/#{id}.diff"]
+      @context.capture cmd, e: true
+    end
   end
 
   def diff_files
     @diff_files ||= begin
       "\n#{raw_diff_data.chomp}"
         .split("\ndiff --git ")
         .slice(1..-1)
-        .map{ |text| DiffFile.new("diff --git #{text}\n") }
+        .map { |text| DiffFile.new "diff --git #{text}\n" }
     end
   end
 
   def diff_line_count
-    @diff_line_count ||= raw_diff_data.count("\n")
+    @diff_line_count ||= raw_diff_data.count "\n"
   end
 
   def lib_name
     unless defined? @lib_name
       @lib_name =
         case title
-        when /^\[CHANGE ME\] Re-generated google-cloud-([\w-]+) to pick up changes in the API or client/
-          Regexp.last_match[1]
-        when /^\[CHANGE ME\] Re-generated ([\w-]+) to pick up changes in the API or client/
-          Regexp.last_match[1]
+        when /^\[CHANGE ME\] Re-generated google-cloud-(?<basename>[\w-]+) to pick up changes in the API or client/
+          Regexp.last_match[:basename]
+        when /^\[CHANGE ME\] Re-generated (?<fullname>[\w-]+) to pick up changes in the API or client/
+          Regexp.last_match[:fullname]
         else
           interpret_lib_name
         end
@@ -628,7 +631,7 @@ def lib_name
 
   def custom_message message
     if lib_name && message =~ /^(\w+):\s+(\S.*)$/
-      "#{$1}(#{lib_name}): #{$2}"
+      "#{Regexp.last_match[1]}(#{lib_name}): #{Regexp.last_match[2]}"
     else
       message
     end
@@ -639,14 +642,11 @@ def custom_message message
   def interpret_lib_name
     name = nil
     diff_files.each do |diff_file|
-      if %r{^([^/]+)/} =~ diff_file.path
-        possible_name = Regexp.last_match[1]
-        if name.nil?
-          name = possible_name
-        elsif name != possible_name
-          return nil
-        end
-      else
+      return nil unless %r{^([^/]+)/} =~ diff_file.path
+      possible_name = Regexp.last_match[1]
+      if name.nil?
+        name = possible_name
+      elsif name != possible_name
         return nil
       end
     end