Skip to content

Commit 8c7ebed

Browse files
fix(npm_and_yarn): handle engines OR constraints and split caret-expanded bounds (#15144)
* fix(npm_and_yarn): handle engines OR constraints and split caret-expanded bounds * chore: trigger PR CI rerun * fix(npm_and_yarn): address copilot review feedback * fix(npm_and_yarn): satisfy sorbet requirement return type * fix(npm_and_yarn): treat wildcard engine branches as no requirement * style(npm_and_yarn): use modifier if for empty constraint groups * test(npm_and_yarn): add explicit comparator OR engines regression * Log selected node version for engines handling
1 parent 23048bb commit 8c7ebed

4 files changed

Lines changed: 221 additions & 11 deletions

File tree

npm_and_yarn/lib/dependabot/npm_and_yarn/helpers.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,9 @@ def self.node_version
334334

335335
# Validate the output format (e.g., "v20.18.1" or "20.18.1")
336336
if version.match?(/^v?\d+(\.\d+){2}$/)
337-
version.strip.delete_prefix("v") # Remove the "v" prefix if present
337+
parsed_version = version.strip.delete_prefix("v") # Remove the "v" prefix if present
338+
Dependabot.logger.info("Using node version: #{parsed_version}")
339+
parsed_version
338340
end
339341
rescue StandardError => e
340342
Dependabot.logger.error("Error retrieving Node.js version: #{e.message}")

npm_and_yarn/lib/dependabot/npm_and_yarn/package_manager.rb

Lines changed: 83 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -193,28 +193,101 @@ def language_requirement
193193
def find_engine_constraints_as_requirement(name)
194194
Dependabot.logger.info("Processing engine constraints for #{name}")
195195

196-
return nil unless @engines.is_a?(Hash) && @engines[name]
197-
198-
raw_constraint = @engines[name].to_s.strip
196+
raw_constraint = raw_engine_constraint(name)
199197
return nil if raw_constraint.empty?
200198

201-
constraints = ConstraintHelper.extract_ruby_constraints(raw_constraint)
202-
# When constraints are invalid we return constraints array nil
203-
if constraints.nil?
199+
constraint_groups = parse_constraint_groups(raw_constraint)
200+
if constraint_groups.nil?
204201
Dependabot.logger.warn(
205202
"Unrecognized constraint format for #{name}: #{raw_constraint}"
206203
)
204+
return nil
207205
end
208206

209-
if constraints && !constraints.empty?
210-
Dependabot.logger.info("Parsed constraints for #{name}: #{constraints.join(', ')}")
211-
Requirement.new(constraints)
212-
end
207+
# A wildcard/latest branch translates to no constraints, which means
208+
# there is effectively no engine requirement.
209+
return nil if constraint_groups.any?(&:empty?)
210+
211+
constraint_groups = constraint_groups.reject(&:empty?)
212+
213+
return nil if constraint_groups.empty?
214+
215+
parsed_constraints = constraint_groups.map { |group| group.join(" ") }.join(" || ")
216+
Dependabot.logger.info("Parsed constraints for #{name}: #{parsed_constraints}")
217+
218+
requirement_for_group(constraint_groups, name)
213219
rescue StandardError => e
214220
Dependabot.logger.error("Error processing constraints for #{name}: #{e.message}")
215221
nil
216222
end
217223

224+
sig { params(name: String).returns(String) }
225+
def raw_engine_constraint(name)
226+
return "" unless @engines.is_a?(Hash) && @engines[name]
227+
228+
@engines[name].to_s.strip
229+
end
230+
231+
sig { params(raw_constraint: String).returns(T.nilable(T::Array[T::Array[String]])) }
232+
def parse_constraint_groups(raw_constraint)
233+
raw_constraint.split("||").map(&:strip).reject(&:empty?).map do |constraint_group|
234+
constraints = ConstraintHelper.extract_ruby_constraints(constraint_group)
235+
return nil if constraints.nil?
236+
237+
expanded_constraints(constraints)
238+
end
239+
end
240+
241+
sig { params(constraints: T::Array[String]).returns(T::Array[String]) }
242+
def expanded_constraints(constraints)
243+
constraints.flat_map do |constraint|
244+
parts = constraint.strip.split(/\s+/)
245+
if parts.length > 1 && parts.all? { |part| part.match?(ConstraintHelper::VALID_CONSTRAINT_REGEX) }
246+
parts
247+
else
248+
[constraint]
249+
end
250+
end
251+
end
252+
253+
sig do
254+
params(
255+
constraint_groups: T::Array[T::Array[String]],
256+
name: String
257+
).returns(Requirement)
258+
end
259+
def requirement_for_group(constraint_groups, name)
260+
requirements = constraint_groups.map { |constraints| Requirement.new(constraints) }
261+
fallback_requirement = T.must(requirements.first)
262+
263+
current_version = current_engine_version(name)
264+
return fallback_requirement unless current_version
265+
266+
matching_requirement = requirements.find { |requirement| requirement.satisfied_by?(current_version) }
267+
matching_requirement || fallback_requirement
268+
end
269+
270+
sig { params(name: String).returns(T.nilable(Dependabot::Version)) }
271+
def current_engine_version(name)
272+
raw_version = if name == Language::NAME
273+
Helpers.node_version
274+
else
275+
@installed_versions[name]
276+
end
277+
278+
return nil if raw_version.to_s.strip.empty?
279+
280+
Version.new(raw_version)
281+
rescue StandardError
282+
nil
283+
end
284+
285+
private :raw_engine_constraint,
286+
:parse_constraint_groups,
287+
:expanded_constraints,
288+
:requirement_for_group,
289+
:current_engine_version
290+
218291
# rubocop:disable Metrics/CyclomaticComplexity
219292
# rubocop:disable Metrics/AbcSize
220293
# rubocop:disable Metrics/PerceivedComplexity

npm_and_yarn/spec/dependabot/npm_and_yarn/helpers_spec.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -610,8 +610,10 @@
610610
"node -v",
611611
fingerprint: "node -v"
612612
).and_return("v16.13.1")
613+
allow(Dependabot.logger).to receive(:info)
613614

614615
expect(described_class.node_version).to eq("16.13.1")
616+
expect(Dependabot.logger).to have_received(:info).with("Using node version: 16.13.1")
615617
end
616618

617619
it "raises an error if the Node.js version command fails" do

npm_and_yarn/spec/dependabot/npm_and_yarn/package_manager_helper_spec.rb

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,16 @@
161161
end
162162
end
163163

164+
context "with OR constraints where the lower branch is on the right" do
165+
let(:lockfiles) { {} }
166+
let(:package_json) { { "engines" => { "pnpm" => ">=10 || >=7 <9" } } }
167+
168+
it "selects the highest matching supported pnpm version" do
169+
expect(helper.package_manager).to be_a(Dependabot::NpmAndYarn::PNPMPackageManager)
170+
expect(helper.package_manager.detected_version).to eq("10")
171+
end
172+
end
173+
164174
context "when neither lockfile, packageManager, nor engines field exists" do
165175
let(:lockfiles) { {} }
166176
let(:package_json) { {} }
@@ -522,6 +532,129 @@
522532
end
523533
end
524534

535+
context "when the engines field contains a caret OR constraint" do
536+
let(:package_json) do
537+
{
538+
"name" => "example",
539+
"version" => "1.0.0",
540+
"engines" => {
541+
"node" => "^22 || >=24"
542+
}
543+
}
544+
end
545+
546+
it "expands caret constraints into separate comparators" do
547+
allow(Dependabot::NpmAndYarn::Helpers).to receive(:node_version).and_return("22.6.0")
548+
549+
requirement = helper.find_engine_constraints_as_requirement("node")
550+
551+
expect(requirement).to be_a(Dependabot::NpmAndYarn::Requirement)
552+
expect(requirement.constraints).to eq([">= 22.0.0", "< 23.0.0"])
553+
end
554+
555+
it "selects the matching OR branch for current node version" do
556+
allow(Dependabot::NpmAndYarn::Helpers).to receive(:node_version).and_return("24.2.0")
557+
558+
requirement = helper.find_engine_constraints_as_requirement("node")
559+
560+
expect(requirement).to be_a(Dependabot::NpmAndYarn::Requirement)
561+
expect(requirement.constraints).to eq([">= 24"])
562+
end
563+
564+
it "falls back to the first OR branch when current node version is unavailable" do
565+
allow(Dependabot::NpmAndYarn::Helpers).to receive(:node_version).and_return(nil)
566+
567+
requirement = helper.find_engine_constraints_as_requirement("node")
568+
569+
expect(requirement).to be_a(Dependabot::NpmAndYarn::Requirement)
570+
expect(requirement.constraints).to eq([">= 22.0.0", "< 23.0.0"])
571+
end
572+
573+
context "when one OR branch is invalid" do
574+
let(:package_json) do
575+
{
576+
"name" => "example",
577+
"version" => "1.0.0",
578+
"engines" => {
579+
"node" => "^22 || invalid"
580+
}
581+
}
582+
end
583+
584+
it "logs a warning and returns nil" do
585+
expect(Dependabot.logger).to receive(:warn).with(/Unrecognized constraint format for node: \^22 \|\| invalid/)
586+
587+
requirement = helper.find_engine_constraints_as_requirement("node")
588+
589+
expect(requirement).to be_nil
590+
end
591+
end
592+
593+
context "when one OR branch is a wildcard" do
594+
let(:package_json) do
595+
{
596+
"name" => "example",
597+
"version" => "1.0.0",
598+
"engines" => {
599+
"node" => "* || >=24"
600+
}
601+
}
602+
end
603+
604+
it "returns nil without logging an unrecognized warning" do
605+
allow(Dependabot.logger).to receive(:warn)
606+
607+
requirement = helper.find_engine_constraints_as_requirement("node")
608+
609+
expect(requirement).to be_nil
610+
expect(Dependabot.logger).not_to have_received(:warn)
611+
.with(/Unrecognized constraint format for node/)
612+
end
613+
end
614+
end
615+
616+
context "when the engines field contains an explicit comparator OR constraint" do
617+
let(:package_json) do
618+
{
619+
"name" => "example",
620+
"version" => "1.0.0",
621+
"engines" => {
622+
"node" => ">=22.0.0 <23.0.0 || >=24"
623+
}
624+
}
625+
end
626+
627+
it "splits the first OR branch into separate comparators" do
628+
allow(Dependabot::NpmAndYarn::Helpers).to receive(:node_version).and_return("22.6.0")
629+
630+
requirement = helper.find_engine_constraints_as_requirement("node")
631+
632+
expect(requirement).to be_a(Dependabot::NpmAndYarn::Requirement)
633+
expect(requirement.constraints).to eq([">= 22.0.0", "< 23.0.0"])
634+
end
635+
636+
context "when the lower branch appears on the right" do
637+
let(:package_json) do
638+
{
639+
"name" => "example",
640+
"version" => "1.0.0",
641+
"engines" => {
642+
"node" => ">=24 || >=22.0.0 <23.0.0"
643+
}
644+
}
645+
end
646+
647+
it "selects the higher matching branch" do
648+
allow(Dependabot::NpmAndYarn::Helpers).to receive(:node_version).and_return("24.2.0")
649+
650+
requirement = helper.find_engine_constraints_as_requirement("node")
651+
652+
expect(requirement).to be_a(Dependabot::NpmAndYarn::Requirement)
653+
expect(requirement.constraints).to eq([">= 24"])
654+
end
655+
end
656+
end
657+
525658
context "when the engines field does not contain the specified package manager" do
526659
it "returns nil" do
527660
requirement = helper.find_engine_constraints_as_requirement("nonexistent")

0 commit comments

Comments
 (0)