Merge pull request #20135 from Homebrew/forbid_dynamic_caveats

rubocops/caveats: check for dynamic caveats.
This commit is contained in:
Mike McQuaid 2025-06-20 07:57:06 +00:00 committed by GitHub
commit 9d357b57d1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 55 additions and 1 deletions

View File

@ -6,13 +6,25 @@ require "rubocops/extend/formula_cop"
module RuboCop module RuboCop
module Cop module Cop
module FormulaAudit module FormulaAudit
# This cop ensures that caveats don't recommend unsupported or unsafe operations. # This cop ensures that caveats don't have problematic text or logic.
# #
# ### Example # ### Example
# #
# ```ruby # ```ruby
# # bad # # bad
# def caveats # def caveats
# if File.exist?("/etc/issue")
# "This caveat only when file exists that won't work with JSON API."
# end
# end
#
# # good
# def caveats
# "This caveat always works regardless of the JSON API."
# end
#
# # bad
# def caveats
# <<~EOS # <<~EOS
# Use `setuid` to allow running the executable by non-root users. # Use `setuid` to allow running the executable by non-root users.
# EOS # EOS
@ -35,6 +47,18 @@ module RuboCop
problem "Don't use ANSI escape codes in the caveats." if regex_match_group(n, /\e/) problem "Don't use ANSI escape codes in the caveats." if regex_match_group(n, /\e/)
end end
# Forbid dynamic logic in caveats (only if/else/unless)
caveats_method = find_method_def(@body, :caveats)
return unless caveats_method
dynamic_nodes = caveats_method.each_descendant.select do |descendant|
descendant.type == :if
end
dynamic_nodes.each do |node|
@offensive_node = node
problem "Don't use dynamic logic (if/else/unless) in caveats."
end
end end
end end
end end

View File

@ -42,5 +42,35 @@ RSpec.describe RuboCop::Cop::FormulaAudit::Caveats do
end end
RUBY RUBY
end end
it "reports an offense if dynamic logic (if/else/unless) is used in caveats" do
expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb")
class Foo < Formula
homepage "https://brew.sh/foo"
url "https://brew.sh/foo-1.0.tgz"
def caveats
if true
^^^^^^^ FormulaAudit/Caveats: Don't use dynamic logic (if/else/unless) in caveats.
"foo"
else
"bar"
end
end
end
RUBY
expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb")
class Foo < Formula
homepage "https://brew.sh/foo"
url "https://brew.sh/foo-1.0.tgz"
def caveats
unless false
^^^^^^^^^^^^ FormulaAudit/Caveats: Don't use dynamic logic (if/else/unless) in caveats.
"foo"
end
end
end
RUBY
end
end end
end end