Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

audit: Move patch checks from brew audit to rubocop #7837

Merged
merged 4 commits into from Jun 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 0 additions & 14 deletions Library/Homebrew/dev-cmd/audit.rb
Expand Up @@ -168,14 +168,6 @@ def without_patch
@text.split("\n__END__").first
end

def data?
/^[^#]*\bDATA\b/ =~ @text
end

def end?
/^__END__$/ =~ @text
end

def trailing_newline?
/\Z\n/ =~ @text
end
Expand Down Expand Up @@ -234,12 +226,6 @@ def audit_style
end

def audit_file
# TODO: check could be in RuboCop
problem "'DATA' was found, but no '__END__'" if text.data? && !text.end?

# TODO: check could be in RuboCop
problem "'__END__' was found, but 'DATA' is not used" if text.end? && !text.data?

# TODO: check could be in RuboCop
if text.to_s.match?(/inreplace [^\n]* do [^\n]*\n[^\n]*\.gsub![^\n]*\n\ *end/m)
problem "'inreplace ... do' was used for a single substitution (use the non-block form instead)."
Expand Down
36 changes: 35 additions & 1 deletion Library/Homebrew/rubocops/patches.rb
Expand Up @@ -8,14 +8,24 @@ module Cop
module FormulaAudit
# This cop audits patches in Formulae.
class Patches < FormulaCop
def audit_formula(_node, _class_node, _parent_class_node, body)
def audit_formula(node, _class_node, _parent_class_node, body)
@full_source_content = source_buffer(node).source

external_patches = find_all_blocks(body, :patch)
external_patches.each do |patch_block|
url_node = find_every_method_call_by_name(patch_block, :url).first
url_string = parameters(url_node).first
patch_problems(url_string)
end

inline_patches = find_every_method_call_by_name(body, :patch)
inline_patches.each { |patch| inline_patch_problems(patch) }

if inline_patches.empty? && patch_end?
offending_patch_end_node(node)
problem "patch is missing 'DATA'"
end

patches_node = find_method_def(body, :patches)
return if patches_node.nil?

Expand Down Expand Up @@ -84,6 +94,30 @@ def patch_problems(patch)
#{patch_url}
EOS
end

def inline_patch_problems(patch)
return unless patch_data?(patch) && !patch_end?

offending_node(patch)
problem "patch is missing '__END__'"
end

def_node_search :patch_data?, <<~AST
(send nil? :patch (:sym :DATA))
AST

def patch_end?
/^__END__$/.match?(@full_source_content)
end

def offending_patch_end_node(node)
@offensive_node = node
@source_buf = source_buffer(node)
@line_no = node.loc.last_line + 1
@column = 0
@length = 7 # "__END__".size
@offense_source_range = source_range(@source_buf, @line_no, @column, @length)
end
end
end
end
Expand Down
41 changes: 0 additions & 41 deletions Library/Homebrew/test/dev-cmd/audit_spec.rb
Expand Up @@ -41,8 +41,6 @@ class #{Formulary.class_s(name)} < Formula
url "https://www.brew.sh/valid-1.0.tar.gz"
RUBY

expect(ft).not_to have_data
expect(ft).not_to have_end
expect(ft).to have_trailing_newline

expect(ft =~ /\burl\b/).to be_truthy
Expand All @@ -55,20 +53,6 @@ class #{Formulary.class_s(name)} < Formula
ft = formula_text "newline"
expect(ft).to have_trailing_newline
end

specify "#data?" do
ft = formula_text "data", <<~RUBY
patch :DATA
RUBY

expect(ft).to have_data
end

specify "#end?" do
ft = formula_text "end", "", patch: "__END__\na patch here"
expect(ft).to have_end
expect(ft.without_patch).to eq("class End < Formula\n \nend")
end
end

describe FormulaAuditor do
Expand Down Expand Up @@ -96,31 +80,6 @@ class Foo < Formula
end

describe "#audit_file" do
specify "DATA but no __END__" do
fa = formula_auditor "foo", <<~RUBY
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
patch :DATA
end
RUBY

fa.audit_file
expect(fa.problems).to eq(["'DATA' was found, but no '__END__'"])
end

specify "__END__ but no DATA" do
fa = formula_auditor "foo", <<~RUBY
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
end
__END__
a patch goes here
RUBY

fa.audit_file
expect(fa.problems).to eq(["'__END__' was found, but 'DATA' is not used"])
end

specify "no issue" do
fa = formula_auditor "foo", <<~RUBY
class Foo < Formula
Expand Down
47 changes: 47 additions & 0 deletions Library/Homebrew/test/rubocops/patches_spec.rb
Expand Up @@ -163,6 +163,53 @@ def patches
end
end

context "When auditing inline patches" do
it "reports no offenses for valid inline patches" do
expect_no_offenses(<<~RUBY)
class Foo < Formula
url 'https://brew.sh/foo-1.0.tgz'
patch :DATA
end
__END__
patch content here
RUBY
end

it "reports no offenses for valid nested inline patches" do
expect_no_offenses(<<~RUBY)
class Foo < Formula
url 'https://brew.sh/foo-1.0.tgz'
stable do
patch :DATA
end
end
__END__
patch content here
RUBY
end

it "reports an offense when DATA is found with no __END__" do
expect_offense(<<~RUBY)
class Foo < Formula
url 'https://brew.sh/foo-1.0.tgz'
patch :DATA
^^^^^^^^^^^ patch is missing '__END__'
end
RUBY
end

it "reports an offense when __END__ is found with no DATA" do
expect_offense(<<~RUBY)
class Foo < Formula
url 'https://brew.sh/foo-1.0.tgz'
end
__END__
^^^^^^^ patch is missing 'DATA'
patch content here
RUBY
end
end

context "When auditing external patches" do
it "Patch URLs" do
patch_urls = [
Expand Down