Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Pronto Integration (v2) #500

Closed
wants to merge 8 commits into from
Closed
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
6 changes: 6 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ gem 'default_value_for', '>= 3.1.0'
gem 'haml', '~> 5.1', :require => false # force newer version of haml
NickLaMuro marked this conversation as resolved.
Show resolved Hide resolved
gem 'haml_lint', '~> 0.35.0', :require => false
gem 'more_core_extensions', '~> 4.0.0', :require => 'more_core_extensions/all'
gem 'manageiq-style', :require => false
gem 'rubocop', '~> 0.82.0', :require => false
gem 'rubocop-performance', :require => false
gem 'rubocop-rails', :require => false
Expand All @@ -50,6 +51,11 @@ gem 'octokit', '~> 4.8.0', :require => false
gem 'faraday', '~> 0.9.2'
gem 'faraday-http-cache', '~> 2.0.0'

gem 'pronto', '~> 0.9.5', :require => false
gem 'pronto-haml', '~> 0.9.0', :require => false
gem 'pronto-rubocop', :require => false
gem 'pronto-yamllint', :require => false

group :development, :test do
gem 'rspec'
gem 'rspec-rails'
Expand Down
44 changes: 42 additions & 2 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,9 @@ GEM
multi_json (~> 1.0)
net-http-persistent (~> 2.9)
net-http-pipeline
gitlab (4.14.1)
httparty (~> 0.14, >= 0.14.0)
terminal-table (~> 1.5, >= 1.5.1)
globalid (0.4.2)
activesupport (>= 4.2.0)
haml (5.1.2)
Expand All @@ -168,6 +171,9 @@ GEM
sysexits (~> 1.1)
hashdiff (1.0.0)
highline (1.7.10)
httparty (0.18.0)
mime-types (~> 3.0)
multi_xml (>= 0.5.2)
i18n (1.8.2)
concurrent-ruby (~> 1.0)
ice_cube (0.14.0)
Expand All @@ -190,9 +196,18 @@ GEM
nokogiri (>= 1.5.9)
mail (2.7.1)
mini_mime (>= 0.1.1)
manageiq-style (1.1.0)
more_core_extensions
optimist
rubocop (~> 0.82.0)
rubocop-performance
rubocop-rails
marcel (0.3.3)
mimemagic (~> 0.3.2)
method_source (0.9.2)
mime-types (3.3.1)
mime-types-data (~> 3.2015)
mime-types-data (3.2020.0425)
mimemagic (0.3.4)
mini_mime (1.0.2)
mini_portile2 (2.4.0)
Expand All @@ -201,6 +216,7 @@ GEM
more_core_extensions (4.0.0)
activesupport
multi_json (1.14.1)
multi_xml (0.6.0)
multipart-post (2.1.1)
mustermann (1.1.1)
ruby2_keywords (~> 0.0.1)
Expand All @@ -212,10 +228,26 @@ GEM
mini_portile2 (~> 2.4.0)
octokit (4.8.0)
sawyer (~> 0.8.0, >= 0.5.3)
optimist (3.0.1)
parallel (1.19.1)
parser (2.7.1.3)
ast (~> 2.4.0)
pg (1.2.2)
pronto (0.9.5)
gitlab (~> 4.0, >= 4.0.0)
httparty (>= 0.13.7)
octokit (~> 4.7, >= 4.7.0)
rainbow (~> 2.1)
rugged (~> 0.24, >= 0.23.0)
thor (~> 0.19.0)
pronto-haml (0.9.0)
haml_lint (~> 0.23)
pronto (~> 0.9.0)
pronto-rubocop (0.9.1)
pronto (~> 0.9.0)
rubocop (~> 0.50, >= 0.49.1)
pronto-yamllint (0.1.1)
pronto (~> 0.9.0)
pry (0.9.12.6)
coderay (~> 1.0)
method_source (~> 0.8)
Expand Down Expand Up @@ -252,7 +284,8 @@ GEM
method_source
rake (>= 0.8.7)
thor (>= 0.19.0, < 2.0)
rainbow (3.0.0)
rainbow (2.2.2)
rake
rake (12.3.3)
rb-fsevent (0.10.3)
rb-inotify (0.10.1)
Expand Down Expand Up @@ -343,11 +376,13 @@ GEM
sprockets (>= 3.0.0)
sysexits (1.2.0)
temple (0.8.2)
terminal-table (1.8.0)
unicode-display_width (~> 1.1, >= 1.1.1)
thin (1.7.2)
daemons (~> 1.0, >= 1.0.9)
eventmachine (~> 1.0, >= 1.0.4)
rack (>= 1, < 3)
thor (1.0.1)
thor (0.19.4)
thread_safe (0.3.6)
tilt (2.0.10)
timecop (0.9.1)
Expand Down Expand Up @@ -414,11 +449,16 @@ DEPENDENCIES
influxdb (~> 0.3.13)
jquery-rails
listen
manageiq-style
minigit (~> 0.0.4)
more_core_extensions (~> 4.0.0)
net-ssh (~> 4.2.0)
octokit (~> 4.8.0)
pg
pronto (~> 0.9.5)
pronto-haml (~> 0.9.0)
pronto-rubocop
pronto-yamllint
rails (~> 5.2.2)
rspec
rspec-rails
Expand Down
1 change: 1 addition & 0 deletions app/workers/code_analyzer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ class CodeAnalyzer

def perform(branch_id)
return unless find_branch(branch_id, :regular)
return unless branch.merge_target

analyze
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ def offenses
SEVERITY_MAP[o["severity"]],
format_message(o),
f["path"],
format_locator(f, o)
format_line(f, o)
)
end
end.flatten
Expand All @@ -96,22 +96,13 @@ def format_cop_name(cop_name)
COP_URIS[cop_name] || cop_name
end

def format_locator(file, offense)
[format_line(file, offense), format_column(offense)].compact.join(", ").presence
end

def format_line(file, offense)
line = offense.fetch_path("location", "line")
line = offense.fetch_path("line")
return nil unless line
uri = File.join(line_uri, "blob", commits.last, file["path"]) << "#L#{line}"
"[Line #{line}](#{uri})"
end

def format_column(offense)
column = offense.fetch_path("location", "column")
column && "Col #{column}"
end

# TODO: Don't reuse the commit_uri. This should probably be its own URI.
def line_uri
branch.commit_uri.chomp("commit/$commit")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def filter_on_diff
@results["files"].each do |f|
f["offenses"].select! do |o|
o["severity"].in?(%w(error fatal)) ||
@diff_details[f["path"]].include?(o["location"]["line"])
@diff_details[f["path"]].include?(o["line"])
end
end
end
Expand Down
141 changes: 132 additions & 9 deletions app/workers/concerns/code_analysis_mixin.rb
Original file line number Diff line number Diff line change
@@ -1,16 +1,27 @@
require 'pronto/runners'
require 'pronto/gem_names'
require 'pronto/git/repository'
require 'pronto/git/patches'
require 'pronto/git/patch'
require 'pronto/git/line'

require 'tempfile'

require 'fileutils'
require 'tmpdir'

module CodeAnalysisMixin
def merged_linter_results
results = {
"files" => [],
"summary" => {
"inspected_file_count" => 0,
"offense_count" => 0,
"target_file_count" => 0,
"offense_count" => 0,
"target_file_count" => 0,
},
}

run_all_linters.each do |result|
%w(offense_count target_file_count inspected_file_count).each do |m|
%w[offense_count target_file_count].each do |m|
results['summary'][m] += result['summary'][m]
end
results['files'] += result['files']
Expand All @@ -19,11 +30,123 @@ def merged_linter_results
results
end

# run linters via pronto and return the pronto result
def pronto_result
Pronto::GemNames.new.to_a.each { |gem_name| require "pronto/#{gem_name}" }

branch.repo.git_fetch
generate_linter_configs

git_service = branch.git_service
merge_base = git_service.merge_base
pronto_repo = Pronto::Git::Repository.new(repo.path.to_s)
pronto_patches = Pronto::Git::Patches.new(pronto_repo, merge_base, git_service.diff.raw_diff)

Pronto::Runners.new.run(pronto_patches)
end

# configs in the repo itself most likely will be invalid as the what the
# `MinigitService` has checked out in `.repos/` probably isn't the same SHA
# as what we are working with.
#
# Any specifics per linter that need to be adjusted need to be done here, but
# RuboCop might be the only one that is necessary for now.
#
def generate_linter_configs
# Ensure any tempfiles created here are kept around through
# `.run_all_linters`, and will be unlinked there.
@config_tempfiles = []

# This is config option found in the `proto-rubocop` plugin README:
#
# > You can also specify a custom .rubocop.yml location with the
# > environment variable RUBOCOP_CONFIG.
#
git_service = branch.git_service
rubocop_config_contents = git_service.content_at(".rubocop.yml")
yamllint_config_contents = git_service.content_at(".yamllint")

if rubocop_config_contents
rubocop_yaml_data = YAML.load(rubocop_config_contents)
(rubocop_yaml_data.delete("inherit_from") || []).each do |local_config_path|
next unless inherit_from_data = git_service.content_at(local_config_path)

rubocop_yaml_data["inherit_from"] ||= []
rubocop_yaml_data["inherit_from"] << generate_temp_config(local_config_path, inherit_from_data)
end

ENV["RUBOCOP_CONFIG"] = generate_temp_config(".rubocop.yml", rubocop_yaml_data.to_yaml)
end

if yamllint_config_contents
yamllint_config_path = generate_temp_config(".yamllint", yamllint_config_contents)
ENV["YAMLLINT_OPTS"] = "-c #{yamllint_config_path}"
end
end

def generate_temp_config(config_filepath, file_contents)
temp_config = Tempfile.new(config_filepath)
temp_config.write file_contents
temp_config.close

@config_tempfiles << temp_config

temp_config.path
end

def run_all_linters
unmerged_results = []
unmerged_results << Linter::Rubocop.new(branch).run
unmerged_results << Linter::Haml.new(branch).run
unmerged_results << Linter::Yaml.new(branch).run
unmerged_results.tap(&:compact!)
pronto_result.group_by(&:runner).values.map do |linted| # group by linter
output = {}

output["files"] = linted.group_by(&:path).map do |path, value| # group by file in linter
{
"path" => path,
"offenses" => value.map do |msg| # put offenses of file in linter into an array
{
"severity" => msg.level.to_s,
"message" => msg.msg,
"cop_name" => msg.runner,
"corrected" => false,
"line" => msg.line.position
}
end
}
end

output["summary"] = {
"offense_count" => output["files"].sum { |item| item['offenses'].length },
"target_file_count" => output["files"].length,
}

output
end
rescue RuboCop::ValidationError => error
[failed_linter_offenses("#{self.class.name} STDERR:\n```\n#{error.message}\n```")]
ensure
@config_tempfiles.each(&:unlink)
end

def failed_linter_offenses(message)
git_service = branch.git_service
files_to_lint = branch.pull_request? ? git_service.diff.new_files : git_service.tip_files
{
"files" => [
{
"path" => "\\*\\*",
"offenses" => [
{
"severity" => "fatal",
"message" => message,
"cop_name" => self.class.name.titleize
}
]
}
],
"summary" => {
"offense_count" => 1,
"target_file_count" => files_to_lint.length,
"inspected_file_count" => files_to_lint.length
}
}
end
end
Loading