diff --git a/Library/Homebrew/rubocops/cask/array_alphabetization.rb b/Library/Homebrew/rubocops/cask/array_alphabetization.rb new file mode 100644 index 0000000000..d6ca7f3ca3 --- /dev/null +++ b/Library/Homebrew/rubocops/cask/array_alphabetization.rb @@ -0,0 +1,70 @@ +# 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?(node.method_name) + + node.each_descendant(:pair).each do |pair| + symbols = pair.children.select(&:sym_type?).map(&:value) + next if 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(s) below so that they remain in the same relative location + combined_source = source.each_with_index.map do |line, index| + next if line.strip.start_with?("#") + + next recursively_find_comments(source, index, line) + 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 + + def recursively_find_comments(source, index, line) + if source[index - 1].strip.start_with?("#") + return recursively_find_comments(source, index - 1, "#{source[index - 1]}\n#{line}") + end + + line + end + end + end + end +end diff --git a/Library/Homebrew/rubocops/rubocop-cask.rb b/Library/Homebrew/rubocops/rubocop-cask.rb index 9f3cb11e6a..279f2ad467 100644 --- a/Library/Homebrew/rubocops/rubocop-cask.rb +++ b/Library/Homebrew/rubocops/rubocop-cask.rb @@ -12,6 +12,7 @@ require_relative "cask/extend/node" 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/discontinued" require_relative "cask/homepage_url_trailing_slash" diff --git a/Library/Homebrew/test/rubocops/cask/array_alphabetization_spec.rb b/Library/Homebrew/test/rubocops/cask/array_alphabetization_spec.rb new file mode 100644 index 0000000000..da981e714b --- /dev/null +++ b/Library/Homebrew/test/rubocops/cask/array_alphabetization_spec.rb @@ -0,0 +1,170 @@ +# 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` stanza 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", + ], + rmdir: [ + ^ The array elements should be ordered alphabetically + "/Applications/foo/nested/blah", + "/Applications/foo/", + ] + 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", + ], + rmdir: [ + "/Applications/foo/", + "/Applications/foo/nested/blah", + ] + 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 "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", + # a really long comment related to Zoo + # and the Zoo comment continues + "~/Library/Application Support/Zoo", + "~/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", + # a really long comment related to Zoo + # and the Zoo comment continues + "~/Library/Application Support/Zoo", + ] + end + CASK + end +end diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/everything.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/everything.rb index b253c3d787..1f0cfe08f5 100644 --- a/Library/Homebrew/test/support/fixtures/cask/Casks/everything.rb +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/everything.rb @@ -32,7 +32,7 @@ cask "everything" do } 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}"], diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/with-installable.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/with-installable.rb index fe7cd83008..d6f8ad75b6 100644 --- a/Library/Homebrew/test/support/fixtures/cask/Casks/with-installable.rb +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/with-installable.rb @@ -12,10 +12,10 @@ cask "with-installable" do 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 diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-delete.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-delete.rb index f40775872e..d38206c3c1 100644 --- a/Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-delete.rb +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-delete.rb @@ -9,9 +9,9 @@ cask "with-uninstall-delete" do 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 diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-trash.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-trash.rb index 4f3fac0313..9fa108133b 100644 --- a/Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-trash.rb +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-trash.rb @@ -9,9 +9,9 @@ cask "with-uninstall-trash" do 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 diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/with-zap-delete.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/with-zap-delete.rb index 706ca7a2f1..1e7f57ea1f 100644 --- a/Library/Homebrew/test/support/fixtures/cask/Casks/with-zap-delete.rb +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/with-zap-delete.rb @@ -9,9 +9,9 @@ cask "with-zap-delete" do zap 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 diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/with-zap-trash.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/with-zap-trash.rb index 11651a4f2f..4cd1c15011 100644 --- a/Library/Homebrew/test/support/fixtures/cask/Casks/with-zap-trash.rb +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/with-zap-trash.rb @@ -9,9 +9,9 @@ cask "with-zap-trash" do 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 diff --git a/Library/Homebrew/test/support/fixtures/cask/everything.json b/Library/Homebrew/test/support/fixtures/cask/everything.json index f29f88a4fa..9f8243f9ae 100644 --- a/Library/Homebrew/test/support/fixtures/cask/everything.json +++ b/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" } }