review and ignore dependencyies at versions

- "*"
- a gem version requirement pattern
- an exact string version match

docs/configuration/ignoring_dependencies.md

@@ -17,3 +17,16 @@ ignored:
   go:
     - github.com/me/my-repo/**/*
 ```
+
+## Ignoring dependencies at specific versions
+
+Ignore a dependency at specific versions by appending `@<version>` to the end of the dependency's name in an `ignore` list.  If a dependency is configured to be ignored at a specific version, licensed will not ignore non-matching versions of the dependency.
+
+The version value can be one of:
+
+1. `"*"` - match any version value
+1. any version string, or version range string, that can be parsed by `Gem::Requirement`
+   - a semantic version - `dependency@1.2.3`
+   - a gem requirement range - `dependency@~> 1.0.0` or `dependency@< 3.0`
+   - see the [Rubygems version guides](https://guides.rubygems.org/patterns/#pessimistic-version-constraint) for more details about specifying gem version requirements
+1. a value that can't be parsed by `Gem::Requirement`, which will only match dependencies with the same version string

docs/configuration/reviewing_dependencies.md

@@ -16,3 +16,16 @@ reviewed:
   bundler:
     - gem-using-unallowed-license
 ```
+
+## Reviewing dependencies at specific versions
+
+Review a dependency at specific versions by appending `@<version>` to the end of the dependency's name in an `reviewed` list.  If a dependency is configured to be reviewed at a specific version, licensed will not recognize non-matching versions of the dependency as being manually reviewed and accepted.
+
+The version value can be one of:
+
+1. `"*"` - match any version value
+1. any version string, or version range string, that can be parsed by `Gem::Requirement`
+   - a semantic version - `dependency@1.2.3`
+   - a gem requirement range - `dependency@~> 1.0.0` or `dependency@< 3.0`
+   - see the [Rubygems version guides](https://guides.rubygems.org/patterns/#pessimistic-version-constraint) for more details about specifying gem version requirements
+1. a value that can't be parsed by `Gem::Requirement`, which will only match dependencies with the same version string

lib/licensed/commands/status.rb

@@ -72,7 +72,7 @@ module Licensed
       # record's license(s) and the app's configuration
       def license_needs_review?(app, record)
         # review is not needed if the record is set as reviewed
-        return false if app.reviewed?(record, match_version: data_source == "configuration")
+        return false if app.reviewed?(record, require_version: data_source == "configuration")
 
         # review is not needed if the top level license is allowed
         return false if app.allowed?(record["license"])
@@ -99,7 +99,7 @@ module Licensed
         error = "dependency needs review"
 
         # look for an unversioned reviewed list match
-        if app.reviewed?(record, match_version: false)
+        if app.reviewed?(record, require_version: false)
           error += ", unversioned 'reviewed' match found: #{record["name"]}"
         end
 

lib/licensed/configuration.rb

@@ -10,6 +10,8 @@ module Licensed
 
     DEFAULT_CACHE_PATH = ".licenses".freeze
 
+    ANY_VERSION_REQUIREMENT = "*".freeze
+
     # Returns the root for a configuration in following order of precedence:
     # 1. explicitly configured "root" property
     # 2. a found git repository root
@@ -73,8 +75,8 @@ module Licensed
     end
 
     # Is the given dependency reviewed?
-    def reviewed?(dependency, match_version: false)
-      any_list_pattern_matched? self["reviewed"][dependency["type"]], dependency, match_version: match_version
+    def reviewed?(dependency, require_version: false)
+      any_list_pattern_matched? self["reviewed"][dependency["type"]], dependency, require_version: require_version
     end
 
     # Find all reviewed dependencies that match the provided dependency's name
@@ -83,8 +85,8 @@ module Licensed
     end
 
     # Is the given dependency ignored?
-    def ignored?(dependency)
-      any_list_pattern_matched? self["ignored"][dependency["type"]], dependency
+    def ignored?(dependency, require_version: false)
+      any_list_pattern_matched? self["ignored"][dependency["type"]], dependency, require_version: require_version
     end
 
     # Is the license of the dependency allowed?
@@ -93,8 +95,10 @@ module Licensed
     end
 
     # Ignore a dependency
-    def ignore(dependency)
-      (self["ignored"][dependency["type"]] ||= []) << dependency["name"]
+    def ignore(dependency, at_version: false)
+      id = dependency["name"]
+      id += "@#{dependency["version"]}" if at_version && dependency["version"]
+      (self["ignored"][dependency["type"]] ||= []) << id
     end
 
     # Set a dependency as reviewed
@@ -117,15 +121,38 @@ module Licensed
 
     private
 
-    def any_list_pattern_matched?(list, dependency, match_version: false)
+    def any_list_pattern_matched?(list, dependency, require_version: false)
       Array(list).any? do |pattern|
-        if match_version
-          at_version = "@#{dependency["version"]}"
-          pattern, pattern_version = pattern.rpartition(at_version).values_at(0, 1)
-          next false if pattern == "" || pattern_version == ""
+        # parse a name and version requirement value from the pattern
+        name, requirement = pattern.rpartition("@").values_at(0, 2).map(&:strip)
+
+        if name == ""
+          # if name == "", then the pattern doesn't contain a valid version value.
+          # treat the entire pattern as the dependency name with no version.
+          name = pattern
+          requirement = nil
+        elsif Gem::Requirement::PATTERN.match?(requirement)
+          # check if the version requirement is a valid Gem::Requirement
+          # for range matching
+          requirement = Gem::Requirement.new(requirement)
         end
 
-        File.fnmatch?(pattern, dependency["name"], File::FNM_PATHNAME | File::FNM_CASEFOLD)
+        # the pattern's name must match the dependency's name
+        next false unless File.fnmatch?(name, dependency["name"], File::FNM_PATHNAME | File::FNM_CASEFOLD)
+
+        # if there is no version requirement configured or if the dependency doesn't have a version
+        # specified, return a value based on whether a version match is required
+        next !require_version if requirement.nil? || dependency["version"].to_s.empty?
+
+        case requirement
+        when String
+          # string match the requirement against "*" or the dependency's version
+          [ANY_VERSION_REQUIREMENT, dependency["version"]].any? { |r| requirement == r }
+        when Gem::Requirement
+          # if the version was parsed as a gem requirement, check whether the version requirement
+          # matches the dependency's version
+          Gem::Version.correct?(dependency["version"]) && requirement.satisfied_by?(Gem::Version.new(dependency["version"]))
+        end
       end
     end
 

lib/licensed/sources/source.rb

@@ -81,7 +81,7 @@ module Licensed
 
       # Returns whether a dependency is ignored in the configuration.
       def ignored?(dependency)
-        config.ignored?("type" => self.class.type, "name" => dependency.name)
+        config.ignored?({ "type" => self.class.type, "name" => dependency.name })
       end
 
       private

test/commands/cache_test.rb

@@ -61,7 +61,7 @@ describe Licensed::Commands::Cache do
     config.apps.each do |app|
       FileUtils.mkdir_p app.cache_path.join("test")
       File.write app.cache_path.join("test/dependency.#{Licensed::DependencyRecord::EXTENSION}"), ""
-      app.ignore "type" => "test", "name" => "dependency"
+      app.ignore({ "type" => "test", "name" => "dependency" })
     end
 
     run_command
@@ -206,7 +206,7 @@ describe Licensed::Commands::Cache do
     config.apps.each do |app|
       FileUtils.mkdir_p app.cache_path.join("test")
       File.write app.cache_path.join("test/dependency.#{Licensed::DependencyRecord::EXTENSION}"), ""
-      app.ignore "type" => "test", "name" => "dependency"
+      app.ignore({ "type" => "test", "name" => "dependency" })
     end
 
     run_command

test/commands/status_test.rb

@@ -82,7 +82,7 @@ describe Licensed::Commands::Status do
       config.apps.each do |app|
         app.sources.each do |source|
           assert dependency_errors(app, source).any?
-          app.ignore "type" => source.class.type, "name" => "dependency"
+          app.ignore({ "type" => source.class.type, "name" => "dependency" })
         end
       end
 
@@ -100,7 +100,7 @@ describe Licensed::Commands::Status do
       config.apps.each do |app|
         app.sources.each do |source|
           assert dependency_errors(app, source).any?
-          app.ignore "type" => source.class.type, "name" => "dependency"
+          app.ignore({ "type" => source.class.type, "name" => "dependency" })
         end
       end
 
@@ -189,7 +189,7 @@ describe Licensed::Commands::Status do
     it "does not warn if cached license data missing for ignored gem" do
       config.apps.each do |app|
         FileUtils.rm app.cache_path.join("test/dependency.#{Licensed::DependencyRecord::EXTENSION}")
-        app.ignore "type" => "test", "name" => "dependency"
+        app.ignore({ "type" => "test", "name" => "dependency" })
       end
 
       run_command
@@ -214,7 +214,7 @@ describe Licensed::Commands::Status do
       count = reporter.report.all_reports.size
 
       config.apps.each do |app|
-        app.ignore "type" => "test", "name" => "dependency"
+        app.ignore({ "type" => "test", "name" => "dependency" })
       end
 
       run_command
@@ -423,7 +423,7 @@ describe Licensed::Commands::Status do
       config.apps.each do |app|
         app.sources.each do |source|
           assert dependency_errors(app, source).any?
-          app.ignore "type" => source.class.type, "name" => "dependency"
+          app.ignore({ "type" => source.class.type, "name" => "dependency" })
         end
       end
 
@@ -503,7 +503,7 @@ describe Licensed::Commands::Status do
       count = reporter.report.all_reports.size
 
       config.apps.each do |app|
-        app.ignore "type" => "test", "name" => "dependency"
+        app.ignore({ "type" => "test", "name" => "dependency" })
       end
 
       run_command

test/configuration_test.rb

@@ -273,149 +273,301 @@ describe Licensed::AppConfiguration do
   end
 
   describe "ignore" do
-    let(:package) { { "type" => "go", "name" => "github.com/github/licensed/package" } }
+    let(:package) { { "type" => "go", "name" => "github.com/github/licensed/package", "version" => "1.0.0"  } }
 
     it "marks the dependency as ignored" do
+      assert_nil config.dig("ignored", package["type"])
+      config.ignore package
+      assert_equal [package["name"]], config.dig("ignored", package["type"])
+    end
+
+    it "marks the dependency ignored at the specific version" do
+      assert_nil config.dig("ignored", package["type"])
+      config.ignore package, at_version: true
+      assert_equal ["#{package["name"]}@#{package["version"]}"], config.dig("ignored", package["type"])
+    end
+  end
+
+  describe "ignored?" do
+    let(:package) { { "type" => "go", "name" => "github.com/github/licensed/package", "version" => "1.0.0"  } }
+
+    it "matches exact name matches" do
       refute config.ignored?(package)
       config.ignore package
       assert config.ignored?(package)
     end
 
-    describe "with glob patterns" do
-      it "does not match trailing ** to multiple path segments" do
-        refute config.ignored?(package)
-        config.ignore package.merge("name" => "github.com/github/**")
-        refute config.ignored?(package)
+    it "does not match trailing ** to multiple path segments" do
+      refute config.ignored?(package)
+      config.ignore package.merge("name" => "github.com/github/**")
+      refute config.ignored?(package)
+    end
+
+    it "matches internal ** to multiple path segments" do
+      refute config.ignored?(package)
+      config.ignore package.merge("name" => "github.com/**/package")
+      assert config.ignored?(package)
+    end
+
+    it "matches trailing * to single path segment" do
+      refute config.ignored?(package)
+      config.ignore package.merge("name" => "github.com/github/licensed/*")
+      assert config.ignored?(package)
+    end
+
+    it "maches internal * to single path segment" do
+      refute config.ignored?(package)
+      config.ignore package.merge("name" => "github.com/*/licensed/package")
+      assert config.ignored?(package)
+    end
+
+    it "matches multiple globstars in a pattern" do
+      refute config.ignored?(package)
+      config.ignore package.merge("name" => "**/licensed/*")
+      assert config.ignored?(package)
+    end
+
+    it "does not match * to multiple path segments" do
+      refute config.ignored?(package)
+      config.ignore package.merge("name" => "github.com/github/*")
+      refute config.ignored?(package)
+    end
+
+    it "is case insensitive" do
+      refute config.ignored?(package)
+      config.ignore package.merge("name" => "GITHUB.com/github/**")
+      refute config.ignored?(package)
+    end
+
+    describe "with required_version" do
+      it "matches a dependency ignored at the same version" do
+        config.ignore package, at_version: true
+        assert config.ignored?(package, require_version: true)
       end
 
-      it "matches internal ** to multiple path segments" do
-        refute config.ignored?(package)
-        config.ignore package.merge("name" => "github.com/**/package")
+      it "does not match a dependency not ignored at a version" do
+        config.ignore package
+        refute config.ignored?(package, require_version: true)
+      end
+
+      it "does not match a dependency ignored at a different version" do
+        config.ignore package.merge("version" => "1.0.1"), at_version: true
+        refute config.ignored?(package, require_version: true)
+      end
+
+      it "matches a dependency ignored at *" do
+        config.ignore package.merge("version" => "*"), at_version: true
+        assert config.ignored?(package, require_version: true)
+      end
+
+      it "matches a dependency ignored at a requirement pattern" do
+        config.ignore package.merge("version" => ">= 1.0.0"), at_version: true
+        assert config.ignored?(package, require_version: true)
+      end
+
+      it "does not match a dependency without a version" do
+        config.ignore package, at_version: true
+        package.delete("version")
+        refute config.ignored?(package, require_version: true)
+      end
+
+      it "matches a dependency ignored at a matching non-gem requirement value" do
+        package["version"] = "abcdef"
+        config.ignore package, at_version: true
+        assert config.ignored?(package, require_version: true)
+      end
+    end
+
+    describe "without required_version" do
+      it "matches a dependency ignored at the same version" do
+        config.ignore package, at_version: true
         assert config.ignored?(package)
       end
 
-      it "matches trailing * to single path segment" do
-        refute config.ignored?(package)
-        config.ignore package.merge("name" => "github.com/github/licensed/*")
+      it "matches a dependency not ignored at a version" do
+        config.ignore package
         assert config.ignored?(package)
       end
 
-      it "maches internal * to single path segment" do
+      it "does not match a dependency ignored at a different version" do
+        config.ignore package.merge("version" => "1.0.1"), at_version: true
         refute config.ignored?(package)
-        config.ignore package.merge("name" => "github.com/*/licensed/package")
+      end
+
+      it "matches a dependency ignored at *" do
+        config.ignore package.merge("version" => "*"), at_version: true
+        assert config.ignored?(package.merge("version" => "1.0.1"))
+      end
+
+      it "matches a dependency ignored at a requirement pattern" do
+        config.ignore package.merge("version" => ">= 1.0.0"), at_version: true
         assert config.ignored?(package)
       end
 
-      it "matches multiple globstars in a pattern" do
-        refute config.ignored?(package)
-        config.ignore package.merge("name" => "**/licensed/*")
+      it "matches a dependency without a version" do
+        config.ignore package, at_version: true
+        package.delete("version")
         assert config.ignored?(package)
       end
-
-      it "does not match * to multiple path segments" do
-        refute config.ignored?(package)
-        config.ignore package.merge("name" => "github.com/github/*")
-        refute config.ignored?(package)
-      end
-
-      it "is case insensitive" do
-        refute config.ignored?(package)
-        config.ignore package.merge("name" => "GITHUB.com/github/**")
-        refute config.ignored?(package)
-      end
     end
   end
 
   describe "review" do
-    let(:package) { { "type" => "go", "name" => "github.com/github/licensed/package" } }
+    let(:package) { { "type" => "go", "name" => "github.com/github/licensed/package", "version" => "1.0.0" } }
 
     it "marks the dependency as reviewed" do
+      assert_nil config.dig("reviewed", package["type"])
+      config.review package
+      assert_equal [package["name"]], config.dig("reviewed", package["type"])
+    end
+
+    it "marks the dependency reviewed at the specific version" do
+      assert_nil config.dig("reviewed", package["type"])
+      config.review package, at_version: true
+      assert_equal ["#{package["name"]}@#{package["version"]}"], config.dig("reviewed", package["type"])
+    end
+  end
+
+  describe "reviewed?" do
+    let(:package) { { "type" => "go", "name" => "github.com/github/licensed/package", "version" => "1.0.0" } }
+
+    it "matches exact name matches" do
       refute config.reviewed?(package)
       config.review package
       assert config.reviewed?(package)
     end
 
-    describe "with glob patterns" do
-      it "does not match trailing ** to multiple path segments" do
-        refute config.reviewed?(package)
-        config.review package.merge("name" => "github.com/github/**")
-        refute config.reviewed?(package)
+    it "does not match trailing ** to multiple path segments" do
+      refute config.reviewed?(package)
+      config.review package.merge("name" => "github.com/github/**")
+      refute config.reviewed?(package)
+    end
+
+    it "matches internal ** to multiple path segments" do
+      refute config.reviewed?(package)
+      config.review package.merge("name" => "github.com/**/package")
+      assert config.reviewed?(package)
+    end
+
+    it "matches trailing * to single path segment" do
+      refute config.reviewed?(package)
+      config.review package.merge("name" => "github.com/github/licensed/*")
+      assert config.reviewed?(package)
+    end
+
+    it "maches internal * to single path segment" do
+      refute config.reviewed?(package)
+      config.review package.merge("name" => "github.com/*/licensed/package")
+      assert config.reviewed?(package)
+    end
+
+    it "matches multiple globstars in a pattern" do
+      refute config.reviewed?(package)
+      config.review package.merge("name" => "**/licensed/*")
+      assert config.reviewed?(package)
+    end
+
+    it "does not match * to multiple path segments" do
+      refute config.reviewed?(package)
+      config.review package.merge("name" => "github.com/github/*")
+      refute config.reviewed?(package)
+    end
+
+    it "is case insensitive" do
+      refute config.reviewed?(package)
+      config.review package.merge("name" => "GITHUB.com/github/**")
+      refute config.reviewed?(package)
+    end
+
+    describe "with required_version" do
+      it "matches a dependency reviewed at the same version" do
+        config.review package, at_version: true
+        assert config.reviewed?(package, require_version: true)
       end
 
-      it "matches internal ** to multiple path segments" do
-        refute config.reviewed?(package)
-        config.review package.merge("name" => "github.com/**/package")
-        assert config.reviewed?(package)
+      it "does not match a dependency not reviewed at a version" do
+        config.review package
+        refute config.reviewed?(package, require_version: true)
       end
 
-      it "matches trailing * to single path segment" do
-        refute config.reviewed?(package)
-        config.review package.merge("name" => "github.com/github/licensed/*")
-        assert config.reviewed?(package)
+      it "does not match a dependency reviewed at a different version" do
+        config.review package.merge("version" => "1.0.1"), at_version: true
+        refute config.reviewed?(package, require_version: true)
       end
 
-      it "maches internal * to single path segment" do
-        refute config.reviewed?(package)
-        config.review package.merge("name" => "github.com/*/licensed/package")
-        assert config.reviewed?(package)
+      it "matches a dependency reviewed at *" do
+        config.review package.merge("version" => "*"), at_version: true
+        assert config.reviewed?(package, require_version: true)
       end
 
-      it "matches multiple globstars in a pattern" do
-        refute config.reviewed?(package)
-        config.review package.merge("name" => "**/licensed/*")
-        assert config.reviewed?(package)
+      it "matches a dependency reviewed at a requirement pattern" do
+        config.review package.merge("version" => ">= 1.0.0"), at_version: true
+        assert config.reviewed?(package, require_version: true)
       end
 
-      it "does not match * to multiple path segments" do
-        refute config.reviewed?(package)
-        config.review package.merge("name" => "github.com/github/*")
-        refute config.reviewed?(package)
+      it "does not match a dependency without a version" do
+        config.review package, at_version: true
+        package.delete("version")
+        refute config.reviewed?(package, require_version: true)
       end
 
-      it "is case insensitive" do
-        refute config.reviewed?(package)
-        config.review package.merge("name" => "GITHUB.com/github/**")
-        refute config.reviewed?(package)
+      it "matches a dependency reviewed at a matching non-gem requirement value" do
+        package["version"] = "abcdef"
+        config.review package, at_version: true
+        assert config.reviewed?(package, require_version: true)
       end
     end
 
-    describe "with version" do
-      it "sets the dependency reviewed at the specific version" do
+    describe "without required_version" do
+      it "matches a dependency reviewed at the same version" do
+        config.review package, at_version: true
+        assert config.reviewed?(package)
+      end
+
+      it "matches a dependency not reviewed at a version" do
         config.review package
         assert config.reviewed?(package)
+      end
 
-        package["version"] = "1.2.3"
+      it "does not match a dependency reviewed at a different version" do
+        config.review package.merge("version" => "1.0.1"), at_version: true
+        refute config.reviewed?(package)
+      end
+
+      it "matches a dependency reviewed at *" do
+        config.review package.merge("version" => "*"), at_version: true
+        assert config.reviewed?(package.merge("version" => "1.0.1"))
+      end
+
+      it "matches a dependency reviewed at a requirement pattern" do
+        config.review package.merge("version" => ">= 1.0.0"), at_version: true
         assert config.reviewed?(package)
-        refute config.reviewed?(package, match_version: true)
+      end
 
+      it "matches a dependency without a version" do
         config.review package, at_version: true
-        refute config.reviewed?(package.merge("version" => "1.0.0"), match_version: true)
-        assert config.reviewed?(package, match_version: true)
-        assert_equal ["#{package["name"]}@#{package["version"]}"], config.reviewed_versions(package)
+        package.delete("version")
+        assert config.reviewed?(package)
       end
+    end
+  end
 
-      it "reviewing dependencies at version will not match dependencies without version" do
-        package = { "type" => "bundler", "name" => "licensed", "version" => "1.0.0" }
-        config.review(package, at_version: true)
-        refute config.reviewed?(package, match_version: false)
-      end
+  describe "reviewed_versions" do
+    it "returns an array of all matching reviewed dependency names at versions" do
+      packages = [
+        { "type" => "npm", "name" => "@github/package", "version" => "1.0.0" },
+        { "type" => "npm", "name" => "@github/package", "version" => "1.0.1" }
+      ]
 
-      it "matches glob patterns for specified versions" do
-        config.review({ "type" => "go", "name" => "github.com/github/**/*", "version" => "1.2.3" }, at_version: true)
-        assert_equal ["github.com/github/**/*@1.2.3"], config.reviewed_versions(package)
+      packages.each { |p| config.review(p, at_version: true) }
+      assert_equal packages.map { |p| "#{p["name"]}@#{p["version"]}" },
+        config.reviewed_versions({ "type" => "npm", "name" => "@github/package" })
+    end
 
-        refute config.reviewed?(package.merge("version" => "1.0.0"), match_version: true)
-        assert config.reviewed?(package.merge("version" => "1.2.3"), match_version: true)
-      end
-
-      it "does not treat leading @ symbols as version separators when listing versions" do
-        package = { "type" => "npm", "name" => "@github/package" }
-        config.review(package)
-        assert_empty config.reviewed_versions(package)
-
-        config.review(package.merge("version" => "1.2.3"), at_version: true)
-        assert_equal ["@github/package@1.2.3"], config.reviewed_versions(package)
-      end
+    it "does not return reviewed dependencies without version" do
+      package = { "type" => "npm", "name" => "@github/package", "version" => "1.0.0" }
+      config.review(package)
+      assert_empty config.reviewed_versions(package)
     end
   end