Merge pull request #8684 from reitermarkus/audit-annotations

Output annotations for `brew audit`.

Library/Homebrew/dev-cmd/audit.rb

@@ -3,6 +3,7 @@
 require "formula"
 require "formula_versions"
 require "utils/curl"
+require "utils/github/actions"
 require "utils/shared_audits"
 require "utils/spdx"
 require "extend/ENV"
@@ -142,7 +143,6 @@ module Homebrew
       fa.audit
       next if fa.problems.empty? && fa.new_formula_problems.empty?
 
-      fa.problems
       formula_count += 1
       problem_count += fa.problems.size
       problem_lines = format_problem_lines(fa.problems)
@@ -153,6 +153,15 @@ module Homebrew
       else
         puts "#{f.full_name}:", problem_lines.map { |s| "  #{s}" }
       end
+
+      next unless ENV["GITHUB_ACTIONS"]
+
+      (fa.problems + fa.new_formula_problems).each do |message:, location:|
+        annotation = GitHub::Actions::Annotation.new(
+          :error, message, file: f.path, line: location&.line, column: location&.column
+        )
+        puts annotation if annotation.relevant?
+      end
     end
 
     new_formula_problem_count += new_formula_problem_lines.size
@@ -169,7 +178,12 @@ module Homebrew
   end
 
   def format_problem_lines(problems)
-    problems.uniq.map { |p| "* #{p.chomp.gsub("\n", "\n    ")}" }
+    problems.uniq
+            .map { |message:, location:| format_problem(message, location) }
+  end
+
+  def format_problem(message, location)
+    "* #{location&.to_s&.dup&.concat(": ")}#{message.chomp.gsub("\n", "\n    ")}"
   end
 
   class FormulaText
@@ -238,7 +252,12 @@ module Homebrew
       return unless @style_offenses
 
       @style_offenses.each do |offense|
-        problem offense.to_s(display_cop_name: @display_cop_names)
+        correction_status = "#{Tty.green}[Corrected]#{Tty.reset} " if offense.corrected?
+
+        cop_name = "#{offense.cop_name}: " if @display_cop_names
+        message = "#{cop_name}#{correction_status}#{offense.message}"
+
+        problem message, location: offense.location
       end
     end
 
@@ -701,9 +720,6 @@ module Homebrew
       "libepoxy"            => "1.5",
     }.freeze
 
-    # version_prefix = stable_version_string.sub(/\d+$/, "")
-    # version_prefix = stable.version.major_minor
-
     def audit_specs
       problem "Head-only (no stable download)" if head_only?(formula)
 
@@ -712,15 +728,17 @@ module Homebrew
         next unless spec = formula.send(spec_name)
 
         ra = ResourceAuditor.new(spec, spec_name, online: @online, strict: @strict).audit
-        problems.concat ra.problems.map { |problem| "#{name}: #{problem}" }
+        ra.problems.each do |message|
+          problem "#{name}: #{message}"
+        end
 
         spec.resources.each_value do |resource|
           problem "Resource name should be different from the formula name" if resource.name == formula.name
 
           ra = ResourceAuditor.new(resource, spec_name, online: @online, strict: @strict).audit
-          problems.concat ra.problems.map { |problem|
+          ra.problems.each do |message|
-            "#{name} resource #{resource.name.inspect}: #{problem}"
+            problem "#{name} resource #{resource.name.inspect}: #{message}"
-          }
+          end
         end
 
         next if spec.patches.empty?
@@ -952,12 +970,12 @@ module Homebrew
 
     private
 
-    def problem(p)
+    def problem(message, location: nil)
-      @problems << p
+      @problems << ({ message: message, location: location })
     end
 
-    def new_formula_problem(p)
+    def new_formula_problem(message, location: nil)
-      @new_formula_problems << p
+      @new_formula_problems << ({ message: message, location: location })
     end
 
     def head_only?(formula)

Library/Homebrew/style.rb

@@ -264,36 +264,18 @@ module Homebrew
       def corrected?
         @corrected
       end
-
-      def correction_status
-        "[Corrected] " if corrected?
-      end
-
-      def to_s(display_cop_name: false)
-        if display_cop_name
-          "#{severity_code}: #{location.to_short_s}: #{cop_name}: " \
-          "#{Tty.green}#{correction_status}#{Tty.reset}#{message}"
-        else
-          "#{severity_code}: #{location.to_short_s}: #{Tty.green}#{correction_status}#{Tty.reset}#{message}"
-        end
-      end
     end
 
     # Source location of a style offense.
     class LineLocation
-      attr_reader :line, :column, :length
+      attr_reader :line, :column
 
       def initialize(json)
         @line = json["line"]
         @column = json["column"]
-        @length = json["length"]
       end
 
       def to_s
-        "#{line}: col #{column} (#{length} chars)"
-      end
-
-      def to_short_s
         "#{line}: col #{column}"
       end
     end

Library/Homebrew/test/dev-cmd/audit_spec.rb

@@ -113,7 +113,7 @@ module Homebrew
         RUBY
 
         fa.audit_license
-        expect(fa.problems.first).to match "Formulae in homebrew/core must specify a license."
+        expect(fa.problems.first[:message]).to match "Formulae in homebrew/core must specify a license."
       end
 
       it "detects if license is not a standard spdx-id" do
@@ -125,7 +125,7 @@ module Homebrew
         RUBY
 
         fa.audit_license
-        expect(fa.problems.first).to match <<~EOS
+        expect(fa.problems.first[:message]).to match <<~EOS
           Formula foo contains non-standard SPDX licenses: ["zzz"].
           For a list of valid licenses check: https://spdx.org/licenses/
         EOS
@@ -140,7 +140,7 @@ module Homebrew
         RUBY
 
         fa.audit_license
-        expect(fa.problems.first).to match <<~EOS
+        expect(fa.problems.first[:message]).to match <<~EOS
           Formula foo contains deprecated SPDX licenses: ["GPL-1.0"].
           You may need to add `-only` or `-or-later` for GNU licenses (e.g. `GPL`, `LGPL`, `AGPL`, `GFDL`).
           For a list of valid licenses check: https://spdx.org/licenses/
@@ -156,7 +156,7 @@ module Homebrew
         RUBY
 
         fa.audit_license
-        expect(fa.problems.first).to match <<~EOS
+        expect(fa.problems.first[:message]).to match <<~EOS
           Formula foo contains non-standard SPDX licenses: ["zzz"].
           For a list of valid licenses check: https://spdx.org/licenses/
         EOS
@@ -171,7 +171,7 @@ module Homebrew
         RUBY
 
         fa.audit_license
-        expect(fa.problems.first).to match <<~EOS
+        expect(fa.problems.first[:message]).to match <<~EOS
           Formula foo contains non-standard SPDX licenses: ["zzz"].
           For a list of valid licenses check: https://spdx.org/licenses/
         EOS
@@ -186,7 +186,7 @@ module Homebrew
         RUBY
 
         fa.audit_license
-        expect(fa.problems.first).to match <<~EOS
+        expect(fa.problems.first[:message]).to match <<~EOS
           Formula foo contains deprecated SPDX licenses: ["GPL-1.0"].
           You may need to add `-only` or `-or-later` for GNU licenses (e.g. `GPL`, `LGPL`, `AGPL`, `GFDL`).
           For a list of valid licenses check: https://spdx.org/licenses/
@@ -351,7 +351,7 @@ module Homebrew
                              spdx_license_data: spdx_license_data, spdx_exception_data: spdx_exception_data
 
         fa.audit_license
-        expect(fa.problems.first).to match <<~EOS
+        expect(fa.problems.first[:message]).to match <<~EOS
           Formula cask contains invalid or deprecated SPDX license exceptions: ["zzz"].
           For a list of valid license exceptions check:
             https://spdx.org/licenses/exceptions-index.html
@@ -370,7 +370,7 @@ module Homebrew
                              spdx_license_data: spdx_license_data, spdx_exception_data: spdx_exception_data
 
         fa.audit_license
-        expect(fa.problems.first).to match <<~EOS
+        expect(fa.problems.first[:message]).to match <<~EOS
           Formula cask contains invalid or deprecated SPDX license exceptions: ["#{deprecated_spdx_exception}"].
           For a list of valid license exceptions check:
             https://spdx.org/licenses/exceptions-index.html
@@ -418,7 +418,8 @@ module Homebrew
                              online: true, core_tap: true, new_formula: true
 
         fa.audit_license
-        expect(fa.problems.first).to match "Formula license [\"0BSD\"] does not match GitHub license [\"GPL-3.0\"]."
+        expect(fa.problems.first[:message])
+          .to eq 'Formula license ["0BSD"] does not match GitHub license ["GPL-3.0"].'
       end
 
       it "checks online and detects that an array of license does not contain "\
@@ -434,7 +435,7 @@ module Homebrew
                              online: true, core_tap: true, new_formula: true
 
         fa.audit_license
-        expect(fa.problems.first).to match "Formula license [\"0BSD\", \"MIT\"] "\
+        expect(fa.problems.first[:message]).to match "Formula license [\"0BSD\", \"MIT\"] "\
           "does not match GitHub license [\"GPL-3.0\"]."
       end
 
@@ -465,7 +466,7 @@ module Homebrew
         RUBY
 
         fa.audit_file
-        expect(fa.problems).to eq([])
+        expect(fa.problems).to be_empty
       end
     end
 
@@ -481,7 +482,7 @@ module Homebrew
         RUBY
 
         fa.audit_github_repository
-        expect(fa.problems).to eq([])
+        expect(fa.problems).to be_empty
       end
     end
 
@@ -495,7 +496,7 @@ module Homebrew
         RUBY
 
         fa.audit_github_repository_archived
-        expect(fa.problems).to eq([])
+        expect(fa.problems).to be_empty
       end
     end
 
@@ -509,7 +510,7 @@ module Homebrew
         RUBY
 
         fa.audit_gitlab_repository
-        expect(fa.problems).to eq([])
+        expect(fa.problems).to be_empty
       end
     end
 
@@ -523,7 +524,7 @@ module Homebrew
         RUBY
 
         fa.audit_gitlab_repository_archived
-        expect(fa.problems).to eq([])
+        expect(fa.problems).to be_empty
       end
     end
 
@@ -537,7 +538,7 @@ module Homebrew
         RUBY
 
         fa.audit_bitbucket_repository
-        expect(fa.problems).to eq([])
+        expect(fa.problems).to be_empty
       end
     end
 
@@ -604,7 +605,9 @@ module Homebrew
             fa.audit_deps
           end
 
-          its(:new_formula_problems) { are_expected.to match([/is provided by macOS/]) }
+          its(:new_formula_problems) {
+            are_expected.to include(a_hash_including(message: a_string_matching(/is provided by macOS/)))
+          }
         end
       end
     end
@@ -613,7 +616,7 @@ module Homebrew
       subject {
         fa = described_class.new(Formulary.factory(formula_path), git: true)
         fa.audit_revision_and_version_scheme
-        fa.problems.first
+        fa.problems.first&.fetch(:message)
       }
 
       let(:origin_tap_path) { Tap::TAP_DIRECTORY/"homebrew/homebrew-foo" }
@@ -829,7 +832,7 @@ module Homebrew
 
         fa.audit_versioned_keg_only
 
-        expect(fa.problems.first)
+        expect(fa.problems.first[:message])
           .to match("Versioned formulae in homebrew/core should use `keg_only :versioned_formula`")
       end
 
@@ -844,7 +847,7 @@ module Homebrew
 
         fa.audit_versioned_keg_only
 
-        expect(fa.problems.first)
+        expect(fa.problems.first[:message])
           .to match("Versioned formulae in homebrew/core should use `keg_only :versioned_formula`")
       end
 
@@ -859,7 +862,7 @@ module Homebrew
 
         fa.audit_versioned_keg_only
 
-        expect(fa.problems).to eq([])
+        expect(fa.problems).to be_empty
       end
     end
 

Library/Homebrew/test/style_spec.rb

@@ -34,32 +34,6 @@ describe Homebrew::Style do
       expect(style_offenses.for_path(formula.realpath).map(&:message))
         .to include("Extra empty line detected at class body beginning.")
     end
-
-    it "corrected offense output format" do
-      formula = dir/"my-formula-2.rb"
-
-      formula.write <<~EOS
-        class MyFormula2 < Formula
-          desc "Test formula"
-          homepage "https://foo.org"
-          url "https://foo.org/foo-1.7.5.tgz"
-          sha256 "cc692fb9dee0cc288757e708fc1a3b6b56ca1210ca181053a371cb11746969da"
-
-          depends_on "foo"
-          depends_on "bar-config" => :build
-
-          test do
-            assert_equal 5, 5
-          end
-        end
-      EOS
-      style_offenses = described_class.check_style_json(
-        [formula],
-        fix: true, only_cops: ["FormulaAudit/DependencyOrder"],
-      )
-      offense_string = style_offenses.for_path(formula.realpath).first.to_s
-      expect(offense_string).to match(/\[Corrected\]/)
-    end
   end
 
   describe ".check_style_and_print" do