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.

Sign up for GitHub

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

Jump to bottom

Pronto integration #406

Closed
wants to merge 6 commits into from
Closed

Pronto integration #406

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a7a279e
Pronto integration
europ Feb 13, 2018
b5b8fc8
Comment includes only line number of the offense
europ Feb 25, 2018
a402028
No longer used files removed (lib/linter/)
europ Feb 26, 2018
a377f2e
Removed unused variable "inspected_file_count"
europ Feb 28, 2018
8c43f3f
Pronto linter dependencies automatically required
europ Apr 3, 2018
7328482
Fixing rubocop issues
europ Apr 9, 2018
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
7 changes: 6 additions & 1 deletion Gemfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ gem 'travis', '~> 1.7.6'

gem 'awesome_spawn', '>= 1.4.1'
gem 'default_value_for'
gem 'haml_lint', '~> 0.20.0', :require => false
gem 'haml_lint', '~> 0.27.0', :require => false
gem 'more_core_extensions', '~> 2.0.0', :require => 'more_core_extensions/all'
gem 'rubocop', '~> 0.52.0', :require => false
gem 'rugged', :require => false
Expand All @@ -56,6 +56,11 @@ gem 'octokit', '~> 4.8.0', :require => false
gem 'faraday', '~> 0.9.1'
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
46 changes: 38 additions & 8 deletions Gemfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,19 +125,26 @@ GEM
multi_json (~> 1.0)
net-http-persistent (~> 2.9)
net-http-pipeline
gitlab (4.3.0)
httparty
terminal-table
globalid (0.4.0)
activesupport (>= 4.2.0)
haml (4.0.7)
haml (5.0.4)
temple (>= 0.8.0)
tilt
haml_lint (0.20.0)
haml (~> 4.0)
haml_lint (0.27.0)
haml (>= 4.0, < 5.1)
rainbow
rake (>= 10, < 13)
rubocop (>= 0.47.0)
rubocop (>= 0.50.0)
sysexits (~> 1.1)
hashdiff (0.3.6)
highline (1.7.8)
hike (1.2.3)
hitimes (1.2.6)
httparty (0.16.0)
multi_xml (>= 0.5.2)
i18n (0.8.6)
ice_cube (0.14.0)
ice_nine (0.11.2)
Expand Down Expand Up @@ -171,6 +178,7 @@ GEM
more_core_extensions (2.0.0)
activesupport (> 3.2)
multi_json (1.12.2)
multi_xml (0.6.0)
multipart-post (2.0.0)
net-http-persistent (2.9.4)
net-http-pipeline (1.0.1)
Expand All @@ -183,6 +191,21 @@ GEM
ast (~> 2.4.0)
pg (0.21.0)
powerpack (0.1.1)
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.0)
pronto (~> 0.9.0)
rubocop (~> 0.38, >= 0.35.0)
pronto-yamllint (0.1.0)
pronto (~> 0.9.0)
pry (0.9.12.6)
coderay (~> 1.0)
method_source (~> 0.8)
Expand Down Expand Up @@ -219,8 +242,9 @@ GEM
activesupport (= 4.2.10)
rake (>= 0.8.7)
thor (>= 0.18.1, < 2.0)
rainbow (3.0.0)
rake (12.1.0)
rainbow (2.2.2)
rake
rake (12.3.0)
rb-fsevent (0.10.2)
rb-inotify (0.9.10)
ffi (>= 0.5.0, < 2)
Expand Down Expand Up @@ -299,14 +323,16 @@ GEM
sprockets (>= 2.8, < 4.0)
sysexits (1.2.0)
temple (0.8.0)
terminal-table (1.8.0)
unicode-display_width (~> 1.1, >= 1.1.1)
therubyracer (0.12.3)
libv8 (~> 3.16.14.15)
ref
thin (1.7.2)
daemons (~> 1.0, >= 1.0.9)
eventmachine (~> 1.0, >= 1.0.4)
rack (>= 1, < 3)
thor (0.20.0)
thor (0.19.4)
thread_safe (0.3.6)
tilt (1.4.1)
timecop (0.9.1)
Expand Down Expand Up @@ -368,14 +394,18 @@ DEPENDENCIES
faraday (~> 0.9.1)
faraday-http-cache (~> 2.0.0)
foreman (~> 0.64.0)
haml_lint (~> 0.20.0)
haml_lint (~> 0.27.0)
influxdb (~> 0.3.13)
jquery-rails
listen
minigit (~> 0.0.4)
more_core_extensions (~> 2.0.0)
octokit (~> 4.8.0)
pg
pronto (~> 0.9.5)
pronto-haml (~> 0.9.0)
pronto-rubocop
pronto-yamllint
rails (~> 4.2.4)
rspec
rspec-rails
Expand Down
13 changes: 2 additions & 11 deletions app/workers/commit_monitor_handlers/commit_range/rubocop_checker/message_builder.rb
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion app/workers/commit_monitor_handlers/commit_range/rubocop_checker/rubocop_results_filter.rb
View file
Open in desktop
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
68 changes: 59 additions & 9 deletions app/workers/concerns/code_analysis_mixin.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,25 @@
require 'pronto/runners'
require 'pronto/gem_names'
require 'pronto/git/repository'
require 'pronto/git/patches'
require 'pronto/git/patch'
require 'pronto/git/line'

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 +28,52 @@ 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}" }

p_result = nil

# temporary solution for: download repo, obtain changes, get pronto result about changes
Dir.mktmpdir do |dir|
FileUtils.copy_entry(@branch.repo.path.to_s, dir)
repo = Pronto::Git::Repository.new(dir)
rg = repo.instance_variable_get(:@repo)
rg.fetch('origin', @branch.name.sub(/^prs/, 'pull'))
rg.checkout('FETCH_HEAD')
rg.reset('HEAD', :hard)
patches = repo.diff(@branch.merge_target)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already possible with the repos from the repos directory. Unfortunately, Pronto::Git::Repository seems to only want to use head, but it doesn't look like a stretch to just "copy" what they do there (or make a PR to pronto to support non-head commits, which should be straightforward). I think you can just do something like

branch.repo.git_fetch # note that branch is available to the CodeAnalysisMixin directly via the BranchWorkerMixin as they are usually together
git_service = branch.git_service
merge_base = git_service.merge_base(commit)
diff = git_service.rugged_repo.diff(merge_base, "refs/remotes/origin/#{branch.merge_target}", options)  # Getting the merge target ref is actually already a part of lib/git_service/branch.rb, but it's not exposed in an easy way, so just hardcoding here for an example
patches = Pronto::Git::Patches.new(Pronto::Git::Repository.new(branch.repo.path, merge_base, diff)
p_result = Pronto::Runners.new.run(patches)

p_result = Pronto::Runners.new.run(patches)
end

p_result
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
end
end
128 changes: 0 additions & 128 deletions lib/linter/base.rb
View file
Open in desktop

This file was deleted.

Loading