diff --git a/lib/manageiq/style/cli.rb b/lib/manageiq/style/cli.rb index b9750bb..2350395 100644 --- a/lib/manageiq/style/cli.rb +++ b/lib/manageiq/style/cli.rb @@ -17,12 +17,16 @@ def parse_cli_options synopsis "\nThe ManageIQ community's style configuration utility." version "v#{ManageIQ::Style::VERSION}\n" - opt :install, "Install or update the style configurations", :default => false, :required => true + opt :format, "TODO: How to format the linter output", :default => "emacs" + opt :install, "Install or update the style configurations", :default => false + opt :lint, "Run linters for current changes", :default => false + opt :linters, "Linters to run for current changes", :default => ["rubocop"] end end def run install if @opts[:install] + lint if @opts[:lint] end def install @@ -38,6 +42,20 @@ def install update_gem_source end + def lint + require 'more_core_extensions/all' + + require 'manageiq/style/git_service/branch' + require 'manageiq/style/git_service/diff' + + require 'manageiq/style/linter/base' + require 'manageiq/style/linter/haml' + require 'manageiq/style/linter/rubocop' + require 'manageiq/style/linter/yaml' + + Linter::Rubocop.new(ARGV).run + end + private def check_for_codeclimate_channel diff --git a/lib/manageiq/style/git_service/branch.rb b/lib/manageiq/style/git_service/branch.rb new file mode 100644 index 0000000..791651f --- /dev/null +++ b/lib/manageiq/style/git_service/branch.rb @@ -0,0 +1,158 @@ +require 'rugged' + +module ManageIQ +module Style +module GitService + class Branch + attr_reader :branch_options + + def initialize(branch_options = {}) + @branch_options = branch_options + end + + def name + @branch_options[:name] || rugged_repo.head.name.sub(/^refs\/heads\//, '') + end + + def pull_request? + !!@branch_options[:pull_request] + end + + def merge_target + @branch_options[:merge_target] + end + + def content_at(path, merged = false) + blob_at(path, merged).try(:content) + end + + def permission_for(path, merged = false) + git_perm = blob_data_for(path, merged).to_h[:filemode] + + # Since git stores file permissions in a different format: + # + # https://unix.stackexchange.com/a/450488 + # + # Convert what we get from Rugged to something that will work with File + # + # git_perm = 0o100755 + # git_perm.to_s(8) + # #=> "100755" + # _.[-3,3] + # #=> "755" + # _.to_i(8) + # #=> 493 + # 0o755 + # #=> 493 + # + git_perm ? git_perm.to_s(8)[-3,3].to_i(8) : 0o644 + end + + def diff(merge_target = nil) + Diff.new(target_for_reference(merge_target_ref_name(merge_target)).diff(merge_tree)) + end + + def exists? + rugged_repo.branches.exists?(ref_name) + end + + def mergeable? + merge_tree + true + rescue UnmergeableError + false + end + + def merge_base(merge_target = nil) + rugged_repo.merge_base(target_for_reference(merge_target_ref_name(merge_target)), target_for_reference(ref_name)) + end + + def merge_index(merge_target = nil) # Rugged::Index for a merge of this branch + rugged_repo.merge_commits(target_for_reference(merge_target_ref_name(merge_target)), target_for_reference(ref_name)) + end + + def merge_tree # Rugged::Tree object for the merge of this branch + tree_ref = merge_index.write_tree(rugged_repo) + rugged_repo.lookup(tree_ref) + rescue Rugged::IndexError + raise UnmergeableError + ensure + # Rugged seems to allocate large C structures, but not many Ruby objects, + # and thus doesn't trigger a GC, so we will trigger one manually. + GC.start + end + + def tip_commit + target_for_reference(ref_name) + end + + def tip_tree + tip_commit.tree + end + + def target_for_reference(reference) # Rugged::Commit for a given refname i.e. "refs/remotes/origin/master" + rugged_repo.references[reference].target + end + + def tip_files + list_files_in_tree(tip_tree.oid) + end + + def commit_ids_since(starting_point) + range_walker(starting_point).collect(&:oid).reverse + end + + def commit(commit_oid) + Commit.new(rugged_repo, commit_oid) + end + + private + + def list_files_in_tree(rugged_tree_oid, current_path = Pathname.new("")) + rugged_repo.lookup(rugged_tree_oid).each_with_object([]) do |i, files| + full_path = current_path.join(i[:name]) + case i[:type] + when :blob + files << full_path.to_s + when :tree + files.concat(list_files_in_tree(i[:oid], full_path)) + end + end + end + + def range_walker(walk_start, walk_end = ref_name) + Rugged::Walker.new(rugged_repo).tap do |walker| + walker.push_range("#{walk_start}..#{walk_end}") + end + end + + def ref_name + return "refs/#{name}" if name.include?("prs/") + "refs/remotes/origin/#{name}" + end + + def merge_target_ref_name(other_merge_target = nil) + ref = other_merge_target || merge_target + "refs/remotes/origin/#{ref}" + end + + # Rugged::Blob object for a given file path on this branch + def blob_at(path, merged = false) + blob = Rugged::Blob.lookup(rugged_repo, blob_data_for(path, merged)[:oid]) + blob.type == :blob ? blob : nil + rescue Rugged::TreeError + nil + end + + def blob_data_for(path, merged = false) + source = merged ? merge_tree : tip_tree + source.path(path) + end + + def rugged_repo + @rugged_repo ||= Rugged::Repository.new(branch_options[:repo_path]) + end + end +end +end +end diff --git a/lib/manageiq/style/git_service/diff.rb b/lib/manageiq/style/git_service/diff.rb new file mode 100644 index 0000000..d4898db --- /dev/null +++ b/lib/manageiq/style/git_service/diff.rb @@ -0,0 +1,57 @@ +module ManageIQ +module Style +module GitService + class Diff + attr_reader :raw_diff + def initialize(raw_diff) + @raw_diff = raw_diff + end + + def new_files + raw_diff.deltas.collect { |delta| delta.try(:new_file).try(:[], :path) }.compact + end + + def with_each_patch + raw_diff.patches.each { |patch| yield(patch) } + end + + def with_each_hunk + with_each_patch do |patch| + patch.hunks.each { |hunk| yield(hunk, patch) } + end + end + + def with_each_line + with_each_hunk do |hunk, parent_patch| + hunk.lines.each { |line| yield(line, hunk, parent_patch) } + end + end + + def file_status + raw_diff.patches.each_with_object({}) do |patch, h| + new_file = patch.delta.new_file.try(:[], :path) + if new_file + additions = h.fetch_path(new_file, :additions) || 0 + h.store_path(new_file, :additions, (additions + patch.additions)) + end + + old_file = patch.delta.old_file.try(:[], :path) + if old_file + deletions = h.fetch_path(old_file, :deletions) || 0 + h.store_path(new_file, :deletions, (deletions + patch.deletions)) + end + end + end + + def status_summary + changed, added, deleted = raw_diff.stat + [ + changed.positive? ? "#{changed} #{"file".pluralize(changed)} changed" : nil, + added.positive? ? "#{added} #{"insertion".pluralize(added)}(+)" : nil, + deleted.positive? ? "#{deleted} #{"deletion".pluralize(deleted)}(-)" : nil + ].compact.join(", ") + end + end +end +end +end diff --git a/lib/manageiq/style/git_service/error.rb b/lib/manageiq/style/git_service/error.rb new file mode 100644 index 0000000..6b73bd9 --- /dev/null +++ b/lib/manageiq/style/git_service/error.rb @@ -0,0 +1,7 @@ +module ManageIQ +module Style +module GitService + Error = Class.new(StandardError) +end +end +end diff --git a/lib/manageiq/style/git_service/unmergeable_error.rb b/lib/manageiq/style/git_service/unmergeable_error.rb new file mode 100644 index 0000000..840c0d1 --- /dev/null +++ b/lib/manageiq/style/git_service/unmergeable_error.rb @@ -0,0 +1,7 @@ +module ManageIQ +module Style +module GitService + UnmergeableError = Class.new(Error) +end +end +end diff --git a/lib/manageiq/style/linter/base.rb b/lib/manageiq/style/linter/base.rb new file mode 100644 index 0000000..c0d9830 --- /dev/null +++ b/lib/manageiq/style/linter/base.rb @@ -0,0 +1,180 @@ +module ManageIQ +module Style +module Linter + class Base + attr_reader :branch_options, :files_from_cli, :logger, :options + + OUTPUT_FORMAT = "%s:%d:%d: %s\n".freeze + DEFAULT_BRANCH_OPTIONS = { + :pull_request => true, + :merge_target => "master", + }.freeze + + + def initialize(files_from_cli, options = {}) + require 'logger' + + branch_option_args = options.delete(:branch_options) { |_| {} } + @branch_options = DEFAULT_BRANCH_OPTIONS.dup.merge(branch_option_args) + @branch_options[:repo_path] ||= Dir.pwd + @branch_options[:repo_name] ||= File.basename(@branch_options[:repo_path]) + + @options = options + @options[:output] = :stdout # :stdout or :logger + @options[:logger_io] = STDOUT if @options[:debug] + + @files_from_cli = files_from_cli || [] + + set_logger + end + + def run + logger.info("#{log_header} Starting run...") + if files_to_lint.empty? + logger.info("#{log_header} Skipping run due to no candidate files.") + return + end + + require 'tempfile' + result = Dir.mktmpdir do |dir| + files = collected_config_files(dir) + if files.blank? + logger.error("#{log_header} Failed to run due to missing config files.") + return failed_linter_offenses("missing config files") + else + files += collected_files_to_lint(dir) + logger.info("#{log_header} Collected #{files.length} files.") + logger.debug { "#{log_header} File list: #{files.inspect}"} + run_linter(dir) + end + end + + offenses = parse_output(result.output) + logger.info("#{log_header} Completed run with #{offenses.fetch_path('summary', 'offense_count')} offenses") + output_offenses offenses + offenses + end + + private + + def output_offenses(offenses_json) + case options[:output] + when :stdout + offenses_json["files"].each do |file| + file["offenses"].each do |offense| + message = "#{offense["severity"]}: #{offense["cop_name"]}: #{offense["message"]}" + STDOUT.printf(OUTPUT_FORMAT, :path => file["path"], + :line => offense["location"]["line"], + :column => offense["location"]["column"], + :message => message) + end + end + when :logger + logger.debug { "#{log_header} Offenses: #{offenses.inspect}" } + end + end + + def parse_output(output) + JSON.parse(output.chomp) + rescue JSON::ParserError => error + logger.error("#{log_header} #{error.message}") + logger.error("#{log_header} Failed to parse JSON result #{output.inspect}") + return failed_linter_offenses("error parsing JSON result") + end + + def collected_config_files(dir) + config_files.select { |path| extract_file(path, dir, branch_service.pull_request?) } + end + + def collected_files_to_lint(dir) + files_to_lint.select { |path| extract_file(path, dir) } + end + + def extract_file(path, destination_dir, merged = false) + content = branch_service.content_at(path, merged) + return false unless content + + perm = branch_service.permission_for(path, merged) + temp_file = File.join(destination_dir, path) + FileUtils.mkdir_p(File.dirname(temp_file)) + + # Use "wb" to prevent Encoding::UndefinedConversionError: "\xD0" from + # ASCII-8BIT to UTF-8 + File.write(temp_file, content, :mode => "wb", :perm => perm) + + true + end + + def branch_service + @branch_service ||= GitService::Branch.new(branch_options) + end + + def diff_service + @diff_service ||= branch_service.diff + end + + def files_to_lint + @files_to_lint ||= begin + unfiltered_files = branch_service.pull_request? ? diff_service.new_files : branch_service.tip_files + unfiltered_files &= files_from_cli unless files_from_cli.empty? + # puts "unfiltered_files & files_from_cli = #{unfiltered_files & files_from_cli}" + # puts "files_from_cli & unfiltered_files = #{files_from_cli & unfiltered_files}" + filtered_files(unfiltered_files) + end + end + + def run_linter(dir) + logger.info("#{log_header} Executing linter...") + require 'awesome_spawn' + result = AwesomeSpawn.run(linter_executable, :params => linter_cli_options, :chdir => dir) + handle_linter_output(result) + end + + def handle_linter_output(result) + # rubocop exits 1 both when there are errors and when there are style issues. + # Instead of relying on just exit_status, we check if there is anything on stderr. + return result if result.exit_status.zero? || result.error.blank? + FailedLinterRun.new(failed_linter_offenses("#{self.class.name} STDERR:\n```\n#{result.error}\n```")) + end + + def failed_linter_offenses(message) + { + "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 + + def log_header + "#{self.class.name} Repo: #{@branch_options[:repo_name]} Branch #{branch_service.name} -" + end + + class FailedLinterRun + attr_reader :output + def initialize(message) + @output = message.to_json + end + end + + def set_logger + logger_io = @options[:logfile] || File::NULL + @logger = Logger.new(logger_io) + end + end +end +end +end diff --git a/lib/manageiq/style/linter/haml.rb b/lib/manageiq/style/linter/haml.rb new file mode 100644 index 0000000..4a7523e --- /dev/null +++ b/lib/manageiq/style/linter/haml.rb @@ -0,0 +1,27 @@ +require 'manageiq/style/linter/base' + +module ManageIQ +module Style +module Linter + class Haml < Base + private + + def config_files + [".haml-lint.yml"] + Linter::Rubocop::CONFIG_FILES + end + + def linter_executable + 'haml-lint *' + end + + def linter_cli_options + {:reporter => 'json'} + end + + def filtered_files(files) + files.select { |file| file.end_with?(".haml") } + end + end +end +end +end diff --git a/lib/manageiq/style/linter/rubocop.rb b/lib/manageiq/style/linter/rubocop.rb new file mode 100644 index 0000000..efef9e6 --- /dev/null +++ b/lib/manageiq/style/linter/rubocop.rb @@ -0,0 +1,35 @@ +require 'manageiq/style/linter/base' + +require 'rubocop' + +module ManageIQ +module Style +module Linter + class Rubocop < Base + CONFIG_FILES = %w[.rubocop.yml .rubocop_base.yml .rubocop_local.yml].freeze + + private + + def config_files + CONFIG_FILES + end + + def linter_executable + 'rubocop' + end + + def linter_cli_options + {:format => 'json', :no_display_cop_names => nil} + end + + def filtered_files(files) + files.select do |file| + file.end_with?(".rb", ".ru", ".rake") || %w[Gemfile Rakefile].include?(File.basename(file)) + end.reject do |file| + file.end_with?("db/schema.rb") + end + end + end +end +end +end diff --git a/lib/manageiq/style/linter/yaml.rb b/lib/manageiq/style/linter/yaml.rb new file mode 100644 index 0000000..af33768 --- /dev/null +++ b/lib/manageiq/style/linter/yaml.rb @@ -0,0 +1,65 @@ +require 'manageiq/style/linter/base' +require 'more_core_extensions/core_ext/hash' + +module ManageIQ +module Style +module Linter + class Yaml < Base + private + + def parse_output(output) + lines = output.chomp.split("\n") + parsed = lines.collect { |line| line_to_hash(line) } + grouped = parsed.group_by { |hash| hash["filename"] } + file_count = parsed.collect { |hash| hash["filename"] }.uniq.count + { + "files" => grouped.collect do |filename, offenses| + { + "path" => filename.sub(%r{\A\./}, ""), + "offenses" => offenses.collect { |offense_hash| offense_hash.deep_delete("filename"); } + } + end, + "summary" => { + "offense_count" => lines.size, + "target_file_count" => file_count, + "inspected_file_count" => file_count + } + } + end + + def linter_executable + "yamllint" + end + + def config_files + [".yamllint"] + end + + def linter_cli_options + {:f => "parsable", nil => ["."]} + end + + def filtered_files(files) + files.select { |f| f.end_with?(".yml", ".yaml") } + end + + def line_to_hash(line) + filename, line, column, severity_message_cop = line.split(":", 4) + severity_message, cop = severity_message_cop.split(/ \((.*)\)\Z/) + severity, message = severity_message.match(/\[(.*)\] (.*)/).captures + + { + "filename" => filename, + "severity" => severity, + "message" => message, + "cop_name" => cop, + "location" => { + "line" => line.to_i, + "column" => column.to_i + } + } + end + end +end +end +end diff --git a/manageiq-style.gemspec b/manageiq-style.gemspec index 0fcc095..8693f9d 100644 --- a/manageiq-style.gemspec +++ b/manageiq-style.gemspec @@ -28,6 +28,7 @@ Gem::Specification.new do |spec| spec.add_runtime_dependency "rubocop", "~> 0.82.0" spec.add_runtime_dependency "rubocop-performance" spec.add_runtime_dependency "rubocop-rails" + spec.add_runtime_dependency "rugged" spec.add_development_dependency "rake", "~> 12.0" spec.add_development_dependency "rspec", "~> 3.0" diff --git a/spec/manageiq/style/linter/yaml_spec.rb b/spec/manageiq/style/linter/yaml_spec.rb new file mode 100644 index 0000000..2663495 --- /dev/null +++ b/spec/manageiq/style/linter/yaml_spec.rb @@ -0,0 +1,98 @@ +require 'manageiq/style/linter/yaml' + +RSpec.describe ManageIQ::Style::Linter::Yaml do + describe "#parse_output" do + it "formats the output in the same form as rubocop" do + output = <<-EOOUTPUT +config/settings.yml:8:5: [warning] wrong indentation: expected 2 but found 4 (indentation) +config/settings.yml:11:1: [error] duplication of key ":a" in mapping (key-duplicates) +lib/generators/provider/templates/config/settings.yml:8:15: [error] syntax error: could not find expected ':' +EOOUTPUT + + actual = described_class.new(double("branch")).send(:parse_output, output) + + expected = { + "files" => [ + { + "path" => "config/settings.yml", + "offenses" => a_collection_containing_exactly( + { + "severity" => "warning", + "message" => "wrong indentation: expected 2 but found 4", + "cop_name" => "indentation", + "location" => { + "line" => 8, + "column" => 5 + } + }, + { + "severity" => "error", + "message" => "duplication of key \":a\" in mapping", + "cop_name" => "key-duplicates", + "location" => { + "line" => 11, + "column" => 1 + } + } + ) + }, + { + "path" => "lib/generators/provider/templates/config/settings.yml", + "offenses" => [ + { + "severity" => "error", + "message" => "syntax error: could not find expected ':'", + "cop_name" => nil, + "location" => { + "line" => 8, + "column" => 15 + } + }, + ] + } + ], + "summary" => { + "offense_count" => 3, + "target_file_count" => 2, + "inspected_file_count" => 2 + }, + } + expect(actual).to include(expected) + end + end + + describe "#line_to_hash" do + it "with a syntax error" do + line = "./lib/generators/provider/templates/config/settings.yml:8:15: [error] syntax error: could not find expected ':'" + expect(described_class.new(double).send(:line_to_hash, line)).to eq( + "cop_name" => nil, + "filename" => "./lib/generators/provider/templates/config/settings.yml", + "location" => {"line" => 8, "column" => 15}, + "message" => "syntax error: could not find expected ':'", + "severity" => "error" + ) + end + + it "with an indentation warning" do + line = "config/settings.yml:8:5: [warning] wrong indentation: expected 2 but found 4 (indentation)" + expect(described_class.new(double).send(:line_to_hash, line)).to eq( + "cop_name" => "indentation", + "filename" => "config/settings.yml", + "location" => {"line" => 8, "column" => 5}, + "message" => "wrong indentation: expected 2 but found 4", + "severity" => "warning" + ) + end + + it "with a duplicate key error" do + line = "config/settings.yml:11:1: [error] duplication of key \":a\" in mapping (key-duplicates)" + expect(described_class.new(double).send(:line_to_hash, line)).to eq( + "cop_name" => "key-duplicates", + "filename" => "config/settings.yml", + "location" => {"line" => 11, "column" => 1}, + "message" => "duplication of key \":a\" in mapping", + "severity" => "error" + ) + end + end +end