Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ jobs:
rubocop:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
- uses: ruby/setup-ruby@v1
- uses: ./
with:
Expand Down
12 changes: 7 additions & 5 deletions lib/github.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
module Github
HttpError = Class.new(StandardError)

# The File class wraps the Github API response for files in a pull requests. Its main purpose
# is to provide the #changed_lines method which returns the line numbers of the lines which
# have changed in a file by parsing its patch attribute.
class File
RANGE_INFORMATION_LINE = /^@@ .+\+(?<line_number>\d+),/.freeze
MODIFIED_LINE = /^\+(?!\+|\+)/.freeze
Expand Down Expand Up @@ -42,23 +45,22 @@ def changed_lines
delete: Net::HTTP::Delete,
}.freeze

# Defines .get, .patch, .post, and .delete methods for making requests to the GitHub API.
# For successful requests, The JSON parsed body is returned, otherwise HttpError is raised.
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

# Returns Array of File objects for all files in a pull request which have the .rb extension.
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}")
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
Expand Down
52 changes: 33 additions & 19 deletions rubocop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,35 @@
# Setup

puts "::group::Installing Rubocop gems"
versioned_rubocop_gems =
if ENV.fetch("RUBOCOP_GEM_VERSIONS").downcase == "gemfile"
require "bundler"

rubocop_config_gems_without_prefix = %w[syntax_tree].to_set
if ENV.fetch("RUBOCOP_GEM_VERSIONS").downcase == "gemfile"
require "bundler"

Bundler::LockfileParser.new(Bundler.read_file("Gemfile.lock")).specs
.select { |spec| spec.name.start_with?("rubocop") || rubocop_config_gems_without_prefix.include?(spec.name) }
.map { |spec| "#{spec.name}:#{spec.version}" }
else
ENV.fetch("RUBOCOP_GEM_VERSIONS").split
gemfile = Bundler::LockfileParser.new(Bundler.read_file("Gemfile.lock"))
to_remove = gemfile.dependencies.keys.reject do |dependency|
dependency.include?("rubocop") || dependency == "syntax_tree"
end
gem_install_command = "gem install #{versioned_rubocop_gems.join(' ')} --no-document --conservative"
puts "Installing gems with:", gem_install_command
system "time #{gem_install_command}"

puts "Removing non rubocop gems from Gemfile"
system("bundle remove #{to_remove.join(' ')}") or abort("ERROR: Failed to remove non rubocop gems from Gemfile")
puts

puts "Resulting Gemfile:"
puts Bundler.read_file("Gemfile")

puts "Installing gems with: bundle install"
system("time bundle install") or abort("ERROR: Failed to install gems")

rubocop_command = "bundle exec rubocop"
else
versioned_rubocop_gems = ENV.fetch("RUBOCOP_GEM_VERSIONS").split
gem_install_command = "gem install #{versioned_rubocop_gems.join(' ')} --no-document --conservative"
puts "Installing gems with:", gem_install_command
system "time #{gem_install_command}"

rubocop_command = "rubocop"
end

puts "::endgroup::"

# Script
Expand All @@ -38,7 +52,7 @@
# JSON reference: https://docs.rubocop.org/rubocop/formatters.html#json-formatter
files_with_offenses =
if changed_ruby_files.any?
command = "rubocop #{changed_ruby_files.map(&:path).join(' ')} --format json --force-exclusion #{ARGV.join(' ')}"
command = "#{rubocop_command} #{changed_ruby_files.map(&:path).join(' ')} --format json --force-exclusion #{ARGV.join(' ')}"

puts "Running rubocop with: #{command}"
JSON.parse(`#{command}`).fetch("files")
Expand All @@ -52,7 +66,7 @@

puts "Fetching PR comments from https://api.github.com/repos/#{owner_and_repository}/pulls/#{pr_number}/comments"

existing_comments = Github.get!("/repos/#{owner_and_repository}/pulls/#{pr_number}/comments")
existing_comments = Github.get("/repos/#{owner_and_repository}/pulls/#{pr_number}/comments")

comments_made_by_rubocop = existing_comments.select do |comment|
comment.fetch("body").include?("rubocop-comment-id")
Expand All @@ -76,7 +90,7 @@

puts "Deleting resolved comment #{comment_id} on #{path} line #{line}"

Github.delete!("/repos/#{owner_and_repository}/pulls/comments/#{comment_id}")
Github.delete("/repos/#{owner_and_repository}/pulls/comments/#{comment_id}")
end

# Comment on the pull request with the offenses found
Expand Down Expand Up @@ -132,7 +146,7 @@ def in_diff?(changed_files, path, 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")

Github.post!(
Github.post(
"/repos/#{owner_and_repository}/pulls/#{pr_number}/comments",
body: body,
path: path,
Expand All @@ -147,7 +161,7 @@ def in_diff?(changed_files, path, line)

# If there are any offenses outside the diff, make a separate comment for them

separate_comments = Github.get!("/repos/#{owner_and_repository}/issues/#{pr_number}/comments")
separate_comments = Github.get("/repos/#{owner_and_repository}/issues/#{pr_number}/comments")
existing_separate_comment = separate_comments.find do |comment|
comment.fetch("body").include?("rubocop-comment-id: outside-diff")
end
Expand All @@ -173,12 +187,12 @@ def in_diff?(changed_files, path, line)
puts "Skipping unchanged separate comment #{existing_comment_id}"
else
puts "Updating separate comment #{existing_comment_id}"
Github.patch!("/repos/#{owner_and_repository}/issues/comments/#{existing_comment_id}", body: body)
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"

Github.post!("/repos/#{owner_and_repository}/issues/#{pr_number}/comments", body: body)
Github.post("/repos/#{owner_and_repository}/issues/#{pr_number}/comments", body: body)
end
elsif existing_separate_comment
existing_comment_id = existing_separate_comment.fetch("id")
Expand Down