From b9f13fc35da5ecefe7a2a47ce64d0f6b58b5ce93 Mon Sep 17 00:00:00 2001
From: Issy Long <me@issyl0.co.uk>
Date: Fri, 22 Dec 2023 00:22:33 +0000
Subject: [PATCH] Better detection and replacement of non-alphabetized arrays

- Use `sort_by` to sort the array, rather than comparing each element
  to the next.
- This doesn't error with complaints about clobbering at all when run on
  `homebrew/cask`, hurray. And it also handles interpolations correctly,
  rather than ignoring them.

Co-authored-by: Bevan Kay <email@bevankay.me>
---
 .../rubocops/cask/array_alphabetization.rb    | 20 ++++++++++---------
 .../cask/array_alphabetization_spec.rb        | 19 ++++++++++++++----
 2 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/Library/Homebrew/rubocops/cask/array_alphabetization.rb b/Library/Homebrew/rubocops/cask/array_alphabetization.rb
index 3ee5dbbf28..ff268be6ce 100644
--- a/Library/Homebrew/rubocops/cask/array_alphabetization.rb
+++ b/Library/Homebrew/rubocops/cask/array_alphabetization.rb
@@ -7,26 +7,28 @@ module RuboCop
       class ArrayAlphabetization < Base
         extend AutoCorrector
 
+        SINGLE_MSG = "Remove the `[]` around a single `zap trash` path".freeze
+        NON_ALPHABETICAL_MSG = "The `zap trash` paths should be in alphabetical order".freeze
+
         def on_send(node)
           return if node.method_name != :zap
 
           node.each_descendant(:pair).each do |pair|
             pair.each_descendant(:array).each do |array|
               if array.children.length == 1
-                add_offense(array, message: "Remove the `[]` around a single `zap trash` path") do |corrector|
+                add_offense(array, message: SINGLE_MSG) do |corrector|
                   corrector.replace(array.source_range, array.children.first.source)
                 end
               end
 
-              array.children.reject(&:dstr_type?).each_cons(2) do |first, second|
-                next if first.source.downcase < second.source.downcase
+              next if array.children.length <= 1
 
-                add_offense(second, message: "The `zap trash` paths should be in alphabetical order") do |corrector|
-                  corrector.insert_before(first.source_range, second.source)
-                  corrector.insert_before(second.source_range, first.source)
-                  # Using `corrector.replace` here trips the clobbering detection.
-                  corrector.remove(first.source_range)
-                  corrector.remove(second.source_range)
+              sorted_array = array.children.sort_by { |child| child.source.downcase }
+              next if sorted_array.map(&:source) == array.children.map(&:source)
+
+              add_offense(array, message: NON_ALPHABETICAL_MSG) do |corrector|
+                array.children.each_with_index do |child, index|
+                  corrector.replace(child.source_range, sorted_array[index].source)
                 end
               end
             end
diff --git a/Library/Homebrew/test/rubocops/cask/array_alphabetization_spec.rb b/Library/Homebrew/test/rubocops/cask/array_alphabetization_spec.rb
index 42e67d6ce5..2d932750b7 100644
--- a/Library/Homebrew/test/rubocops/cask/array_alphabetization_spec.rb
+++ b/Library/Homebrew/test/rubocops/cask/array_alphabetization_spec.rb
@@ -28,12 +28,11 @@ describe RuboCop::Cop::Cask::ArrayAlphabetization, :config do
         url "https://example.com/foo.zip"
 
         zap trash: [
+                   ^ The `zap trash` paths should be in alphabetical order
           "/Library/Application Support/Foo",
           "/Library/Application Support/Baz",
-          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ The `zap trash` paths should be in alphabetical order
           "~/Library/Application Support/Foo",
           "~/.dotfiles/thing",
-          ^^^^^^^^^^^^^^^^^^^ The `zap trash` paths should be in alphabetical order
           "~/Library/Application Support/Bar",
         ]
       end
@@ -54,16 +53,28 @@ describe RuboCop::Cop::Cask::ArrayAlphabetization, :config do
     CASK
   end
 
-  it "ignores zap trash paths that have interpolation" do
-    expect_no_offenses(<<~CASK)
+  it "autocorrects alphabetization in zap trash paths with interpolation" do
+    expect_offense(<<~CASK)
       cask "foo" do
         url "https://example.com/foo.zip"
 
         zap trash: [
+                   ^ The `zap trash` paths should be in alphabetical order
           "~/Library/Application Support/Foo",
           "~/Library/Application Support/Bar\#{version.major}",
         ]
       end
     CASK
+
+    expect_correction(<<~CASK)
+      cask "foo" do
+        url "https://example.com/foo.zip"
+
+        zap trash: [
+          "~/Library/Application Support/Bar\#{version.major}",
+          "~/Library/Application Support/Foo",
+        ]
+      end
+    CASK
   end
 end