Merge pull request #877 from G-Rath/gsub_file-error-on-no-change

feat: support `gsub_file` erroring if gsub doesn't change anything, and add `gsub_file!`

Author: rafaelfranca

lib/thor/actions/file_manipulation.rb

@@ -242,6 +242,35 @@ def inject_into_module(path, module_name, *args, &block)
       insert_into_file(path, *(args << config), &block)
     end
 
+    # Run a regular expression replacement on a file, raising an error if the
+    # contents of the file are not changed.
+    #
+    # ==== Parameters
+    # path<String>:: path of the file to be changed
+    # flag<Regexp|String>:: the regexp or string to be replaced
+    # replacement<String>:: the replacement, can be also given as a block
+    # config<Hash>:: give :verbose => false to not log the status, and
+    #                :force => true, to force the replacement regardless of runner behavior.
+    #
+    # ==== Example
+    #
+    #   gsub_file! 'app/controllers/application_controller.rb', /#\s*(filter_parameter_logging :password)/, '\1'
+    #
+    #   gsub_file! 'README', /rake/, :green do |match|
+    #     match << " no more. Use thor!"
+    #   end
+    #
+    def gsub_file!(path, flag, *args, &block)
+      config = args.last.is_a?(Hash) ? args.pop : {}
+
+      return unless behavior == :invoke || config.fetch(:force, false)
+
+      path = File.expand_path(path, destination_root)
+      say_status :gsub, relative_to_original_destination_root(path), config.fetch(:verbose, true)
+
+      actually_gsub_file(path, flag, args, true, &block) unless options[:pretend]
+    end
+
     # Run a regular expression replacement on a file.
     #
     # ==== Parameters
@@ -267,11 +296,7 @@ def gsub_file(path, flag, *args, &block)
       path = File.expand_path(path, destination_root)
       say_status :gsub, relative_to_original_destination_root(path), config.fetch(:verbose, true)
 
-      unless options[:pretend]
-        content = File.binread(path)
-        content.gsub!(flag, *args, &block)
-        File.open(path, "wb") { |file| file.write(content) }
-      end
+      actually_gsub_file(path, flag, args, false, &block) unless options[:pretend]
     end
 
     # Uncomment all lines matching a given regex. Preserves indentation before
@@ -357,6 +382,17 @@ def with_output_buffer(buf = "".dup) #:nodoc:
       self.output_buffer = old_buffer
     end
 
+    def actually_gsub_file(path, flag, args, error_on_no_change, &block)
+      content = File.binread(path)
+      success = content.gsub!(flag, *args, &block)
+
+      if success.nil? && error_on_no_change
+        raise Thor::Error, "The content of #{path} did not change"
+      end
+
+      File.open(path, "wb") { |file| file.write(content) }
+    end
+
     # Thor::Actions#capture depends on what kind of buffer is used in ERB.
     # Thus CapturableERB fixes ERB to use String buffer.
     class CapturableERB < ERB

spec/actions/file_manipulation_spec.rb

@@ -316,6 +316,104 @@ def file
       end
     end
 
+    describe "#gsub_file!" do
+      context "with invoke behavior" do
+        it "replaces the content in the file" do
+          action :gsub_file!, "doc/README", "__start__", "START"
+          expect(File.binread(file)).to eq("START\nREADME\n__end__\n")
+        end
+
+        it "does not replace if pretending" do
+          runner(pretend: true)
+          action :gsub_file!, "doc/README", "__start__", "START"
+          expect(File.binread(file)).to eq("__start__\nREADME\n__end__\n")
+        end
+
+        it "accepts a block" do
+          action(:gsub_file!, "doc/README", "__start__") { |match| match.gsub("__", "").upcase }
+          expect(File.binread(file)).to eq("START\nREADME\n__end__\n")
+        end
+
+        it "logs status" do
+          expect(action(:gsub_file!, "doc/README", "__start__", "START")).to eq("        gsub  doc/README\n")
+        end
+
+        it "does not log status if required" do
+          expect(action(:gsub_file!, file, "__", verbose: false) { |match| match * 2 }).to be_empty
+        end
+
+        it "cares if the file contents did not change" do
+          expect do
+            action :gsub_file!, "doc/README", "___start___", "START"
+          end.to raise_error(Thor::Error, "The content of #{destination_root}/doc/README did not change")
+
+          expect(File.binread(file)).to eq("__start__\nREADME\n__end__\n")
+        end
+      end
+
+      context "with revoke behavior" do
+        context "and no force option" do
+          it "does not replace the content in the file" do
+            runner({}, :revoke)
+            action :gsub_file!, "doc/README", "__start__", "START"
+            expect(File.binread(file)).to eq("__start__\nREADME\n__end__\n")
+          end
+
+          it "does not replace if pretending" do
+            runner({pretend: true}, :revoke)
+            action :gsub_file!, "doc/README", "__start__", "START"
+            expect(File.binread(file)).to eq("__start__\nREADME\n__end__\n")
+          end
+
+          it "does not replace the content in the file when given a block" do
+            runner({}, :revoke)
+            action(:gsub_file!, "doc/README", "__start__") { |match| match.gsub("__", "").upcase }
+            expect(File.binread(file)).to eq("__start__\nREADME\n__end__\n")
+          end
+
+          it "does not log status" do
+            runner({}, :revoke)
+            expect(action(:gsub_file!, "doc/README", "__start__", "START")).to be_empty
+          end
+
+          it "does not log status if required" do
+            runner({}, :revoke)
+            expect(action(:gsub_file!, file, "__", verbose: false) { |match| match * 2 }).to be_empty
+          end
+        end
+
+        context "and force option" do
+          it "replaces the content in the file" do
+            runner({}, :revoke)
+            action :gsub_file!, "doc/README", "__start__", "START", force: true
+            expect(File.binread(file)).to eq("START\nREADME\n__end__\n")
+          end
+
+          it "does not replace if pretending" do
+            runner({pretend: true}, :revoke)
+            action :gsub_file!, "doc/README", "__start__", "START", force: true
+            expect(File.binread(file)).to eq("__start__\nREADME\n__end__\n")
+          end
+
+          it "replaces the content in the file when given a block" do
+            runner({}, :revoke)
+            action(:gsub_file!, "doc/README", "__start__", force: true) { |match| match.gsub("__", "").upcase }
+            expect(File.binread(file)).to eq("START\nREADME\n__end__\n")
+          end
+
+          it "logs status" do
+            runner({}, :revoke)
+            expect(action(:gsub_file!, "doc/README", "__start__", "START", force: true)).to eq("        gsub  doc/README\n")
+          end
+
+          it "does not log status if required" do
+            runner({}, :revoke)
+            expect(action(:gsub_file!, file, "__", verbose: false, force: true) { |match| match * 2 }).to be_empty
+          end
+        end
+      end
+    end
+
     describe "#gsub_file" do
       context "with invoke behavior" do
         it "replaces the content in the file" do
@@ -341,6 +439,11 @@ def file
         it "does not log status if required" do
           expect(action(:gsub_file, file, "__", verbose: false) { |match| match * 2 }).to be_empty
         end
+
+        it "does not care if the file contents did not change" do
+          action :gsub_file, "doc/README", "___start___", "START"
+          expect(File.binread(file)).to eq("__start__\nREADME\n__end__\n")
+        end
       end
 
       context "with revoke behavior" do