Merge pull request #16377 from razvanazamfirei/rubocop-uninstall-methods-order

rubocop: order uninstall/zap methods
This commit is contained in:
Mike McQuaid 2024-01-29 16:32:24 +00:00 committed by GitHub
commit 2cb8efc51d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 451 additions and 14 deletions

View File

@ -65,6 +65,20 @@ module RuboCop
end.freeze
STANZA_ORDER = STANZA_GROUPS.flatten.freeze
UNINSTALL_METHODS_ORDER = [
:early_script,
:launchctl,
:quit,
:signal,
:login_item,
:kext,
:script,
:pkgutil,
:delete,
:trash,
:rmdir,
].freeze
end
end
end

View File

@ -0,0 +1,65 @@
# typed: strict
# frozen_string_literal: true
require "rubocops/shared/helper_functions"
module RuboCop
module Cop
module Cask
# This cop checks for the correct order of methods within the 'uninstall' and 'zap' stanzas.
class UninstallMethodsOrder < Base
extend AutoCorrector
include HelperFunctions
MSG = T.let("`%<method>s` method out of order", String)
sig { params(node: AST::SendNode).void }
def on_send(node)
return unless [:zap, :uninstall].include?(node.method_name)
hash_node = node.arguments.first
return if hash_node.nil? || (!hash_node.is_a?(AST::Node) && !hash_node.hash_type?)
method_nodes = hash_node.pairs.map(&:key)
expected_order = method_nodes.sort_by { |method| method_order_index(method) }
comments = processed_source.comments
method_nodes.each_with_index do |method, index|
next if method == expected_order[index]
report_and_correct_offense(method, hash_node, expected_order, comments)
end
end
sig {
params(method: AST::Node,
hash_node: AST::HashNode,
expected_order: T::Array[AST::Node],
comments: T::Array[Parser::Source::Comment]).void
}
def report_and_correct_offense(method, hash_node, expected_order, comments)
add_offense(method, message: format(MSG, method: method.children.first)) do |corrector|
indentation = " " * (start_column(method) - line_start_column(method))
new_code = expected_order.map do |expected_method|
method_pair = hash_node.pairs.find { |pair| pair.key == expected_method }
source = method_pair.source
# Find and attach a comment on the same line as the method_pair, if any
inline_comment = comments.find do |comment|
comment.location.line == method_pair.loc.line && comment.location.column > method_pair.loc.column
end
inline_comment ? "#{source} #{inline_comment.text}" : source
end.join(",\n#{indentation}")
corrector.replace(hash_node.source_range, new_code)
end
end
sig { params(method_node: AST::SymbolNode).returns(Integer) }
def method_order_index(method_node)
method_name = method_node.children.first
RuboCop::Cask::Constants::UNINSTALL_METHODS_ORDER.index(method_name) || -1
end
end
end
end
end

View File

@ -20,6 +20,7 @@ require_relative "cask/no_overrides"
require_relative "cask/on_system_conditionals"
require_relative "cask/stanza_order"
require_relative "cask/stanza_grouping"
require_relative "cask/uninstall_methods_order"
require_relative "cask/url"
require_relative "cask/url_legacy_comma_separators"
require_relative "cask/variables"

View File

@ -0,0 +1,357 @@
# frozen_string_literal: true
require "rubocops/rubocop-cask"
describe RuboCop::Cop::Cask::UninstallMethodsOrder, :config do
context "with uninstall blocks" do
context "when methods are incorrectly ordered" do
it "detects and corrects ordering offenses in the uninstall block when each method contains a single item" do
expect_offense(<<~CASK)
cask 'foo' do
uninstall quit: "com.example.foo",
^^^^ `quit` method out of order
launchctl: "com.example.foo"
^^^^^^^^^ `launchctl` method out of order
end
CASK
expect_correction(<<~CASK)
cask 'foo' do
uninstall launchctl: "com.example.foo",
quit: "com.example.foo"
end
CASK
end
it "detects and corrects ordering offenses in the uninstall block when methods contain arrays" do
expect_offense(<<~CASK)
cask "foo" do
url "https://example.com/foo.zip"
uninstall delete: [
^^^^^^ `delete` method out of order
"/usr/local/bin/foo",
"/usr/local/bin/foobar",
],
script: {
^^^^^^ `script` method out of order
executable: "/usr/local/bin/foo",
sudo: false,
},
pkgutil: "org.foo.bar"
^^^^^^^ `pkgutil` method out of order
end
CASK
expect_correction(<<~CASK)
cask "foo" do
url "https://example.com/foo.zip"
uninstall script: {
executable: "/usr/local/bin/foo",
sudo: false,
},
pkgutil: "org.foo.bar",
delete: [
"/usr/local/bin/foo",
"/usr/local/bin/foobar",
]
end
CASK
end
end
context "when methods are correctly ordered" do
it "does not report an offense" do
expect_no_offenses(<<~CASK)
cask "foo" do
url "https://example.com/foo.zip"
uninstall script: {
executable: "/usr/local/bin/foo",
sudo: false,
},
pkgutil: "org.foo.bar",
delete: [
"/usr/local/bin/foo",
"/usr/local/bin/foobar",
]
end
CASK
end
end
context "with a single method" do
it "does not report an offense when a single item is present in the method" do
expect_no_offenses(<<~CASK)
cask "foo" do
url "https://example.com/foo.zip"
uninstall delete: "/usr/local/bin/foo"
end
CASK
end
it "does not report an offense when the method contains an array" do
expect_no_offenses(<<~CASK)
cask "foo" do
url "https://example.com/foo.zip"
uninstall pkgutil: [
"org.foo.bar",
"org.foobar.bar",
]
end
CASK
end
end
end
context "with zap blocks" do
context "when methods are incorrectly ordered" do
it "detects and corrects ordering offenses in the zap block when each method contains a single item" do
expect_offense(<<~CASK)
cask 'foo' do
zap rmdir: "/Library/Foo",
^^^^^ `rmdir` method out of order
trash: "com.example.foo"
^^^^^ `trash` method out of order
end
CASK
expect_correction(<<~CASK)
cask 'foo' do
zap trash: "com.example.foo",
rmdir: "/Library/Foo"
end
CASK
end
it "detects and corrects ordering offenses in the zap block when methods contain arrays" do
expect_offense(<<~CASK)
cask "foo" do
url "https://example.com/foo.zip"
zap delete: [
"~/Library/Application Support/Foo",
"~/Library/Application Support/Bar",
],
rmdir: "~/Library/Application Support",
^^^^^ `rmdir` method out of order
trash: "~/Library/Application Support/FooBar"
^^^^^ `trash` method out of order
end
CASK
expect_correction(<<~CASK)
cask "foo" do
url "https://example.com/foo.zip"
zap delete: [
"~/Library/Application Support/Foo",
"~/Library/Application Support/Bar",
],
trash: "~/Library/Application Support/FooBar",
rmdir: "~/Library/Application Support"
end
CASK
end
end
context "when methods are correctly ordered" do
it "does not report an offense" do
expect_no_offenses(<<~CASK)
cask "foo" do
url "https://example.com/foo.zip"
zap delete: [
"~/Library/Application Support/Bar",
"~/Library/Application Support/Foo",
],
trash: "~/Library/Application Support/FooBar",
rmdir: "~/Library/Application Support"
end
CASK
end
end
context "with a single method" do
it "does not report an offense when a single item is present in the method" do
expect_no_offenses(<<~CASK)
cask "foo" do
url "https://example.com/foo.zip"
zap trash: "~/Library/Application Support/FooBar"
end
CASK
end
it "does not report an offense when the method contains an array" do
expect_no_offenses(<<~CASK)
cask "foo" do
url "https://example.com/foo.zip"
zap trash: [
"~/Library/Application Support/FooBar",
"~/Library/Application Support/FooBarBar",
]
end
CASK
end
end
end
context "with both uninstall and zap blocks" do
context "when both uninstall and zap methods are incorrectly ordered" do
it "detects offenses and auto-corrects to the correct order" do
expect_offense(<<~CASK)
cask "foo" do
url "https://example.com/foo.zip"
uninstall delete: [
^^^^^^ `delete` method out of order
"/usr/local/bin/foo",
"/usr/local/bin/foobar",
],
script: {
^^^^^^ `script` method out of order
executable: "/usr/local/bin/foo",
sudo: false,
},
pkgutil: "org.foo.bar"
^^^^^^^ `pkgutil` method out of order
zap delete: [
"~/Library/Application Support/Bar",
"~/Library/Application Support/Foo",
],
rmdir: "~/Library/Application Support",
^^^^^ `rmdir` method out of order
trash: "~/Library/Application Support/FooBar"
^^^^^ `trash` method out of order
end
CASK
expect_correction(<<~CASK)
cask "foo" do
url "https://example.com/foo.zip"
uninstall script: {
executable: "/usr/local/bin/foo",
sudo: false,
},
pkgutil: "org.foo.bar",
delete: [
"/usr/local/bin/foo",
"/usr/local/bin/foobar",
]
zap delete: [
"~/Library/Application Support/Bar",
"~/Library/Application Support/Foo",
],
trash: "~/Library/Application Support/FooBar",
rmdir: "~/Library/Application Support"
end
CASK
end
end
context "when uninstall and zap methods are correctly ordered" do
it "does not report an offense" do
expect_no_offenses(<<~CASK)
cask 'foo' do
uninstall early_script: {
executable: "foo.sh",
args: ["--unattended"],
},
launchctl: "com.example.foo",
quit: "com.example.foo",
signal: ["TERM", "com.example.foo"],
login_item: "FooApp",
kext: "com.example.foo",
script: {
executable: "foo.sh",
args: ["--unattended"],
},
pkgutil: "com.example.foo",
delete: "~/Library/Preferences/com.example.foo",
trash: "~/Library/Preferences/com.example.foo",
rmdir: "~/Library/Foo"
zap early_script: {
executable: "foo.sh",
args: ["--unattended"],
},
launchctl: "com.example.foo",
quit: "com.example.foo",
signal: ["TERM", "com.example.foo"],
login_item: "FooApp",
kext: "com.example.foo",
script: {
executable: "foo.sh",
args: ["--unattended"],
},
pkgutil: "com.example.foo",
delete: "~/Library/Preferences/com.example.foo",
trash: "~/Library/Preferences/com.example.foo",
rmdir: "~/Library/Foo"
end
CASK
end
end
end
context "when in-line comments are present" do
it "keeps associated comments when auto-correcting" do
expect_offense <<~CASK
cask 'foo' do
uninstall quit: "com.example.foo", # comment on same line
^^^^ `quit` method out of order
launchctl: "com.example.foo"
^^^^^^^^^ `launchctl` method out of order
end
CASK
expect_correction <<~CASK
cask 'foo' do
uninstall launchctl: "com.example.foo",
quit: "com.example.foo" # comment on same line
end
CASK
end
end
context "when methods are inside an `on_os` block" do
it "detects and corrects offenses within OS-specific blocks" do
expect_offense <<~CASK
cask "foo" do
on_catalina do
uninstall trash: "com.example.foo",
^^^^^ `trash` method out of order
launchctl: "com.example.foo"
^^^^^^^^^ `launchctl` method out of order
end
on_ventura do
uninstall quit: "com.example.foo",
^^^^ `quit` method out of order
launchctl: "com.example.foo"
^^^^^^^^^ `launchctl` method out of order
end
end
CASK
expect_correction <<~CASK
cask "foo" do
on_catalina do
uninstall launchctl: "com.example.foo",
trash: "com.example.foo"
end
on_ventura do
uninstall launchctl: "com.example.foo",
quit: "com.example.foo"
end
end
CASK
end
end
end

View File

@ -32,12 +32,12 @@ cask "everything" do
}
uninstall launchctl: "com.every.thing.agent",
delete: "/Library/EverythingHelperTools",
kext: "com.every.thing.driver",
signal: [
["TERM", "com.every.thing.controller#{version.major}"],
["TERM", "com.every.thing.bin"],
]
],
kext: "com.every.thing.driver",
delete: "/Library/EverythingHelperTools"
zap trash: [
"~/.everything",

View File

@ -7,9 +7,9 @@ cask "with-installable" do
pkg "MyFancyPkg/Fancy.pkg"
uninstall script: { executable: "MyFancyPkg/FancyUninstaller.tool", args: ["--please"] },
quit: "my.fancy.package.app",
uninstall quit: "my.fancy.package.app",
login_item: "Fancy",
script: { executable: "MyFancyPkg/FancyUninstaller.tool", args: ["--please"] },
delete: [
"#{TEST_TMPDIR}/absolute_path",
"#{TEST_TMPDIR}/glob_path*",

View File

@ -7,7 +7,7 @@ cask "with-pkgutil-zap" do
pkg "Fancy.pkg"
zap pkgutil: "my.fancy.package.*",
zap launchctl: "my.fancy.package.service",
kext: "my.fancy.package.kernelextension",
launchctl: "my.fancy.package.service"
pkgutil: "my.fancy.package.*"
end

View File

@ -9,11 +9,11 @@ cask "with-zap" do
uninstall quit: "my.fancy.package.app.from.uninstall"
zap script: {
zap quit: "my.fancy.package.app",
login_item: "Fancy",
script: {
executable: "MyFancyPkg/FancyUninstaller.tool",
args: ["--please"],
},
quit: "my.fancy.package.app",
login_item: "Fancy",
delete: "~/Library/Preferences/my.fancy.app.plist"
end

View File

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