diff --git a/Library/Homebrew/rubocops/service.rb b/Library/Homebrew/rubocops/service.rb index 3905d34668..c439f8a276 100644 --- a/Library/Homebrew/rubocops/service.rb +++ b/Library/Homebrew/rubocops/service.rb @@ -21,15 +21,39 @@ module RuboCop share: :opt_share, }.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) service_node = find_block(body_node, :service) 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 - # because their `opt_` variants are more portable and work with the - # API. + # because their `opt_` variants are more portable and work with the API. 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) problem "Use `#{opt_path}` instead of `#{path}` in service blocks." do |corrector| corrector.replace(node.source_range, opt_path) diff --git a/Library/Homebrew/test/rubocops/service_spec.rb b/Library/Homebrew/test/rubocops/service_spec.rb index 61da097159..2a723f4c2d 100644 --- a/Library/Homebrew/test/rubocops/service_spec.rb +++ b/Library/Homebrew/test/rubocops/service_spec.rb @@ -5,6 +5,48 @@ require "rubocops/service" describe RuboCop::Cop::FormulaAudit::Service do 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 expect_offense(<<~RUBY) class Foo < Formula @@ -31,7 +73,7 @@ describe RuboCop::Cop::FormulaAudit::Service do RUBY 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) class Bin < Formula url "https://brew.sh/foo-1.0.tgz"