[Fixes #9760] Make RangeHelp#range_with_surrounding_space params consistent

[pirj]

fixes #9760.

⚠️ breaking change. Target to RuboCop 2.0.

---

Before submitting the PR make sure the following are checked:

• The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
• Wrote good commit messages.
• [-] Commit message starts with [Fix #issue-number] (if the related issue exists).
• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• Added tests.
• Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
• Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

[koic]

First, breaking changes should be avoided as much as possible.

It's better to provide a migration path instead of breaking changes.

• Step 1. Accept old and new arguments (Compatible)
• Step 2. Display deprecation warnings (Compatible)
• Step 3. Make an error (Breaking)

Progress to step 1 and 2 with RuboCop 1.x. Progress to step 3 with RuboCop 2.0.

[koic]

Follow #10727 (comment)

Below is an example from step 1.

diff:

diff --git a/lib/rubocop/cop/mixin/range_help.rb b/lib/rubocop/cop/mixin/range_help.rb
index edcd0a97d..c43c46471 100644
--- a/lib/rubocop/cop/mixin/range_help.rb
+++ b/lib/rubocop/cop/mixin/range_help.rb
@@ -7,6 +7,7 @@ module RuboCop
       private

       BYTE_ORDER_MARK = 0xfeff # The Unicode codepoint
+      NOT_GIVEN = Object.new

       def source_range(source_buffer, line_number, column, length = 1)
         if column.is_a?(Range)
@@ -51,16 +52,15 @@ module RuboCop
         Parser::Source::Range.new(buffer, begin_pos, end_pos)
       end

-      OMITTED = Module.new
-      def range_with_surrounding_space(range,
-                                       side: :both,
+      # rubocop:disable Metrics/ParameterLists
+      def range_with_surrounding_space(range_obj = NOT_GIVEN, range: nil, side: :both,
                                        newlines: true, whitespace: false,
                                        continuations: false)
-        if range.is_a?(Hash) && range.key?(:range)
-          raise 'Pass range as the first positional argument to ' \
-                '`range_with_surrounding_space`. ' \
-                'Passing with the `range:` kwarg is not supported.'
-        end
+        # Passing `range` keyword argument is soft deprecated. Use range as
+        # the first positional argument to instead.
+        # A deprecation warning will be displayed in the future and it will
+        # be removed in RuboCop 2.0.
+        range = range_obj if range_obj != NOT_GIVEN

         buffer = @processed_source.buffer
         src = buffer.source
@@ -73,6 +73,7 @@ module RuboCop
         end_pos = final_pos(src, end_pos, 1, continuations, newlines, whitespace) if go_right
         Parser::Source::Range.new(buffer, begin_pos, end_pos)
       end
+      # rubocop:enable Metrics/ParameterLists

       def range_by_whole_lines(range, include_final_newline: false)
         buffer = @processed_source.buffer

plain code:

      # rubocop:disable Metrics/ParameterLists
      def range_with_surrounding_space(range_obj = NOT_GIVEN, range: nil, side: :both,
                                        newlines: true, whitespace: false,
                                        continuations: false)
        # Passing `range` keyword argument is soft deprecated. Use range as
        # the first positional argument to instead.
        # A deprecation warning will be displayed in the future and it will
        # be removed in RuboCop 2.0.
        range = range_obj if range_obj != NOT_GIVEN

        buffer = @processed_source.buffer
        src = buffer.source

        go_left, go_right = directions(side)

        begin_pos = range.begin_pos
        begin_pos = final_pos(src, begin_pos, -1, continuations, newlines, whitespace) if go_left
        end_pos = range.end_pos
        end_pos = final_pos(src, end_pos, 1, continuations, newlines, whitespace) if go_right
        Parser::Source::Range.new(buffer, begin_pos, end_pos)
      end
      # rubocop:enable Metrics/ParameterLists

This way would allow third party cops to have a transition period, which would reduce the impact on end users.

[pirj]

Thank you so much for the suggestions!
Using a default value for a positional argument should work seamlessly in Ruby 3+.

Do you think there is a good reason to separate steps 1 & 2?
I've sent a PR that adds a deprecation warning, and can be merged at some point when 1.x comes to its last releases.

[koic]

3rd party cop users will always encounter unexpected warnings.
We can suppress this by releasing step 1 without warning, and then 3rd party gem releases with the new positional argument.
At least rubocop org-managed gems like rubocop-(performance, rails, rspec, minitest), and etc will be able to respond between step 1 and step 2 when the warning is displayed.
So, the reason for separating step 1 and step 2 is to make a release to reduce user impact.

[pirj]

Fair enough, my apologies for not figuring this out myself. 👍
I'll take care of a Phase 1 PR and updates to extensions in our namespace.

[pirj]

I've sent #10748, and will close this PR and #10729. We can revive them if we ever decide to really deprecate passing range as a kwarg.