Merge pull request rails#42333 from p8/didyoumean-remove-checks

Clean up checks to see if DidYouMean.correct_error is defined

Author: kamipo

actionpack/lib/abstract_controller/base.rb

@@ -16,28 +16,13 @@ def initialize(message = nil, controller = nil, action = nil)
       super(message)
     end
 
-    class Correction
-      def initialize(error)
-        @error = error
-      end
+    if defined?(DidYouMean::Correctable) && defined?(DidYouMean::SpellChecker)
+      include DidYouMean::Correctable
 
       def corrections
-        if @error.action
-          maybe_these = @error.controller.class.action_methods
-
-          maybe_these.sort_by { |n|
-            DidYouMean::Jaro.distance(@error.action.to_s, n)
-          }.reverse.first(4)
-        else
-          []
-        end
+        @corrections ||= DidYouMean::SpellChecker.new(dictionary: controller.class.action_methods).correct(action)
       end
     end
-
-    # We may not have DYM, and DYM might not let us register error handlers
-    if defined?(DidYouMean) && DidYouMean.respond_to?(:correct_error)
-      DidYouMean.correct_error(self, Correction)
-    end
   end
 
   # AbstractController::Base is a low-level API. Nobody should be

actionpack/lib/action_controller/metal/exceptions.rb

@@ -33,29 +33,18 @@ def initialize(message, routes = nil, route_name = nil, method_name = nil)
       super(message)
     end
 
-    class Correction
-      def initialize(error)
-        @error = error
-      end
+    if defined?(DidYouMean::Correctable) && defined?(DidYouMean::SpellChecker)
+      include DidYouMean::Correctable
 
       def corrections
-        if @error.method_name
-          maybe_these = @error.routes.named_routes.helper_names.grep(/#{@error.route_name}/)
-          maybe_these -= [@error.method_name.to_s] # remove exact match
-
-          maybe_these.sort_by { |n|
-            DidYouMean::Jaro.distance(@error.route_name, n)
-          }.reverse.first(4)
-        else
-          []
+        @corrections ||= begin
+          maybe_these = routes&.named_routes&.helper_names&.grep(/#{route_name}/) || []
+          maybe_these -= [method_name.to_s] # remove exact match
+
+          DidYouMean::SpellChecker.new(dictionary: maybe_these).correct(route_name)
         end
       end
     end
-
-    # We may not have DYM, and DYM might not let us register error handlers
-    if defined?(DidYouMean) && DidYouMean.respond_to?(:correct_error)
-      DidYouMean.correct_error(self, Correction)
-    end
   end
 
   class MethodNotAllowed < ActionControllerError #:nodoc:

actionpack/lib/action_controller/metal/strong_parameters.rb

@@ -27,28 +27,13 @@ def initialize(param, keys = nil) # :nodoc:
       super("param is missing or the value is empty: #{param}")
     end
 
-    class Correction
-      def initialize(error)
-        @error = error
-      end
+    if defined?(DidYouMean::Correctable) && defined?(DidYouMean::SpellChecker)
+      include DidYouMean::Correctable
 
       def corrections
-        if @error.param && @error.keys
-          maybe_these = @error.keys
-
-          maybe_these.sort_by { |n|
-            DidYouMean::Jaro.distance(@error.param.to_s, n)
-          }.reverse.first(4)
-        else
-          []
-        end
+        @corrections ||= DidYouMean::SpellChecker.new(dictionary: keys).correct(param.to_s)
       end
     end
-
-    # We may not have DYM, and DYM might not let us register error handlers
-    if defined?(DidYouMean) && DidYouMean.respond_to?(:correct_error)
-      DidYouMean.correct_error(self, Correction)
-    end
   end
 
   # Raised when a supplied parameter is not expected and

actionpack/test/controller/base_test.rb

@@ -172,14 +172,12 @@ def test_process_should_be_precise
     assert_equal "The action 'non_existent' could not be found for EmptyController", exception.message
   end
 
-  if defined?(DidYouMean) && DidYouMean.respond_to?(:correct_error)
-    def test_exceptions_have_suggestions_for_fix
-      use_controller SimpleController
-      exception = assert_raise AbstractController::ActionNotFound do
-        get :non_existent
-      end
-      assert_match "Did you mean?", exception.message
+  def test_exceptions_have_suggestions_for_fix
+    use_controller SimpleController
+    exception = assert_raise AbstractController::ActionNotFound do
+      get :ello
     end
+    assert_match "Did you mean?", exception.message
   end
 
   def test_action_missing_should_work

actionpack/test/controller/required_params_test.rb

@@ -22,18 +22,16 @@ class ActionControllerRequiredParamsTest < ActionController::TestCase
     end
   end
 
-  if defined?(DidYouMean) && DidYouMean.respond_to?(:correct_error)
-    test "exceptions have suggestions for fix" do
-      error = assert_raise ActionController::ParameterMissing do
-        post :create, params: { magazine: { name: "Mjallo!" } }
-      end
-      assert_match "Did you mean?", error.message
-
-      error = assert_raise ActionController::ParameterMissing do
-        post :create, params: { book: { title: "Mjallo!" } }
-      end
-      assert_match "Did you mean?", error.message
+  test "exceptions have suggestions for fix" do
+    error = assert_raise ActionController::ParameterMissing do
+      post :create, params: { boko: { name: "Mjallo!" } }
     end
+    assert_match "Did you mean?", error.message
+
+    error = assert_raise ActionController::ParameterMissing do
+      post :create, params: { book: { naem: "Mjallo!" } }
+    end
+    assert_match "Did you mean?", error.message
   end
 
   test "required parameters that are present will not raise" do

actionpack/test/dispatch/debug_exceptions_test.rb

@@ -64,7 +64,7 @@ def call(env)
         [404, { "X-Cascade" => "pass" }, self]
       when "/not_found"
         controller = SimpleController.new
-        raise AbstractController::ActionNotFound.new(nil, controller, :not_found)
+        raise AbstractController::ActionNotFound.new(nil, controller, :ello)
       when "/runtime_error"
         raise RuntimeError
       when "/method_not_allowed"
@@ -98,7 +98,7 @@ def call(env)
       when "/missing_keys"
         raise ActionController::UrlGenerationError, "No route matches"
       when "/parameter_missing"
-        raise ActionController::ParameterMissing.new(:missing_param_key, %w(valid_param_key))
+        raise ActionController::ParameterMissing.new(:invalid_param_key, %w(valid_param_key))
       when "/original_syntax_error"
         eval "broke_syntax =" # `eval` need for raise native SyntaxError at runtime
       when "/syntax_error_into_view"
@@ -318,20 +318,18 @@ def call(env)
     assert_match(/ActionDispatch::Http::MimeNegotiation::InvalidType/, body)
   end
 
-  if defined?(DidYouMean) && DidYouMean.respond_to?(:correct_error)
-    test "rescue with suggestions" do
-      @app = DevelopmentApp
+  test "rescue with suggestions" do
+    @app = DevelopmentApp
 
-      get "/not_found", headers: { "action_dispatch.show_exceptions" => true }
-      assert_response 404
-      assert_select("b", /Did you mean\?/)
-      assert_select("li", "hello")
+    get "/not_found", headers: { "action_dispatch.show_exceptions" => true }
+    assert_response 404
+    assert_select("b", /Did you mean\?/)
+    assert_select("li", "hello")
 
-      get "/parameter_missing", headers: { "action_dispatch.show_exceptions" => true }
-      assert_response 400
-      assert_select("b", /Did you mean\?/)
-      assert_select("li", "valid_param_key")
-    end
+    get "/parameter_missing", headers: { "action_dispatch.show_exceptions" => true }
+    assert_response 400
+    assert_select("b", /Did you mean\?/)
+    assert_select("li", "valid_param_key")
   end
 
   test "rescue with HTML format for HTML API request" do

actionpack/test/dispatch/routing_test.rb

@@ -4913,11 +4913,9 @@ def app; APP end
     assert_match message, error.message
   end
 
-  if defined?(DidYouMean) && DidYouMean.respond_to?(:correct_error)
-    test "exceptions have suggestions for fix" do
-      error = assert_raises(ActionController::UrlGenerationError) { product_path(nil, "id" => "url-tested") }
-      assert_match "Did you mean?", error.message
-    end
+  test "exceptions have suggestions for fix" do
+    error = assert_raises(ActionController::UrlGenerationError) { product_path(nil, "id" => "url-tested") }
+    assert_match "Did you mean?", error.message
   end
 
   # FIXME: we should fix all locations that raise this exception to provide
@@ -4926,8 +4924,8 @@ def app; APP end
   # we don't want to break other code.
   test "correct for empty UrlGenerationError" do
     err = ActionController::UrlGenerationError.new("oh no!")
-    correction = ActionController::UrlGenerationError::Correction.new(err)
-    assert_equal [], correction.corrections
+
+    assert_equal [], err.corrections
   end
 end
 

actionview/test/template/render_test.rb

@@ -627,31 +627,29 @@ def test_render_partial_with_layout_raises_descriptive_error
     assert_match "Missing partial /_true with", e.message
   end
 
-  if defined?(DidYouMean) && DidYouMean.respond_to?(:correct_error)
-    def test_render_partial_provides_spellcheck
-      e = assert_raises(ActionView::MissingTemplate) { @view.render(partial: "test/partail") }
-      assert_match %r{Did you mean\?  test/partial\n *test/partialhtml}, e.message
-    end
+  def test_render_partial_provides_spellcheck
+    e = assert_raises(ActionView::MissingTemplate) { @view.render(partial: "test/partail") }
+    assert_match %r{Did you mean\?  test/partial\n *test/partialhtml}, e.message
+  end
 
-    def test_spellcheck_doesnt_list_directories
-      e = assert_raises(ActionView::MissingTemplate) { @view.render(partial: "test/directory") }
-      assert_match %r{Did you mean\?}, e.message
-      assert_no_match %r{Did you mean\?  test/directory\n}, e.message # test/hello is a directory
-    end
+  def test_spellcheck_doesnt_list_directories
+    e = assert_raises(ActionView::MissingTemplate) { @view.render(partial: "test/directory") }
+    assert_match %r{Did you mean\?}, e.message
+    assert_no_match %r{Did you mean\?  test/directory\n}, e.message # test/hello is a directory
+  end
 
-    def test_spellcheck_only_lists_templates
-      e = assert_raises(ActionView::MissingTemplate) { @view.render(template: "test/partial") }
+  def test_spellcheck_only_lists_templates
+    e = assert_raises(ActionView::MissingTemplate) { @view.render(template: "test/partial") }
 
-      assert_match %r{Did you mean\?}, e.message
-      assert_no_match %r{Did you mean\?  test/partial\n}, e.message
-    end
+    assert_match %r{Did you mean\?}, e.message
+    assert_no_match %r{Did you mean\?  test/partial\n}, e.message
+  end
 
-    def test_spellcheck_only_lists_partials
-      e = assert_raises(ActionView::MissingTemplate) { @view.render(partial: "test/template") }
+  def test_spellcheck_only_lists_partials
+    e = assert_raises(ActionView::MissingTemplate) { @view.render(partial: "test/template") }
 
-      assert_match %r{Did you mean\?}, e.message
-      assert_no_match %r{Did you mean\?  test/template\n}, e.message
-    end
+    assert_match %r{Did you mean\?}, e.message
+    assert_no_match %r{Did you mean\?  test/template\n}, e.message
   end
 
   def test_render_partial_wrong_details_no_spellcheck

activerecord/lib/active_record/associations.rb

@@ -13,28 +13,16 @@ def initialize(record = nil, association_name = nil)
       end
     end
 
-    class Correction
-      def initialize(error)
-        @error = error
-      end
+    if defined?(DidYouMean::Correctable) && defined?(DidYouMean::SpellChecker)
+      include DidYouMean::Correctable
 
       def corrections
-        if @error.association_name
-          maybe_these = @error.record.class.reflections.keys
-
-          maybe_these.sort_by { |n|
-            DidYouMean::Jaro.distance(@error.association_name.to_s, n)
-          }.reverse.first(4)
-        else
-          []
+        @corrections ||= begin
+          maybe_these = record&.class&.reflections&.keys
+          DidYouMean::SpellChecker.new(dictionary: maybe_these).correct(association_name.to_s)
         end
       end
     end
-
-    # We may not have DYM, and DYM might not let us register error handlers
-    if defined?(DidYouMean) && DidYouMean.respond_to?(:correct_error)
-      DidYouMean.correct_error(self, Correction)
-    end
   end
 
   class InverseOfAssociationNotFoundError < ActiveRecordError #:nodoc:
@@ -49,28 +37,20 @@ def initialize(reflection = nil, associated_class = nil)
       end
     end
 
-    class Correction
-      def initialize(error)
-        @error = error
-      end
+    if defined?(DidYouMean::Correctable) && defined?(DidYouMean::SpellChecker)
+      include DidYouMean::Correctable
 
       def corrections
-        if @error.reflection && @error.associated_class
-          maybe_these = @error.associated_class.reflections.keys
-
-          maybe_these.sort_by { |n|
-            DidYouMean::Jaro.distance(@error.reflection.options[:inverse_of].to_s, n)
-          }.reverse.first(4)
+        if reflection && associated_class
+          @corrections ||= begin
+            maybe_these = associated_class.reflections.keys
+            DidYouMean::SpellChecker.new(dictionary: maybe_these).correct(reflection.options[:inverse_of].to_s)
+          end
         else
           []
         end
       end
     end
-
-    # We may not have DYM, and DYM might not let us register error handlers
-    if defined?(DidYouMean) && DidYouMean.respond_to?(:correct_error)
-      DidYouMean.correct_error(self, Correction)
-    end
   end
 
   class HasManyThroughAssociationNotFoundError < ActiveRecordError #:nodoc:
@@ -86,29 +66,21 @@ def initialize(owner_class = nil, reflection = nil)
       end
     end
 
-    class Correction
-      def initialize(error)
-        @error = error
-      end
+    if defined?(DidYouMean::Correctable) && defined?(DidYouMean::SpellChecker)
+      include DidYouMean::Correctable
 
       def corrections
-        if @error.reflection && @error.owner_class
-          maybe_these = @error.owner_class.reflections.keys
-          maybe_these -= [@error.reflection.name.to_s] # remove failing reflection
-
-          maybe_these.sort_by { |n|
-            DidYouMean::Jaro.distance(@error.reflection.options[:through].to_s, n)
-          }.reverse.first(4)
+        if owner_class && reflection
+          @corrections ||= begin
+            maybe_these = owner_class.reflections.keys
+            maybe_these -= [reflection.name.to_s] # remove failing reflection
+            DidYouMean::SpellChecker.new(dictionary: maybe_these).correct(reflection.options[:through].to_s)
+          end
         else
           []
         end
       end
     end
-
-    # We may not have DYM, and DYM might not let us register error handlers
-    if defined?(DidYouMean) && DidYouMean.respond_to?(:correct_error)
-      DidYouMean.correct_error(self, Correction)
-    end
   end
 
   class HasManyThroughAssociationPolymorphicSourceError < ActiveRecordError #:nodoc: