Add a new RuboCop for alphabetizing zap trash array elements

Library/Homebrew/rubocops/cask/array_alphabetization.rb

@@ -0,0 +1,69 @@
+# typed: true
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Cask
+      class ArrayAlphabetization < Base
+        extend AutoCorrector
+
+        def on_send(node)
+          return unless [:zap, :uninstall].include?(name = node.method_name)
+
+          node.each_descendant(:pair).each do |pair|
+            symbols = pair.children.select(&:sym_type?).map(&:value)
+            # For `zap`s, we only care about `trash` arrays.
+            next if name == :zap && !symbols.include?(:trash)
+            # Don't order `uninstall` arrays that contain commands.
+            next if name == :uninstall && symbols.intersect?([:signal, :script, :early_script, :args, :input])
+
+            pair.each_descendant(:array).each do |array|
+              if array.children.length == 1
+                add_offense(array, message: "Avoid single-element arrays by removing the []") do |corrector|
+                  corrector.replace(array.source_range, array.children.first.source)
+                end
+              end
+
+              next if array.children.length <= 1
+
+              sorted_array = sort_array(array.source.split("\n")).join("\n")
+
+              next if array.source == sorted_array
+
+              add_offense(array, message: "The array elements should be ordered alphabetically") do |corrector|
+                corrector.replace(array.source_range, sorted_array)
+              end
+            end
+          end
+        end
+
+        def sort_array(source)
+          # Combine each comment with the line below it so that they remain connected to the line they comment
+          combined_source = source.each_with_index.map do |line, index|
+            if line.strip.start_with?("#") && index < source.length - 1
+              "#{line}\n#{source[index + 1]}"
+            elsif source[index - 1]&.strip&.start_with?("#")
+              nil
+            else
+              line
+            end
+          end.compact
+
+          # Separate the lines into those that should be sorted and those that should not
+          # ie. skip the opening and closing brackets of the array
+          to_sort, to_keep = combined_source.partition { |line| !line.include?("[") && !line.include?("]") }
+
+          # Sort the lines that should be sorted
+          to_sort.sort! do |a, b|
+            a_non_comment = a.split("\n").reject { |line| line.strip.start_with?("#") }.first
+            b_non_comment = b.split("\n").reject { |line| line.strip.start_with?("#") }.first
+            a_non_comment.downcase <=> b_non_comment.downcase
+          end
+
+          # Merge the sorted lines and the unsorted lines, preserving the original positions of the unsorted lines
+          combined_source.map { |line| to_keep.include?(line) ? line : to_sort.shift }
+        end
+      end
+    end
+  end
+end

Library/Homebrew/rubocops/rubocop-cask.rb

@@ -12,6 +12,7 @@
 require_relative "cask/mixin/cask_help"
 require_relative "cask/mixin/on_homepage_stanza"
 require_relative "cask/mixin/on_url_stanza"
+require_relative "cask/array_alphabetization"
 require_relative "cask/desc"
 require_relative "cask/homepage_url_trailing_slash"
 require_relative "cask/no_overrides"

Library/Homebrew/test/rubocops/cask/array_alphabetization_spec.rb

@@ -0,0 +1,168 @@
+# frozen_string_literal: true
+
+require "rubocops/rubocop-cask"
+
+describe RuboCop::Cop::Cask::ArrayAlphabetization, :config do
+  it "reports an offense when a single `zap trash` path is specified in an array" do
+    expect_offense(<<~CASK)
+      cask "foo" do
+        url "https://example.com/foo.zip"
+
+        zap trash: ["~/Library/Application Support/Foo"]
+                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid single-element arrays by removing the []
+      end
+    CASK
+
+    expect_correction(<<~CASK)
+      cask "foo" do
+        url "https://example.com/foo.zip"
+
+        zap trash: "~/Library/Application Support/Foo"
+      end
+    CASK
+  end
+
+  it "reports an offense when the `zap trash` paths are not in alphabetical order" do
+    expect_offense(<<~CASK)
+      cask "foo" do
+        url "https://example.com/foo.zip"
+
+        zap trash: [
+                   ^ The array elements should be ordered alphabetically
+          "/Library/Application Support/Foo",
+          "/Library/Application Support/Baz",
+          "~/Library/Application Support/Foo",
+          "~/.dotfiles/thing",
+          "~/Library/Application Support/Bar",
+        ]
+      end
+    CASK
+
+    expect_correction(<<~CASK)
+      cask "foo" do
+        url "https://example.com/foo.zip"
+
+        zap trash: [
+          "/Library/Application Support/Baz",
+          "/Library/Application Support/Foo",
+          "~/.dotfiles/thing",
+          "~/Library/Application Support/Bar",
+          "~/Library/Application Support/Foo",
+        ]
+      end
+    CASK
+  end
+
+  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 array elements should be ordered alphabetically
+          "~/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
+
+  it "ignores `zap` methods other than `trash`" do
+    expect_no_offenses(<<~CASK)
+      cask "foo" do
+        url "https://example.com/foo.zip"
+
+        zap delete: [
+          "~/Library/Application Support/Foo",
+          "~/Library/Application Support/Bar",
+        ]
+      end
+    CASK
+  end
+
+  it "autocorrects alphabetization in `uninstall` methods" do
+    expect_offense(<<~CASK)
+      cask "foo" do
+        url "https://example.com/foo.zip"
+
+        uninstall pkgutil: [
+                           ^ The array elements should be ordered alphabetically
+          "something",
+          "other",
+        ],
+        script: [
+          "ordered",
+          "differently",
+        ]
+      end
+    CASK
+
+    expect_correction(<<~CASK)
+      cask "foo" do
+        url "https://example.com/foo.zip"
+
+        uninstall pkgutil: [
+          "other",
+          "something",
+        ],
+        script: [
+          "ordered",
+          "differently",
+        ]
+      end
+    CASK
+  end
+
+  it "ignores `uninstall` methods with commands" do
+    expect_no_offenses(<<~CASK)
+      cask "foo" do
+        url "https://example.com/foo.zip"
+
+        uninstall script: {
+          args: ["--mode=something", "--another-mode"],
+          executable: "thing",
+        }
+      end
+    CASK
+  end
+
+  it "moves comments when autocorrecting" do
+    expect_offense(<<~CASK)
+      cask "foo" do
+        url "https://example.com/foo.zip"
+
+        zap trash: [
+                   ^ The array elements should be ordered alphabetically
+          # comment related to foo
+          "~/Library/Application Support/Foo",
+          "~/Library/Application Support/Bar",
+          "~/Library/Application Support/Baz", # in-line comment
+        ]
+      end
+    CASK
+
+    expect_correction(<<~CASK)
+      cask "foo" do
+        url "https://example.com/foo.zip"
+
+        zap trash: [
+          "~/Library/Application Support/Bar",
+          "~/Library/Application Support/Baz", # in-line comment
+          # comment related to foo
+          "~/Library/Application Support/Foo",
+        ]
+      end
+    CASK
+  end
+end

Library/Homebrew/test/support/fixtures/cask/Casks/everything.rb

@@ -32,7 +32,7 @@
   }
 
   uninstall launchctl: "com.every.thing.agent",
-            delete:    ["/Library/EverythingHelperTools"],
+            delete:    "/Library/EverythingHelperTools",
             kext:      "com.every.thing.driver",
             signal:    [
               ["TERM", "com.every.thing.controller#{version.major}"],

Library/Homebrew/test/support/fixtures/cask/Casks/with-installable.rb

@@ -12,10 +12,10 @@
             login_item: "Fancy",
             delete:     [
               "#{TEST_TMPDIR}/absolute_path",
-              "~/path_with_tilde",
               "#{TEST_TMPDIR}/glob_path*",
-              "impermissible/relative/path",
               "/another/impermissible/../relative/path",
+              "impermissible/relative/path",
+              "~/path_with_tilde",
             ],
             rmdir:      "#{TEST_TMPDIR}/empty_directory_path"
 end

Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-delete.rb

@@ -9,9 +9,9 @@
 
   uninstall delete: [
     "#{TEST_TMPDIR}/absolute_path",
-    "~/path_with_tilde",
     "#{TEST_TMPDIR}/glob_path*",
-    "impermissible/relative/path",
     "/another/impermissible/../relative/path",
+    "impermissible/relative/path",
+    "~/path_with_tilde",
   ]
 end

Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-trash.rb

@@ -9,9 +9,9 @@
 
   uninstall trash: [
     "#{TEST_TMPDIR}/absolute_path",
-    "~/path_with_tilde",
     "#{TEST_TMPDIR}/glob_path*",
-    "impermissible/relative/path",
     "/another/impermissible/../relative/path",
+    "impermissible/relative/path",
+    "~/path_with_tilde",
   ]
 end

Library/Homebrew/test/support/fixtures/cask/Casks/with-zap-trash.rb

@@ -9,9 +9,9 @@
 
   zap trash: [
     "#{TEST_TMPDIR}/absolute_path",
-    "~/path_with_tilde",
     "#{TEST_TMPDIR}/glob_path*",
-    "impermissible/relative/path",
     "/another/impermissible/../relative/path",
+    "impermissible/relative/path",
+    "~/path_with_tilde",
   ]
 end

Library/Homebrew/test/support/fixtures/cask/everything.json

@@ -28,9 +28,7 @@
       "uninstall": [
         {
           "launchctl": "com.every.thing.agent",
-          "delete": [
-            "/Library/EverythingHelperTools"
-          ],
+          "delete": "/Library/EverythingHelperTools",
           "kext": "com.every.thing.driver",
           "signal": [
             [
@@ -103,6 +101,6 @@
   ],
   "ruby_source_path": "Casks/everything.rb",
   "ruby_source_checksum": {
-    "sha256": "b2707d1952f02c3fa566b7ad2a707a847a959d36f51d3dee642dbe5deec12f27"
+    "sha256": "0c4af571cce1632fc6a3dcf3e75ba82a3283077ef12399428192c26f9d6f779b"
   }
 }