From a3c9b6bbf38f6dd241c3f8519c9cdb33c7945db8 Mon Sep 17 00:00:00 2001
From: Ewoud Kohl van Wijngaarden <ewoud@kohlvanwijngaarden.nl>
Date: Sun, 9 Jun 2024 20:06:09 +0200
Subject: [PATCH 1/2] Use max_by to determine the maximum value

This gives the same result, but it's much easier to read what's going
on.
---
 lib/rspec-puppet-facts.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/rspec-puppet-facts.rb b/lib/rspec-puppet-facts.rb
index 6577fb6..5f5a4d2 100644
--- a/lib/rspec-puppet-facts.rb
+++ b/lib/rspec-puppet-facts.rb
@@ -412,7 +412,7 @@ def self.facter_version_for_puppet_version(puppet_version)
       return Facter.version
     end
 
-    applicable_versions.sort { |a, b| b.first <=> a.first }.first.last
+    applicable_versions.max_by { |p, _| p }.last
   rescue JSON::ParserError
     warning "#{json_path} contains invalid JSON, defaulting to Facter #{Facter.version}"
     Facter.version

From c69be0a0754db09a966d62b2c81bbd27fff4f118 Mon Sep 17 00:00:00 2001
From: Ewoud Kohl van Wijngaarden <ewoud@kohlvanwijngaarden.nl>
Date: Sun, 9 Jun 2024 20:15:45 +0200
Subject: [PATCH 2/2] Rewrite facter_version_for_puppet_version for a better
 style

This rewrites the code to be easier to follow.

First of all, JSON is parsed and all error handling is local. Using a
self closing block avoids having to close a file descriptor and
exception handling near the actual code helps understanding.

Then it uses filter_map that Ruby 2.7 introduced to avoid calling
.map.compact. It also moves the selection of applicable versions into
the mapping.
---
 lib/rspec-puppet-facts.rb | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/lib/rspec-puppet-facts.rb b/lib/rspec-puppet-facts.rb
index 5f5a4d2..7af7ad3 100644
--- a/lib/rspec-puppet-facts.rb
+++ b/lib/rspec-puppet-facts.rb
@@ -394,30 +394,34 @@ def self.facter_version_for_puppet_version(puppet_version)
       return Facter.version
     end
 
-    fd = File.open(json_path, 'rb:UTF-8')
-    data = JSON.parse(fd.read)
+    data = File.open(json_path, 'rb:UTF-8') do |fd|
+      JSON.parse(fd.read)
+    rescue JSON::ParserError
+      warning "#{json_path} contains invalid JSON, defaulting to Facter #{Facter.version}"
+      return Facter.version
+    end
+
+    puppet_gem_version = Gem::Version.new(puppet_version)
 
-    version_map = data.map { |_, versions|
+    applicable_versions = data.filter_map do |_, versions|
       if versions['puppet'].nil? || versions['facter'].nil?
         nil
       else
-        [Gem::Version.new(versions['puppet']), versions['facter']]
+        puppet_version = Gem::Version.new(versions['puppet'])
+        if puppet_gem_version >= puppet_version
+          [puppet_version, versions['facter']]
+        else
+          nil
+        end
       end
-    }.compact
+    end
 
-    puppet_gem_version = Gem::Version.new(puppet_version)
-    applicable_versions = version_map.select { |p, _| puppet_gem_version >= p }
     if applicable_versions.empty?
       warning "Unable to find Puppet #{puppet_version} in #{json_path}, defaulting to Facter #{Facter.version}"
       return Facter.version
     end
 
     applicable_versions.max_by { |p, _| p }.last
-  rescue JSON::ParserError
-    warning "#{json_path} contains invalid JSON, defaulting to Facter #{Facter.version}"
-    Facter.version
-  ensure
-    fd.close if fd
   end
 end