Java: Add PropertyValueFinder class, which handle property inheritance

lib/dependabot/file_parsers/java/maven.rb

@@ -10,6 +10,7 @@ module FileParsers
     module Java
       class Maven < Dependabot::FileParsers::Base
         require "dependabot/file_parsers/base/dependency_set"
+        require_relative "maven/property_value_finder"
 
         DEPENDENCY_SELECTOR = "parent, dependencies > dependency,
                                plugins > plugin"
@@ -34,10 +35,10 @@ def pomfile_dependencies(pom)
             dependency_set <<
               Dependency.new(
                 name: name,
-                version: dependency_version(dependency_node),
+                version: dependency_version(pom, dependency_node),
                 package_manager: "maven",
                 requirements: [{
-                  requirement: dependency_requirement(dependency_node),
+                  requirement: dependency_requirement(pom, dependency_node),
                   file: pom.name,
                   groups: [],
                   source: nil
@@ -58,8 +59,8 @@ def dependency_name(dependency_node)
           ].join(":")
         end
 
-        def dependency_version(dependency_node)
-          requirement = dependency_requirement(dependency_node)
+        def dependency_version(pom, dependency_node)
+          requirement = dependency_requirement(pom, dependency_node)
           return nil unless requirement
 
           # If a range is specified then we can't tell the exact version
@@ -69,7 +70,7 @@ def dependency_version(dependency_node)
           requirement.gsub(/[\(\)\[\]]/, "").strip
         end
 
-        def dependency_requirement(dependency_node)
+        def dependency_requirement(pom, dependency_node)
           return unless dependency_node.at_css("version")
           version_content = dependency_node.at_css("version").content.strip
 
@@ -78,28 +79,22 @@ def dependency_requirement(dependency_node)
           prop_name = version_content.match(PROPERTY_REGEX).
                       named_captures.fetch("property")
 
-          property_value = value_for_property(prop_name)
+          property_value = value_for_property(prop_name, pom)
           version_content.gsub(PROPERTY_REGEX, property_value)
         end
 
-        def value_for_property(property_name)
-          pomfiles.each do |pom|
-            doc = Nokogiri::XML(pom.content)
-            doc.remove_namespaces!
-
-            value =
-              if property_name.start_with?("project.")
-                path = "//project/#{property_name.gsub(/^project\./, '')}"
-                doc.at_xpath(path)&.content&.strip ||
-                  doc.at_xpath("//properties/#{property_name}")&.content&.strip
-              else
-                doc.at_xpath("//properties/#{property_name}")&.content&.strip
-              end
-
-            return value if value
-          end
+        def value_for_property(property_name, pom)
+          value = property_value_finder.property_value(
+            property_name: property_name,
+            callsite_pom: pom
+          )
+
+          raise "Property not found: #{prop_name}" unless value
+          value
+        end
 
-          raise "Property not found: #{prop_name}"
+        def property_value_finder
+          PropertyValueFinder.new(dependency_files: dependency_files)
         end
 
         def pomfiles
@@ -108,7 +103,7 @@ def pomfiles
         end
 
         def internal_dependency_names
-          @internal_dependency_names =
+          @internal_dependency_names ||=
             pomfiles.map do |pom|
               doc = Nokogiri::XML(pom.content)
               group_id    = doc.at_css("project > groupId") ||

lib/dependabot/file_parsers/java/maven/property_value_finder.rb

@@ -0,0 +1,98 @@
+# frozen_string_literal: true
+
+require "nokogiri"
+
+require "dependabot/file_parsers/java/maven"
+
+# For documentation, see the "Available Variables" section of
+# http://maven.apache.org/guides/introduction/introduction-to-the-pom.html
+module Dependabot
+  module FileParsers
+    module Java
+      class Maven
+        class PropertyValueFinder
+          def initialize(dependency_files:)
+            @dependency_files = dependency_files
+          end
+
+          def property_value(property_name:, callsite_pom:)
+            pom = dependency_files.find { |f| f.name == callsite_pom.name }
+
+            doc = Nokogiri::XML(pom.content)
+            doc.remove_namespaces!
+
+            # Loop through the paths that would satisfy this property name,
+            # looking for one that exists in this POM
+            temp_name = sanitize_property_name(property_name)
+            property_value =
+              loop do
+                node =
+                  doc.at_xpath("//#{temp_name}") ||
+                  doc.at_xpath("//properties/#{temp_name}")
+                break node.content if node
+                break unless temp_name.include?(".")
+                temp_name = temp_name.sub(".", "/")
+              end
+
+            # If we found a property, return it
+            return property_value if property_value
+
+            # Otherwise, look for a value in this pom's parent
+            return unless (parent = parent_pom(pom))
+            property_value(
+              property_name: property_name,
+              callsite_pom: parent
+            )
+          end
+
+          private
+
+          attr_reader :dependency_files
+
+          def pomfiles
+            @pomfiles ||=
+              dependency_files.select { |f| f.name.end_with?("pom.xml") }
+          end
+
+          def internal_dependency_poms
+            return @internal_dependency_poms if @internal_dependency_poms
+
+            @internal_dependency_poms = {}
+            pomfiles.each do |pom|
+              doc = Nokogiri::XML(pom.content)
+              group_id    = doc.at_css("project > groupId") ||
+                            doc.at_css("project > parent > groupId")
+              artifact_id = doc.at_css("project > artifactId")
+
+              next unless group_id && artifact_id
+
+              dependency_name = [
+                group_id.content.strip,
+                artifact_id.content.strip
+              ].join(":")
+
+              @internal_dependency_poms[dependency_name] = pom
+            end
+
+            @internal_dependency_poms
+          end
+
+          def sanitize_property_name(property_name)
+            property_name.sub(/^pom\./, "").sub(/^project\./, "")
+          end
+
+          def parent_pom(pom)
+            doc = Nokogiri::XML(pom.content)
+            doc.remove_namespaces!
+            group_id = doc.at_xpath("//parent/groupId")
+            artifact_id = doc.at_xpath("//parent/artifactId")
+
+            return unless group_id && artifact_id
+            name = [group_id.content.strip, artifact_id.content.strip].join(":")
+            internal_dependency_poms[name]
+          end
+        end
+      end
+    end
+  end
+end

spec/dependabot/file_parsers/java/maven/property_value_finder_spec.rb

@@ -0,0 +1,61 @@
+# frozen_string_literal: true
+
+require "spec_helper"
+require "dependabot/dependency_file"
+require "dependabot/file_parsers/java/maven/property_value_finder"
+
+RSpec.describe Dependabot::FileParsers::Java::Maven::PropertyValueFinder do
+  let(:finder) { described_class.new(dependency_files: dependency_files) }
+
+  let(:dependency_files) { [base_pom] }
+  let(:base_pom) do
+    Dependabot::DependencyFile.new(
+      name: "pom.xml",
+      content: fixture("java", "poms", base_pom_fixture_name)
+    )
+  end
+  let(:base_pom_fixture_name) { "property_pom.xml" }
+
+  describe "#property_value" do
+    subject(:property_value) do
+      finder.property_value(
+        property_name: property_name,
+        callsite_pom: callsite_pom
+      )
+    end
+
+    context "when the property is declared in the calling pom" do
+      let(:base_pom_fixture_name) { "property_pom.xml" }
+      let(:property_name) { "springframework.version" }
+      let(:callsite_pom) { base_pom }
+      it { is_expected.to eq("4.3.12.RELEASE") }
+
+      context "and the property is an attribute on the project" do
+        let(:base_pom_fixture_name) { "project_version_pom.xml" }
+        let(:property_name) { "project.version" }
+        it { is_expected.to eq("0.0.2-RELEASE") }
+      end
+    end
+
+    context "when the property is declared in a parent pom" do
+      let(:dependency_files) { [base_pom, child_pom, grandchild_pom] }
+      let(:child_pom) do
+        Dependabot::DependencyFile.new(
+          name: "legacy/pom.xml",
+          content: fixture("java", "poms", "legacy_pom.xml")
+        )
+      end
+      let(:grandchild_pom) do
+        Dependabot::DependencyFile.new(
+          name: "legacy/some-spring-project/pom.xml",
+          content: fixture("java", "poms", "some_spring_project_pom.xml")
+        )
+      end
+
+      let(:base_pom_fixture_name) { "multimodule_pom.xml" }
+      let(:property_name) { "spring.version" }
+      let(:callsite_pom) { grandchild_pom }
+      it { is_expected.to eq("2.5.6") }
+    end
+  end
+end

spec/dependabot/file_parsers/java/maven_spec.rb

@@ -437,11 +437,11 @@
           expect(dependency).to be_a(Dependabot::Dependency)
           expect(dependency.name).
             to eq("com.google.guava:guava")
-          expect(dependency.version).to eq("24.0-jre")
+          expect(dependency.version).to eq("23.0-jre")
           expect(dependency.requirements).to eq(
             [
               {
-                requirement: "24.0-jre",
+                requirement: "23.0-jre",
                 file: "pom.xml",
                 groups: [],
                 source: nil

spec/fixtures/java/poms/multimodule_pom.xml

@@ -18,7 +18,7 @@
     <description>A sample Maven multi-module project</description>
 
     <properties>
-        <guava.version>24.0-jre</guava.version>
+        <guava.version>23.0-jre</guava.version>
         <spring.version>2.5.6</spring.version>
         <java.version>1.8</java.version>
         <maven.compiler.source>${java.version}</maven.compiler.source>