Code cleanup

exe/importmap-update

@@ -67,7 +67,7 @@ end
 
 rails_root = ENV.fetch("RAILS_ROOT", ".")
 runner = Importmap::Update::Commands::ShellRunner.new(cwd: rails_root)
-gh = Importmap::Update::GitHubClient.new(repo: repo, token: token)
+gh = Importmap::Update::GitHubClient.new(repo:, token:)
 git = Importmap::Update::GitClient.new(
   repo: Git.open(rails_root),
   author_name: ENV.fetch("IMPORTMAP_AUTHOR_NAME", "github-actions[bot]"),
@@ -77,14 +77,14 @@ git = Importmap::Update::GitClient.new(
 outdated = Importmap::Update::Parsers::OutdatedParser.parse(ENV.fetch("IMPORTMAP_OUTDATED_OUTPUT"))
 vulnerabilities = Importmap::Update::Parsers::AuditParser.parse(ENV.fetch("IMPORTMAP_AUDIT_OUTPUT"))
 plan = Importmap::Update::Planner.new(
-  outdated: outdated, vulnerabilities: vulnerabilities, config: config
+  outdated:, vulnerabilities:, config:
 ).call
 
 existing_prs = gh.list_open_prs(branch_prefix: config.branch_prefix)
-reconciled = Importmap::Update::Reconciler.new(plan: plan, existing_prs: existing_prs).call
+reconciled = Importmap::Update::Reconciler.new(plan:, existing_prs:).call
 
 executor = Importmap::Update::Executor.new(
-  gh: gh, git: git, runner: runner,
+  gh:, git:, runner:,
   base_branch: ENV.fetch("IMPORTMAP_BASE_BRANCH", "main"),
   commit_message_prefix: config.commit_message.prefix,
   labels: config.labels,

lib/commands.rb

@@ -19,7 +19,7 @@ module Update
     # safer (no shell-injection surprises from package names) and easier to
     # match against fixture keys.
     module Commands
-      Result = Struct.new(:stdout, :stderr, :exit_code, keyword_init: true) do
+      Result = Data.define(:stdout, :stderr, :exit_code) do
         def success?
           exit_code == 0
         end
@@ -47,7 +47,7 @@ def run(*argv)
           opts[:chdir] = @cwd if @cwd
           Bundler.with_unbundled_env do
             stdout, stderr, status = Open3.capture3(*argv, opts)
-            Result.new(stdout: stdout, stderr: stderr, exit_code: status.exitstatus)
+            Result.new(stdout:, stderr:, exit_code: status.exitstatus)
           end
         end
 

lib/config.rb

@@ -24,7 +24,7 @@ class Config
 
       VALID_STRATEGIES = %i[grouped individual].freeze
 
-      Bucket = Struct.new(:strategy, keyword_init: true) do
+      Bucket = Data.define(:strategy) do
         def grouped?
           strategy == :grouped
         end
@@ -34,7 +34,7 @@ def individual?
         end
       end
 
-      CommitMessage = Struct.new(:prefix, keyword_init: true)
+      CommitMessage = Data.define(:prefix)
 
       attr_reader :version,
         :grouping,
@@ -62,13 +62,13 @@ def self.default
 
       def to_h
         {
-          version: version,
+          version:,
           grouping: grouping.transform_values { |b| {strategy: b.strategy} },
-          open_pull_requests_limit: open_pull_requests_limit,
-          labels: labels,
-          reviewers: reviewers,
-          commit_message: {prefix: commit_message.prefix},
-          branch_prefix: branch_prefix
+          open_pull_requests_limit:,
+          labels:,
+          reviewers:,
+          branch_prefix:,
+          commit_message: {prefix: commit_message.prefix}
         }
       end
 

lib/executor.rb

@@ -40,7 +40,7 @@ def failed?
         end
       end
 
-      Report = Struct.new(:outcomes, :warnings, keyword_init: true)
+      Report = Data.define(:outcomes, :warnings)
 
       def initialize(
         gh:,

lib/git_client.rb

@@ -47,7 +47,7 @@ def commit_changes(message:)
       end
 
       def push(branch:, force: false)
-        @repo.push("origin", branch, force: force)
+        @repo.push("origin", branch, force:)
         nil
       end
     end

lib/github_client.rb

@@ -62,7 +62,7 @@ def ensure_labels(labels)
 
       # Edits an existing PR's title and body.
       def update_pr(number:, title:, body:)
-        @client.update_pull_request(@repo, number, title: title, body: body)
+        @client.update_pull_request(@repo, number, title:, body:)
         nil
       end
 

lib/parsers/audit_parser.rb

@@ -20,7 +20,7 @@ module Parsers
       # free-form text. If it ever contains a literal `|`, we rejoin the
       # overflow cells so the description survives intact.
       class AuditParser
-        Vulnerability = Struct.new(:name, :severity, :vulnerable_versions, :advisory, keyword_init: true)
+        Vulnerability = Data.define(:name, :severity, :vulnerable_versions, :advisory)
 
         SEVERITIES = %w[low moderate high critical].freeze
 
@@ -62,11 +62,11 @@ def split_row(line)
         # If a description contained a `|`, cells.size will be >4. Rejoin the
         # tail into the advisory column so we don't lose information.
         def build_row(cells)
-          name, severity, vuln_versions, *advisory_parts = cells
+          name, severity, vulnerable_versions, *advisory_parts = cells
           Vulnerability.new(
-            name: name,
-            severity: severity,
-            vulnerable_versions: vuln_versions,
+            name:,
+            severity:,
+            vulnerable_versions:,
             advisory: advisory_parts.join(" | ")
           )
         end

lib/parsers/outdated_parser.rb

@@ -21,7 +21,7 @@ module Parsers
       # underlying OutdatedPackage. Those rows are returned with `error: ...`
       # set and `latest: nil`, so callers can decide whether to skip them.
       class OutdatedParser
-        OutdatedPackage = Struct.new(:name, :current, :latest, :error, keyword_init: true) do
+        OutdatedPackage = Data.define(:name, :current, :latest, :error) do
           def parseable?
             !latest.nil?
           end
@@ -69,12 +69,11 @@ def split_row(line)
         end
 
         def build_row(cells)
-          name, current, latest_or_error = cells[0], cells[1], cells[2]
-          if VERSION_SHAPE_RE.match?(latest_or_error)
-            OutdatedPackage.new(name: name, current: current, latest: latest_or_error, error: nil)
-          else
-            OutdatedPackage.new(name: name, current: current, latest: nil, error: latest_or_error)
-          end
+          name, current, latest_or_error = cells
+          latest = latest_or_error if VERSION_SHAPE_RE.match?(latest_or_error)
+          error = latest_or_error unless VERSION_SHAPE_RE.match?(latest_or_error)
+
+          OutdatedPackage.new(name:, current:, latest:, error:)
         end
       end
     end

lib/planner.rb

@@ -37,24 +37,18 @@ module Update
     #     attention than patches.
     class Planner
       # An individual package update slated for inclusion in some PR.
-      PackageBump = Struct.new(
-        :name, :from, :to, :semver_kind, :advisory,
-        keyword_init: true
-      )
+      PackageBump = Data.define(:name, :from, :to, :semver_kind, :advisory)
 
       # A planned pull request. The reconciler (step 4) will compare these against
       # existing open PRs on GitHub to decide what to open, force-push, or close.
-      PRSpec = Struct.new(
-        :kind, :packages, :branch, :title, :metadata,
-        keyword_init: true
-      ) do
+      PRSpec = Data.define(:kind, :packages, :branch, :title, :metadata) do
         def security?
           kind == :security
         end
       end
 
       # The full output of a planning pass.
-      Plan = Struct.new(:pr_specs, :warnings, keyword_init: true) do
+      Plan = Data.define(:pr_specs, :warnings) do
         def empty?
           pr_specs.empty?
         end
@@ -196,7 +190,7 @@ def build_non_security_specs(bumps)
 
       def build_grouped_spec(kind, bumps)
         PRSpec.new(
-          kind: kind,
+          kind:,
           packages: bumps,
           branch: "#{@config.branch_prefix}/#{kind}",
           title: grouped_title(kind, bumps),
@@ -206,7 +200,7 @@ def build_grouped_spec(kind, bumps)
 
       def build_individual_spec(kind, bump)
         PRSpec.new(
-          kind: kind,
+          kind:,
           packages: [bump],
           branch: "#{@config.branch_prefix}/#{kind}-#{sanitize(bump.name)}",
           title: with_prefix("bump #{bump.name} #{bump.from} → #{bump.to}"),
@@ -235,7 +229,7 @@ def with_prefix(message)
       def metadata_for(kind, bumps)
         {
           tool: "importmap-update",
-          kind: kind,
+          kind:,
           packages: bumps.map { |b|
             entry = {name: b.name, from: b.from, to: b.to, semver_kind: b.semver_kind}
             entry[:severity] = b.advisory[:severity] if b.advisory

lib/reconciler.rb

@@ -31,7 +31,7 @@ module Update
     class Reconciler
       # Input describing an open PR that GitHub returned. Step 5's client
       # will fill these in from the list-PRs API call.
-      ExistingPR = Struct.new(:number, :branch, :body, :title, keyword_init: true)
+      ExistingPR = Data.define(:number, :branch, :body, :title)
 
       Action = Struct.new(:type, :pr_spec, :existing_pr, :reason, keyword_init: true) do
         def open?
@@ -51,7 +51,7 @@ def noop?
         end
       end
 
-      Result = Struct.new(:actions, :ignored, keyword_init: true) do
+      Result = Data.define(:actions, :ignored) do
         def opens
           actions.select(&:open?)
         end

test/executor_test.rb

@@ -30,19 +30,19 @@ def ensure_labels(labels)
     end
 
     def create_pr(branch:, base:, title:, body:, labels: [])
-      @created << {branch: branch, base: base, title: title, body: body, labels: labels}
+      @created << {branch:, base:, title:, body:, labels:}
       n = @next_pr_number
       @next_pr_number += 1
       n
     end
 
     def update_pr(number:, title:, body:)
-      @updated << {number: number, title: title, body: body}
+      @updated << {number:, title:, body:}
       nil
     end
 
     def close_pr(number:, comment: nil)
-      @closed << {number: number, comment: comment}
+      @closed << {number:, comment:}
       nil
     end
   end
@@ -60,7 +60,7 @@ def initialize
     end
 
     def checkout_fresh_branch(branch:, base:)
-      @checkouts << {branch: branch, base: base}
+      @checkouts << {branch:, base:}
       nil
     end
 
@@ -70,30 +70,30 @@ def commit_changes(message:)
     end
 
     def push(branch:, force: false)
-      @pushes << {branch: branch, force: force}
+      @pushes << {branch:, force:}
       nil
     end
   end
 
   # ---- builders mirroring the planner's output ----
 
   def bump(name, from, to, kind: :patch, severity: nil)
-    advisory = severity ? {severity: severity} : nil
-    Planner::PackageBump.new(name: name, from: from, to: to, semver_kind: kind, advisory: advisory)
+    advisory = severity ? {severity:} : nil
+    Planner::PackageBump.new(name:, from:, to:, semver_kind: kind, advisory:)
   end
 
   def spec(branch:, packages:, kind: :patch, title: "spec title")
     Planner::PRSpec.new(
-      kind: kind, packages: packages, branch: branch, title: title,
+      kind:, packages:, branch:, title:,
       metadata: {
-        tool: "importmap-update", kind: kind,
+        tool: "importmap-update", kind:,
         packages: packages.map { |p| {name: p.name, from: p.from, to: p.to, semver_kind: p.semver_kind} }
       }
     )
   end
 
   def existing_pr(number:, branch:)
-    Reconciler::ExistingPR.new(number: number, branch: branch, body: "", title: "old")
+    Reconciler::ExistingPR.new(number:, branch:, body: "", title: "old")
   end
 
   def setup
@@ -108,7 +108,7 @@ def make_executor(dry_run: false)
       base_branch: "main",
       commit_message_prefix: "",
       labels: %w[dependencies],
-      dry_run: dry_run,
+      dry_run:,
       # Override the body renderer so tests don't depend on the exact body string.
       body_renderer: ->(s) { "body for #{s.branch}" }
     )

test/fixture_runner.rb

@@ -6,7 +6,7 @@ module Importmap
   module Update
     module Commands
       class FixtureRunner
-        Fixture = Struct.new(:pattern, :result, keyword_init: true)
+        Fixture = Data.define(:pattern, :result)
 
         def initialize(fixtures = [])
           @fixtures = fixtures
@@ -17,8 +17,8 @@ def initialize(fixtures = [])
 
         def add(pattern:, stdout: "", stderr: "", exit_code: 0)
           @fixtures << Fixture.new(
-            pattern: pattern,
-            result: Result.new(stdout: stdout, stderr: stderr, exit_code: exit_code)
+            pattern:,
+            result: Result.new(stdout:, stderr:, exit_code:)
           )
         end
 

test/planner_test.rb

@@ -15,15 +15,15 @@ class PlannerTest < Minitest::Test
   # ---- helpers ----
 
   def outdated(name, from, to, error: nil)
-    Outdated.new(name: name, current: from, latest: to, error: error)
+    Outdated.new(name:, current: from, latest: to, error:)
   end
 
   def vuln(name, severity, vulnerable: "<#{name}", advisory: "Vulnerability in #{name}")
-    Vuln.new(name: name, severity: severity, vulnerable_versions: vulnerable, advisory: advisory)
+    Vuln.new(name:, severity:, vulnerable_versions: vulnerable, advisory:)
   end
 
   def plan_with(outdated:, vulnerabilities: [], config: Config.default)
-    Planner.new(outdated: outdated, vulnerabilities: vulnerabilities, config: config).call
+    Planner.new(outdated:, vulnerabilities:, config:).call
   end
 
   # ---- happy paths: grouping ----
@@ -178,7 +178,7 @@ def test_individual_patch_strategy_emits_one_pr_per_package
         outdated("lodash", "4.17.20", "4.17.21"),
         outdated("axios", "1.7.0", "1.7.1")
       ],
-      config: config
+      config:
     )
     assert_equal 2, plan.pr_specs.size
     assert(plan.pr_specs.all? { |s| s.kind == :patch })
@@ -198,7 +198,7 @@ def test_grouped_major_strategy_combines_into_single_pr
         outdated("react", "18.2.0", "19.0.0"),
         outdated("vue", "2.7.0", "3.0.0")
       ],
-      config: config
+      config:
     )
     assert_equal 1, plan.pr_specs.size
     assert_equal :major, plan.pr_specs.first.kind
@@ -276,7 +276,7 @@ def test_open_pull_requests_limit_throttles_non_security_prs
         outdated("b", "1.0.0", "2.0.0"),
         outdated("c", "1.0.0", "2.0.0")
       ],
-      config: config
+      config:
     )
     assert_equal 2, plan.pr_specs.size
     assert(plan.warnings.any? { |w| w.include?("Dropped 1") })
@@ -296,7 +296,7 @@ def test_security_prs_are_never_throttled_even_above_the_limit
         vuln("vulnB", "high"),
         vuln("vulnC", "high")
       ],
-      config: config
+      config:
     )
     security = plan.pr_specs.select(&:security?)
     assert_equal 3, security.size, "security PRs should not be throttled"
@@ -312,7 +312,7 @@ def test_budget_keeps_higher_priority_prs_when_truncating
         outdated("patch-pkg", "1.0.0", "1.0.1"),  # patch (grouped)
         outdated("major-pkg", "1.0.0", "2.0.0")  # major (individual)
       ],
-      config: config
+      config:
     )
     # Major should win over patch when only 1 slot is available.
     assert_equal 1, plan.pr_specs.size