Merge pull request #8564 from reitermarkus/rubocop-json

Output GitHub Actions annotations for `brew style`.

Library/Homebrew/dev-cmd/audit.rb

@@ -117,7 +117,7 @@ module Homebrew
     end
 
     # Check style in a single batch run up front for performance
-    style_results = Style.check_style_json(style_files, options) if style_files
+    style_offenses = Style.check_style_json(style_files, options) if style_files
     # load licenses
     spdx_license_data = SPDX.license_data
     spdx_exception_data = SPDX.exception_data
@@ -134,7 +134,7 @@ module Homebrew
         spdx_license_data:   spdx_license_data,
         spdx_exception_data: spdx_exception_data,
       }
-      options[:style_offenses] = style_results.file_offenses(f.path) if style_results
+      options[:style_offenses] = style_offenses.for_path(f.path) if style_offenses
       options[:display_cop_names] = args.display_cop_names?
       options[:build_stable] = args.build_stable?
 

Library/Homebrew/style.rb

@@ -12,10 +12,17 @@ module Homebrew
     # Checks style for a list of files, printing simple RuboCop output.
     # Returns true if violations were found, false otherwise.
     def check_style_and_print(files, **options)
-      check_style_impl(files, :print, **options)
+      success = check_style_impl(files, :print, **options)
+
+      if ENV["GITHUB_ACTIONS"] && !success
+        offenses = check_style_json(files, **options)
+        puts offenses.to_github_annotations
+      end
+
+      success
     end
 
-    # Checks style for a list of files, returning results as a RubocopResults
+    # Checks style for a list of files, returning results as `Offenses`
     # object parsed from its JSON output.
     def check_style_json(files, **options)
       check_style_impl(files, :json, **options)
@@ -49,7 +56,7 @@ module Homebrew
       end
 
       if output_type == :json
-        RubocopResults.new(rubocop_result + shellcheck_result)
+        Offenses.new(rubocop_result + shellcheck_result)
       else
         rubocop_result && shellcheck_result
       end
@@ -161,12 +168,12 @@ module Homebrew
         system shellcheck, "--format=tty", *args
         $CHILD_STATUS.success?
       when :json
-        result = system_command shellcheck, args: ["--format=json1", *args]
+        result = system_command shellcheck, args: ["--format=json", *args]
         json = json_result!(result)
 
         # Convert to same format as RuboCop offenses.
-        json["comments"].group_by { |v| v["file"] }
+        json.group_by { |v| v["file"] }
-                        .map do |k, v|
+            .map do |k, v|
           {
             "path"     => k,
             "offenses" => v.map do |o|
@@ -208,25 +215,51 @@ module Homebrew
       JSON.parse(result.stdout)
     end
 
-    # Result of a RuboCop run.
+    # Collection of style offenses.
-    class RubocopResults
+    class Offenses
-      def initialize(files)
+      include Enumerable
-        @file_offenses = {}
+
-        files.each do |f|
+      def initialize(paths)
+        @offenses = {}
+        paths.each do |f|
           next if f["offenses"].empty?
 
-          file = File.realpath(f["path"])
+          path = Pathname(f["path"]).realpath
-          @file_offenses[file] = f["offenses"].map { |x| RubocopOffense.new(x) }
+          @offenses[path] = f["offenses"].map { |x| Offense.new(x) }
         end
       end
 
-      def file_offenses(path)
+      def for_path(path)
-        @file_offenses.fetch(path.to_s, [])
+        @offenses.fetch(Pathname(path), [])
+      end
+
+      def each(*args, &block)
+        @offenses.each(*args, &block)
+      end
+
+      def to_github_annotations
+        workspace = ENV["GITHUB_WORKSPACE"]
+        return [] if workspace.blank?
+
+        workspace = Pathname(workspace).realpath
+
+        @offenses.flat_map do |path, offenses|
+          relative_path = path.relative_path_from(workspace)
+
+          # Only generate annotations for paths relative to the `GITHUB_WORKSPACE` directory.
+          next [] if relative_path.descend.next.to_s == ".."
+
+          offenses.map do |o|
+            line = o.location.line
+            column = o.location.line
+            GitHub::Actions::Annotation.new(:error, o.message, file: relative_path, line: line, column: column)
+          end
+        end
       end
     end
 
-    # A RuboCop offense.
+    # A style offense.
-    class RubocopOffense
+    class Offense
       attr_reader :severity, :message, :corrected, :location, :cop_name
 
       def initialize(json)
@@ -234,7 +267,7 @@ module Homebrew
         @message = json["message"]
         @cop_name = json["cop_name"]
         @corrected = json["corrected"]
-        @location = RubocopLineLocation.new(json["location"])
+        @location = LineLocation.new(json["location"])
       end
 
       def severity_code
@@ -259,8 +292,8 @@ module Homebrew
       end
     end
 
-    # Source location of a RuboCop offense.
+    # Source location of a style offense.
-    class RubocopLineLocation
+    class LineLocation
       attr_reader :line, :column, :length
 
       def initialize(json)

Library/Homebrew/test/style_spec.rb

@@ -20,7 +20,7 @@ describe Homebrew::Style do
   describe ".check_style_json" do
     let(:dir) { mktmpdir }
 
-    it "returns RubocopResults when RuboCop reports offenses" do
+    it "returns offenses when RuboCop reports offenses" do
       formula = dir/"my-formula.rb"
 
       formula.write <<~'EOS'
@@ -29,9 +29,9 @@ describe Homebrew::Style do
         end
       EOS
 
-      rubocop_result = described_class.check_style_json([formula])
+      style_offenses = described_class.check_style_json([formula])
 
-      expect(rubocop_result.file_offenses(formula.realpath.to_s).map(&:message))
+      expect(style_offenses.for_path(formula.realpath).map(&:message))
         .to include("Extra empty line detected at class body beginning.")
     end
 
@@ -53,11 +53,11 @@ describe Homebrew::Style do
           end
         end
       EOS
-      rubocop_result = described_class.check_style_json(
+      style_offenses = described_class.check_style_json(
         [formula],
         fix: true, only_cops: ["FormulaAudit/DependencyOrder"],
       )
-      offense_string = rubocop_result.file_offenses(formula.realpath).first.to_s
+      offense_string = style_offenses.for_path(formula.realpath).first.to_s
       expect(offense_string).to match(/\[Corrected\]/)
     end
   end
@@ -70,9 +70,9 @@ describe Homebrew::Style do
       # but not regular, cop violations
       target_file = HOMEBREW_LIBRARY_PATH/"utils.rb"
 
-      rubocop_result = described_class.check_style_and_print([target_file])
+      style_result = described_class.check_style_and_print([target_file])
 
-      expect(rubocop_result).to eq true
+      expect(style_result).to eq true
     end
   end
 end

Library/Homebrew/test/utils/github/actions_spec.rb

@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+require "utils/github/actions"
+
+describe GitHub::Actions::Annotation do
+  let(:message) { "lorem ipsum" }
+
+  describe "#new" do
+    it "fails when the type is wrong" do
+      expect {
+        described_class.new(:fatal, message)
+      }.to raise_error(ArgumentError)
+    end
+  end
+
+  describe "#to_s" do
+    it "escapes newlines" do
+      annotation = described_class.new(:warning, <<~EOS)
+        lorem
+        ipsum
+      EOS
+
+      expect(annotation.to_s).to eq "::warning::lorem%0Aipsum%0A"
+    end
+
+    it "allows specifying the file" do
+      annotation = described_class.new(:warning, "lorem ipsum", file: "file.txt")
+
+      expect(annotation.to_s).to eq "::warning file=file.txt::lorem ipsum"
+    end
+
+    it "allows specifying the file and line" do
+      annotation = described_class.new(:error, "lorem ipsum", file: "file.txt", line: 3)
+
+      expect(annotation.to_s).to eq "::error file=file.txt,line=3::lorem ipsum"
+    end
+
+    it "allows specifying the file, line and column" do
+      annotation = described_class.new(:error, "lorem ipsum", file: "file.txt", line: 3, column: 18)
+
+      expect(annotation.to_s).to eq "::error file=file.txt,line=3,col=18::lorem ipsum"
+    end
+  end
+end

Library/Homebrew/utils/github.rb

@@ -2,6 +2,7 @@
 
 require "tempfile"
 require "uri"
+require "utils/github/actions"
 
 # Helper functions for interacting with the GitHub API.
 #

Library/Homebrew/utils/github/actions.rb

@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+
+module GitHub
+  # Helper functions for interacting with GitHub Actions.
+  #
+  # @api private
+  module Actions
+    def self.escape(string)
+      string.gsub(/\r/, "%0D")
+            .gsub(/\n/, "%0A")
+            .gsub(/]/, "%5D")
+            .gsub(/;/, "%3B")
+    end
+
+    # Helper class for formatting annotations on GitHub Actions.
+    class Annotation
+      def initialize(type, message, file: nil, line: nil, column: nil)
+        raise ArgumentError, "Unsupported type: #{type.inspect}" unless [:warning, :error].include?(type)
+
+        @type = type
+        @message = String(message)
+        @file = Pathname(file) if file
+        @line = Integer(line) if line
+        @column = Integer(column) if column
+      end
+
+      def to_s
+        file = "file=#{Actions.escape(@file.to_s)}" if @file
+        line = "line=#{@line}" if @line
+        column = "col=#{@column}" if @column
+
+        metadata = [*file, *line, *column].join(",").presence&.prepend(" ")
+
+        "::#{@type}#{metadata}::#{Actions.escape(@message)}"
+      end
+    end
+  end
+end

bin/brew

@@ -94,7 +94,7 @@ then
   # Filter all but the specific variables.
   for VAR in HOME SHELL PATH TERM TERMINFO COLUMNS DISPLAY LOGNAME USER CI SSH_AUTH_SOCK SUDO_ASKPASS \
              http_proxy https_proxy ftp_proxy no_proxy all_proxy HTTPS_PROXY FTP_PROXY ALL_PROXY \
-             GITHUB_ACTIONS GITHUB_ACTIONS_HOMEBREW_SELF_HOSTED \
+             GITHUB_ACTIONS GITHUB_WORKSPACE GITHUB_ACTIONS_HOMEBREW_SELF_HOSTED \
              GITHUB_REPOSITORY GITHUB_RUN_ID GITHUB_SHA GITHUB_HEAD_REF GITHUB_BASE_REF GITHUB_REF \
              "${!HOMEBREW_@}"
   do