From 65744119caf26694d0ecd3be9cb8b74ae7f6660f Mon Sep 17 00:00:00 2001 From: Landon Wilkins Date: Wed, 21 Dec 2016 16:30:57 -0700 Subject: [PATCH] dry up linters test plan: * see test commit verifying linters still work Change-Id: I406c218309e824618869c9b5f3841af8387bf836 Reviewed-on: https://gerrit.instructure.com/98329 Reviewed-by: Simon Williams Tested-by: Jenkins Reviewed-by: Shawn Meredith Product-Review: Shawn Meredith QA-Review: Shawn Meredith --- script/brakeman | 56 +++---------------- script/eslint | 71 ++++++------------------ script/linter.rb | 138 +++++++++++++++++++++++++++++++++++++++++++++++ script/rlint | 98 ++++++++------------------------- script/stylelint | 60 ++++----------------- 5 files changed, 195 insertions(+), 228 deletions(-) create mode 100644 script/linter.rb diff --git a/script/brakeman b/script/brakeman index 7c0de0bc6e6..64c7a0555de 100755 --- a/script/brakeman +++ b/script/brakeman @@ -1,53 +1,13 @@ #!/usr/bin/env ruby -$:.push File.expand_path("../../gems/dr_diff/lib", __FILE__) -require 'dr_diff' -require 'json' +require_relative './linter' -def pretty_comment(comment) - message = "" - message += "[#{comment[:severity]}]".colorize(:yellow) - message += " #{comment[:path].colorize(:light_blue)}:#{comment[:position]}" - message += " => #{comment[:message].colorize(:green)}" - puts message -end - -def gerrit_patchset? - !!ENV['GERRIT_PATCHSET_REVISION'] -end - -env_sha = ENV['SHA'] || ENV['GERRIT_PATCHSET_REVISION'] - -dr_diff = DrDiff::Manager.new(sha: env_sha) -ruby_files = dr_diff.files(/\.rb$/) - -if ruby_files.size == 0 - puts "No ruby file changes found, skipping Brakeman check!" - exit 0 -end - -comments = dr_diff.comments( +linter_options = { + linter_name: "brakeman", + file_regex: /\.rb$/, format: 'brakeman', command: 'bundle exec brakeman --quiet --format json --confidence-level 2', - include_git_dir_in_output: !gerrit_patchset? -) + append_files_to_command: false, +} -unless comments.size > 0 - puts '-- -- -- -- -- -- -- -- -- -- --' - puts 'No relevant Brakeman errors found!' - puts '-- -- -- -- -- -- -- -- -- -- --' - exit 0 -end - -if gerrit_patchset? - require "gergich" - draft = Gergich::Draft.new - comments.each do |comment| - draft.add_comment comment[:path], - comment[:position], - comment[:message], - comment[:severity] - end -else - require 'colorize' - comments.each { |comment| pretty_comment(comment) } -end +brakeman = Linter.new(linter_options) +brakeman.run diff --git a/script/eslint b/script/eslint index 2faf229988b..d96898b1f00 100755 --- a/script/eslint +++ b/script/eslint @@ -1,58 +1,19 @@ #!/usr/bin/env ruby -$:.push File.expand_path("../../gems/dr_diff/lib", __FILE__) -require 'dr_diff' -require 'json' +require_relative './linter' -def pretty_comment(comment) - message = "" - message += "[#{comment[:severity]}]".colorize(:yellow) - message += " #{comment[:path].colorize(:light_blue)}:#{comment[:position]}" - message += " => #{comment[:message].colorize(:green)}" - puts message -end - -def gerrit_patchset? - !!ENV['GERRIT_PATCHSET_REVISION'] -end - -env_sha = ENV['SHA'] || ENV['GERRIT_PATCHSET_REVISION'] - -dr_diff = DrDiff::Manager.new(sha: env_sha) -ESLINT_FILE_REGEX = /\.jsx?$/ -js_files = dr_diff.files(ESLINT_FILE_REGEX) - -if js_files.size == 0 - puts "No js or jsx file changes found!" - exit 0 -end - -comments = dr_diff.comments(format: "eslint", - command: "node_modules/.bin/eslint #{js_files.join(' ')}", - include_git_dir_in_output: !gerrit_patchset?) - -# This section should be removed when we start -2ing patchsets -comments = comments.map do |comment| - comment[:severity] = 'warn' - comment -end - -unless comments.size > 0 - puts "-- -- -- -- -- -- -- -- -- -- --" - puts "No relevant js or jsx errors found!" - puts "-- -- -- -- -- -- -- -- -- -- --" - exit(0) -end - -if gerrit_patchset? - require "gergich" - draft = Gergich::Draft.new - comments.each do |comment| - draft.add_comment comment[:path], - comment[:position], - comment[:message], - comment[:severity] +linter_options = { + linter_name: "eslint", + file_regex: /\.jsx?$/, + format: "eslint", + command: "node_modules/.bin/eslint", + append_files_to_command: true, + comment_post_processing: proc do |comments| + # This section should be removed when we start -2ing patchsets + comments.each do |comment| + comment[:severity] = 'warn' + end end -else - require 'colorize' - comments.each { |comment| pretty_comment(comment) } -end +} + +eslint = Linter.new(linter_options) +eslint.run diff --git a/script/linter.rb b/script/linter.rb new file mode 100644 index 00000000000..21b3d05e069 --- /dev/null +++ b/script/linter.rb @@ -0,0 +1,138 @@ +$LOAD_PATH.push File.expand_path("../../gems/dr_diff/lib", __FILE__) +require 'dr_diff' +require 'json' + +class Linter + DEFAULT_OPTIONS = { + append_files_to_command: false, + boyscout_mode: true, + comment_post_processing: proc {}, + env_sha: ENV['SHA'] || ENV['GERRIT_PATCHSET_REVISION'], + file_regex: /./, + gerrit_patchset: !!ENV['GERRIT_PATCHSET_REVISION'], + heavy_mode: false, + heavy_mode_proc: proc {}, + include_git_dir_in_output: !!!ENV['GERRIT_PATCHSET_REVISION'], + plugin: ENV['GERRIT_PROJECT'], + skip_file_size_check: false, + }.freeze + + def initialize(options = {}) + options = DEFAULT_OPTIONS.merge(options) + + if options[:plugin] == 'canvas-lms' + options[:plugin] = nil + end + + options.each do |key, value| + instance_variable_set("@#{key}", value) + end + end + + def run + if git_dir && !Dir.exist?(git_dir) + puts "No plugin #{plugin} found" + exit 0 + end + + if !skip_file_size_check && files.size == 0 + puts "No #{file_regex} file changes found, skipping #{linter_name} check!" + exit 0 + end + + if heavy_mode + heavy_mode_proc.call(files) + else + publish_comments + end + end + + private + + attr_reader :append_files_to_command, + :boyscout_mode, + :command, + :comment_post_processing, + :default_boyscout_mode, + :env_sha, + :file_regex, + :format, + :gerrit_patchset, + :heavy_mode, + :heavy_mode_proc, + :include_git_dir_in_output, + :linter_name, + :plugin, + :severe_levels, + :skip_file_size_check + + def git_dir + @git_dir ||= plugin && "gems/plugins/#{plugin}/" + end + + def severe_levels + boyscout_mode ? %w(error warn info) : %w(error warn) + end + + def dr_diff + @dr_diff ||= ::DrDiff::Manager.new(git_dir: git_dir, sha: env_sha) + end + + def files + @files ||= dr_diff.files(file_regex) + end + + def full_command + if append_files_to_command + "#{command} #{files.join(' ')}" + else + command + end + end + + def comments + @comments ||= dr_diff.comments(format: format, + command: full_command, + include_git_dir_in_output: include_git_dir_in_output, + severe_levels: severe_levels) + end + + def publish_comments + unless comments.size > 0 + puts "-- -- -- -- -- -- -- -- -- -- --" + puts "No relevant #{linter_name} errors found!" + puts "-- -- -- -- -- -- -- -- -- -- --" + exit(0) + end + + if gerrit_patchset + publish_gergich_comments(comments) + else + publish_local_comments(comments) + end + end + + def publish_gergich_comments(comments) + require "gergich" + draft = Gergich::Draft.new + comments.each do |comment| + draft.add_comment comment[:path], + comment[:position], + comment[:message], + comment[:severity] + end + end + + def publish_local_comments(comments) + require 'colorize' + comments.each { |comment| pretty_comment(comment) } + end + + def pretty_comment(comment) + message = "" + message += "[#{comment[:severity]}]".colorize(:yellow) + message += " #{comment[:path].colorize(:light_blue)}:#{comment[:position]}" + message += " => #{comment[:message].colorize(:green)}" + puts message + end +end diff --git a/script/rlint b/script/rlint index 35aaf170572..f2d70098437 100755 --- a/script/rlint +++ b/script/rlint @@ -1,84 +1,30 @@ #!/usr/bin/env ruby -$:.push File.expand_path("../../gems/dr_diff/lib", __FILE__) -require 'dr_diff' -require 'json' +require_relative './linter' require 'optparse' -require 'rubocop' -def pretty_comment(comment) - message = "" - message += "[#{comment[:severity]}]".colorize(:yellow) - message += " #{comment[:path].colorize(:light_blue)}:#{comment[:position]}" - message += " => #{comment[:message].colorize(:green)}" - puts message -end - -def gerrit_patchset? - !!ENV['GERRIT_PATCHSET_REVISION'] -end - -def run_heavy_mode(ruby_files) - args = ruby_files - args_index = ARGV.index('--') - if args_index - args.concat(ARGV[(args_index + 1)..-1]) +linter_options = { + linter_name: "Rubocop", + file_regex: /\.rb$/, + format: "rubocop", + command: "bundle exec rubocop", + append_files_to_command: true, + severe_levels: [], + default_boyscout_mode: false, + heavy_mode_proc: proc do |ruby_files| + args = ruby_files + args_index = ARGV.index('--') + if args_index + args.concat(ARGV[(args_index + 1)..-1]) + end + RuboCop::CLI.new.run(args) end - RuboCop::CLI.new.run(args) -end - -env_sha = ENV['SHA'] || ENV['GERRIT_PATCHSET_REVISION'] -heavy_mode = false -boyscout_mode = false -plugin = ENV['GERRIT_PROJECT'] -plugin = nil if plugin == 'canvas-lms' +} OptionParser.new do |opts| - opts.on("--heavy") { heavy_mode = true } - opts.on("--boy-scout") { boyscout_mode = true } - opts.on("--plugin PLUGIN") { |v| plugin = v } + opts.on("--heavy") { linter_options[:heavy_mode] = true } + opts.on("--boy-scout") { linter_options[:boyscout_mode] = true } + opts.on("--plugin PLUGIN") { |v| linter_options[:plugin] = v } end.parse! -git_dir = "gems/plugins/#{plugin}/" if plugin -if git_dir && !Dir.exist?(git_dir) - puts "No plugin #{plugin} found" - exit 0 -end - -dr_diff = DrDiff::Manager.new(git_dir: git_dir, sha: env_sha) -ruby_files = dr_diff.files(/\.rb$/) - -if ruby_files.size == 0 - puts "No ruby file changes found, skipping Rubocop check!" - exit 0 -end - -if heavy_mode - run_heavy_mode(ruby_files) -else - severe_levels = boyscout_mode ? %w(error warn info) : %w(error warn) - comments = dr_diff.comments(format: "rubocop", - command: "bundle exec rubocop #{ruby_files.join(' ')}", - include_git_dir_in_output: !gerrit_patchset?, - severe_levels: severe_levels) - - unless comments.size > 0 - puts "-- -- -- -- -- -- -- -- -- -- --" - puts "No relevant rubocop errors found!" - puts "-- -- -- -- -- -- -- -- -- -- --" - exit(0) - end - - if gerrit_patchset? - require "gergich" - draft = Gergich::Draft.new - comments.each do |comment| - draft.add_comment comment[:path], - comment[:position], - comment[:message], - comment[:severity] - end - else - require 'colorize' - comments.each { |comment| pretty_comment(comment) } - end -end +rlint = Linter.new(linter_options) +rlint.run diff --git a/script/stylelint b/script/stylelint index db739a4f2b3..6d9ccc35ee6 100755 --- a/script/stylelint +++ b/script/stylelint @@ -1,52 +1,14 @@ #!/usr/bin/env ruby -$:.push File.expand_path("../../gems/dr_diff/lib", __FILE__) -require 'dr_diff' -require 'json' +require_relative './linter' -def pretty_comment(comment) - message = "" - message += "[#{comment[:severity]}]".colorize(:yellow) - message += " #{comment[:path].colorize(:light_blue)}:#{comment[:position]}" - message += " => #{comment[:message].colorize(:green)}" - puts message -end +linter_options = { + linter_name: "stylelint", + file_regex: /\.s?css$/, + format: 'stylelint', + command: 'node_modules/.bin/stylelint', + append_files_to_command: true, + default_boyscout_mode: false +} -def gerrit_patchset? - !!ENV['GERRIT_PATCHSET_REVISION'] -end - -env_sha = ENV['SHA'] || ENV['GERRIT_PATCHSET_REVISION'] - -dr_diff = DrDiff::Manager.new(sha: env_sha) -STYLELINT_FILE_REGEX = /\.s?css$/ -scss_files = dr_diff.files(STYLELINT_FILE_REGEX) - -if scss_files.size == 0 - puts "No scss or css file changes found!" - exit 0 -end - -comments = dr_diff.comments(format: "stylelint", - command: "node_modules/.bin/stylelint #{scss_files.join(' ')}", - include_git_dir_in_output: !gerrit_patchset?) - -unless comments.size > 0 - puts "-- -- -- -- -- -- -- -- -- -- --" - puts "No relevant scss or css errors found!" - puts "-- -- -- -- -- -- -- -- -- -- --" - exit(0) -end - -if gerrit_patchset? - require "gergich" - draft = Gergich::Draft.new - comments.each do |comment| - draft.add_comment comment[:path], - comment[:position], - comment[:message], - comment[:severity] - end -else - require 'colorize' - comments.each { |comment| pretty_comment(comment) } -end +stylelint = Linter.new(linter_options) +stylelint.run