Skip to content

Commit 9d357b5

Browse files
authored
Merge pull request #20135 from Homebrew/forbid_dynamic_caveats
rubocops/caveats: check for dynamic caveats.
2 parents e6d4db8 + f77c986 commit 9d357b5

2 files changed

Lines changed: 55 additions & 1 deletion

File tree

Library/Homebrew/rubocops/caveats.rb

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,25 @@
66
module RuboCop
77
module Cop
88
module FormulaAudit
9-
# This cop ensures that caveats don't recommend unsupported or unsafe operations.
9+
# This cop ensures that caveats don't have problematic text or logic.
1010
#
1111
# ### Example
1212
#
1313
# ```ruby
1414
# # bad
1515
# def caveats
16+
# if File.exist?("/etc/issue")
17+
# "This caveat only when file exists that won't work with JSON API."
18+
# end
19+
# end
20+
#
21+
# # good
22+
# def caveats
23+
# "This caveat always works regardless of the JSON API."
24+
# end
25+
#
26+
# # bad
27+
# def caveats
1628
# <<~EOS
1729
# Use `setuid` to allow running the executable by non-root users.
1830
# EOS
@@ -35,6 +47,18 @@ def audit_formula(_formula_nodes)
3547

3648
problem "Don't use ANSI escape codes in the caveats." if regex_match_group(n, /\e/)
3749
end
50+
51+
# Forbid dynamic logic in caveats (only if/else/unless)
52+
caveats_method = find_method_def(@body, :caveats)
53+
return unless caveats_method
54+
55+
dynamic_nodes = caveats_method.each_descendant.select do |descendant|
56+
descendant.type == :if
57+
end
58+
dynamic_nodes.each do |node|
59+
@offensive_node = node
60+
problem "Don't use dynamic logic (if/else/unless) in caveats."
61+
end
3862
end
3963
end
4064
end

Library/Homebrew/test/rubocops/caveats_spec.rb

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,5 +42,35 @@ def caveats
4242
end
4343
RUBY
4444
end
45+
46+
it "reports an offense if dynamic logic (if/else/unless) is used in caveats" do
47+
expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb")
48+
class Foo < Formula
49+
homepage "https://brew.sh/foo"
50+
url "https://brew.sh/foo-1.0.tgz"
51+
def caveats
52+
if true
53+
^^^^^^^ FormulaAudit/Caveats: Don't use dynamic logic (if/else/unless) in caveats.
54+
"foo"
55+
else
56+
"bar"
57+
end
58+
end
59+
end
60+
RUBY
61+
62+
expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb")
63+
class Foo < Formula
64+
homepage "https://brew.sh/foo"
65+
url "https://brew.sh/foo-1.0.tgz"
66+
def caveats
67+
unless false
68+
^^^^^^^^^^^^ FormulaAudit/Caveats: Don't use dynamic logic (if/else/unless) in caveats.
69+
"foo"
70+
end
71+
end
72+
end
73+
RUBY
74+
end
4575
end
4676
end

0 commit comments

Comments
 (0)