From 644ae115f56de90a72ab0fac68d5bdac906cf82e Mon Sep 17 00:00:00 2001 From: Frank Lam Date: Thu, 25 Jun 2020 22:29:34 +0800 Subject: [PATCH 1/4] Prepare inline patch specs --- .../Homebrew/test/rubocops/patches_spec.rb | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/Library/Homebrew/test/rubocops/patches_spec.rb b/Library/Homebrew/test/rubocops/patches_spec.rb index f2a0fc25a7..6b0fc2a1a5 100644 --- a/Library/Homebrew/test/rubocops/patches_spec.rb +++ b/Library/Homebrew/test/rubocops/patches_spec.rb @@ -163,6 +163,40 @@ describe RuboCop::Cop::FormulaAudit::Patches do 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 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 = [ From afb844538048055d3a5ce42ce051a21fb3ff8cf2 Mon Sep 17 00:00:00 2001 From: Frank Lam Date: Sat, 27 Jun 2020 01:10:52 +0800 Subject: [PATCH 2/4] Add inline patch checks to patches cop --- Library/Homebrew/rubocops/patches.rb | 36 +++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/rubocops/patches.rb b/Library/Homebrew/rubocops/patches.rb index eeae550780..5a0e0effae 100644 --- a/Library/Homebrew/rubocops/patches.rb +++ b/Library/Homebrew/rubocops/patches.rb @@ -8,7 +8,9 @@ module RuboCop 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 @@ -16,6 +18,14 @@ module RuboCop patch_problems(url_string) end + inline_patches = find_method_calls_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? @@ -84,6 +94,30 @@ module RuboCop #{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 From b2eafdf11d1bec99479e9a097f68cf5be31c2146 Mon Sep 17 00:00:00 2001 From: Frank Lam Date: Sat, 27 Jun 2020 02:37:57 +0800 Subject: [PATCH 3/4] Remove patch checks from audit --- Library/Homebrew/dev-cmd/audit.rb | 14 ------- Library/Homebrew/test/dev-cmd/audit_spec.rb | 41 --------------------- 2 files changed, 55 deletions(-) diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb index aade8065e9..9210c975f9 100644 --- a/Library/Homebrew/dev-cmd/audit.rb +++ b/Library/Homebrew/dev-cmd/audit.rb @@ -168,14 +168,6 @@ module Homebrew @text.split("\n__END__").first end - def data? - /^[^#]*\bDATA\b/ =~ @text - end - - def end? - /^__END__$/ =~ @text - end - def trailing_newline? /\Z\n/ =~ @text end @@ -234,12 +226,6 @@ module Homebrew 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)." diff --git a/Library/Homebrew/test/dev-cmd/audit_spec.rb b/Library/Homebrew/test/dev-cmd/audit_spec.rb index 0a437b20d6..992281d5c1 100644 --- a/Library/Homebrew/test/dev-cmd/audit_spec.rb +++ b/Library/Homebrew/test/dev-cmd/audit_spec.rb @@ -41,8 +41,6 @@ module Homebrew 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 @@ -55,20 +53,6 @@ module Homebrew 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 @@ -96,31 +80,6 @@ module Homebrew 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 From ffb1cc8e19abf6da810470d4ce266ab05fe69115 Mon Sep 17 00:00:00 2001 From: Frank Lam Date: Sat, 27 Jun 2020 03:13:50 +0800 Subject: [PATCH 4/4] Find patch nodes nested inside blocks --- Library/Homebrew/rubocops/patches.rb | 2 +- Library/Homebrew/test/rubocops/patches_spec.rb | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/rubocops/patches.rb b/Library/Homebrew/rubocops/patches.rb index 5a0e0effae..13cffc6dca 100644 --- a/Library/Homebrew/rubocops/patches.rb +++ b/Library/Homebrew/rubocops/patches.rb @@ -18,7 +18,7 @@ module RuboCop patch_problems(url_string) end - inline_patches = find_method_calls_by_name(body, :patch) + inline_patches = find_every_method_call_by_name(body, :patch) inline_patches.each { |patch| inline_patch_problems(patch) } if inline_patches.empty? && patch_end? diff --git a/Library/Homebrew/test/rubocops/patches_spec.rb b/Library/Homebrew/test/rubocops/patches_spec.rb index 6b0fc2a1a5..42883c8458 100644 --- a/Library/Homebrew/test/rubocops/patches_spec.rb +++ b/Library/Homebrew/test/rubocops/patches_spec.rb @@ -175,6 +175,19 @@ describe RuboCop::Cop::FormulaAudit::Patches do 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