diff --git a/lib/github.rb b/lib/github.rb new file mode 100644 index 0000000..d7e6c1c --- /dev/null +++ b/lib/github.rb @@ -0,0 +1,80 @@ +require "json" +require "net/http" + +module Github + HttpError = Class.new(StandardError) + + class File + RANGE_INFORMATION_LINE = /^@@ .+\+(?\d+),/.freeze + MODIFIED_LINE = /^\+(?!\+|\+)/.freeze + NOT_REMOVED_LINE = /^[^-]/.freeze + + def initialize(file) + @file = file + end + + def path + @file.fetch("filename") + end + + def changed_lines + patch = @file.fetch("patch") || "" + line_number = 0 + patch.each_line.with_object([]) do |line_content, lines| + case line_content + when RANGE_INFORMATION_LINE + line_number = Regexp.last_match[:line_number].to_i + when MODIFIED_LINE + lines << line_number + line_number += 1 + when NOT_REMOVED_LINE + line_number += 1 + end + end + end + end + + CONNECTION = Net::HTTP.new("api.github.com", 443).tap { |http| http.use_ssl = true } + REQUEST_METHOD_TO_CLASS = { + get: Net::HTTP::Get, + patch: Net::HTTP::Patch, + post: Net::HTTP::Post, + delete: Net::HTTP::Delete, + }.freeze + + REQUEST_METHOD_TO_CLASS.each do |method, klass| + define_singleton_method(method) do |path, params = nil| + request(klass, path, params) + end + + define_singleton_method("#{method}!") do |path, params = nil| + response = request(klass, path, params) + raise HttpError, "status: #{response.code}, body: #{response.body}" unless response.is_a?(Net::HTTPSuccess) + + JSON.parse(response.body) if response.body + end + end + + def self.pull_request_ruby_files(owner_and_repository, pr_number) + changed_files = [] + 1.step do |page| + files = Github.get!("/repos/#{owner_and_repository}/pulls/#{pr_number}/files?per_page=100&page=#{page}") + changed_files.concat(files) + break if files.length < 100 + end + changed_files + .reject { |file| file.fetch("status") == "removed" } + .select { |file| file.fetch("filename").end_with?(".rb") } + .map { |file| File.new(file) } + end + + def self.request(request_class, path, params = nil) + request = request_class.new(path) + request.content_type = "application/json" + request.body = params&.to_json + request["Authorization"] = "Bearer #{ENV.fetch('GITHUB_TOKEN')}" + request["Accept"] = "application/vnd.github.v3+json" + + CONNECTION.request(request) + end +end diff --git a/rubocop.rb b/rubocop.rb index 548a9a8..ce9c77f 100644 --- a/rubocop.rb +++ b/rubocop.rb @@ -23,43 +23,7 @@ # Script -require "json" -require "net/http" - -module Github - HttpError = Class.new(StandardError) - - CONNECTION = Net::HTTP.new("api.github.com", 443).tap { |http| http.use_ssl = true } - REQUEST_METHOD_TO_CLASS = { - get: Net::HTTP::Get, - patch: Net::HTTP::Patch, - post: Net::HTTP::Post, - delete: Net::HTTP::Delete, - }.freeze - - REQUEST_METHOD_TO_CLASS.each do |method, klass| - define_singleton_method(method) do |path, params = nil| - request(klass, path, params) - end - - define_singleton_method("#{method}!") do |path, params = nil| - response = request(klass, path, params) - raise HttpError, "status: #{response.code}, body: #{response.body}" unless response.is_a?(Net::HTTPSuccess) - - JSON.parse(response.body) if response.body - end - end - - def self.request(request_class, path, params = nil) - request = request_class.new(path) - request.content_type = "application/json" - request.body = params&.to_json - request["Authorization"] = "Bearer #{ENV.fetch('GITHUB_TOKEN')}" - request["Accept"] = "application/vnd.github.v3+json" - - CONNECTION.request(request) - end -end +require_relative "lib/github" # Figure out which ruby files have changed and run Rubocop on them @@ -67,26 +31,15 @@ def self.request(request_class, path, params = nil) pr_number = github_event.fetch("pull_request").fetch("number") owner_and_repository = ENV.fetch("GITHUB_REPOSITORY") -changed_files = [] -1.step do |page| - files = Github.get!("/repos/#{owner_and_repository}/pulls/#{pr_number}/files?per_page=100&page=#{page}") - changed_files.concat(files) - break if files.length < 100 -end -changed_ruby_files = changed_files - .reject { |file| file.fetch("status") == "removed" } - .select { |file| file.fetch("filename").end_with?(".rb") } - .map { |file| file.fetch("filename") } +changed_ruby_files = Github.pull_request_ruby_files(owner_and_repository, pr_number) # JSON reference: https://docs.rubocop.org/rubocop/formatters.html#json-formatter files_with_offenses = if changed_ruby_files.any? - command = "rubocop #{changed_ruby_files.join(' ')} --format json --force-exclusion" + command = "rubocop #{changed_ruby_files.map(&:path).join(' ')} --format json --force-exclusion" puts "Running rubocop with: #{command}" - rubocop_output = `#{command}` - - JSON.parse(rubocop_output).fetch("files") + JSON.parse(`#{command}`).fetch("files") else puts "No changed Ruby files, skipping rubocop" @@ -126,6 +79,11 @@ def self.request(request_class, path, params = nil) # Comment on the pull request with the offenses found +def in_diff?(changed_files, path, line) + file = changed_files.find { |changed_file| changed_file.path == path } + file&.changed_lines&.include?(line) +end + offences_outside_diff = [] files_with_offenses.each do |file| @@ -166,26 +124,21 @@ def self.request(request_class, path, params = nil) puts "Updating comment #{comment_id} on #{path} line #{line}" Github.patch("/repos/#{owner_and_repository}/pulls/comments/#{comment_id}", body: body) - else + elsif in_diff?(changed_ruby_files, path, line) puts "Commenting on #{path} line #{line}" # Somehow the commit_id should not be just the HEAD SHA: https://stackoverflow.com/a/71431370/1075108 commit_id = github_event.fetch("pull_request").fetch("head").fetch("sha") - response = Github.post( + Github.post!( "/repos/#{owner_and_repository}/pulls/#{pr_number}/comments", body: body, path: path, commit_id: commit_id, line: line, ) - - # Rubocop might hit errors on lines which are not part of the diff and thus cannot be commented on. - if response.code == "422" && response.body.include?("line must be part of the diff") - puts "Deferring comment on #{path} line #{line} because it isn't part of the diff" - - offences_outside_diff << { path: path, line: line, message: message } - end + else + offences_outside_diff << { path: path, line: line, message: message } end end end @@ -196,7 +149,9 @@ def self.request(request_class, path, params = nil) existing_separate_comment = separate_comments.find do |comment| comment.fetch("body").include?("rubocop-comment-id: outside-diff") end + if offences_outside_diff.any? + puts "Found #{offences_outside_diff.count} offenses outside of the diff" body = <<~BODY @@ -215,8 +170,8 @@ def self.request(request_class, path, params = nil) if existing_separate_comment.fetch("body") == body puts "Skipping unchanged separate comment #{existing_comment_id}" else - puts "Updating comment #{existing_comment_id} on pull request" - Github.patch!("/repos/#{owner_and_repository}/pulls/comments/#{existing_comment_id}", body: body) + puts "Updating separate comment #{existing_comment_id}" + Github.patch!("/repos/#{owner_and_repository}/issues/comments/#{existing_comment_id}", body: body) end else puts "Commenting on pull request with offenses found outside the diff" @@ -227,6 +182,8 @@ def self.request(request_class, path, params = nil) existing_comment_id = existing_separate_comment.fetch("id") puts "Deleting resolved separate comment #{existing_comment_id}" Github.delete("/repos/#{owner_and_repository}/issues/comments/#{existing_comment_id}") +else + puts "No offenses found outside of the diff and no existing separate comment to remove" end # Fail the build if there were any offenses