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 <jmadsen@instructure.com> Product-Review: Ethan Vizitei <evizitei@instructure.com> QA-Review: Ethan Vizitei <evizitei@instructure.com>
This commit is contained in:
parent
0463872031
commit
19a71c6b64
|
@ -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
|
|
@ -0,0 +1,10 @@
|
||||||
|
inherit_from: ../.rubocop.yml
|
||||||
|
|
||||||
|
Metrics/LineLength:
|
||||||
|
Enabled: false
|
||||||
|
Style/IndentationConsistency:
|
||||||
|
Enabled: false
|
||||||
|
Metrics/MethodLength:
|
||||||
|
Enabled: false
|
||||||
|
Metrics/ClassLength:
|
||||||
|
Enabled: false
|
|
@ -17,6 +17,11 @@ group :test do
|
||||||
gem 'rspec-rails', '3.2.0'
|
gem 'rspec-rails', '3.2.0'
|
||||||
gem 'rspec-legacy_formatters', '1.0.0'
|
gem 'rspec-legacy_formatters', '1.0.0'
|
||||||
gem 'rspec-collection_matchers', '1.1.2'
|
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 'once-ler', '0.0.15'
|
||||||
|
|
||||||
gem 'sequel', '4.5.0', require: false
|
gem 'sequel', '4.5.0', require: false
|
||||||
|
|
|
@ -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
|
|
@ -16,6 +16,7 @@
|
||||||
# with this program. If not, see <http://www.gnu.org/licenses/>.
|
# with this program. If not, see <http://www.gnu.org/licenses/>.
|
||||||
#
|
#
|
||||||
|
|
||||||
|
# rubocop:disable Migration/PrimaryKey
|
||||||
class InitCanvasDb < ActiveRecord::Migration
|
class InitCanvasDb < ActiveRecord::Migration
|
||||||
def self.up
|
def self.up
|
||||||
create_table "abstract_courses", :force => true do |t|
|
create_table "abstract_courses", :force => true do |t|
|
||||||
|
|
|
@ -2,6 +2,8 @@ class AddMaterializedDiscussions < ActiveRecord::Migration
|
||||||
tag :predeploy
|
tag :predeploy
|
||||||
|
|
||||||
def self.up
|
def self.up
|
||||||
|
# this is fixed in a later migration
|
||||||
|
# rubocop:disable Migration/PrimaryKey
|
||||||
create_table :discussion_topic_materialized_views, :id => false do |t|
|
create_table :discussion_topic_materialized_views, :id => false do |t|
|
||||||
t.integer :discussion_topic_id, :limit => 8
|
t.integer :discussion_topic_id, :limit => 8
|
||||||
t.text :json_structure, :limit => 10.megabytes
|
t.text :json_structure, :limit => 10.megabytes
|
||||||
|
|
|
@ -1,6 +1,7 @@
|
||||||
class AddDiscussionTopicType < ActiveRecord::Migration
|
class AddDiscussionTopicType < ActiveRecord::Migration
|
||||||
tag :predeploy
|
tag :predeploy
|
||||||
|
|
||||||
|
# rubocop:disable Migration/RemoveColumn
|
||||||
def self.up
|
def self.up
|
||||||
remove_column :discussion_topics, :threaded
|
remove_column :discussion_topics, :threaded
|
||||||
add_column :discussion_topics, :discussion_type, :string
|
add_column :discussion_topics, :discussion_type, :string
|
||||||
|
|
|
@ -1,6 +1,7 @@
|
||||||
class UpdateCollectionItemImageColumns < ActiveRecord::Migration
|
class UpdateCollectionItemImageColumns < ActiveRecord::Migration
|
||||||
tag :predeploy
|
tag :predeploy
|
||||||
|
|
||||||
|
# rubocop:disable Migration/RemoveColumn
|
||||||
def self.up
|
def self.up
|
||||||
add_column :collection_item_datas, :image_pending, :boolean
|
add_column :collection_item_datas, :image_pending, :boolean
|
||||||
add_column :collection_item_datas, :image_attachment_id, :integer, :limit => 8
|
add_column :collection_item_datas, :image_attachment_id, :integer, :limit => 8
|
||||||
|
|
|
@ -1,6 +1,7 @@
|
||||||
class CreatePollSessionsAndModifyPolls < ActiveRecord::Migration
|
class CreatePollSessionsAndModifyPolls < ActiveRecord::Migration
|
||||||
tag :predeploy
|
tag :predeploy
|
||||||
|
|
||||||
|
# rubocop:disable Migration/RemoveColumn
|
||||||
def self.up
|
def self.up
|
||||||
create_table :polling_poll_sessions do |t|
|
create_table :polling_poll_sessions do |t|
|
||||||
t.boolean :is_published, null: false, default: false
|
t.boolean :is_published, null: false, default: false
|
||||||
|
|
|
@ -0,0 +1,14 @@
|
||||||
|
/.bundle/
|
||||||
|
/.yardoc
|
||||||
|
/Gemfile.lock
|
||||||
|
/_yardoc/
|
||||||
|
/coverage/
|
||||||
|
/doc/
|
||||||
|
/pkg/
|
||||||
|
/spec/reports/
|
||||||
|
/tmp/
|
||||||
|
*.bundle
|
||||||
|
*.so
|
||||||
|
*.o
|
||||||
|
*.a
|
||||||
|
mkmf.log
|
|
@ -0,0 +1 @@
|
||||||
|
--require spec_helper
|
|
@ -0,0 +1,4 @@
|
||||||
|
source 'https://rubygems.org'
|
||||||
|
|
||||||
|
# Specify your gem's dependencies in rubocop-canvas.gemspec
|
||||||
|
gemspec
|
|
@ -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"
|
|
@ -0,0 +1,2 @@
|
||||||
|
require "bundler/gem_tasks"
|
||||||
|
|
|
@ -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"
|
|
@ -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!
|
|
@ -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
|
|
@ -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
|
|
@ -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
|
|
@ -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
|
|
@ -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
|
|
@ -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
|
|
@ -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
|
|
@ -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
|
|
@ -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
|
|
@ -0,0 +1,5 @@
|
||||||
|
module Rubocop
|
||||||
|
module Canvas
|
||||||
|
VERSION = "1.0.0"
|
||||||
|
end
|
||||||
|
end
|
|
@ -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
|
|
@ -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 <evizitei@instructure.com>
|
||||||
|
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
|
|
@ -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 <ethan.vizitei@gmail.com>
|
||||||
|
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 <ethan.vizitei@gmail.com>
|
||||||
|
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
|
|
@ -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
|
|
@ -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
|
|
@ -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
|
|
@ -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
|
|
@ -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
|
|
@ -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
|
|
@ -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'
|
|
@ -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
|
|
@ -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
|
|
@ -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
|
Loading…
Reference in New Issue