Merge pull request #467 from bkeepers/refactor

Refactor `Dotenv.load` and friends

.standard.yml

@@ -1,4 +1,4 @@
-ruby_version: 2.5
+ruby_version: 3.0
 
 ignore:
   - lib/dotenv/parser.rb:

lib/dotenv.rb

@@ -10,58 +10,48 @@ class << self
 
   module_function
 
-  def load(*filenames)
-    with(*filenames) do |f|
-      ignoring_nonexistent_files do
-        env = Environment.new(f, true)
-        instrument("dotenv.load", env: env) { env.apply }
-      end
+  # Loads environment variables from one or more `.env` files. See `#parse` for more details.
+  def load(*filenames, **kwargs)
+    parse(*filenames, **kwargs) do |env|
+      instrument("dotenv.load", env: env) { env.apply }
     end
   end
 
-  # same as `load`, but raises Errno::ENOENT if any files don't exist
+  # Same as `#load`, but raises Errno::ENOENT if any files don't exist
   def load!(*filenames)
-    with(*filenames) do |f|
-      env = Environment.new(f, true)
-      instrument("dotenv.load", env: env) { env.apply }
-    end
+    load(*filenames, ignore: false)
   end
 
-  # same as `load`, but will override existing values in `ENV`
+  # same as `#load`, but will override existing values in `ENV`
   def overload(*filenames)
-    with(*filenames.reverse) do |f|
-      ignoring_nonexistent_files do
-        env = Environment.new(f, false)
-        instrument("dotenv.overload", env: env) { env.apply! }
-      end
-    end
+    load(*filenames, overwrite: true)
   end
 
-  # same as `overload`, but raises Errno::ENOENT if any files don't exist
+  # same as `#overload`, but raises Errno::ENOENT if any files don't exist
   def overload!(*filenames)
-    with(*filenames.reverse) do |f|
-      env = Environment.new(f, false)
-      instrument("dotenv.overload", env: env) { env.apply! }
-    end
+    load(*filenames, overwrite: true, ignore: false)
   end
 
-  # returns a hash of parsed key/value pairs but does not modify ENV
-  def parse(*filenames)
-    with(*filenames) do |f|
-      ignoring_nonexistent_files do
-        Environment.new(f, false)
-      end
-    end
-  end
-
-  # Internal: Helper to expand list of filenames.
+  # Parses the given files, yielding for each file if a block is given.
   #
-  # Returns a hash of all the loaded environment variables.
-  def with(*filenames)
+  # @param filenames [String, Array<String>] Files to parse
+  # @param overwrite [Boolean] Overwrite existing `ENV` values
+  # @param ignore [Boolean] Ignore non-existent files
+  # @param block [Proc] Block to yield for each parsed `Dotenv::Environment`
+  # @return [Hash] parsed key/value pairs
+  def parse(*filenames, overwrite: false, ignore: true, &block)
     filenames << ".env" if filenames.empty?
+    filenames = filenames.reverse if overwrite
 
     filenames.reduce({}) do |hash, filename|
-      hash.merge!(yield(File.expand_path(filename)) || {})
+      begin
+        env = Environment.new(File.expand_path(filename), overwrite: overwrite)
+        env = block.call(env) if block
+      rescue Errno::ENOENT
+        raise unless ignore
+      end
+
+      hash.merge! env || {}
     end
   end
 
@@ -78,9 +68,4 @@ def require_keys(*keys)
     return if missing_keys.empty?
     raise MissingKeys, missing_keys
   end
-
-  def ignoring_nonexistent_files
-    yield
-  rescue Errno::ENOENT
-  end
 end

lib/dotenv/environment.rb

@@ -4,25 +4,28 @@ module Dotenv
   class Environment < Hash
     attr_reader :filename
 
-    def initialize(filename, is_load = false)
+    def initialize(filename, overwrite: false)
       @filename = filename
-      load(is_load)
+      @overwrite = overwrite
+      load
     end
 
-    def load(is_load = false)
-      update Parser.call(read, is_load)
+    def load
+      update Parser.call(read, overwrite: @overwrite)
     end
 
     def read
       File.open(@filename, "rb:bom|utf-8", &:read)
     end
 
     def apply
-      each { |k, v| ENV[k] ||= v }
-    end
-
-    def apply!
-      each { |k, v| ENV[k] = v }
+      each do |k, v|
+        if @overwrite
+          ENV[k] = v
+        else
+          ENV[k] ||= v
+        end
+      end
     end
   end
 end

lib/dotenv/parser.rb

@@ -32,15 +32,15 @@ class Parser
     class << self
       attr_reader :substitutions
 
-      def call(string, is_load = false)
-        new(string, is_load).call
+      def call(...)
+        new(...).call
       end
     end
 
-    def initialize(string, is_load = false)
+    def initialize(string, overwrite: false)
       @string = string
       @hash = {}
-      @is_load = is_load
+      @overwrite = overwrite
     end
 
     def call
@@ -88,7 +88,7 @@ def expand_newlines(value)
     end
 
     def variable_not_set?(line)
-      !line.split[1..-1].all? { |var| @hash.member?(var) }
+      !line.split[1..].all? { |var| @hash.member?(var) }
     end
 
     def unescape_value(value, maybe_quote)
@@ -104,7 +104,7 @@ def unescape_value(value, maybe_quote)
     def perform_substitutions(value, maybe_quote)
       if maybe_quote != "'"
         self.class.substitutions.each do |proc|
-          value = proc.call(value, @hash, @is_load)
+          value = proc.call(value, @hash, overwrite: @overwrite)
         end
       end
       value

lib/dotenv/substitutions/command.rb

@@ -20,15 +20,15 @@ class << self
           )
         /x
 
-        def call(value, _env, _is_load)
+        def call(value, _env, overwrite: false)
           # Process interpolated shell commands
           value.gsub(INTERPOLATED_SHELL_COMMAND) do |*|
             # Eliminate opening and closing parentheses
             command = $LAST_MATCH_INFO[:cmd][1..-2]
 
             if $LAST_MATCH_INFO[:backslash]
               # Command is escaped, don't replace it.
-              $LAST_MATCH_INFO[0][1..-1]
+              $LAST_MATCH_INFO[0][1..]
             else
               # Execute the command and return the value
               `#{command}`.chomp

lib/dotenv/substitutions/variable.rb

@@ -18,8 +18,8 @@ class << self
           \}?           # closing brace
         /xi
 
-        def call(value, env, is_load)
-          combined_env = is_load ? env.merge(ENV) : ENV.to_h.merge(env)
+        def call(value, env, overwrite: false)
+          combined_env = overwrite ? ENV.to_h.merge(env) : env.merge(ENV)
           value.gsub(VARIABLE) do |variable|
             match = $LAST_MATCH_INFO
             substitute(match, variable, combined_env)
@@ -30,7 +30,7 @@ def call(value, env, is_load)
 
         def substitute(match, variable, env)
           if match[1] == "\\"
-            variable[1..-1]
+            variable[1..]
           elsif match[3]
             env.fetch(match[3], "")
           else

spec/dotenv/environment_spec.rb

@@ -11,7 +11,7 @@
 
     it "fails if file does not exist" do
       expect do
-        Dotenv::Environment.new(".does_not_exists", true)
+        Dotenv::Environment.new(".does_not_exists")
       end.to raise_error(Errno::ENOENT)
     end
   end
@@ -27,27 +27,29 @@
       subject.apply
       expect(ENV["OPTION_A"]).to eq("predefined")
     end
-  end
 
-  describe "apply!" do
-    it "sets variables in the ENV" do
-      subject.apply!
-      expect(ENV["OPTION_A"]).to eq("1")
-    end
+    context "with overwrite: true" do
+      subject { env("OPTION_A=1\nOPTION_B=2", overwrite: true) }
 
-    it "overrides defined variables" do
-      ENV["OPTION_A"] = "predefined"
-      subject.apply!
-      expect(ENV["OPTION_A"]).to eq("1")
+      it "sets variables in the ENV" do
+        subject.apply
+        expect(ENV["OPTION_A"]).to eq("1")
+      end
+
+      it "overrides defined variables" do
+        ENV["OPTION_A"] = "predefined"
+        subject.apply
+        expect(ENV["OPTION_A"]).to eq("1")
+      end
     end
   end
 
   require "tempfile"
-  def env(text)
+  def env(text, ...)
     file = Tempfile.new("dotenv")
     file.write text
     file.close
-    env = Dotenv::Environment.new(file.path, true)
+    env = Dotenv::Environment.new(file.path, ...)
     file.unlink
     env
   end

spec/dotenv/parser_spec.rb

@@ -2,7 +2,7 @@
 
 describe Dotenv::Parser do
   def env(string)
-    Dotenv::Parser.call(string, true)
+    Dotenv::Parser.call(string)
   end
 
   it "parses unquoted values" do
@@ -129,14 +129,14 @@ def env(string)
     ENV["DOTENV_LINEBREAK_MODE"] = "strict"
 
     contents = [
-      'DOTENV_LINEBREAK_MODE=legacy',
+      "DOTENV_LINEBREAK_MODE=legacy",
       'FOO="bar\nbaz\rfizz"'
     ].join("\n")
     expect(env(contents)).to eql("DOTENV_LINEBREAK_MODE" => "legacy", "FOO" => "bar\nbaz\rfizz")
   end
 
   it 'expands \n and \r in quoted strings with DOTENV_LINEBREAK_MODE=legacy in ENV' do
-    ENV['DOTENV_LINEBREAK_MODE'] = 'legacy'
+    ENV["DOTENV_LINEBREAK_MODE"] = "legacy"
     contents = 'FOO="bar\nbaz\rfizz"'
     expect(env(contents)).to eql("FOO" => "bar\nbaz\rfizz")
   end

spec/dotenv_spec.rb

@@ -10,8 +10,7 @@
       let(:env_files) { [] }
 
       it "defaults to .env" do
-        expect(Dotenv::Environment).to receive(:new).with(expand(".env"), anything)
-          .and_return(double(apply: {}, apply!: {}))
+        expect(Dotenv::Environment).to receive(:new).with(expand(".env"), anything).and_call_original
         subject
       end
     end
@@ -23,7 +22,7 @@
         expected = expand("~/.env")
         allow(File).to receive(:exist?) { |arg| arg == expected }
         expect(Dotenv::Environment).to receive(:new).with(expected, anything)
-          .and_return(double(apply: {}, apply!: {}))
+          .and_return(Dotenv::Environment.new(".env"))
         subject
       end
     end
@@ -84,9 +83,9 @@
 
     it_behaves_like "load"
 
-    it "initializes the Environment with a truthy is_load" do
-      expect(Dotenv::Environment).to receive(:new).with(anything, true)
-        .and_return(double(apply: {}, apply!: {}))
+    it "initializes the Environment with overwrite: false" do
+      expect(Dotenv::Environment).to receive(:new).with(anything, overwrite: false)
+        .and_call_original
       subject
     end
 
@@ -106,9 +105,9 @@
 
     it_behaves_like "load"
 
-    it "initializes Environment with truthy is_load" do
-      expect(Dotenv::Environment).to receive(:new).with(anything, true)
-        .and_return(double(apply: {}, apply!: {}))
+    it "initializes Environment with overwrite: false" do
+      expect(Dotenv::Environment).to receive(:new).with(anything, overwrite: false)
+        .and_call_original
       subject
     end
 
@@ -127,9 +126,9 @@
     it_behaves_like "load"
     it_behaves_like "overload"
 
-    it "initializes the Environment with a falsey is_load" do
-      expect(Dotenv::Environment).to receive(:new).with(anything, false)
-        .and_return(double(apply: {}, apply!: {}))
+    it "initializes the Environment overwrite: true" do
+      expect(Dotenv::Environment).to receive(:new).with(anything, overwrite: true)
+        .and_call_original
       subject
     end
 
@@ -161,9 +160,9 @@
     it_behaves_like "load"
     it_behaves_like "overload"
 
-    it "initializes the Environment with a falsey is_load" do
-      expect(Dotenv::Environment).to receive(:new).with(anything, false)
-        .and_return(double(apply: {}, apply!: {}))
+    it "initializes the Environment with overwrite: true" do
+      expect(Dotenv::Environment).to receive(:new).with(anything, overwrite: true)
+        .and_call_original
       subject
     end
 
@@ -271,8 +270,8 @@
       end
     end
 
-    it "initializes the Environment with a falsey is_load" do
-      expect(Dotenv::Environment).to receive(:new).with(anything, false)
+    it "initializes the Environment with overwrite: false" do
+      expect(Dotenv::Environment).to receive(:new).with(anything, overwrite: false)
       subject
     end