rubocops/service: add checks for custom service names

It now checks for two new edge cases.
1. Are the required methods defined in the block?
2. Is the `run` command defined if we're using methods other
   than `service_name` or `plist_name`?

It has also been updated to only get the list of method call nodes
once instead of doing it repeatedly for each cellar path check.
This commit is contained in:
apainintheneck 2023-05-07 12:51:46 -07:00
parent 1111706378
commit 1bae81cba2
2 changed files with 70 additions and 4 deletions

View File

@ -21,15 +21,39 @@ module RuboCop
share: :opt_share, share: :opt_share,
}.freeze }.freeze
# These can be defined without using the `run` command.
RENAME_METHOD_CALLS = [:plist_name, :service_name].freeze
# At least one of these methods must be defined in a service block.
REQUIRED_METHOD_CALLS = [:run, *RENAME_METHOD_CALLS].freeze
def audit_formula(_node, _class_node, _parent_class_node, body_node) def audit_formula(_node, _class_node, _parent_class_node, body_node)
service_node = find_block(body_node, :service) service_node = find_block(body_node, :service)
return if service_node.blank? return if service_node.blank?
method_calls = service_node.each_descendant(:send).group_by(&:method_name)
method_calls.delete(:service)
# NOTE: Solving the first problem here might solve the second one too
# so we don't show both of them at the same time.
if (method_calls.keys & REQUIRED_METHOD_CALLS).empty?
offending_node(service_node)
problem "Service blocks require `run`, `plist_name` or `service_name` to be defined."
elsif !method_calls.key?(:run)
other_method_calls = method_calls.keys - RENAME_METHOD_CALLS
if other_method_calls.any?
offending_node(service_node)
problem "`run` must be defined to use methods other than " \
"`service_name` and `plist_name` like #{other_method_calls}."
end
end
# This check ensures that cellar paths like `bin` are not referenced # This check ensures that cellar paths like `bin` are not referenced
# because their `opt_` variants are more portable and work with the # because their `opt_` variants are more portable and work with the API.
# API.
CELLAR_PATH_AUDIT_CORRECTIONS.each do |path, opt_path| CELLAR_PATH_AUDIT_CORRECTIONS.each do |path, opt_path|
find_every_method_call_by_name(service_node, path).each do |node| next unless method_calls.key?(path)
method_calls.fetch(path).each do |node|
offending_node(node) offending_node(node)
problem "Use `#{opt_path}` instead of `#{path}` in service blocks." do |corrector| problem "Use `#{opt_path}` instead of `#{path}` in service blocks." do |corrector|
corrector.replace(node.source_range, opt_path) corrector.replace(node.source_range, opt_path)

View File

@ -5,6 +5,48 @@ require "rubocops/service"
describe RuboCop::Cop::FormulaAudit::Service do describe RuboCop::Cop::FormulaAudit::Service do
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
it "reports offenses when a service block is missing a required command" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
service do
^^^^^^^^^^ FormulaAudit/Service: Service blocks require `run`, `plist_name` or `service_name` to be defined.
run_type :cron
working_dir "/tmp/example"
end
end
RUBY
end
it "reports no offenses when a service block only includes custom names" do
expect_no_offenses(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
service do
plist_name "custom.mcxl.foo"
service_name "custom.foo"
end
end
RUBY
end
it "reports offenses when a service block includes more than custom names and no run command" do
expect_offense(<<~RUBY)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
service do
^^^^^^^^^^ FormulaAudit/Service: `run` must be defined to use methods other than `service_name` and `plist_name` like [:working_dir].
plist_name "custom.mcxl.foo"
service_name "custom.foo"
working_dir "/tmp/example"
end
end
RUBY
end
it "reports offenses when a formula's service block uses cellar paths" do it "reports offenses when a formula's service block uses cellar paths" do
expect_offense(<<~RUBY) expect_offense(<<~RUBY)
class Foo < Formula class Foo < Formula
@ -31,7 +73,7 @@ describe RuboCop::Cop::FormulaAudit::Service do
RUBY RUBY
end end
it "reports no offenses when a formula's service block only uses opt paths" do it "reports no offenses when a service block only uses opt paths" do
expect_no_offenses(<<~RUBY) expect_no_offenses(<<~RUBY)
class Bin < Formula class Bin < Formula
url "https://brew.sh/foo-1.0.tgz" url "https://brew.sh/foo-1.0.tgz"