Make `@view_paths` on the lookup context mostly read-only

The `with_fallbacks` method will temporarily mutate the lookup context
instance, but nobody can call the setter, and we don't have to do a push
/ pop dance.

2 files changed, 17 insertions, 17 deletions

diff --git a/actionview/lib/action_view/lookup_context.rb b/actionview/lib/action_view/lookup_context.rb
index 554d223c0e..cc262dc2b7 100644
--- a/actionview/lib/action_view/lookup_context.rb
+++ b/actionview/lib/action_view/lookup_context.rb
@@ -106,12 +106,6 @@ module ActionView
     module ViewPaths
       attr_reader :view_paths, :html_fallback_for_js
 
-      # Whenever setting view paths, makes a copy so that we can manipulate them in
-      # instance objects as we wish.
-      def view_paths=(paths)
-        @view_paths = ActionView::PathSet.new(Array(paths))
-      end
-
       def find(name, prefixes = [], partial = false, keys = [], options = {})
         @view_paths.find(*args_for_lookup(name, prefixes, partial, keys, options))
       end
@@ -138,19 +132,21 @@ module ActionView
       # Adds fallbacks to the view paths. Useful in cases when you are rendering
       # a :file.
       def with_fallbacks
-        added_resolvers = 0
-        self.class.fallbacks.each do |resolver|
-          next if view_paths.include?(resolver)
-          view_paths.push(resolver)
-          added_resolvers += 1
-        end
+        view_paths = @view_paths
+        @view_paths = build_view_paths((view_paths.paths + self.class.fallbacks).uniq)
         yield
       ensure
-        added_resolvers.times { view_paths.pop }
+        @view_paths = view_paths
       end
 
     private
 
+      # Whenever setting view paths, makes a copy so that we can manipulate them in
+      # instance objects as we wish.
+      def build_view_paths(paths)
+        ActionView::PathSet.new(Array(paths))
+      end
+
       def args_for_lookup(name, prefixes, partial, keys, details_options)
         name, prefixes = normalize_name(name, prefixes)
         details, details_key = detail_args_for(details_options)
@@ -226,7 +222,7 @@ module ActionView
       @rendered_format = nil
 
       @details = initialize_details({}, details)
-      self.view_paths = view_paths
+      @view_paths = build_view_paths(view_paths)
     end
 
     def digest_cache
diff --git a/actionview/test/template/lookup_context_test.rb b/actionview/test/template/lookup_context_test.rb
index 02b5d48e5a..0894925c9f 100644
--- a/actionview/test/template/lookup_context_test.rb
+++ b/actionview/test/template/lookup_context_test.rb
@@ -5,10 +5,14 @@ require "abstract_controller/rendering"
 
 class LookupContextTest < ActiveSupport::TestCase
   def setup
-    @lookup_context = ActionView::LookupContext.new(FIXTURE_LOAD_PATH, {})
+    @lookup_context = build_lookup_context(FIXTURE_LOAD_PATH, {})
     ActionView::LookupContext::DetailsKey.clear
   end
 
+  def build_lookup_context(paths, details)
+    ActionView::LookupContext.new(paths, details)
+  end
+
   def teardown
     I18n.locale = :en
   end
@@ -156,7 +160,7 @@ class LookupContextTest < ActiveSupport::TestCase
   end
 
   test "gives the key forward to the resolver, so it can be used as cache key" do
-    @lookup_context.view_paths = ActionView::FixtureResolver.new("test/_foo.erb" => "Foo")
+    @lookup_context = build_lookup_context(ActionView::FixtureResolver.new("test/_foo.erb" => "Foo"), {})
     template = @lookup_context.find("foo", %w(test), true)
     assert_equal "Foo", template.source
 
@@ -185,7 +189,7 @@ class LookupContextTest < ActiveSupport::TestCase
   end
 
   test "can disable the cache on demand" do
-    @lookup_context.view_paths = ActionView::FixtureResolver.new("test/_foo.erb" => "Foo")
+    @lookup_context = build_lookup_context(ActionView::FixtureResolver.new("test/_foo.erb" => "Foo"), {})
     old_template = @lookup_context.find("foo", %w(test), true)
 
     template = @lookup_context.find("foo", %w(test), true)