rubocop: go back to a single .rubocop.yml
it was just too confusing on which one an editor is using, double comments in jenkins, etc. this is accomplished by several things: * required cops are just marked as severe, instead of using a separate config for them, and failing if anything shows up from that config * get rid of all the logic to only include certain directories for certain cops. turns out it's not _that_ ominous to correct errors across the entire repository before marking a cop as required. * but still auto-generate config to turn _off_ autocorrect for non-severe cops. this is important because auto-correct must run for entire files, and we don't want it auto-correcting optional things that you didn't touch. * update gergich to get more details from the parsed comments. this plus the prior point means we _don't_ have to have heavy mode when in autocorrecting, but we still display out-of-context lines that were autocorrected this also makes it so we can use per-dir .rubocop.yml files again, so take some of the exceptions out of the root and put them in their own directory Change-Id: Ie936d1a9920b68910acd250ba817c7b4a670b958 Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/274394 Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com> Reviewed-by: Simon Williams <simon@instructure.com> QA-Review: Cody Cutrer <cody@instructure.com> Product-Review: Cody Cutrer <cody@instructure.com>
This commit is contained in:
Cody Cutrer
parent
432f3cde03
commit
b68d11de70
|
|
@ -10,10 +10,15 @@ require:
|
|||
|
||||
AllCops:
|
||||
TargetRubyVersion: 2.7
|
||||
NewCops: enable
|
||||
|
||||
Bundler:
|
||||
Severity: error
|
||||
Bundler/OrderedGems:
|
||||
Enabled: false # this isn't good for us because of how we pin dependencies
|
||||
|
||||
Gemspec:
|
||||
Severity: error
|
||||
Gemspec/RequiredRubyVersion:
|
||||
# all the gemspecs in this repo are non-published gems
|
||||
# the root Gemfile enforces the Ruby version, and we purposely
|
||||
|
|
@ -21,12 +26,8 @@ Gemspec/RequiredRubyVersion:
|
|||
# maintenance pain when updating ruby versions
|
||||
Enabled: false
|
||||
|
||||
Layout/EmptyLineAfterMagicComment:
|
||||
Exclude:
|
||||
- gems/tatl_tael/spec/lib/tatl_tael/linters/fixtures/**/* # fixtures purposely have errors
|
||||
Layout/IndentationConsistency:
|
||||
Exclude:
|
||||
- "**/Gemfile.d/*" # we purposely indent dependent gems
|
||||
Layout:
|
||||
Severity: error
|
||||
Layout/LineLength:
|
||||
Enabled: false # TODO. Maybe.
|
||||
|
||||
|
|
@ -48,18 +49,21 @@ Naming/FileName:
|
|||
Exclude:
|
||||
- "**/Gemfile.d/~after.rb"
|
||||
|
||||
Rails:
|
||||
Exclude:
|
||||
- "**/Gemfile.d/*" # Rails isn't loaded yet, so can't use their helpers in the Gemfile
|
||||
Rails/ApplicationRecord:
|
||||
Enabled: false # we never bothered creating an ApplicationRecord
|
||||
Rails/HasManyOrHasOneDependent:
|
||||
Enabled: false # legacy code + most things we soft delete anyway
|
||||
Rails/SkipsModelValidations:
|
||||
Enabled: false # Canvas skips validations in many places for optimization reasons
|
||||
|
||||
RSpec/DescribedClass:
|
||||
Enabled: false # we haven't used it, and it seems antithetical to RSpec/NamedSubject
|
||||
RSpec/EmptyExampleGroup:
|
||||
Severity: error
|
||||
RSpec/ExampleLength:
|
||||
Enabled: false # this is a Metrics-style cop
|
||||
RSpec/ExampleWording:
|
||||
Severity: error
|
||||
RSpec/InstanceVariable:
|
||||
Enabled: false # legacy code
|
||||
RSpec/MessageSpies:
|
||||
|
|
@ -80,11 +84,110 @@ Style/Documentation:
|
|||
Enabled: false # most things don't need to be documented
|
||||
Style/FrozenStringLiteralComment:
|
||||
Severity: error
|
||||
Style/HashSyntax:
|
||||
Enabled: false # TODO. Maybe.
|
||||
Style/PerlBackrefs:
|
||||
Enabled: false # Regexp.last_match(1) is far worse than $1
|
||||
Style/SpecialGlobalVars:
|
||||
Enabled: false # $! and $? are fine
|
||||
|
||||
# the following cops are currently silenced, but we may want to enable them IF
|
||||
# we correct all instances in the codebase first
|
||||
|
||||
Lint/AmbiguousBlockAssociation: # TODO
|
||||
Exclude:
|
||||
- spec/**/*
|
||||
|
||||
Naming/VariableNumber: # TODO
|
||||
Enabled: false
|
||||
|
||||
Rails/ReadWriteAttribute: # TODO: autocorrect (unsafe)
|
||||
Enabled: false # accessors are often defined in terms of read_attribute
|
||||
Rails/TimeZone: # TODO: autocorrect (unsafe)
|
||||
Enabled: false
|
||||
|
||||
RSpec/ContextWording: # TODO
|
||||
Enabled: false
|
||||
RSpec/DescribeClass: # TODO
|
||||
Enabled: false
|
||||
RSpec/ExpectChange: # TODO: autocorrect (unsafe)
|
||||
Enabled: false
|
||||
RSpec/HookArgument: # TODO: autocorrect
|
||||
Enabled: false
|
||||
RSpec/NamedSubject: # TODO
|
||||
Enabled: false
|
||||
RSpec/NotToNot: # TODO: autocorrect
|
||||
Enabled: false
|
||||
RSpec/ScatteredSetup: # TODO: investigate if these are required
|
||||
Enabled: false
|
||||
RSpec/VerifiedDoubles: # TODO
|
||||
Enabled: false
|
||||
|
||||
Style/BlockDelimiters: # TODO: autocorrect
|
||||
Enabled: true
|
||||
Exclude:
|
||||
- spec/**/*_spec.rb
|
||||
- spec/shared_examples/**/*.rb
|
||||
AutoCorrect: false
|
||||
Style/ClassAndModuleChildren: # TODO: autocorrect (unsafe)
|
||||
Enabled: false
|
||||
Style/DoubleNegation: # TODO: autocorrect (unsafe)
|
||||
Enabled: false
|
||||
Style/Dir: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/FormatStringToken: # TODO: investigate if this complains about our i18n library
|
||||
Enabled: false
|
||||
Style/GuardClause: # TODO
|
||||
Enabled: false
|
||||
Style/HashSyntax:
|
||||
EnforcedStyle: ruby19_no_mixed_keys
|
||||
Enabled: false # TODO: autocorrect
|
||||
Style/IfUnlessModifier: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/Lambda: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/MethodCallWithArgsParentheses: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/MethodCallWithoutArgsParentheses: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/NumericLiterals: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/NumericPredicate: # TODO: autocorrect (unsafe)
|
||||
Enabled: false
|
||||
Style/ParallelAssignment: # TODO: autocorrect (with probable exceptions)
|
||||
Enabled: false
|
||||
Style/PercentLiteralDelimiters: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/PercentQLiterals: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/RedundantSelf: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/RegexpLiteral: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/RescueModifier: # TODO
|
||||
Severity: warning
|
||||
AutoCorrect: false
|
||||
Style/RescueStandardError: # TODO: autocorrect
|
||||
EnforcedStyle: implicit
|
||||
Enabled: false
|
||||
Style/ReturnNil: # TODO: autocorrect (investigate violations)
|
||||
Enabled: false
|
||||
Style/SignalException: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/StderrPuts: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/StringLiterals:
|
||||
Enabled: false # TODO. Maybe.
|
||||
EnforcedStyle: double_quotes
|
||||
Enabled: false # TODO: auotcorrect
|
||||
Style/SymbolArray: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/TrailingCommaInArguments: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/TrailingCommaInArrayLiteral: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/TrailingCommaInHashLiteral: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/WhileUntilModifier: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/WordArray: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/ZeroLengthPredicate: # TODO: autocorrect (unsafe)
|
||||
Enabled: false
|
||||
|
|
|
|||
|
|
@ -1,94 +0,0 @@
|
|||
inherit_from: .rubocop.common.yml
|
||||
|
||||
inherit_mode:
|
||||
merge:
|
||||
- Exclude
|
||||
|
||||
AllCops:
|
||||
NewCops: disable
|
||||
|
||||
<%=
|
||||
|
||||
# if you want to see what this all evaluates to, you can run
|
||||
# `require 'erb'; puts ERB.new(File.read(".rubocop.enforced.yml")).result(binding)` from IRB
|
||||
|
||||
# keys are cops you want to opt in for (nil being all cops)
|
||||
# values are an array of directories to opt in for (nil being all directories)
|
||||
OPT_IN = {
|
||||
nil => %w[Gemfile.d].freeze,
|
||||
'Bundler' => nil,
|
||||
'Gemspec' => nil,
|
||||
'Layout' => nil,
|
||||
'RSpec/ExampleWording' => nil
|
||||
}.freeze
|
||||
|
||||
# this code generates a configuration that disables all cops for all files
|
||||
# _unless_ the cop is already configured in .rubocop.common.yml, OR the file
|
||||
# is in one of the OPT_IN directories. It does this by generating an Exclude
|
||||
# configuration for every cop (except already configured) that lists all
|
||||
# directories (except OPT_IN). AllCops does not support an Include, and
|
||||
# even if it did, inheritance to individual cops would not work correctly.
|
||||
|
||||
def generate_excludes(opt_in_array)
|
||||
return nil unless opt_in_array
|
||||
|
||||
excludes = []
|
||||
dirs_to_exclude_siblings_of = []
|
||||
|
||||
opt_in_array.each do |dir|
|
||||
components = dir.split("/")
|
||||
(0...components.length).each do |i|
|
||||
ancestor = components[0..i].join("/")
|
||||
exclude = "#{ancestor}/*"
|
||||
excludes << exclude unless excludes.include?(exclude) || opt_in_array.include?(ancestor)
|
||||
dirs_to_exclude_siblings_of << ancestor unless dirs_to_exclude_siblings_of.include?(ancestor)
|
||||
end
|
||||
end
|
||||
|
||||
dirs_to_find_siblings_of = dirs_to_exclude_siblings_of.map do |dir|
|
||||
File.dirname(dir)
|
||||
end.uniq
|
||||
|
||||
dirs_to_find_siblings_of.each do |dir|
|
||||
dirs = Dir["#{dir}/*"]
|
||||
.select { |dir| File.directory?(dir) }
|
||||
.map { |dir| dir.sub(%r{^\./}, "") }
|
||||
dirs -= dirs_to_exclude_siblings_of
|
||||
excludes.concat(dirs.map { |d| "#{d}/**/*" })
|
||||
end
|
||||
|
||||
excludes.sort
|
||||
end
|
||||
|
||||
resolved_excludes = OPT_IN.transform_values do |dirs|
|
||||
next nil unless dirs
|
||||
|
||||
generate_excludes((Array(dirs) + OPT_IN[nil]).uniq)
|
||||
end
|
||||
|
||||
require 'yaml'
|
||||
|
||||
require 'rubocop'
|
||||
common_config = YAML.safe_load(File.read(".rubocop.common.yml"))
|
||||
common_config["require"].each { |f| require f }
|
||||
# already configured cops in common.yml are intended to apply to all files already
|
||||
already_configured_cops = common_config.keys.select { |k| k.include?("/") && !common_config[k]['Exclude'] }.to_set
|
||||
|
||||
config = {}
|
||||
RuboCop::Cop::Registry.all.each do |cop|
|
||||
next if cop.department == :Metrics
|
||||
next if cop.cop_name == 'Lint/Syntax'
|
||||
next if already_configured_cops.include?(cop.cop_name)
|
||||
next if ENV['RUBOCOP_INCLUDE_AUTOCORRECTS'] && cop.support_autocorrect? && cop.new.safe_autocorrect?
|
||||
|
||||
key = [cop.cop_name, cop.department.to_s, nil].find do |key|
|
||||
resolved_excludes.key?(key)
|
||||
end
|
||||
excludes = resolved_excludes[key]
|
||||
next if excludes.nil?
|
||||
|
||||
config[cop.cop_name] = { "Exclude" => excludes }
|
||||
end
|
||||
|
||||
config.to_yaml.sub(/^---\n/, "")
|
||||
%>
|
||||
118
.rubocop.yml
118
.rubocop.yml
|
|
@ -1,100 +1,30 @@
|
|||
inherit_from: .rubocop.common.yml
|
||||
# if you want to see what this all evaluates to, you can run
|
||||
# `require 'erb'; puts ERB.new(File.read(".rubocop.yml")).result(binding)` from IRB
|
||||
|
||||
AllCops:
|
||||
NewCops: enable # TODO
|
||||
inherit_from:
|
||||
- .rubocop.common.yml
|
||||
|
||||
Lint/AmbiguousBlockAssociation: # TODO
|
||||
Exclude:
|
||||
- spec/**/*
|
||||
<%=
|
||||
|
||||
Naming/VariableNumber: # TODO
|
||||
Enabled: false
|
||||
# disable auto-correct on all non-explicitly-configured cops
|
||||
unless ENV['RUBOCOP_INCLUDE_AUTOCORRECTS']
|
||||
require 'yaml'
|
||||
|
||||
Rails/HasManyOrHasOneDependent: # TODO
|
||||
Enabled: false
|
||||
Rails/ReadWriteAttribute: # TODO: autocorrect (unsafe)
|
||||
Enabled: false # accessors are often defined in terms of read_attribute
|
||||
Rails/TimeZone: # TODO: autocorrect (unsafe)
|
||||
Enabled: false
|
||||
require 'rubocop'
|
||||
common_config = YAML.safe_load(File.read(".rubocop.common.yml"))
|
||||
common_config["require"].each { |f| require f }
|
||||
already_configured_cops = common_config.keys.select { |k| k.include?("/") && !common_config[k]['Exclude'] }.to_set
|
||||
already_configured_departments = common_config.keys.select { |k| !k.include?("/") }.map(&:to_sym).to_set
|
||||
|
||||
RSpec/ContextWording: # TODO
|
||||
Enabled: false
|
||||
RSpec/DescribeClass: # TODO
|
||||
Enabled: false
|
||||
RSpec/DescribedClass: # TODO: autocorrect (unsafe)
|
||||
Enabled: false
|
||||
RSpec/ExpectChange: # TODO: autocorrect (unsafe)
|
||||
Enabled: false
|
||||
RSpec/HookArgument: # TODO: autocorrect
|
||||
Enabled: false
|
||||
RSpec/NamedSubject: # TODO
|
||||
Enabled: false
|
||||
RSpec/NotToNot: # TODO: autocorrect
|
||||
Enabled: false
|
||||
RSpec/ScatteredSetup: # TODO: investigate if these are required
|
||||
Enabled: false
|
||||
RSpec/VerifiedDoubles: # TODO
|
||||
Enabled: false
|
||||
config = {}
|
||||
RuboCop::Cop::Registry.all.each do |cop|
|
||||
next if already_configured_departments.include?(cop.department)
|
||||
next if already_configured_cops.include?(cop.cop_name)
|
||||
next unless cop.support_autocorrect?
|
||||
|
||||
Style/BlockDelimiters: # TODO: autocorrect
|
||||
Enabled: true
|
||||
Exclude:
|
||||
- spec/**/*_spec.rb
|
||||
- spec/shared_examples/**/*.rb
|
||||
Style/ClassAndModuleChildren: # TODO: autocorrect (unsafe)
|
||||
Enabled: false
|
||||
Style/DoubleNegation: # TODO: autocorrect (unsafe)
|
||||
Enabled: false
|
||||
Style/Dir: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/FormatStringToken: # TODO: investigate if this complains about our i18n library
|
||||
Enabled: false
|
||||
Style/GuardClause: # TODO
|
||||
Enabled: false
|
||||
Style/IfUnlessModifier: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/Lambda: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/MethodCallWithArgsParentheses: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/MethodCallWithoutArgsParentheses: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/NumericLiterals: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/NumericPredicate: # TODO: autocorrect (unsafe)
|
||||
Enabled: false
|
||||
Style/ParallelAssignment: # TODO: autocorrect (with probable exceptions)
|
||||
Enabled: false
|
||||
Style/PercentLiteralDelimiters: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/PercentQLiterals: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/RedundantSelf: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/RegexpLiteral: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/RescueModifier: # TODO
|
||||
Severity: warning
|
||||
Style/RescueStandardError: # TODO: autocorrect
|
||||
EnforcedStyle: implicit
|
||||
Enabled: false
|
||||
Style/ReturnNil: # TODO: autocorrect (investigate violations)
|
||||
Enabled: false
|
||||
Style/SignalException: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/StderrPuts: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/SymbolArray: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/TrailingCommaInArguments: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/TrailingCommaInArrayLiteral: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/TrailingCommaInHashLiteral: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/WhileUntilModifier: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/WordArray: # TODO: autocorrect
|
||||
Enabled: false
|
||||
Style/ZeroLengthPredicate: # TODO: autocorrect (unsafe)
|
||||
Enabled: false
|
||||
config[cop.cop_name] = { "AutoCorrect" => false }
|
||||
end
|
||||
|
||||
config.to_yaml.sub(/^---\n/, "")
|
||||
end
|
||||
%>
|
||||
|
|
|
|||
|
|
@ -0,0 +1,8 @@
|
|||
inherit_from:
|
||||
- ../.rubocop.yml
|
||||
|
||||
Layout/IndentationConsistency:
|
||||
Enabled: false # we purposely indent dependent gems
|
||||
|
||||
Rails:
|
||||
Enabled: false # Rails isn't loaded yet
|
||||
|
|
@ -23,7 +23,7 @@
|
|||
group :test do
|
||||
dedicated_gemfile = ENV['BUNDLE_GEMFILE']&.end_with?('rubocop.rb')
|
||||
|
||||
gem 'gergich', '1.2.3', require: false
|
||||
gem 'gergich', '2.0.0', require: false
|
||||
gem 'mime-types-data', '3.2021.0901', require: false
|
||||
|
||||
gem 'rubocop', '1.19.1', require: false
|
||||
|
|
|
|||
|
|
@ -22,7 +22,7 @@ GEM
|
|||
zeitwerk (~> 2.2, >= 2.2.2)
|
||||
ast (2.4.2)
|
||||
concurrent-ruby (1.1.9)
|
||||
gergich (1.2.3)
|
||||
gergich (2.0.0)
|
||||
httparty (~> 0.17)
|
||||
sqlite3 (~> 1.4)
|
||||
httparty (0.18.1)
|
||||
|
|
@ -79,7 +79,7 @@ PLATFORMS
|
|||
x86_64-darwin-20
|
||||
|
||||
DEPENDENCIES
|
||||
gergich (= 1.2.3)
|
||||
gergich (= 2.0.0)
|
||||
mime-types-data (= 3.2021.0901)
|
||||
rainbow (= 3.0.0)
|
||||
rubocop (= 1.19.1)
|
||||
|
|
|
|||
|
|
@ -27,8 +27,7 @@ gergich capture i18nliner 'rake i18n:check'
|
|||
ruby script/brakeman
|
||||
ruby script/tatl_tael
|
||||
ruby script/stylelint
|
||||
ruby script/rlint --optional --no-fail-on-offense
|
||||
ruby script/rlint --boy-scout --heavy --no-fail-on-offense
|
||||
ruby script/rlint --no-fail-on-offense
|
||||
[ "${SKIP_ESLINT-}" != "true" ] && ruby script/eslint
|
||||
ruby script/lint_commit_message
|
||||
|
||||
|
|
|
|||
|
|
@ -1,4 +1,3 @@
|
|||
# coding: utf-8
|
||||
# frozen_string_literal: true
|
||||
|
||||
lib = File.expand_path('../lib', __FILE__)
|
||||
|
|
@ -15,7 +14,9 @@ Gem::Specification.new do |spec|
|
|||
spec.test_files = spec.files.grep(%r{^spec/})
|
||||
spec.require_paths = ["lib"]
|
||||
|
||||
spec.add_dependency "gergich", "1.2.1"
|
||||
spec.add_dependency "gergich", "2.0.0"
|
||||
|
||||
spec.add_development_dependency "byebug", "~> 11.1"
|
||||
spec.add_development_dependency "pry"
|
||||
spec.add_development_dependency "pry-nav"
|
||||
spec.add_development_dependency "rspec", "~> 3.5.0"
|
||||
|
|
|
|||
|
|
@ -22,13 +22,13 @@ module DrDiff
|
|||
attr_reader :diff
|
||||
attr_reader :campsite
|
||||
|
||||
# campsite = true: consider relevant the severe linter errors near lines changed
|
||||
def initialize(input, raw = true, campsite = true)
|
||||
# campsite = true: consider relevant linter errors near lines changed
|
||||
def initialize(input, raw: true, campsite: true)
|
||||
@diff = (raw ? parse_raw_diff(input) : input)
|
||||
@campsite = campsite
|
||||
end
|
||||
|
||||
def relevant?(path, line_number, severe = false)
|
||||
def relevant?(path, line_number, severe: false)
|
||||
return false unless diff[path]
|
||||
return true if line_number == 1 # whole-file comments are addressed to line 1
|
||||
|
||||
|
|
|
|||
|
|
@ -21,30 +21,25 @@ require "forwardable"
|
|||
|
||||
module DrDiff
|
||||
class Manager
|
||||
attr_reader :git
|
||||
attr_reader :git, :git_dir, :campsite, :heavy, :base_dir, :severe_anywhere
|
||||
|
||||
private :git
|
||||
|
||||
attr_reader :git_dir
|
||||
private :git_dir
|
||||
|
||||
attr_reader :campsite
|
||||
private :campsite
|
||||
|
||||
attr_reader :heavy
|
||||
private :heavy
|
||||
|
||||
attr_reader :base_dir
|
||||
private :base_dir
|
||||
private :severe_anywhere
|
||||
|
||||
# all levels: %w(error warn info)
|
||||
SEVERE_LEVELS = %w(error warn).freeze
|
||||
|
||||
def initialize(git: nil, git_dir: nil, sha: nil, campsite: true, heavy: false, base_dir: nil)
|
||||
def initialize(git: nil, git_dir: nil, sha: nil, campsite: true, heavy: false, base_dir: nil, severe_anywhere: true)
|
||||
@git_dir = git_dir
|
||||
@git = git || GitProxy.new(git_dir: git_dir, sha: sha)
|
||||
@campsite = campsite
|
||||
@heavy = heavy
|
||||
@base_dir = base_dir || ""
|
||||
@severe_anywhere = severe_anywhere
|
||||
end
|
||||
|
||||
extend Forwardable
|
||||
|
|
@ -67,17 +62,21 @@ module DrDiff
|
|||
severe_levels: SEVERE_LEVELS)
|
||||
|
||||
command_comments = CommandCapture.run(format, command)
|
||||
diff = DiffParser.new(git.diff, true, campsite)
|
||||
diff = DiffParser.new(git.diff, raw: true, campsite: campsite)
|
||||
|
||||
result = []
|
||||
|
||||
command_comments.each do |comment|
|
||||
path = comment[:path]
|
||||
path = path[git_dir.length..-1] if git_dir
|
||||
if heavy || diff.relevant?(path, comment[:position], severe?(comment[:severity], severe_levels))
|
||||
comment[:path] = path unless include_git_dir_in_output
|
||||
result << comment
|
||||
end
|
||||
severe = severe?(comment[:severity], severe_levels)
|
||||
next unless heavy ||
|
||||
severe && severe_anywhere ||
|
||||
diff.relevant?(path, comment[:position], severe: severe) ||
|
||||
comment[:corrected]
|
||||
|
||||
comment[:path] = path unless include_git_dir_in_output
|
||||
result << comment
|
||||
end
|
||||
|
||||
result
|
||||
|
|
|
|||
|
|
@ -161,30 +161,30 @@ module DrDiff
|
|||
end
|
||||
|
||||
it "is relevant anywhere in ranges if severe" do
|
||||
expect(parser.relevant?("some_file.rb", 56, true)).to be(false)
|
||||
expect(parser.relevant?("some_file.rb", 57, true)).to be(true)
|
||||
expect(parser.relevant?("some_file.rb", 66, true)).to be(true)
|
||||
expect(parser.relevant?("some_file.rb", 67, true)).to be(false)
|
||||
expect(parser.relevant?("some_file.rb", 56, severe: true)).to be(false)
|
||||
expect(parser.relevant?("some_file.rb", 57, severe: true)).to be(true)
|
||||
expect(parser.relevant?("some_file.rb", 66, severe: true)).to be(true)
|
||||
expect(parser.relevant?("some_file.rb", 67, severe: true)).to be(false)
|
||||
end
|
||||
|
||||
context "with campsite mode turned off" do
|
||||
let(:parser) { described_class.new(add_diff, true, false) }
|
||||
let(:parser) { described_class.new(add_diff, raw: true, campsite: false) }
|
||||
|
||||
it "is only true for touched lines" do
|
||||
expect(parser.relevant?("some_file.rb", 60, true)).to be(true)
|
||||
expect(parser.relevant?("some_file.rb", 61, true)).to be(true)
|
||||
expect(parser.relevant?("some_file.rb", 62, true)).to be(true)
|
||||
expect(parser.relevant?("some_file.rb", 60, severe: true)).to be(true)
|
||||
expect(parser.relevant?("some_file.rb", 61, severe: true)).to be(true)
|
||||
expect(parser.relevant?("some_file.rb", 62, severe: true)).to be(true)
|
||||
end
|
||||
|
||||
it "is not relevant for files in ranges that are not touched" do
|
||||
expect(parser.relevant?("some_file.rb", 57, true)).to be(false)
|
||||
expect(parser.relevant?("some_file.rb", 58, true)).to be(false)
|
||||
expect(parser.relevant?("some_file.rb", 59, true)).to be(false)
|
||||
expect(parser.relevant?("some_file.rb", 63, true)).to be(false)
|
||||
expect(parser.relevant?("some_file.rb", 64, true)).to be(false)
|
||||
expect(parser.relevant?("some_file.rb", 65, true)).to be(false)
|
||||
expect(parser.relevant?("some_file.rb", 66, true)).to be(false)
|
||||
expect(parser.relevant?("some_file.rb", 67, true)).to be(false)
|
||||
expect(parser.relevant?("some_file.rb", 57, severe: true)).to be(false)
|
||||
expect(parser.relevant?("some_file.rb", 58, severe: true)).to be(false)
|
||||
expect(parser.relevant?("some_file.rb", 59, severe: true)).to be(false)
|
||||
expect(parser.relevant?("some_file.rb", 63, severe: true)).to be(false)
|
||||
expect(parser.relevant?("some_file.rb", 64, severe: true)).to be(false)
|
||||
expect(parser.relevant?("some_file.rb", 65, severe: true)).to be(false)
|
||||
expect(parser.relevant?("some_file.rb", 66, severe: true)).to be(false)
|
||||
expect(parser.relevant?("some_file.rb", 67, severe: true)).to be(false)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
@ -193,20 +193,20 @@ module DrDiff
|
|||
let(:parser) { described_class.new(subtractive_diff) }
|
||||
|
||||
it 'respects the boundary of a removed diff set' do
|
||||
expect(parser.relevant?("some_file.rb", 55, true)).to be(false)
|
||||
expect(parser.relevant?("some_file.rb", 56, true)).to be(true)
|
||||
expect(parser.relevant?("some_file.rb", 60, true)).to be(true)
|
||||
expect(parser.relevant?("some_file.rb", 61, true)).to be(false)
|
||||
expect(parser.relevant?("some_file.rb", 55, severe: true)).to be(false)
|
||||
expect(parser.relevant?("some_file.rb", 56, severe: true)).to be(true)
|
||||
expect(parser.relevant?("some_file.rb", 60, severe: true)).to be(true)
|
||||
expect(parser.relevant?("some_file.rb", 61, severe: true)).to be(false)
|
||||
end
|
||||
|
||||
context "with campsite mode turned off" do
|
||||
let(:parser) { described_class.new(subtractive_diff, true, false) }
|
||||
let(:parser) { described_class.new(subtractive_diff, raw: true, campsite: false) }
|
||||
|
||||
it "does not count deletions as relevant" do
|
||||
expect(parser.relevant?("some_file.rb", 55, true)).to be(false)
|
||||
expect(parser.relevant?("some_file.rb", 56, true)).to be(false)
|
||||
expect(parser.relevant?("some_file.rb", 60, true)).to be(false)
|
||||
expect(parser.relevant?("some_file.rb", 61, true)).to be(false)
|
||||
expect(parser.relevant?("some_file.rb", 55, severe: true)).to be(false)
|
||||
expect(parser.relevant?("some_file.rb", 56, severe: true)).to be(false)
|
||||
expect(parser.relevant?("some_file.rb", 60, severe: true)).to be(false)
|
||||
expect(parser.relevant?("some_file.rb", 61, severe: true)).to be(false)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -75,12 +75,12 @@ spec/dr_diff_spec.rb}
|
|||
:message =>
|
||||
"[rubocop] Avoid using sleep.\n\n sleep 1\n ^^^^^^^\n",
|
||||
:position => 5,
|
||||
:severity => "warn" }
|
||||
:severity => "convention" }
|
||||
]
|
||||
end
|
||||
|
||||
before :each do
|
||||
expect(DiffParser).to receive(:new).with(git.diff, true, true).and_return(diff_parser)
|
||||
expect(DiffParser).to receive(:new).with(git.diff, raw: true, campsite: true).and_return(diff_parser)
|
||||
expect(CommandCapture).to receive(:run).with(format, command).and_return(command_capture_comments)
|
||||
allow(diff_parser).to receive(:relevant?).and_return(true)
|
||||
end
|
||||
|
|
@ -105,7 +105,7 @@ spec/dr_diff_spec.rb}
|
|||
path_without_git_dir = comment[:path][git_dir.length..-1]
|
||||
expect(diff_parser).to receive(:relevant?).with(path_without_git_dir,
|
||||
comment[:position],
|
||||
true)
|
||||
severe: false)
|
||||
subject.comments(format: format, command: command)
|
||||
end
|
||||
|
||||
|
|
@ -134,7 +134,7 @@ spec/dr_diff_spec.rb}
|
|||
comment = command_capture_comments.first
|
||||
expect(diff_parser).to receive(:relevant?).with(comment[:path],
|
||||
comment[:position],
|
||||
true)
|
||||
severe: false)
|
||||
subject.comments(format: format, command: command)
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -17,5 +17,6 @@
|
|||
# You should have received a copy of the GNU Affero General Public License along
|
||||
# with this program. If not, see <http://www.gnu.org/licenses/>.
|
||||
|
||||
$LOAD_PATH.unshift File.expand_path('../../lib', __FILE__)
|
||||
require 'dr_diff'
|
||||
|
||||
require "byebug"
|
||||
|
|
|
|||
|
|
@ -0,0 +1,5 @@
|
|||
inherit_from:
|
||||
- ../../../../../../../.rubocop.yml
|
||||
|
||||
Layout/EmptyLineAfterMagicComment:
|
||||
Enabled: false # fixtures purposely have errors
|
||||
|
|
@ -9,6 +9,7 @@ linter_options = {
|
|||
format: 'brakeman',
|
||||
command: 'bundle exec brakeman --rails6 --quiet --format json --confidence-level 2',
|
||||
append_files_to_command: false,
|
||||
severe_anywhere: false
|
||||
}
|
||||
|
||||
brakeman = Linter.new(linter_options)
|
||||
|
|
|
|||
|
|
@ -42,7 +42,8 @@ class Linter
|
|||
plugin: ENV['GERRIT_PROJECT'],
|
||||
skip_file_size_check: false,
|
||||
skip_wips: false,
|
||||
base_dir: nil
|
||||
base_dir: nil,
|
||||
severe_anywhere: true
|
||||
}.freeze
|
||||
|
||||
def initialize(options = {})
|
||||
|
|
@ -87,7 +88,7 @@ class Linter
|
|||
|
||||
private
|
||||
|
||||
attr_reader *DEFAULT_OPTIONS.keys
|
||||
attr_reader(*DEFAULT_OPTIONS.keys)
|
||||
|
||||
def git_dir
|
||||
@git_dir ||= plugin && "gems/plugins/#{plugin}/"
|
||||
|
|
@ -98,7 +99,7 @@ class Linter
|
|||
end
|
||||
|
||||
def dr_diff
|
||||
@dr_diff ||= ::DrDiff::Manager.new(git_dir: git_dir, sha: env_sha, campsite: campsite_mode, heavy: heavy_mode, base_dir: base_dir)
|
||||
@dr_diff ||= ::DrDiff::Manager.new(git_dir: git_dir, sha: env_sha, campsite: campsite_mode, heavy: heavy_mode, base_dir: base_dir, severe_anywhere: severe_anywhere)
|
||||
end
|
||||
|
||||
def wip?
|
||||
|
|
@ -172,9 +173,13 @@ class Linter
|
|||
end
|
||||
|
||||
comments.each do |comment|
|
||||
message = +"[#{comment[:source]}] "
|
||||
message << "#{comment[:rule]}: " if comment[:rule]
|
||||
message << comment[:message]
|
||||
|
||||
draft.add_comment comment[:path],
|
||||
comment[:position],
|
||||
comment[:message],
|
||||
message,
|
||||
comment[:severity]
|
||||
end
|
||||
|
||||
|
|
@ -189,12 +194,18 @@ class Linter
|
|||
end
|
||||
|
||||
def pretty_comment(comment)
|
||||
message = ""
|
||||
message += "[#{comment[:severity]}]".colorize(:yellow)
|
||||
message = +""
|
||||
severity_color = severe_levels.include?(comment[:severity]) ? :red : :yellow
|
||||
severity_color = :green if comment[:corrected]
|
||||
message << "[#{comment[:severity]}]".colorize(severity_color)
|
||||
unless comment[:cover_message]
|
||||
message += " #{comment[:path].colorize(:light_blue)}:#{comment[:position]}"
|
||||
message << " #{comment[:path].colorize(:light_blue)}:#{comment[:position]}"
|
||||
end
|
||||
message += " => #{comment[:message].colorize(:green)}"
|
||||
message << " => "
|
||||
message << "[Correctable] ".colorize(:yellow) if comment[:correctable]
|
||||
message << "[Corrected] ".colorize(:green) if comment[:corrected]
|
||||
message << "#{comment[:rule]}: " if comment[:rule]
|
||||
message << comment[:message].colorize(:green)
|
||||
puts message
|
||||
end
|
||||
end
|
||||
|
|
|
|||
15
script/rlint
15
script/rlint
|
|
@ -18,14 +18,12 @@ linter_options = {
|
|||
boyscout_mode: false
|
||||
}
|
||||
no_fail = false
|
||||
optional = false
|
||||
|
||||
OptionParser.new do |opts|
|
||||
# boy scout means treat everything as an error
|
||||
opts.on("--boy-scout", "Treat all comments as errors") { linter_options[:boyscout_mode] = true }
|
||||
# heavy means inspect entire files if a file was changed
|
||||
opts.on("--heavy") { linter_options[:heavy_mode] = true }
|
||||
opts.on("--optional", "Run against the optional rubocop ruleset, instead of the default") { optional = true }
|
||||
opts.on("--plugin PLUGIN", "Inspect changes from the given plugin, instead of canvas-lms") do |v|
|
||||
linter_options[:plugin] = v
|
||||
end
|
||||
|
|
@ -42,10 +40,7 @@ OptionParser.new do |opts|
|
|||
end
|
||||
opts.on("--summary", "Print a summary of offense counts by cop, instead of individual offenses") do
|
||||
linter_options[:comment_post_processing] = proc do |comments|
|
||||
grouped_comments = comments.group_by do |comment|
|
||||
comment[:message] =~ %r{^\[rubocop\]( \[Corrected\])?( \[Correctable\])? ([A-Za-z/]+)}
|
||||
$3
|
||||
end
|
||||
grouped_comments = comments.group_by { |comment| comment[:rule] }
|
||||
|
||||
require 'pp'
|
||||
pp grouped_comments.transform_values(&:length).to_a.sort_by(&:last).reverse.to_h
|
||||
|
|
@ -54,17 +49,14 @@ OptionParser.new do |opts|
|
|||
end
|
||||
opts.on("-a", "--auto-correct") do |_v|
|
||||
linter_options[:auto_correct] = true
|
||||
linter_options[:heavy_mode] = true
|
||||
linter_options[:command] << " -a"
|
||||
end
|
||||
opts.on("-A", "--auto-correct-all") do |_v|
|
||||
linter_options[:auto_correct] = true
|
||||
linter_options[:heavy_mode] = true
|
||||
linter_options[:command] << " -A"
|
||||
end
|
||||
opts.on("-x", "--fix-layout") do
|
||||
linter_options[:auto_correct] = true
|
||||
linter_options[:heavy_mode] = true
|
||||
linter_options[:command] << " -x"
|
||||
end
|
||||
opts.on("-h", "--help", "Display this usage information") do
|
||||
|
|
@ -73,10 +65,5 @@ OptionParser.new do |opts|
|
|||
end
|
||||
end.parse!
|
||||
|
||||
# without optional, we only run against the "enforced" config. this makes it so
|
||||
# that jenkins will warn about a larger config, but the pre-commit hook can
|
||||
# auto-correct against a much smaller config.
|
||||
linter_options[:command] << " -c .rubocop.enforced.yml" unless optional
|
||||
|
||||
rlint = Linter.new(linter_options)
|
||||
exit 1 if !rlint.run && !no_fail
|
||||
|
|
|
|||
Loading…
Reference in New Issue