From 19a71c6b649fe9d2672224d8cee95a944097e3e8 Mon Sep 17 00:00:00 2001 From: Ethan Vizitei Date: Sat, 4 Apr 2015 18:49:25 -0600 Subject: [PATCH] add some ruby linting for jenkins to take advantage of add migration lint as rubocop cops create frozen constant linter find_ids datafixup lint send_later lint lint send_later in predeploys add freeze_constant cop to default config don't forget to include rubocop when running get the rubocop runner into the script lint for wrong algorithm name lint primary key lint remove_column in predeploys get rubocop output as parsed json diff munging for gergich disable a few style cops tweak rubocop setup to allow IDE plugins to work get gergich comment format right shell out to gergich if we're in jenkins-land Change-Id: I6eecc8d8ede17a755c9d9a86121c3658776de9cd Reviewed-on: https://gerrit.instructure.com/51755 Tested-by: Jenkins Reviewed-by: Jason Madsen Product-Review: Ethan Vizitei QA-Review: Ethan Vizitei --- .rubocop.yml | 68 ++++++++++ Gemfile.d/.rubocop.yml | 10 ++ Gemfile.d/test.rb | 5 + db/migrate/.rubocop.yml | 12 ++ db/migrate/20101210192618_init_canvas_db.rb | 1 + ...0307190206_add_materialized_discussions.rb | 2 + ...0120328162105_add_discussion_topic_type.rb | 1 + ...14_update_collection_item_image_columns.rb | 1 + ...9_create_poll_sessions_and_modify_polls.rb | 1 + gems/rubocop-canvas/.gitignore | 14 +++ gems/rubocop-canvas/.rspec | 1 + gems/rubocop-canvas/Gemfile | 4 + gems/rubocop-canvas/README.md | 46 +++++++ gems/rubocop-canvas/Rakefile | 2 + gems/rubocop-canvas/config/default.yml | 22 ++++ gems/rubocop-canvas/lib/rubocop_canvas.rb | 32 +++++ .../rubocop_canvas/cops/datafixup/find_ids.rb | 20 +++ .../cops/lint/freeze_constants.rb | 43 +++++++ .../cops/migration/concurrent_index.rb | 47 +++++++ .../cops/migration/primary_key.rb | 26 ++++ .../cops/migration/remove_column.rb | 35 ++++++ .../cops/migration/send_later.rb | 30 +++++ .../lib/rubocop_canvas/helpers/comments.rb | 53 ++++++++ .../lib/rubocop_canvas/helpers/diff_parser.rb | 48 ++++++++ .../rubocop_canvas/helpers/migration_tags.rb | 13 ++ .../lib/rubocop_canvas/version.rb | 5 + gems/rubocop-canvas/rubocop-canvas.gemspec | 22 ++++ .../spec/rubocop/canvas/comments_spec.rb | 116 ++++++++++++++++++ .../spec/rubocop/canvas/diff_parser_spec.rb | 99 +++++++++++++++ .../rubocop/canvas/migration_tags_spec.rb | 9 ++ .../rubocop/cop/lint/freeze_constants_spec.rb | 67 ++++++++++ .../cop/migration/concurrent_index_spec.rb | 53 ++++++++ .../rubocop/cop/migration/primary_key_spec.rb | 28 +++++ .../cop/migration/remove_column_spec.rb | 46 +++++++ .../rubocop/cop/migration/send_later_spec.rb | 30 +++++ gems/rubocop-canvas/spec/spec_helper.rb | 18 +++ .../rubocop-canvas/spec/support/cop_helper.rb | 81 ++++++++++++ gems/rubocop-canvas/test.sh | 15 +++ script/rlint | 62 ++++++++++ 39 files changed, 1188 insertions(+) create mode 100644 .rubocop.yml create mode 100644 Gemfile.d/.rubocop.yml create mode 100644 db/migrate/.rubocop.yml create mode 100644 gems/rubocop-canvas/.gitignore create mode 100644 gems/rubocop-canvas/.rspec create mode 100644 gems/rubocop-canvas/Gemfile create mode 100644 gems/rubocop-canvas/README.md create mode 100644 gems/rubocop-canvas/Rakefile create mode 100644 gems/rubocop-canvas/config/default.yml create mode 100644 gems/rubocop-canvas/lib/rubocop_canvas.rb create mode 100644 gems/rubocop-canvas/lib/rubocop_canvas/cops/datafixup/find_ids.rb create mode 100644 gems/rubocop-canvas/lib/rubocop_canvas/cops/lint/freeze_constants.rb create mode 100644 gems/rubocop-canvas/lib/rubocop_canvas/cops/migration/concurrent_index.rb create mode 100644 gems/rubocop-canvas/lib/rubocop_canvas/cops/migration/primary_key.rb create mode 100644 gems/rubocop-canvas/lib/rubocop_canvas/cops/migration/remove_column.rb create mode 100644 gems/rubocop-canvas/lib/rubocop_canvas/cops/migration/send_later.rb create mode 100644 gems/rubocop-canvas/lib/rubocop_canvas/helpers/comments.rb create mode 100644 gems/rubocop-canvas/lib/rubocop_canvas/helpers/diff_parser.rb create mode 100644 gems/rubocop-canvas/lib/rubocop_canvas/helpers/migration_tags.rb create mode 100644 gems/rubocop-canvas/lib/rubocop_canvas/version.rb create mode 100644 gems/rubocop-canvas/rubocop-canvas.gemspec create mode 100644 gems/rubocop-canvas/spec/rubocop/canvas/comments_spec.rb create mode 100644 gems/rubocop-canvas/spec/rubocop/canvas/diff_parser_spec.rb create mode 100644 gems/rubocop-canvas/spec/rubocop/canvas/migration_tags_spec.rb create mode 100644 gems/rubocop-canvas/spec/rubocop/cop/lint/freeze_constants_spec.rb create mode 100644 gems/rubocop-canvas/spec/rubocop/cop/migration/concurrent_index_spec.rb create mode 100644 gems/rubocop-canvas/spec/rubocop/cop/migration/primary_key_spec.rb create mode 100644 gems/rubocop-canvas/spec/rubocop/cop/migration/remove_column_spec.rb create mode 100644 gems/rubocop-canvas/spec/rubocop/cop/migration/send_later_spec.rb create mode 100644 gems/rubocop-canvas/spec/spec_helper.rb create mode 100644 gems/rubocop-canvas/spec/support/cop_helper.rb create mode 100755 gems/rubocop-canvas/test.sh create mode 100755 script/rlint diff --git a/.rubocop.yml b/.rubocop.yml new file mode 100644 index 00000000000..8b82b83c1ac --- /dev/null +++ b/.rubocop.yml @@ -0,0 +1,68 @@ +require: + - rubocop-rspec + # this odd relative path is so that rubocop works when run without "bundle + # exec", such as from most editors/IDEs. + - ./gems/rubocop-canvas/lib/rubocop_canvas + +AllCops: + RunRailsCops: true + +# our style changes: disabling style rules we aren't interested in +Style/StringLiterals: + Enabled: false +Style/SignalException: + Enabled: false +Style/SpaceBeforeBlockBraces: + Enabled: false +Style/SpaceInsideBlockBraces: + Enabled: false +Style/NumericLiterals: + Enabled: false +Style/BracesAroundHashParameters: + Enabled: false +Style/SpaceInsideBrackets: + Enabled: false +Style/SpaceInsideHashLiteralBraces: + Enabled: false +Style/PercentLiteralDelimiters: + Enabled: false +Style/SpaceAroundOperators: + Enabled: false +Style/SpaceAfterColon: + Enabled: false +Style/SpaceAfterComma: + Enabled: false +Style/Documentation: + Enabled: false +Style/ClassAndModuleChildren: + Enabled: false +Style/RegexpLiteral: + Enabled: false +Style/EmptyLines: + Enabled: false +Style/EmptyLinesAroundMethodBody: + Enabled: false +Style/EmptyLinesAroundClassBody: + Enabled: false +Style/EmptyLinesAroundModuleBody: + Enabled: false +Style/GuardClause: + Enabled: false +Style/RedundantSelf: + Enabled: false +Style/SpaceAroundEqualsInParameterDefault: + Enabled: false +Style/SingleSpaceBeforeFirstArg: + Enabled: false +Style/HashSyntax: + Enabled: false +Style/EmptyLinesAroundAccessModifier: + Enabled: false +Style/IfUnlessModifier: + Enabled: false +Style/WordArray: + Enabled: false +Style/PercentQLiterals: + Enabled: false +Style/IndentHash: + Enabled: false diff --git a/Gemfile.d/.rubocop.yml b/Gemfile.d/.rubocop.yml new file mode 100644 index 00000000000..4894c825a75 --- /dev/null +++ b/Gemfile.d/.rubocop.yml @@ -0,0 +1,10 @@ +inherit_from: ../.rubocop.yml + +Metrics/LineLength: + Enabled: false +Style/IndentationConsistency: + Enabled: false +Metrics/MethodLength: + Enabled: false +Metrics/ClassLength: + Enabled: false diff --git a/Gemfile.d/test.rb b/Gemfile.d/test.rb index 59630c2eb25..ce8697c57f5 100644 --- a/Gemfile.d/test.rb +++ b/Gemfile.d/test.rb @@ -17,6 +17,11 @@ group :test do gem 'rspec-rails', '3.2.0' gem 'rspec-legacy_formatters', '1.0.0' gem 'rspec-collection_matchers', '1.1.2' + + gem 'rubocop', require: false + gem 'rubocop-rspec', require: false + gem 'rubocop-canvas', require: false, path: 'gems/rubocop-canvas' + gem 'once-ler', '0.0.15' gem 'sequel', '4.5.0', require: false diff --git a/db/migrate/.rubocop.yml b/db/migrate/.rubocop.yml new file mode 100644 index 00000000000..6f9a1cda6ac --- /dev/null +++ b/db/migrate/.rubocop.yml @@ -0,0 +1,12 @@ +inherit_from: ../../.rubocop.yml + +Metrics/LineLength: + Enabled: false +Style/IndentationConsistency: + Enabled: false +Metrics/MethodLength: + Enabled: false +Metrics/ClassLength: + Enabled: false +Metrics/AbcSize: + Enabled: false diff --git a/db/migrate/20101210192618_init_canvas_db.rb b/db/migrate/20101210192618_init_canvas_db.rb index cc29fd5baf8..8bda7336c55 100644 --- a/db/migrate/20101210192618_init_canvas_db.rb +++ b/db/migrate/20101210192618_init_canvas_db.rb @@ -16,6 +16,7 @@ # with this program. If not, see . # +# rubocop:disable Migration/PrimaryKey class InitCanvasDb < ActiveRecord::Migration def self.up create_table "abstract_courses", :force => true do |t| diff --git a/db/migrate/20120307190206_add_materialized_discussions.rb b/db/migrate/20120307190206_add_materialized_discussions.rb index e1a730299b2..b3bc53bb206 100644 --- a/db/migrate/20120307190206_add_materialized_discussions.rb +++ b/db/migrate/20120307190206_add_materialized_discussions.rb @@ -2,6 +2,8 @@ class AddMaterializedDiscussions < ActiveRecord::Migration tag :predeploy def self.up + # this is fixed in a later migration + # rubocop:disable Migration/PrimaryKey create_table :discussion_topic_materialized_views, :id => false do |t| t.integer :discussion_topic_id, :limit => 8 t.text :json_structure, :limit => 10.megabytes diff --git a/db/migrate/20120328162105_add_discussion_topic_type.rb b/db/migrate/20120328162105_add_discussion_topic_type.rb index 30cfb59451d..cb0f8a4c077 100644 --- a/db/migrate/20120328162105_add_discussion_topic_type.rb +++ b/db/migrate/20120328162105_add_discussion_topic_type.rb @@ -1,6 +1,7 @@ class AddDiscussionTopicType < ActiveRecord::Migration tag :predeploy + # rubocop:disable Migration/RemoveColumn def self.up remove_column :discussion_topics, :threaded add_column :discussion_topics, :discussion_type, :string diff --git a/db/migrate/20120511173314_update_collection_item_image_columns.rb b/db/migrate/20120511173314_update_collection_item_image_columns.rb index 12fcd3ee5c0..76fba364f07 100644 --- a/db/migrate/20120511173314_update_collection_item_image_columns.rb +++ b/db/migrate/20120511173314_update_collection_item_image_columns.rb @@ -1,6 +1,7 @@ class UpdateCollectionItemImageColumns < ActiveRecord::Migration tag :predeploy + # rubocop:disable Migration/RemoveColumn def self.up add_column :collection_item_datas, :image_pending, :boolean add_column :collection_item_datas, :image_attachment_id, :integer, :limit => 8 diff --git a/db/migrate/20140505211339_create_poll_sessions_and_modify_polls.rb b/db/migrate/20140505211339_create_poll_sessions_and_modify_polls.rb index 28851d5d5f2..78360e25a9b 100644 --- a/db/migrate/20140505211339_create_poll_sessions_and_modify_polls.rb +++ b/db/migrate/20140505211339_create_poll_sessions_and_modify_polls.rb @@ -1,6 +1,7 @@ class CreatePollSessionsAndModifyPolls < ActiveRecord::Migration tag :predeploy + # rubocop:disable Migration/RemoveColumn def self.up create_table :polling_poll_sessions do |t| t.boolean :is_published, null: false, default: false diff --git a/gems/rubocop-canvas/.gitignore b/gems/rubocop-canvas/.gitignore new file mode 100644 index 00000000000..ae3fdc2989a --- /dev/null +++ b/gems/rubocop-canvas/.gitignore @@ -0,0 +1,14 @@ +/.bundle/ +/.yardoc +/Gemfile.lock +/_yardoc/ +/coverage/ +/doc/ +/pkg/ +/spec/reports/ +/tmp/ +*.bundle +*.so +*.o +*.a +mkmf.log diff --git a/gems/rubocop-canvas/.rspec b/gems/rubocop-canvas/.rspec new file mode 100644 index 00000000000..c99d2e7396e --- /dev/null +++ b/gems/rubocop-canvas/.rspec @@ -0,0 +1 @@ +--require spec_helper diff --git a/gems/rubocop-canvas/Gemfile b/gems/rubocop-canvas/Gemfile new file mode 100644 index 00000000000..7533ef49c2b --- /dev/null +++ b/gems/rubocop-canvas/Gemfile @@ -0,0 +1,4 @@ +source 'https://rubygems.org' + +# Specify your gem's dependencies in rubocop-canvas.gemspec +gemspec diff --git a/gems/rubocop-canvas/README.md b/gems/rubocop-canvas/README.md new file mode 100644 index 00000000000..2db31f5c0ca --- /dev/null +++ b/gems/rubocop-canvas/README.md @@ -0,0 +1,46 @@ +# Rubocop::Canvas + +This is an extension of the RuboCop gem that provides some extra linters +("cops") that might be handy, and that provides a neat way to do +inline commenting in gerrit as a result of linted source + +## Usage + +### At Instructure +You don't really have to do anything if you're at Instructure! +The linter runs automatically on the CI server, using the diff-tree from the +HEAD commit to decide what files to look at. Then it takes the linted warnings, +compares them to the lines you changed +(based on chunk headers in "git show"), and posts them as inline gerrit +comments via gergich. + +### No CI Server? +No problem, you can still benefit from rubocop-canvas. In the +"script" directory of canvas-lms, there's a little glue code called +"rlint", just an executable that does some git-fu to get the right +files, and then will print out linting information for your most +recent change right to your console. Fix and repeat. :) + +## Modifying Rubocop::Canvas +You may have several ideas of things you want to do: + +#### Turn off a linter for a given directory +It doesn't make sense to lint database migrations as aggressively +as the rest of the source code, for example, so with the config file +in "db/migrate/.rubocop.yml" we turn off several linters that we might still +want elsewhere + +#### Turn off a given cop everywhere +Are you sure you aren't just frustrated at how much messy code you've written? +;) Ok, ok, you can screw with the config file in the root directory of +canvas-lms (".rubocop.yml") there are already examples in there of linters +that have been disabled because they're just too much noise. + +#### Add my own cop! +Have you discovered some new thing that we should try to detect automatically +from now on? Have a look at "freeze_constants.rb" in this project, you could +build something similar for whatever it is you need. Checkout +all the callbacks that are available for Cops at https://github.com/bbatsov/rubocop, +don't forget to write specs! + +Also, don't forget to enable your new cop by default in "config/default.yml" diff --git a/gems/rubocop-canvas/Rakefile b/gems/rubocop-canvas/Rakefile new file mode 100644 index 00000000000..809eb5616ad --- /dev/null +++ b/gems/rubocop-canvas/Rakefile @@ -0,0 +1,2 @@ +require "bundler/gem_tasks" + diff --git a/gems/rubocop-canvas/config/default.yml b/gems/rubocop-canvas/config/default.yml new file mode 100644 index 00000000000..41f3c2ed0ff --- /dev/null +++ b/gems/rubocop-canvas/config/default.yml @@ -0,0 +1,22 @@ +Lint/FreezeConstants: + enabled: true + +Migration/ConcurrentIndex: &Migration + Enabled: true + Include: + - "**/db/migrate/*.rb" + +Migration/SendLater: + <<: *Migration + +Migration/PrimaryKey: + <<: *Migration + +Migration/RemoveColumn: + <<: *Migration + +Datafixup/FindIds: + Enabled: true + Include: + - "**/db/migrate/*.rb" + - "**/lib/data_fixup/*.rb" diff --git a/gems/rubocop-canvas/lib/rubocop_canvas.rb b/gems/rubocop-canvas/lib/rubocop_canvas.rb new file mode 100644 index 00000000000..f142af55e0b --- /dev/null +++ b/gems/rubocop-canvas/lib/rubocop_canvas.rb @@ -0,0 +1,32 @@ +$LOAD_PATH << File.dirname(__FILE__) + +require 'rubocop' +require 'rubocop_canvas/version' + +require 'rubocop_canvas/helpers/migration_tags' +require 'rubocop_canvas/helpers/comments' +require 'rubocop_canvas/helpers/diff_parser' + +require 'rubocop_canvas/cops/lint/freeze_constants' +require 'rubocop_canvas/cops/datafixup/find_ids' +require 'rubocop_canvas/cops/migration/concurrent_index' +require 'rubocop_canvas/cops/migration/primary_key' +require 'rubocop_canvas/cops/migration/remove_column' +require 'rubocop_canvas/cops/migration/send_later' + +module RuboCop + module Canvas + module Inject + DEFAULT_FILE = File.expand_path("../../config/default.yml", __FILE__) + + def self.defaults! + hash = YAML.load_file(DEFAULT_FILE) + config = ConfigLoader.merge_with_default(hash, DEFAULT_FILE) + + ConfigLoader.instance_variable_set(:@default_configuration, config) + end + end + end +end + +RuboCop::Canvas::Inject.defaults! diff --git a/gems/rubocop-canvas/lib/rubocop_canvas/cops/datafixup/find_ids.rb b/gems/rubocop-canvas/lib/rubocop_canvas/cops/datafixup/find_ids.rb new file mode 100644 index 00000000000..5ac8b848a17 --- /dev/null +++ b/gems/rubocop-canvas/lib/rubocop_canvas/cops/datafixup/find_ids.rb @@ -0,0 +1,20 @@ +module RuboCop + module Cop + module Datafixup + class FindIds < Cop + def on_class(node) + @with_exclusive_scope = node.to_sexp =~ /with_exclusive_scope/ + end + + def on_send(node) + _receiver, method_name, *_args = *node + + if method_name.to_s =~ /find_ids_in_/ && !@with_exclusive_scope + add_offense(node, :expression, "find_ids_in without "\ + "with_exclusive_scope might be dangerous", :warning) + end + end + end + end + end +end diff --git a/gems/rubocop-canvas/lib/rubocop_canvas/cops/lint/freeze_constants.rb b/gems/rubocop-canvas/lib/rubocop_canvas/cops/lint/freeze_constants.rb new file mode 100644 index 00000000000..8ecead6df03 --- /dev/null +++ b/gems/rubocop-canvas/lib/rubocop_canvas/cops/lint/freeze_constants.rb @@ -0,0 +1,43 @@ +module RuboCop + module Cop + module Lint + class FreezeConstants < Cop + def on_casgn(node) + _scope, _const_name, value = *node + check_for_unfrozen_structures(value) + end + + def check_for_unfrozen_structures(node, safe_at_this_level=false) + safe_at_next_level = false + if node.type == :send + safe_at_next_level = send_is_safe?(node) + elsif node.type == :array || node.type == :hash + mark_offense!(node) unless safe_at_this_level + end + check_children(node, safe_at_next_level) + end + + private + def send_is_safe?(node) + return true if node.children.include?(:freeze) + mark_offense!(node) + false + end + + def check_children(node, safe_at_next_level) + node.children.each do |child| + if child.respond_to?(:type) + check_for_unfrozen_structures(child, safe_at_next_level) + end + end + end + + def mark_offense!(node) + add_offense(node, :expression, "data structure constants"\ + " should be frozen") + end + + end + end + end +end diff --git a/gems/rubocop-canvas/lib/rubocop_canvas/cops/migration/concurrent_index.rb b/gems/rubocop-canvas/lib/rubocop_canvas/cops/migration/concurrent_index.rb new file mode 100644 index 00000000000..c62b292eac0 --- /dev/null +++ b/gems/rubocop-canvas/lib/rubocop_canvas/cops/migration/concurrent_index.rb @@ -0,0 +1,47 @@ +module RuboCop + module Cop + module Migration + class ConcurrentIndex < Cop + def on_send(node) + _receiver, method_name, *args = *node + + case method_name + when :disable_ddl_transaction! + @disable_ddl_transaction = true + when :add_index + check_add_index(node, args) + end + end + + ALGORITHM = AST::Node.new(:sym, [:algorithm]) + + def check_add_index(node, args) + options = args.last + return unless options.hash_type? + + algorithm = options.children.find do |pair| + pair.children.first == ALGORITHM + end + return unless algorithm + algorithm_name = algorithm.children.last.children.first + + add_offenses(node, algorithm_name) + end + + private + + def add_offenses(node, algorithm_name) + if algorithm_name != :concurrently + add_offense(node, :expression, "Unknown algorithm name"\ + " `#{algorithm_name}`, did you mean `:concurrently`?") + end + + if algorithm_name == :concurrently && !@disable_ddl_transaction + add_offense(node, :expression, "Concurrent index adds require"\ + " `disable_ddl_transaction!`") + end + end + end + end + end +end diff --git a/gems/rubocop-canvas/lib/rubocop_canvas/cops/migration/primary_key.rb b/gems/rubocop-canvas/lib/rubocop_canvas/cops/migration/primary_key.rb new file mode 100644 index 00000000000..b89b656da23 --- /dev/null +++ b/gems/rubocop-canvas/lib/rubocop_canvas/cops/migration/primary_key.rb @@ -0,0 +1,26 @@ +module RuboCop + module Cop + module Migration + class PrimaryKey < Cop + def on_send(node) + _receiver, method_name, *args = *node + if method_name == :create_table + check_create_table(node, args) + end + end + + NO_PK = Parser::CurrentRuby.parse("{ id: false }").children.first + + def check_create_table(node, args) + options = args.last + return unless options.hash_type? + + if options.children.find { |pair| pair == NO_PK } + message = "Please always include a primary key" + add_offense(node, :expression, message) + end + end + end + end + end +end diff --git a/gems/rubocop-canvas/lib/rubocop_canvas/cops/migration/remove_column.rb b/gems/rubocop-canvas/lib/rubocop_canvas/cops/migration/remove_column.rb new file mode 100644 index 00000000000..6b1da316953 --- /dev/null +++ b/gems/rubocop-canvas/lib/rubocop_canvas/cops/migration/remove_column.rb @@ -0,0 +1,35 @@ +module RuboCop + module Cop + module Migration + class RemoveColumn < Cop + include RuboCop::Canvas::MigrationTags + + def on_def(node) + method_name, *_args = *node + @current_def = method_name + end + + def on_defs(node) + _receiver, method_name, *_args = *node + @current_def = method_name + end + + def on_send(node) + super + _receiver, method_name, *_args = *node + if remove_column_in_predeploy?(method_name) + message = "`remove_column` needs to be in a postdeploy migration" + add_offense(node, :expression, message) + end + end + + private + def remove_column_in_predeploy?(method_name) + tags.include?(:predeploy) && + method_name == :remove_column && + @current_def == :up + end + end + end + end +end diff --git a/gems/rubocop-canvas/lib/rubocop_canvas/cops/migration/send_later.rb b/gems/rubocop-canvas/lib/rubocop_canvas/cops/migration/send_later.rb new file mode 100644 index 00000000000..44f14c5df8f --- /dev/null +++ b/gems/rubocop-canvas/lib/rubocop_canvas/cops/migration/send_later.rb @@ -0,0 +1,30 @@ +module RuboCop + module Cop + module Migration + class SendLater < Cop + include RuboCop::Canvas::MigrationTags + + def on_send(node) + super + _receiver, method_name = *node + if method_name.to_s =~ /^send_later/ + check_send_later(node, method_name) + end + end + + def check_send_later(node, method_name) + if method_name.to_s !~ /if_production/ + add_offense(node, :expression, "All `send_later`s in migrations "\ + "should be `send_later_if_production`") + end + + if tags.include?(:predeploy) + add_offense(node, :expression, "`send_later` cannot be used in a "\ + "predeploy migration, since job servers won't"\ + " have the new code yet") + end + end + end + end + end +end diff --git a/gems/rubocop-canvas/lib/rubocop_canvas/helpers/comments.rb b/gems/rubocop-canvas/lib/rubocop_canvas/helpers/comments.rb new file mode 100644 index 00000000000..bc1925cd3b6 --- /dev/null +++ b/gems/rubocop-canvas/lib/rubocop_canvas/helpers/comments.rb @@ -0,0 +1,53 @@ +require_relative "./diff_parser" +module RuboCop::Canvas + class Comments + def self.build(raw_diff_tree, cop_output) + diff = DiffParser.new(raw_diff_tree) + comments = self.new(diff) + comments.on_output(cop_output) + end + + attr_reader :diff + def initialize(diff) + @diff = diff + end + + def on_output(cop_output) + comments = [] + cop_output['files'].each do |file| + path = file['path'] + file['offenses'].each do |offense| + if diff.relevant?(path, line_number(offense)) + comments << transform_to_gergich_comment(path, offense) + end + end + end + comments + end + + private + + SEVERITY_MAPPING = { + 'refactor' => 'info', + 'convention' => 'info', + 'warning' => 'warn', + 'error' => 'error', + 'fatal' => 'error' + }.freeze + + def transform_to_gergich_comment(path, offense) + { + path: path, + position: line_number(offense), + message: offense['message'], + severity: SEVERITY_MAPPING[offense['severity']] + } + end + + def line_number(offense) + offense['location']['line'] + end + + end + +end diff --git a/gems/rubocop-canvas/lib/rubocop_canvas/helpers/diff_parser.rb b/gems/rubocop-canvas/lib/rubocop_canvas/helpers/diff_parser.rb new file mode 100644 index 00000000000..ca58f87f4e8 --- /dev/null +++ b/gems/rubocop-canvas/lib/rubocop_canvas/helpers/diff_parser.rb @@ -0,0 +1,48 @@ +module RuboCop::Canvas + class DiffParser + + attr_reader :diff + + def initialize(input, raw = true) + @diff = (raw ? parse_raw_diff(input) : input) + end + + def relevant?(path, line_number) + return false unless diff[path] + diff[path].any?{|range| range.include?(line_number)} + end + + private + + def parse_raw_diff(raw_diff) + parsed = {} + key = "" + raw_diff.each_line.map(&:strip).each do |line| + if file_line?(line) + key = path_from_file_line(line) + parsed[key] ||= [] + end + line_range?(line) && (parsed[key] << range_from_file_line(line)) + end + parsed + end + + def file_line?(line) + line =~ /^\+\+\+ b\/.*\./ + end + + def line_range?(line) + line =~ /^@@ -\d.*\+\d.* @@/ + end + + def path_from_file_line(line) + line.split(/\s/).last.gsub(/^b\//, "") + end + + def range_from_file_line(line) + line_plus_size = line.split(/\s/)[2].split(",") + start = line_plus_size[0].gsub("+",'').to_i + (start..(start + line_plus_size[1].to_i)) + end + end +end diff --git a/gems/rubocop-canvas/lib/rubocop_canvas/helpers/migration_tags.rb b/gems/rubocop-canvas/lib/rubocop_canvas/helpers/migration_tags.rb new file mode 100644 index 00000000000..19b2e5b7f62 --- /dev/null +++ b/gems/rubocop-canvas/lib/rubocop_canvas/helpers/migration_tags.rb @@ -0,0 +1,13 @@ +module RuboCop::Canvas + module MigrationTags + def on_send(node) + receiver, method_name, *args = *node + return unless !receiver && method_name == :tag + @tags = args.map { |n| n.children.first } + end + + def tags + @tags || [] + end + end +end diff --git a/gems/rubocop-canvas/lib/rubocop_canvas/version.rb b/gems/rubocop-canvas/lib/rubocop_canvas/version.rb new file mode 100644 index 00000000000..99cb2b77504 --- /dev/null +++ b/gems/rubocop-canvas/lib/rubocop_canvas/version.rb @@ -0,0 +1,5 @@ +module Rubocop + module Canvas + VERSION = "1.0.0" + end +end diff --git a/gems/rubocop-canvas/rubocop-canvas.gemspec b/gems/rubocop-canvas/rubocop-canvas.gemspec new file mode 100644 index 00000000000..9f5477a67ab --- /dev/null +++ b/gems/rubocop-canvas/rubocop-canvas.gemspec @@ -0,0 +1,22 @@ +# coding: utf-8 +lib = File.expand_path('../lib', __FILE__) +$LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) +require 'rubocop_canvas/version' + +Gem::Specification.new do |spec| + spec.name = "rubocop-canvas" + spec.version = Rubocop::Canvas::VERSION + spec.authors = ["Brian Palmer"] + spec.email = ["brianp@instructure.com"] + spec.summary = %q{custom cops for canvas} + + spec.files = Dir.glob("{lib,spec}/**/*") + %w(test.sh) + spec.test_files = spec.files.grep(%r{^(test|spec|features)/}) + spec.require_paths = ["lib"] + + spec.add_runtime_dependency "rubocop" + spec.add_development_dependency "bundler", "~> 1.7" + spec.add_development_dependency "rake", "~> 10.0" + spec.add_development_dependency "rspec", "~> 3.2.0" + spec.add_development_dependency "pry", "~> 0.10.1" +end diff --git a/gems/rubocop-canvas/spec/rubocop/canvas/comments_spec.rb b/gems/rubocop-canvas/spec/rubocop/canvas/comments_spec.rb new file mode 100644 index 00000000000..6f3c143e821 --- /dev/null +++ b/gems/rubocop-canvas/spec/rubocop/canvas/comments_spec.rb @@ -0,0 +1,116 @@ + +require 'spec_helper' + +describe RuboCop::Canvas::Comments do + it "picks out which comments are applicable" do + output = TWO_FILE_OFFENSE_OUTPUT + diff = { + 'app/controllers/quizzes/quizzes_controller.rb' => [(1..3), (200..250)], + 'app/controllers/application_controller.rb' => [(80..110)] + } + parsed_diff = ::RuboCop::Canvas::DiffParser.new(diff, false) + comments = described_class.new(parsed_diff) + results = comments.on_output(output) + expected = [{ + path: "app/controllers/application_controller.rb", + position: 100, + message: "Class definition is too long. [748/100]", + severity: "info" + }] + expect(results).to eq(expected) + end + + it "integrates a real dataset and makes legitimate comments" do + # rubocop:disable all + raw_diff = %Q{ + commit cd4249a9bbc70ce0d7f767b137ca15671d34a277 +Author: Ethan Vizitei +Date: Mon Apr 6 19:50:34 2015 -0600 + + Line too long + + Change-Id: I0aa8af857501ae3fbc21366f742ea014cbcd2564 + +diff --git a/app/helpers/calendars_helper.rb b/app/helpers/calendars_helper.rb +index 00f8cde..087ac4a 100644 +--- a/app/helpers/calendars_helper.rb ++++ b/app/helpers/calendars_helper.rb +@@ -18,7 +18,7 @@ + + module CalendarsHelper + def pastel_color_index(idx) +- colors = ['#c0deca', '#f8ca35', '#d8dec0', '#b6b6b6', '#b3d1d1', '#cde5ab', '#c8b3d1', '#d1beb3', '#bfafaf', '#ddac81', '#d5d5d5', '#d49abe', '#c1ee82', '#98cbd1', '#b7b29c'] ++ colors = ['#c0deca', '#f8ca35', '#d8dec0', '#b6b6b6', '#b3d1d1', '#cde5ab', '#c8b3d1', '#d1beb3', '#bfafaf', '#ddac81', '#d5d5d5', '#d49abe', '#c1ee82', '#98cbd1', '#b7b29c', 'Make a long line longer'] + colors[idx % colors.length] + end + } + # rubocop:enable all + + parsed_rubocop_output = LARGE_OFFENSE_LIST + diff = ::RuboCop::Canvas::DiffParser.new(raw_diff) + comments = described_class.new(diff) + results = comments.on_output(parsed_rubocop_output) + expect(results.length).to eq(3) + end + + + # rubocop:disable all + LARGE_OFFENSE_LIST = { + "files"=>[ + { + "path"=>"app/helpers/calendars_helper.rb", + "offenses"=> + [ + { + "severity"=>"convention", + "message"=>"Missing top-level module documentation comment.", + "cop_name"=>"Style/Documentation", + "corrected"=>nil, + "location"=>{"line"=>19, "column"=>1, "length"=>6} + },{ + "severity"=>"convention", + "message"=>"Line is too long. [178/80]", + "cop_name"=>"Metrics/LineLength", + "corrected"=>nil, + "location"=>{"line"=>21, "column"=>81, "length"=>98} + },{ + "severity"=>"convention", + "message"=>"Trailing whitespace detected.", + "cop_name"=>"Style/TrailingWhitespace", + "corrected"=>false, + "location"=>{"line"=>24, "column"=>1, "length"=>2} + },{ + "severity"=>"convention", + "message"=>"Trailing whitespace detected.", + "cop_name"=>"Style/TrailingWhitespace", + "corrected"=>false, + "location"=>{"line"=>28, "column"=>1, "length"=>2} + },{ + "severity"=>"convention", + "message"=>"Trailing whitespace detected.", + "cop_name"=>"Style/TrailingWhitespace", + "corrected"=>false, + "location"=>{"line"=>32, "column"=>1, "length"=>2} + } + ] + } + ] + } + + TWO_FILE_OFFENSE_OUTPUT = { + "metadata" => { "rubocop_version" => "0.30.0" }, + "files" => [ + { + "path" => "app/controllers/quizzes/quizzes_controller.rb","offenses" => + [{"severity" => "convention","message" => "Class definition is too long. [748/100]", + "cop_name" => "Metrics/ClassLength","corrected" =>nil,"location"=>{"line"=>19,"column"=>1,"length"=>5}}] + }, + { + "path"=>"app/controllers/application_controller.rb","offenses"=> + [{"severity"=>"convention","message"=>"Class definition is too long. [748/100]", + "cop_name"=>"Metrics/ClassLength","corrected" => nil,"location"=>{"line"=>100,"column"=>1,"length"=>5}}] + } + ] + } + # rubocop:enable all +end diff --git a/gems/rubocop-canvas/spec/rubocop/canvas/diff_parser_spec.rb b/gems/rubocop-canvas/spec/rubocop/canvas/diff_parser_spec.rb new file mode 100644 index 00000000000..e7f46217bd8 --- /dev/null +++ b/gems/rubocop-canvas/spec/rubocop/canvas/diff_parser_spec.rb @@ -0,0 +1,99 @@ +require 'spec_helper' + +describe RuboCop::Canvas::DiffParser do + describe "#relevant?" do + let(:parser){ described_class.new(raw_diff) } + + context "diff with new code" do + let(:raw_diff) do + %{ + commit 5f073b464f0274b3035039b74c8dee84c21835dc + Author: Ethan Vizitei + Date: Mon Apr 6 15:31:52 2015 -0600 + + first commit + + diff --git a/some_file.rb b/some_file.rb + index eede799..c949884 100644 + --- a/some_file.rb + +++ b/some_file.rb + @@ -1,5 +1,6 @@ + # encoding: utf-8 + + +puts "DIFF TWO" + require 'spec_helper' + + describe RuboCop::Cop::Style::ConstantName do + @@ -56,6 +57,9 @@ describe RuboCop::Cop::Style::ConstantName do + expect(cop.offenses).to be_empty + end + + + + + puts "DIFF ONE" + + + it 'checks qualified const names' do + inspect_source(cop, + ['::AnythingGoes = 30', + + } + end + + it "is true for same file with a line number in the range" do + expect(parser.relevant?("some_file.rb", 3)).to be(true) + end + + it "is false for a different file" do + expect(parser.relevant?("some_other_file.rb", 3)).to be(false) + end + + it "is false for a line number outside the change ranges" do + expect(parser.relevant?("some_file.rb", 8)).to be(false) + end + + it "is true at the boundary of a diff set" do + expect(parser.relevant?("some_file.rb", 56)).to be(false) + expect(parser.relevant?("some_file.rb", 57)).to be(true) + expect(parser.relevant?("some_file.rb", 66)).to be(true) + expect(parser.relevant?("some_file.rb", 67)).to be(false) + end + end + + context "for diffs with deletions" do + let(:raw_diff) do + %{commit 6321b044912b5162a07c2f4f88c335ee1d781d1e + Author: Ethan Vizitei + Date: Wed Apr 8 14:23:26 2015 -0600 + + remove something + + diff --git a/some_file.rb b/some_file.rb + index c949884..150ed80 100644 + --- a/some_file.rb + +++ b/some_file.rb + @@ -56,14 +56,4 @@ describe RuboCop::Cop::Style::ConstantName do + 'Parser::CurrentRuby = Parser::Ruby20') + expect(cop.offenses).to be_empty + end + - + - + - puts "DIFF ONE" + - + - it 'checks qualified const names' do + - inspect_source(cop, + - ['::AnythingGoes = 30', + - 'a::Bar_foo = 10']) + - expect(cop.offenses.size).to eq(2) + - end + end + } + end + + it 'respects the boundary of a removed diff set' do + expect(parser.relevant?("some_file.rb", 55)).to be(false) + expect(parser.relevant?("some_file.rb", 56)).to be(true) + expect(parser.relevant?("some_file.rb", 60)).to be(true) + expect(parser.relevant?("some_file.rb", 61)).to be(false) + end + end + end +end diff --git a/gems/rubocop-canvas/spec/rubocop/canvas/migration_tags_spec.rb b/gems/rubocop-canvas/spec/rubocop/canvas/migration_tags_spec.rb new file mode 100644 index 00000000000..860b7330d99 --- /dev/null +++ b/gems/rubocop-canvas/spec/rubocop/canvas/migration_tags_spec.rb @@ -0,0 +1,9 @@ +describe RuboCop::Canvas::MigrationTags do + subject { Class.new.tap { |c| c.include(described_class) }.new } + + it 'collects the list of tags for the migration' do + node = Parser::CurrentRuby.parse('tag :predeploy, :cassandra') + subject.on_send(node) + expect(subject.tags).to eq([:predeploy, :cassandra]) + end +end diff --git a/gems/rubocop-canvas/spec/rubocop/cop/lint/freeze_constants_spec.rb b/gems/rubocop-canvas/spec/rubocop/cop/lint/freeze_constants_spec.rb new file mode 100644 index 00000000000..000362875b9 --- /dev/null +++ b/gems/rubocop-canvas/spec/rubocop/cop/lint/freeze_constants_spec.rb @@ -0,0 +1,67 @@ +describe RuboCop::Cop::Lint::FreezeConstants do + subject(:cop) { described_class.new } + + it 'doesnt care about single literals' do + inspect_source(cop, %{ MAX_SOMETHING = 5 }) + expect(cop.offenses.size).to eq(0) + end + + it 'warns about unfrozen arrays' do + inspect_source(cop, %{ BLAH = [1,2,3,4] }) + expect(cop.offenses.size).to eq(1) + expect(cop.messages.first).to match(/should be frozen/) + end + + it 'likes frozen arrays' do + inspect_source(cop, %{ BLAH = [1,2,3,4].freeze }) + expect(cop.offenses.size).to eq(0) + end + + it 'warns for unfrozen hashes' do + inspect_source(cop, %{ FOO = {one: 'two', three: 'four'} }) + expect(cop.offenses.size).to eq(1) + expect(cop.messages.first).to match(/should be frozen/) + end + + it 'is ok with frozen hashes' do + inspect_source(cop, %{ FOO = {one: 'two', three: 'four'}.freeze }) + expect(cop.offenses.size).to eq(0) + end + + it 'catches nested arrays within a hash' do + inspect_source(cop, %{ FOO = {one: 'two', three: ['a', 'b']}.freeze }) + expect(cop.offenses.size).to eq(1) + end + + it 'will go several levels deep with one offense for each structure' do + inspect_source(cop, %{ + FOO = { + one: { + two: { + three: { + four: 5 + } + } + } + } + }) + expect(cop.offenses.size).to eq(4) + end + + it 'also catches unfrozen nested arrays' do + inspect_source(cop, %{ MATRIX = [[[1,2], [3,4]], [[5,6], [7,8]]] }) + expect(cop.offenses.size).to eq(7) + end + + it 'doesnt interrupt code with no constant assignments' do + inspect_source(cop, %{ + class TestMigration < ActiveRecord::Migration + + def up + add_index :my_index + end + end + }) + expect(cop.offenses.size).to eq(0) + end +end diff --git a/gems/rubocop-canvas/spec/rubocop/cop/migration/concurrent_index_spec.rb b/gems/rubocop-canvas/spec/rubocop/cop/migration/concurrent_index_spec.rb new file mode 100644 index 00000000000..184f390efb8 --- /dev/null +++ b/gems/rubocop-canvas/spec/rubocop/cop/migration/concurrent_index_spec.rb @@ -0,0 +1,53 @@ +describe RuboCop::Cop::Migration::ConcurrentIndex do + subject(:cop) { described_class.new } + + it 'complains about concurrent indexes in ddl transaction' do + inspect_source(cop, %{ + class TestMigration < ActiveRecord::Migration + + def up + add_index :my_index, algorithm: :concurrently + end + end + }) + expect(cop.offenses.size).to eq(1) + expect(cop.messages.first).to match(/disable_ddl/) + end + + it 'ignores non-concurrent indexes' do + inspect_source(cop, %{ + class TestMigration < ActiveRecord::Migration + + def up + add_index :my_index + end + end + }) + expect(cop.offenses.size).to eq(0) + end + + it 'is ok with concurrent indexes added non-transactionally' do + inspect_source(cop, %{ + class TestMigration < ActiveRecord::Migration + disable_ddl_transaction! + + def up + add_index :my_index, algorithm: :concurrently + end + end + }) + expect(cop.offenses.size).to eq(0) + end + + it 'complains about unknown algorithm' do + inspect_source(cop, %{ + class TestMigration < ActiveRecord::Migration + def up + add_index :my_index, algorithm: :concurrent + end + end + }) + expect(cop.offenses.size).to eq(1) + expect(cop.messages.first).to match(/unknown algorithm/i) + end +end diff --git a/gems/rubocop-canvas/spec/rubocop/cop/migration/primary_key_spec.rb b/gems/rubocop-canvas/spec/rubocop/cop/migration/primary_key_spec.rb new file mode 100644 index 00000000000..e38d6931ca5 --- /dev/null +++ b/gems/rubocop-canvas/spec/rubocop/cop/migration/primary_key_spec.rb @@ -0,0 +1,28 @@ +describe RuboCop::Cop::Migration::PrimaryKey do + subject(:cop) { described_class.new } + + it 'catches explicit id disabled' do + inspect_source(cop, %{ + class CreateNotificationEndpoints < ActiveRecord::Migration + tag :predeploy + + def self.up + create_table(:notification_endpoints, id: false) do |t| + t.integer :access_token_id, limit: 8, null: false + t.string :token, null: false + t.string :arn, null: false + t.timestamps + end + add_index :notification_endpoints, :access_token_id + add_foreign_key :notification_endpoints, :access_tokens + end + + def self.down + drop_table :notification_endpoints + end + end + }) + expect(cop.offenses.size).to eq(1) + expect(cop.messages.first).to match(/include a primary key/) + end +end diff --git a/gems/rubocop-canvas/spec/rubocop/cop/migration/remove_column_spec.rb b/gems/rubocop-canvas/spec/rubocop/cop/migration/remove_column_spec.rb new file mode 100644 index 00000000000..a7a94b9080f --- /dev/null +++ b/gems/rubocop-canvas/spec/rubocop/cop/migration/remove_column_spec.rb @@ -0,0 +1,46 @@ +describe RuboCop::Cop::Migration::RemoveColumn do + subject(:cop) { described_class.new } + + context 'predeploy' do + it 'disallows remove_column in `up`' do + inspect_source(cop, %{ + class MyMigration < ActiveRecord::Migration + tag :predeploy + + def up + remove_column :x, :y + end + end + }) + expect(cop.offenses.size).to eq(1) + expect(cop.messages.first).to match(/remove_column/) + end + + it 'disallows remove_column in `self.up`' do + inspect_source(cop, %{ + class MyMigration < ActiveRecord::Migration + tag :predeploy + + def self.up + remove_column :x, :y + end + end + }) + expect(cop.offenses.size).to eq(1) + expect(cop.messages.first).to match(/remove_column/) + end + + it 'allows remove_column in `down`' do + inspect_source(cop, %{ + class MyMigration < ActiveRecord::Migration + tag :predeploy + + def down + remove_column :x, :y + end + end + }) + expect(cop.offenses.size).to eq(0) + end + end +end diff --git a/gems/rubocop-canvas/spec/rubocop/cop/migration/send_later_spec.rb b/gems/rubocop-canvas/spec/rubocop/cop/migration/send_later_spec.rb new file mode 100644 index 00000000000..9dfa1c42f06 --- /dev/null +++ b/gems/rubocop-canvas/spec/rubocop/cop/migration/send_later_spec.rb @@ -0,0 +1,30 @@ +describe RuboCop::Cop::Migration::SendLater do + subject(:cop) { described_class.new } + + it 'catches other forms of send_later' do + inspect_source(cop, %{ + class TestMigration < ActiveRecord::Migration + + def up + MyClass.send_later_enqueue_args(:run, max_attempts: 1) + end + end + }) + expect(cop.offenses.size).to eq(1) + expect(cop.messages.first).to match(/if_production/) + end + + it 'disallows send_later in predeploys' do + inspect_source(cop, %{ + class TestMigration < ActiveRecord::Migration + tag :predeploy + + def up + MyClass.send_later_if_production(:run, max_attempts: 1) + end + end + }) + expect(cop.offenses.size).to eq(1) + expect(cop.messages.first).to match(/predeploy/) + end +end diff --git a/gems/rubocop-canvas/spec/spec_helper.rb b/gems/rubocop-canvas/spec/spec_helper.rb new file mode 100644 index 00000000000..cd2cac932f3 --- /dev/null +++ b/gems/rubocop-canvas/spec/spec_helper.rb @@ -0,0 +1,18 @@ +require 'rubocop' + +RSpec.configure do |config| + config.order = :random + + config.expect_with :rspec do |expectations| + expectations.syntax = :expect # Disable `should` + end + + config.mock_with :rspec do |mocks| + mocks.syntax = :expect # Disable `should_receive` and `stub` + end +end + +$LOAD_PATH.unshift(File.join(File.dirname(__FILE__), '..', 'lib')) +$LOAD_PATH.unshift(File.dirname(__FILE__)) +require 'support/cop_helper' +require 'rubocop_canvas' diff --git a/gems/rubocop-canvas/spec/support/cop_helper.rb b/gems/rubocop-canvas/spec/support/cop_helper.rb new file mode 100644 index 00000000000..245f5340c73 --- /dev/null +++ b/gems/rubocop-canvas/spec/support/cop_helper.rb @@ -0,0 +1,81 @@ +# encoding: utf-8 + +require 'tempfile' + +# This provides many of the helper hooks +# that rubocop itself uses inside it's gem tests, along +# with some hooks for inspecting messages generated from offenses +# to make tests more focused on the cop conditions they're building +module CopHelper + def inspect_source_file(cop, source) + Tempfile.open('tmp') { |f| inspect_source(cop, source, f) } + end + + def inspect_source(cop, source, file = nil) + if source.is_a?(Array) && source.size == 1 + fail "Don't use an array for a single line of code: #{source}" + end + RuboCop::Formatter::DisabledConfigFormatter.config_to_allow_offenses = {} + processed_source = parse_source(source, file) + fail 'Error parsing example code' unless processed_source.valid_syntax? + _investigate(cop, processed_source) + end + + def parse_source(source, file = nil) + source = source.join($RS) if source.is_a?(Array) + + if file && file.respond_to?(:write) + file.write(source) + file.rewind + file = file.path + end + + RuboCop::ProcessedSource.new(source, file) + end + + def autocorrect_source_file(cop, source) + Tempfile.open('tmp') { |f| autocorrect_source(cop, source, f) } + end + + def autocorrect_source(cop, source, file = nil) + cop.instance_variable_get(:@options)[:auto_correct] = true + processed_source = parse_source(source, file) + _investigate(cop, processed_source) + + corrector = + RuboCop::Cop::Corrector.new(processed_source.buffer, cop.corrections) + corrector.rewrite + end + + def _investigate(cop, processed_source) + forces = RuboCop::Cop::Force.all.each_with_object([]) do |klass, instances| + next unless cop.join_force?(klass) + instances << klass.new([cop]) + end + + commissioner = + RuboCop::Cop::Commissioner.new([cop], forces, raise_error: true) + commissioner.investigate(processed_source) + commissioner + end +end + +module RuboCop + module Cop + # This re-opens the internal rubocop class just to get some hooks in + # for inspecting offenses more tersely in specs + class Cop + def messages + offenses.sort.map(&:message) + end + + def highlights + offenses.sort.map { |o| o.location.source } + end + end + end +end + +RSpec.configure do |config| + config.include CopHelper +end diff --git a/gems/rubocop-canvas/test.sh b/gems/rubocop-canvas/test.sh new file mode 100755 index 00000000000..350363e52d2 --- /dev/null +++ b/gems/rubocop-canvas/test.sh @@ -0,0 +1,15 @@ +#!/bin/bash +result=0 + +echo "################ csv_diff ################" +bundle check || bundle install +bundle exec rspec spec +result+=$? + +if [ $result -eq 0 ]; then + echo "SUCCESS" +else + echo "FAILURE" +fi + +exit $result diff --git a/script/rlint b/script/rlint new file mode 100755 index 00000000000..5a0d22cb795 --- /dev/null +++ b/script/rlint @@ -0,0 +1,62 @@ +#!/usr/bin/env ruby +$:.push File.expand_path("../../gems/rubocop-canvas/lib", __FILE__) + +def git_changes(include_untracked=false) + if include_untracked + `git status --porcelain` + else + `git status --porcelain -uno` + end +end + +def git_is_dirty? + !git_changes.strip.empty? +end + +def head_sha + `git log --oneline -n 1 | cut -d " " -f 1` +end + +def pick_ruby_files(env_sha) + sha = env_sha + if sha.nil? + if git_is_dirty? + return git_changes(true).split("\n").map{|f| f.strip.split(/\s/).last }.select{|f| f =~ /\.rb$/} + else + # no changes, no sha specified, use the HEAD commit + sha = head_sha + end + end + files = `git diff-tree --no-commit-id --name-only -r #{sha}` + return files.split("\n").select{|f| f =~ /\.rb$/} +end + +def git_diff(env_sha) + sha = env_sha + if sha.nil? + return `git diff` if git_is_dirty? + sha = head_sha + end + `git show #{sha}` +end + +require 'rubocop_canvas' + +env_sha = ENV['SHA'] +ruby_files = pick_ruby_files(env_sha) +cli = RuboCop::CLI.new +if ENV['GERRIT_REPO'] + # we're on a jenkins box, lets send diff info via gergich + require 'rubocop_canvas/helpers/comments' + require 'shellwords' + cli.run(ruby_files + ["--format", "json", "--out", "rubocop.json"]) + results_json = JSON.parse(File.read("rubocop.json")) + diff = git_diff(env_sha) + comments = RuboCop::Canvas::Comments.build(diff, results_json) + comments.each do |comment| + `gergich comment #{Shellwords.escape(comment.to_json)}` + end +else + #local run, just spit out everything wrong with these files to output + cli.run(ruby_files) +end