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>
This commit is contained in:
Issy Long 2023-12-22 00:22:33 +00:00
parent f34accfcde
commit b9f13fc35d
No known key found for this signature in database
GPG Key ID: 8247C390DADC67D4
2 changed files with 26 additions and 13 deletions

View File

@ -7,26 +7,28 @@ module RuboCop
class ArrayAlphabetization < Base class ArrayAlphabetization < Base
extend AutoCorrector 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) def on_send(node)
return if node.method_name != :zap return if node.method_name != :zap
node.each_descendant(:pair).each do |pair| node.each_descendant(:pair).each do |pair|
pair.each_descendant(:array).each do |array| pair.each_descendant(:array).each do |array|
if array.children.length == 1 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) corrector.replace(array.source_range, array.children.first.source)
end end
end end
array.children.reject(&:dstr_type?).each_cons(2) do |first, second| next if array.children.length <= 1
next if first.source.downcase < second.source.downcase
add_offense(second, message: "The `zap trash` paths should be in alphabetical order") do |corrector| sorted_array = array.children.sort_by { |child| child.source.downcase }
corrector.insert_before(first.source_range, second.source) next if sorted_array.map(&:source) == array.children.map(&:source)
corrector.insert_before(second.source_range, first.source)
# Using `corrector.replace` here trips the clobbering detection. add_offense(array, message: NON_ALPHABETICAL_MSG) do |corrector|
corrector.remove(first.source_range) array.children.each_with_index do |child, index|
corrector.remove(second.source_range) corrector.replace(child.source_range, sorted_array[index].source)
end end
end end
end end

View File

@ -28,12 +28,11 @@ describe RuboCop::Cop::Cask::ArrayAlphabetization, :config do
url "https://example.com/foo.zip" url "https://example.com/foo.zip"
zap trash: [ zap trash: [
^ The `zap trash` paths should be in alphabetical order
"/Library/Application Support/Foo", "/Library/Application Support/Foo",
"/Library/Application Support/Baz", "/Library/Application Support/Baz",
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ The `zap trash` paths should be in alphabetical order
"~/Library/Application Support/Foo", "~/Library/Application Support/Foo",
"~/.dotfiles/thing", "~/.dotfiles/thing",
^^^^^^^^^^^^^^^^^^^ The `zap trash` paths should be in alphabetical order
"~/Library/Application Support/Bar", "~/Library/Application Support/Bar",
] ]
end end
@ -54,16 +53,28 @@ describe RuboCop::Cop::Cask::ArrayAlphabetization, :config do
CASK CASK
end end
it "ignores zap trash paths that have interpolation" do it "autocorrects alphabetization in zap trash paths with interpolation" do
expect_no_offenses(<<~CASK) expect_offense(<<~CASK)
cask "foo" do cask "foo" do
url "https://example.com/foo.zip" url "https://example.com/foo.zip"
zap trash: [ zap trash: [
^ The `zap trash` paths should be in alphabetical order
"~/Library/Application Support/Foo", "~/Library/Application Support/Foo",
"~/Library/Application Support/Bar\#{version.major}", "~/Library/Application Support/Bar\#{version.major}",
] ]
end end
CASK 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
end end