From 8cf58e36e6479ffa0f56608856573e90269dfb77 Mon Sep 17 00:00:00 2001 From: Issy Long Date: Tue, 19 Dec 2023 23:35:16 +0000 Subject: [PATCH] Add a new RuboCop for alphabetizing `zap trash` array elements - Part of issue 16323. - Previously this was being done manually by Cask maintainers. - While we're here, enforce that the `zap trash` path is not in `[]` if it only contains a single element. - This is buggy on actual Casks, hence the draft PR. --- .../rubocops/cask/array_alphabetization.rb | 38 ++++++++++++ Library/Homebrew/rubocops/rubocop-cask.rb | 1 + .../cask/array_alphabetization_spec.rb | 59 +++++++++++++++++++ 3 files changed, 98 insertions(+) create mode 100644 Library/Homebrew/rubocops/cask/array_alphabetization.rb create mode 100644 Library/Homebrew/test/rubocops/cask/array_alphabetization_spec.rb diff --git a/Library/Homebrew/rubocops/cask/array_alphabetization.rb b/Library/Homebrew/rubocops/cask/array_alphabetization.rb new file mode 100644 index 0000000000..394a97f8cd --- /dev/null +++ b/Library/Homebrew/rubocops/cask/array_alphabetization.rb @@ -0,0 +1,38 @@ +# typed: true +# frozen_string_literal: true + +module RuboCop + module Cop + module Cask + class ArrayAlphabetization < Base + extend AutoCorrector + + 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| + corrector.replace(array.source_range, array.children.first.source) + end + end + + array.each_descendant(:str).each_cons(2) do |first, second| + next if first.source < second.source + + 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) + end + end + end + end + end + end + end + end +end diff --git a/Library/Homebrew/rubocops/rubocop-cask.rb b/Library/Homebrew/rubocops/rubocop-cask.rb index a5ee744b8d..f174ba76e9 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/homepage_url_trailing_slash" require_relative "cask/no_overrides" 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..fddcf0ac2a --- /dev/null +++ b/Library/Homebrew/test/rubocops/cask/array_alphabetization_spec.rb @@ -0,0 +1,59 @@ + +# 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 + source = <<~CASK + cask "foo" do + url "https://example.com/foo.zip" + + zap trash: ["~/Library/Application Support/Foo"] + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Remove the `[]` around a single `zap trash` path + end + CASK + + expect_offense(source) + 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 + source = <<~CASK + cask "foo" do + url "https://example.com/foo.zip" + + zap trash: [ + "/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 + CASK + + expect_offense(source) + 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 +end