From 9f915a6a623992552e72880a0ff4958509d4b411 Mon Sep 17 00:00:00 2001 From: Issy Long Date: Sat, 4 May 2024 22:04:04 +0100 Subject: [PATCH] Replace `FormulaTextAuditor` usage - Only two audits were using this: `audit_keg_only_reason` and `audit_text`, and they weren't using any of its text processing methods, so there's little reason to keep it around. - The "`keg_only_reason` shouldn't contain 'HOMEBREW_PREFIX'" audit can easily be replaced with a RuboCop since that's "just" text parsing. - The "tests should invoke binaries with `bin/`" audit had to stay as a FormulaAudit because it requires accessing attributes about the Formula like its name, aliases, which RuboCop can't get to, but it was easy to move the singular "read the text in the file" line from `FormulaTextAuditor`. --- Library/Homebrew/formula_auditor.rb | 13 +----- Library/Homebrew/formula_text_auditor.rb | 43 ------------------- Library/Homebrew/rubocops/text.rb | 11 ++++- Library/Homebrew/test/formula_auditor_spec.rb | 2 +- .../test/formula_text_auditor_spec.rb | 39 ----------------- 5 files changed, 11 insertions(+), 97 deletions(-) delete mode 100644 Library/Homebrew/formula_text_auditor.rb delete mode 100644 Library/Homebrew/test/formula_text_auditor_spec.rb diff --git a/Library/Homebrew/formula_auditor.rb b/Library/Homebrew/formula_auditor.rb index 67d61397ca93b..d8fcd03cb38f3 100644 --- a/Library/Homebrew/formula_auditor.rb +++ b/Library/Homebrew/formula_auditor.rb @@ -2,7 +2,6 @@ # frozen_string_literal: true require "deprecate_disable" -require "formula_text_auditor" require "formula_versions" require "resource_auditor" require "utils/shared_audits" @@ -32,7 +31,7 @@ def initialize(formula, options = {}) @core_tap = formula.tap&.core_tap? || options[:core_tap] @problems = [] @new_formula_problems = [] - @text = FormulaTextAuditor.new(formula.path) + @text = formula.path.open("rb", &:read) @specs = %w[stable head].filter_map { |s| formula.send(s) } @spdx_license_data = options[:spdx_license_data] @spdx_exception_data = options[:spdx_exception_data] @@ -510,16 +509,6 @@ def audit_relicensed_formulae "It must not be upgraded to version #{relicensed_version} or newer." end - def audit_keg_only_reason - return unless @core_tap - return unless formula.keg_only? - - keg_only_message = text.to_s.match(/keg_only\s+["'](.*)["']/)&.captures&.first - return unless keg_only_message&.include?("HOMEBREW_PREFIX") - - problem "`keg_only` reason should not include `HOMEBREW_PREFIX` as it creates confusing `brew info` output." - end - def audit_versioned_keg_only return unless @versioned_formula return unless @core_tap diff --git a/Library/Homebrew/formula_text_auditor.rb b/Library/Homebrew/formula_text_auditor.rb deleted file mode 100644 index 5600b037bff85..0000000000000 --- a/Library/Homebrew/formula_text_auditor.rb +++ /dev/null @@ -1,43 +0,0 @@ -# typed: true -# frozen_string_literal: true - -module Homebrew - # Auditor for checking common violations in {Formula} text content. - class FormulaTextAuditor - def initialize(path) - @text = path.open("rb", &:read) - @lines = @text.lines.to_a - end - - def without_patch - @text.split("\n__END__").first - end - - def trailing_newline? - /\Z\n/.match?(@text) - end - - def =~(other) - other =~ @text - end - - def include?(string) - @text.include? string - end - - sig { returns(String) } - def to_s - @text - end - - def line_number(regex, skip = 0) - index = @lines.drop(skip).index { |line| line.match?(regex) } - index ? index + 1 : nil - end - - def reverse_line_number(regex) - index = @lines.reverse.index { |line| line.match?(regex) } - index ? @lines.count - index : nil - end - end -end diff --git a/Library/Homebrew/rubocops/text.rb b/Library/Homebrew/rubocops/text.rb index 56601d0c174ba..3c6113c8b828b 100644 --- a/Library/Homebrew/rubocops/text.rb +++ b/Library/Homebrew/rubocops/text.rb @@ -30,8 +30,15 @@ def audit_formula(node, _class_node, _parent_class_node, body_node) problem "Formulae should not depend on both OpenSSL and LibreSSL (even optionally)." end - if formula_tap == "homebrew-core" && (depends_on?("veclibfort") || depends_on?("lapack")) - problem "Formulae in homebrew/core should use OpenBLAS as the default serial linear algebra library." + if formula_tap == "homebrew-core" + if depends_on?("veclibfort") || depends_on?("lapack") + problem "Formulae in homebrew/core should use OpenBLAS as the default serial linear algebra library." + end + + if find_node_method_by_name(body_node, :keg_only)&.source&.include?("HOMEBREW_PREFIX") + problem "`keg_only` reason should not include `HOMEBREW_PREFIX` " \ + "as it creates confusing `brew info` output." + end end unless method_called_ever?(body_node, :go_resource) diff --git a/Library/Homebrew/test/formula_auditor_spec.rb b/Library/Homebrew/test/formula_auditor_spec.rb index 930f8bb794850..f8103cda3eb73 100644 --- a/Library/Homebrew/test/formula_auditor_spec.rb +++ b/Library/Homebrew/test/formula_auditor_spec.rb @@ -1238,7 +1238,7 @@ class FooAT11 < Formula describe "#audit_conflicts" do before do - # We don't really test FormulaTextAuditor here + # We don't really test the formula text retrieval here allow(File).to receive(:open).and_return("") end diff --git a/Library/Homebrew/test/formula_text_auditor_spec.rb b/Library/Homebrew/test/formula_text_auditor_spec.rb deleted file mode 100644 index b754613b49ddd..0000000000000 --- a/Library/Homebrew/test/formula_text_auditor_spec.rb +++ /dev/null @@ -1,39 +0,0 @@ -# frozen_string_literal: true - -require "formula_text_auditor" - -RSpec.describe Homebrew::FormulaTextAuditor do - alias_matcher :have_trailing_newline, :be_trailing_newline - - let(:dir) { mktmpdir } - - def formula_text(name, body = nil) - path = dir/"#{name}.rb" - - path.write <<~RUBY - class #{Formulary.class_s(name)} < Formula - #{body} - end - RUBY - - described_class.new(path) - end - - specify "simple valid Formula" do - ft = formula_text "valid", <<~RUBY - url "https://www.brew.sh/valid-1.0.tar.gz" - RUBY - - expect(ft).to have_trailing_newline - - expect(ft =~ /\burl\b/).to be_truthy - expect(ft.line_number(/desc/)).to be_nil - expect(ft.line_number(/\burl\b/)).to eq(2) - expect(ft).to include("Valid") - end - - specify "#trailing_newline?" do - ft = formula_text "newline" - expect(ft).to have_trailing_newline - end -end