Merge pull request #7837 from FTLam11/7392-migrate-audit-rules-to-rubocop

audit: Move patch checks from brew audit to rubocop

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)."

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_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?
 
@@ -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

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

Library/Homebrew/test/rubocops/patches_spec.rb

@@ -163,6 +163,53 @@ 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 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 = [