Skip to content

Commit 3e38fe3

Browse files
committed
Parse Job inputs into typed structs instead of untyped hashes Replaces T::Hash[String, T.untyped] with T::ImmutableStruct for the three main untyped collections in the Job class: - AllowedUpdate: dependency_name, dependency_type, update_type, update_types fields replace hash key lookups like update.fetch("dependency-name") - IgnoreCondition (Job-scoped): dependency_name, version_requirement, update_types, source fields replace ic["dependency-name"] etc. - SecurityAdvisoryEntry: dependency_name, affected_versions, patched_versions, unaffected_versions replace adv["affected-versions"] Each struct has a from_hash class method that parses the raw JSON hash at the boundary (in the Job constructor). Downstream code gets typed field access instead of T.untyped hash lookups.
1 parent 75393ad commit 3e38fe3

5 files changed

Lines changed: 155 additions & 40 deletions

File tree

updater/lib/dependabot/dependency_change.rb

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,13 @@ def pr_message
9595
dependency_group: dependency_group,
9696
pr_message_max_length: pr_message_max_length,
9797
pr_message_encoding: pr_message_encoding,
98-
ignore_conditions: job.ignore_conditions,
98+
ignore_conditions: job.ignore_conditions.map do |ic|
99+
{
100+
"dependency-name" => ic.dependency_name,
101+
"version-requirement" => ic.version_requirement,
102+
"source" => ic.source
103+
}.compact
104+
end,
99105
notices: notices
100106
).message
101107

updater/lib/dependabot/job.rb

Lines changed: 44 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@
1515
require "dependabot/package/release_cooldown_options"
1616
require "dependabot/updater/update_type_helper"
1717

18+
require "dependabot/job/allowed_update"
19+
require "dependabot/job/ignore_condition"
20+
require "dependabot/job/security_advisory_entry"
21+
1822
# Describes a single Dependabot workload within the GitHub-integrated Service
1923
#
2024
# This primarily acts as a value class to hold inputs for various Core objects
@@ -61,7 +65,7 @@ class Job # rubocop:disable Metrics/ClassLength
6165
T::Array[Symbol]
6266
)
6367

64-
sig { returns(T::Array[T::Hash[String, T.untyped]]) }
68+
sig { returns(T::Array[Dependabot::Job::AllowedUpdate]) }
6569
attr_reader :allowed_updates
6670

6771
sig { returns(T::Array[Dependabot::Credential]) }
@@ -85,7 +89,7 @@ class Job # rubocop:disable Metrics/ClassLength
8589
sig { returns(String) }
8690
attr_reader :command
8791

88-
sig { returns(T::Array[T.untyped]) }
92+
sig { returns(T::Array[Dependabot::Job::IgnoreCondition]) }
8993
attr_reader :ignore_conditions
9094

9195
sig { returns(String) }
@@ -94,7 +98,7 @@ class Job # rubocop:disable Metrics/ClassLength
9498
sig { returns(T.nilable(Dependabot::RequirementsUpdateStrategy)) }
9599
attr_reader :requirements_update_strategy
96100

97-
sig { returns(T::Array[T.untyped]) }
101+
sig { returns(T::Array[Dependabot::Job::SecurityAdvisoryEntry]) }
98102
attr_reader :security_advisories
99103

100104
sig { returns(T::Boolean) }
@@ -161,8 +165,11 @@ def self.standardise_keys(hash)
161165
def initialize(attributes) # rubocop:disable Metrics/AbcSize,Metrics/MethodLength
162166
@id = T.let(attributes.fetch(:id), String)
163167
@command = T.let(attributes.fetch(:command, ""), String)
164-
@allowed_updates = T.let(attributes.fetch(:allowed_updates), T::Array[T.untyped])
165-
@commit_message_options = T.let(
168+
@allowed_updates = T.let(
169+
attributes.fetch(:allowed_updates).map { |h| AllowedUpdate.from_hash(h) },
170+
T::Array[AllowedUpdate]
171+
)
172+
@commit_message_options = T.let(
166173
attributes.fetch(:commit_message_options, {}),
167174
T.nilable(T::Hash[T.untyped, T.untyped])
168175
)
@@ -188,7 +195,10 @@ def initialize(attributes) # rubocop:disable Metrics/AbcSize,Metrics/MethodLengt
188195
attributes.fetch(:experiments, {}),
189196
T.nilable(T::Hash[T.untyped, T.untyped])
190197
)
191-
@ignore_conditions = T.let(attributes.fetch(:ignore_conditions), T::Array[T.untyped])
198+
@ignore_conditions = T.let(
199+
attributes.fetch(:ignore_conditions).map { |h| Job::IgnoreCondition.from_hash(h) },
200+
T::Array[Job::IgnoreCondition]
201+
)
192202
@package_manager = T.let(attributes.fetch(:package_manager), String)
193203
@reject_external_code = T.let(attributes.fetch(:reject_external_code, false), T::Boolean)
194204
@repo_contents_path = T.let(attributes.fetch(:repo_contents_path, nil), T.nilable(String))
@@ -200,7 +210,10 @@ def initialize(attributes) # rubocop:disable Metrics/AbcSize,Metrics/MethodLengt
200210
T.nilable(Dependabot::RequirementsUpdateStrategy)
201211
)
202212

203-
@security_advisories = T.let(attributes.fetch(:security_advisories), T::Array[T.untyped])
213+
@security_advisories = T.let(
214+
attributes.fetch(:security_advisories).map { |h| SecurityAdvisoryEntry.from_hash(h) },
215+
T::Array[SecurityAdvisoryEntry]
216+
)
204217
@security_updates_only = T.let(attributes.fetch(:security_updates_only), T::Boolean)
205218
@source = T.let(build_source(attributes.fetch(:source)), Dependabot::Source)
206219
@token = T.let(attributes.fetch(:token, nil), T.nilable(String))
@@ -316,18 +329,17 @@ def allowed_update?(dependency, check_previous_version: false)
316329

317330
allowed_updates.any? do |update|
318331
# Check the update-type (defaulting to all)
319-
update_type = update.fetch("update-type", "all")
320332
# NOTE: Preview supports specifying a "security" update type whereas
321333
# native will say "security-updates-only"
322-
security_update = update_type == "security" || security_updates_only?
334+
security_update = update.update_type == "security" || security_updates_only?
323335
next false if security_update && !vulnerable?(dependency, check_previous_version)
324336

325337
# Check the dependency-name (defaulting to matching)
326-
condition_name = update.fetch("dependency-name", dependency.name)
338+
condition_name = update.dependency_name || dependency.name
327339
next false unless name_match?(condition_name, dependency.name)
328340

329341
# Check the dependency-type (defaulting to all)
330-
dep_type = update.fetch("dependency-type", "all")
342+
dep_type = update.dependency_type
331343
next false if dep_type == "indirect" &&
332344
dependency.requirements.any?
333345
# In dependabot-api, dependency-type is defaulting to "direct" not "all". Ignoring
@@ -417,18 +429,14 @@ def commit_message_options
417429
def security_advisories_for(dependency)
418430
relevant_advisories =
419431
security_advisories
420-
.select { |adv| adv.fetch("dependency-name").casecmp(dependency.name).zero? }
432+
.select { |adv| T.must(adv.dependency_name.casecmp(dependency.name)).zero? }
421433

422434
relevant_advisories.map do |adv|
423-
vulnerable_versions = adv["affected-versions"] || []
424-
safe_versions = (adv["patched-versions"] || []) +
425-
(adv["unaffected-versions"] || [])
426-
427435
Dependabot::SecurityAdvisory.new(
428436
dependency_name: dependency.name,
429437
package_manager: package_manager,
430-
vulnerable_versions: vulnerable_versions,
431-
safe_versions: safe_versions
438+
vulnerable_versions: T.unsafe(adv.affected_versions),
439+
safe_versions: T.unsafe(adv.patched_versions + adv.unaffected_versions)
432440
)
433441
end
434442
end
@@ -459,17 +467,15 @@ def ignore_conditions_for(dependency)
459467
# were created via "@dependabot ignore version" commands
460468
sig { params(dependency: Dependabot::Dependency).void }
461469
def log_ignore_conditions_for(dependency)
462-
conditions = ignore_conditions.select { |ic| name_match?(ic["dependency-name"], dependency.name) }
470+
conditions = ignore_conditions.select { |ic| name_match?(ic.dependency_name, dependency.name) }
463471
return if conditions.empty?
464472

465473
Dependabot.logger.info("Ignored versions:")
466474
conditions.each do |ic|
467-
unless ic["version-requirement"].nil?
468-
Dependabot.logger.info(" #{ic['version-requirement']} - from #{ic['source']}")
469-
end
475+
Dependabot.logger.info(" #{ic.version_requirement} - from #{ic.source}") unless ic.version_requirement.nil?
470476

471-
ic["update-types"]&.each do |update_type|
472-
msg = " #{update_type} - from #{ic['source']}"
477+
ic.update_types&.each do |update_type|
478+
msg = " #{update_type} - from #{ic.source}"
473479
msg += " (doesn't apply to security update)" if security_updates_only?
474480
Dependabot.logger.info(msg)
475481
end
@@ -524,26 +530,26 @@ def collect_permitted_update_types(dependency)
524530
return [] if matching_rules.any? { |r| allow_rule_permits_all_types?(r) }
525531

526532
matching_rules
527-
.flat_map { |r| r.fetch("update-types", []) }
533+
.flat_map(&:update_types)
528534
.filter_map { |t| t.is_a?(String) ? t.downcase.strip : nil }
529535
.select { |t| Dependabot::Updater::UpdateTypeHelper::ALL_SEMVER_UPDATE_TYPES.include?(t) }
530536
.uniq
531537
end
532538

533-
sig { params(dependency: Dependabot::Dependency).returns(T::Array[T::Hash[String, T.untyped]]) }
539+
sig { params(dependency: Dependabot::Dependency).returns(T::Array[AllowedUpdate]) }
534540
def matching_allow_rules(dependency)
535541
allowed_updates.select do |update|
536542
allow_rule_matches_dependency?(update, dependency)
537543
end
538544
end
539545

540-
sig { params(update: T::Hash[String, T.untyped], dependency: Dependabot::Dependency).returns(T::Boolean) }
546+
sig { params(update: AllowedUpdate, dependency: Dependabot::Dependency).returns(T::Boolean) }
541547
def allow_rule_matches_dependency?(update, dependency)
542-
condition_name = update.fetch("dependency-name", nil)
548+
condition_name = update.dependency_name
543549
return false if condition_name && !name_match?(condition_name, dependency.name)
544550

545-
dep_type = update.fetch("dependency-type", nil)
546-
return true if dep_type.nil? || dep_type == "all"
551+
dep_type = update.dependency_type
552+
return true if dep_type == "all"
547553

548554
# Indirect deps don't match top-level type rules (matching allowed_update? behavior)
549555
return false if dependency.requirements.none? && TOP_LEVEL_DEPENDENCY_TYPES.include?(dep_type)
@@ -557,9 +563,9 @@ def allow_rule_matches_dependency?(update, dependency)
557563
end
558564
end
559565

560-
sig { params(rule: T::Hash[String, T.untyped]).returns(T::Boolean) }
566+
sig { params(rule: AllowedUpdate).returns(T::Boolean) }
561567
def allow_rule_permits_all_types?(rule)
562-
!rule.key?("update-types") || !rule["update-types"].is_a?(Array) || rule["update-types"].empty?
568+
rule.update_types.empty?
563569
end
564570

565571
sig { params(dependency: Dependabot::Dependency).returns(T.nilable(Dependabot::Version)) }
@@ -678,17 +684,16 @@ def clean_directories(source_details)
678684
def calculate_update_config
679685
update_config_ignore_conditions = ignore_conditions.map do |ic|
680686
Dependabot::Config::IgnoreCondition.new(
681-
dependency_name: T.let(ic["dependency-name"], String),
682-
versions: T.let([ic["version-requirement"]].compact, T::Array[String]),
683-
update_types: T.let(ic["update-types"], T.nilable(T::Array[String]))
687+
dependency_name: ic.dependency_name,
688+
versions: [ic.version_requirement].compact,
689+
update_types: ic.update_types
684690
)
685691
end
686692

687-
update_config = Dependabot::Config::UpdateConfig.new(
688-
ignore_conditions: T.let(update_config_ignore_conditions, T::Array[Dependabot::Config::IgnoreCondition]),
689-
exclude_paths: T.let(exclude_paths, T.nilable(T::Array[String]))
693+
Dependabot::Config::UpdateConfig.new(
694+
ignore_conditions: update_config_ignore_conditions,
695+
exclude_paths: exclude_paths
690696
)
691-
T.let(update_config, Dependabot::Config::UpdateConfig)
692697
end
693698
end
694699
end
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
# typed: strict
2+
# frozen_string_literal: true
3+
4+
require "sorbet-runtime"
5+
6+
module Dependabot
7+
class Job
8+
# Parsed representation of an allowed update rule from the job definition.
9+
#
10+
# Replaces the raw T::Hash[String, T.untyped] with a typed struct so
11+
# downstream code gets compile-time checked field access instead of
12+
# hash key lookups that return T.untyped.
13+
class AllowedUpdate < T::ImmutableStruct
14+
extend T::Sig
15+
16+
const :dependency_name, T.nilable(String)
17+
const :dependency_type, String, default: "all"
18+
const :update_type, String, default: "all"
19+
const :update_types, T::Array[String], default: []
20+
21+
sig { params(hash: T::Hash[String, T.untyped]).returns(AllowedUpdate) }
22+
def self.from_hash(hash)
23+
new(
24+
dependency_name: hash["dependency-name"],
25+
dependency_type: hash.fetch("dependency-type", "all"),
26+
update_type: hash.fetch("update-type", "all"),
27+
update_types: hash.fetch("update-types", []) || []
28+
)
29+
end
30+
31+
# Temporary bridge: returns the original hash format for code that
32+
# still expects hash access. Remove once all callers are migrated.
33+
sig { returns(T::Hash[String, T.untyped]) }
34+
def to_hash
35+
h = T.let({}, T::Hash[String, T.untyped])
36+
h["dependency-name"] = dependency_name if dependency_name
37+
h["dependency-type"] = dependency_type
38+
h["update-type"] = update_type
39+
h["update-types"] = update_types unless update_types.empty?
40+
h
41+
end
42+
end
43+
end
44+
end
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
# typed: strict
2+
# frozen_string_literal: true
3+
4+
require "sorbet-runtime"
5+
6+
module Dependabot
7+
class Job
8+
# Parsed representation of an ignore condition from the job definition.
9+
#
10+
# The raw JSON uses string keys like "dependency-name" and "version-requirement".
11+
# This struct parses those into typed fields at the boundary so the rest of
12+
# the codebase gets compile-time field access.
13+
class IgnoreCondition < T::ImmutableStruct
14+
extend T::Sig
15+
16+
const :dependency_name, String
17+
const :version_requirement, T.nilable(String)
18+
const :update_types, T.nilable(T::Array[String])
19+
const :source, T.nilable(String)
20+
21+
sig { params(hash: T::Hash[String, T.untyped]).returns(IgnoreCondition) }
22+
def self.from_hash(hash)
23+
new(
24+
dependency_name: hash.fetch("dependency-name"),
25+
version_requirement: hash["version-requirement"],
26+
update_types: hash["update-types"],
27+
source: hash["source"]
28+
)
29+
end
30+
end
31+
end
32+
end
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
# typed: strict
2+
# frozen_string_literal: true
3+
4+
require "sorbet-runtime"
5+
6+
module Dependabot
7+
class Job
8+
# Parsed representation of a security advisory from the job definition.
9+
class SecurityAdvisoryEntry < T::ImmutableStruct
10+
extend T::Sig
11+
12+
const :dependency_name, String
13+
const :affected_versions, T::Array[String], default: []
14+
const :patched_versions, T::Array[String], default: []
15+
const :unaffected_versions, T::Array[String], default: []
16+
17+
sig { params(hash: T::Hash[String, T.untyped]).returns(SecurityAdvisoryEntry) }
18+
def self.from_hash(hash)
19+
new(
20+
dependency_name: hash.fetch("dependency-name"),
21+
affected_versions: hash.fetch("affected-versions", []) || [],
22+
patched_versions: hash.fetch("patched-versions", []) || [],
23+
unaffected_versions: hash.fetch("unaffected-versions", []) || []
24+
)
25+
end
26+
end
27+
end
28+
end

0 commit comments

Comments
 (0)