From e9d63396ff1d02e40ab416a48d3cf5b8861156c5 Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Mon, 20 Sep 2021 10:20:02 -0600 Subject: [PATCH] rubocop: split configuration * remove spurious .rubocop.yml override files * split the configuration into an enforced and optional * run both configurations in jenkins (may result in some duplicate comments at different levels) * auto-correct the enforced configuration in the pre-commit hook * fix comments for Gemfile.d and the root dir; enforced configuration is only applied to that directory for now Change-Id: I8da21073d74e19138b1b580d66c7aae6465348d4 Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/273898 Reviewed-by: Simon Williams Tested-by: Service Cloud Jenkins QA-Review: Cody Cutrer Product-Review: Cody Cutrer --- .rubocop.common.yml | 71 ++++ .rubocop.enforced.yml | 85 +++++ .rubocop.yml | 305 +++++------------- Gemfile | 26 +- Gemfile.d/.rubocop.yml | 10 - Gemfile.d/_before.rb | 9 +- Gemfile.d/app.rb | 6 +- Gemfile.d/cassandra.rb | 7 +- Gemfile.d/i18n_tools_and_rake_tasks.rb | 2 - Gemfile.d/pulsar.rb | 2 +- Gemfile.d/rubocop.rb | 1 + Gemfile.d/rubocop.rb.lock | 3 + Gemfile.d/test.rb | 2 +- Gemfile.d/~after.rb | 3 +- Rakefile | 2 +- app/.rubocop.yml | 4 - .../linters/run-gergich-linters.sh | 3 +- config.ru | 2 +- db/migrate/.rubocop.yml | 12 - hooks/pre-commit | 3 + script/linter.rb | 5 + script/rlint | 43 ++- spec/spec_helper.rb | 1 - 23 files changed, 326 insertions(+), 281 deletions(-) create mode 100644 .rubocop.common.yml create mode 100644 .rubocop.enforced.yml delete mode 100644 Gemfile.d/.rubocop.yml delete mode 100644 app/.rubocop.yml delete mode 100644 db/migrate/.rubocop.yml diff --git a/.rubocop.common.yml b/.rubocop.common.yml new file mode 100644 index 00000000000..324680334c3 --- /dev/null +++ b/.rubocop.common.yml @@ -0,0 +1,71 @@ +require: + - rubocop-rails + - rubocop-rake + - rubocop-rspec + - rubocop-performance + # 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 + - outrigger/cops/migration/tagged + +AllCops: + TargetRubyVersion: 2.7 + +Bundler/OrderedGems: + Enabled: false # this isn't good for us because of how we pin dependencies + +Gemspec/OrderedDependencies: + Enabled: false # this isn't good for us because of how we pin dependencies + +Layout/IndentationConsistency: + Exclude: + - "**/Gemfile.d/*" # we purposely indent dependent gems + +Lint/Debugger: + Severity: error + +Metrics: + Enabled: false # SnR is just too low to have this enabled + +Migration/Tagged: + Severity: error + AllowedTags: + - predeploy + - postdeploy + - cassandra + - dynamodb + +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 + +RSpec/EmptyExampleGroup: + Severity: error +RSpec/ExampleLength: + Enabled: false +RSpec/InstanceVariable: + Enabled: false +RSpec/MultipleExpectations: + Enabled: false +RSpec/NestedGroups: + Max: 5 +RSpec/RepeatedDescription: + Severity: error + +Specs/EnsureSpecExtension: + Exclude: + - spec/shared_examples/**/* +Style/AsciiComments: + Enabled: false +Style/Documentation: + Enabled: false +Style/FrozenStringLiteralComment: + Severity: error +Style/SpecialGlobalVars: + Enabled: false +Style/StringLiterals: + Enabled: false diff --git a/.rubocop.enforced.yml b/.rubocop.enforced.yml new file mode 100644 index 00000000000..bf095cddb04 --- /dev/null +++ b/.rubocop.enforced.yml @@ -0,0 +1,85 @@ +inherit_from: .rubocop.common.yml + +inherit_mode: + merge: + - Exclude + +AllCops: + NewCops: disable + +<%= + +# 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 +}.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' + +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) + + 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/, "") +%> diff --git a/.rubocop.yml b/.rubocop.yml index 470f093d8f1..694e6544c93 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,97 +1,109 @@ -require: - - rubocop-rails - - rubocop-rspec - - rubocop-performance - # 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 - - outrigger/cops/migration/tagged +inherit_from: .rubocop.common.yml AllCops: - TargetRubyVersion: 2.7 NewCops: enable -# our style changes: disabling style rules we aren't interested in -Layout/ParameterAlignment: - Enabled: false -Layout/ElseAlignment: - Enabled: false -Layout/EmptyLines: - Enabled: false -Layout/EmptyLinesAroundAccessModifier: - Enabled: false -Layout/EmptyLinesAroundArguments: - Enabled: false -Layout/EmptyLinesAroundBlockBody: - Enabled: false -Layout/EmptyLinesAroundClassBody: - Enabled: false -Layout/EmptyLinesAroundMethodBody: - Enabled: false -Layout/EmptyLinesAroundModuleBody: - Enabled: false -Layout/FirstHashElementIndentation: - Enabled: false -Layout/IndentationConsistency: - Enabled: false -Layout/IndentationWidth: - Enabled: false -Layout/MultilineOperationIndentation: - Enabled: false -Layout/SpaceAfterColon: - Enabled: false -Layout/SpaceAfterComma: - Enabled: false -Layout/SpaceAroundEqualsInParameterDefault: - Enabled: false -Layout/SpaceAroundOperators: - Enabled: false -Layout/SpaceBeforeBlockBraces: - Enabled: false -Layout/SpaceBeforeFirstArg: - Enabled: false -Layout/SpaceInLambdaLiteral: - Enabled: false -Layout/SpaceInsideArrayLiteralBrackets: - Enabled: false -Layout/SpaceInsideBlockBraces: - Enabled: false -Layout/SpaceInsideHashLiteralBraces: - Enabled: false -Layout/SpaceInsideReferenceBrackets: - Enabled: false -Layout/TrailingEmptyLines: - Enabled: false -Layout/TrailingWhitespace: +Gemspec/RequiredRubyVersion: Enabled: false -Style/FormatStringToken: +Lint/AmbiguousBlockAssociation: + Exclude: + - spec/**/* + +Naming/VariableNumber: Enabled: false -Style/StringLiterals: + +Rails/ApplicationRecord: + Enabled: false # we never bothered creating an ApplicationRecord +Rails/HasManyOrHasOneDependent: + # It whines about update_all too much, which we use a lot specifically to + # bypass validations and any other AR-ness Enabled: false -Style/SignalException: +Rails/ReadWriteAttribute: + Enabled: false # accessors are often defined in terms of read_attribute +Rails/SkipsModelValidations: Enabled: false -Style/NumericLiterals: +Rails/TimeZone: Enabled: false -Style/PercentLiteralDelimiters: + +RSpec/ContextWording: Enabled: false -Style/Documentation: +RSpec/DescribeClass: Enabled: false +RSpec/DescribedClass: + Enabled: false +RSpec/ExampleWording: + Enabled: false +RSpec/ExpectChange: + Enabled: false +RSpec/HookArgument: + Enabled: false +RSpec/MessageSpies: + Enabled: false +RSpec/NamedSubject: + Enabled: false +RSpec/NotToNot: + Enabled: false +RSpec/ScatteredSetup: + Enabled: false +RSpec/VerifiedDoubles: + Enabled: false + +Style/BlockDelimiters: + Enabled: true + Exclude: + - spec/**/*_spec.rb + - spec/shared_examples/**/*.rb Style/ClassAndModuleChildren: Enabled: false -Style/RegexpLiteral: +Style/DateTime: Enabled: false -Style/GuardClause: +Style/DoubleNegation: Enabled: false -Style/RedundantSelf: +Style/Dir: + Enabled: false +Style/FormatStringToken: Enabled: false Style/IfUnlessModifier: Enabled: false -Style/WordArray: +Style/GuardClause: + Enabled: false +Style/HashSyntax: + Enabled: false +Style/Lambda: + Enabled: false +Style/MethodCallWithArgsParentheses: + Enabled: false +Style/MethodCallWithoutArgsParentheses: + Enabled: false +Style/NumericLiterals: + Enabled: false +Style/NumericPredicate: + Enabled: false +Style/ParallelAssignment: + Enabled: false +Style/PercentLiteralDelimiters: Enabled: false Style/PercentQLiterals: Enabled: false -Style/DoubleNegation: +Style/RedundantSelf: + Enabled: false +Style/RegexpLiteral: + Enabled: false +Style/RescueModifier: + Severity: warning +Style/RescueStandardError: + EnforcedStyle: implicit + Enabled: false +Style/ReturnNil: + Enabled: false +Style/SignalException: + Enabled: false +Style/StderrPuts: + Enabled: false +Style/StringLiterals: + Enabled: false +Style/SymbolArray: Enabled: false Style/TrailingCommaInArguments: Enabled: false @@ -99,156 +111,9 @@ Style/TrailingCommaInArrayLiteral: Enabled: false Style/TrailingCommaInHashLiteral: Enabled: false -Style/MethodCallWithoutArgsParentheses: - Enabled: false -Style/MethodCallWithArgsParentheses: - Enabled: false -Layout/HashAlignment: - Enabled: false -Style/Lambda: - Enabled: false Style/WhileUntilModifier: Enabled: false -Style/ParallelAssignment: +Style/WordArray: Enabled: false Style/ZeroLengthPredicate: Enabled: false -Style/NumericPredicate: - Enabled: false -Naming/VariableNumber: - Enabled: false -Style/Dir: - Enabled: false -Style/ReturnNil: - Enabled: false -Style/StderrPuts: - Enabled: false -Style/DateTime: - Enabled: false -Style/SymbolArray: - Enabled: false -Style/FrozenStringLiteralComment: - Severity: error -Style/AsciiComments: - Enabled: false -Style/BlockDelimiters: - Enabled: true - Exclude: - - spec/**/*_spec.rb - - spec/shared_examples/**/*.rb - -# RSpec cops we don't care about -RSpec/MessageSpies: - Enabled: false -RSpec/HookArgument: - Enabled: false -RSpec/VerifiedDoubles: - Enabled: false -RSpec/NamedSubject: - Enabled: false -RSpec/NotToNot: - Enabled: false -# RSpec cops we care extra about -RSpec/EmptyExampleGroup: - Severity: error -RSpec/RepeatedDescription: - Severity: error -RSpec/ExpectChange: - Enabled: false - -# this isn't good for us because of how we pin dependencies -Bundler/OrderedGems: - Enabled: false -Gemspec/OrderedDependencies: - Enabled: false -Gemspec/RequiredRubyVersion: - Enabled: false - -# Rails style changes -Migration/Tagged: - Enabled: true - Severity: error - AllowedTags: - - predeploy - - postdeploy - - cassandra - - dynamodb -Rails: - Enabled: true -Rails/TimeZone: - Enabled: false -# accessors are often defined in terms of read_attribute -Rails/ReadWriteAttribute: - Enabled: false -# we never bothered creating an ApplicationRecord -Rails/ApplicationRecord: - Enabled: false -Rails/HasManyOrHasOneDependent: - Enabled: false -# It whines about update_all too much, which we use a lot specifically to -# bypass validations and any other AR-ness -Rails/SkipsModelValidations: - Enabled: false - -# Lint changes -Lint/AmbiguousRegexpLiteral: - Severity: convention -Lint/AmbiguousBlockAssociation: - Exclude: - - spec/**/* -Lint/UselessAssignment: - Severity: convention -Lint/Debugger: - Severity: error -Layout/EndAlignment: - EnforcedStyleAlignWith: variable - Severity: convention - -# Performance changes -Performance/Detect: - Severity: warning -Performance/TimesMap: - Exclude: - - spec/**/* - -# these need better configuration than the default: -Style/AndOr: - EnforcedStyle: conditionals -Style/RescueModifier: - Severity: warning -Layout/MultilineMethodCallIndentation: - EnforcedStyle: indented -Layout/FirstArrayElementIndentation: - EnforcedStyle: consistent - -# SnR is just too low to have this enabled -Metrics: - Enabled: false - -RSpec/InstanceVariable: - Enabled: false -RSpec/ExampleWording: - Enabled: false -RSpec/ContextWording: - Enabled: false -RSpec/ExampleLength: - Max: 48 -RSpec/NestedGroups: - Max: 5 -RSpec/DescribedClass: - Enabled: false -RSpec/DescribeClass: - Enabled: false -Style/HashSyntax: - Enabled: false -RSpec/MultipleExpectations: - Enabled: false -RSpec/ScatteredSetup: - Enabled: false -Style/RescueStandardError: - Enabled: false - -Specs/EnsureSpecExtension: - Enabled: true - Exclude: - - spec/shared_examples/**/* diff --git a/Gemfile b/Gemfile index e16ffd4c23c..d1a9bd7fcac 100644 --- a/Gemfile +++ b/Gemfile @@ -2,23 +2,27 @@ # What have they done to the Gemfile??? # -# Relax. Breathe deep. All the gems are still there; they're just loaded in various files in Gemfile.d/ -# This allows us to require gems locally that we might not want to commit to our public repo. We can maintain -# a customized list of gems for development and debuggery, without affecting our ability to merge with canvas-lms +# Relax. Breathe deep. All the gems are still there; they're just loaded in +# various files in Gemfile.d/. This allows us to require gems locally that we +# might not want to commit to our public repo. We can maintain a customized +# list of gems for development and debuggery, without affecting our ability to +# merge with canvas-lms # -# NOTE: some files in Gemfile.d/ will have certain required gems indented. While this may seem arbitrary, -# it actually has semantic significance. An indented gem required in Gemfile is a gem that is NOT -# directly used by Canvas, but required by a gem that is used by Canvas. We lock into specific versions of -# these gems to prevent regression, and the indentation serves to alert us to the relationship between the gem and canvas-lms +# NOTE: some files in Gemfile.d/ will have certain required gems indented. +# While this may seem arbitrary, it actually has semantic significance. An +# indented gem required in Gemfile is a gem that is NOT directly used by +# Canvas, but required by a gem that is used by Canvas. We lock into specific +# versions of these gems to prevent regression, and the indentation serves to +# alert us to the relationship between the gem and canvas-lms + source 'https://rubygems.org/' - Dir[File.join(File.dirname(__FILE__), 'gems/plugins/*/Gemfile.d/_before.rb')].each do |file| - eval(File.read(file), nil, file) + eval(File.read(file), nil, file) # rubocop:disable Security/Eval end -require File.expand_path("../config/canvas_rails_switcher", __FILE__) +require File.expand_path('config/canvas_rails_switcher', __dir__) Dir.glob(File.join(File.dirname(__FILE__), 'Gemfile.d', '*.rb')).sort.each do |file| - eval(File.read(file), nil, file) + eval(File.read(file), nil, file) # rubocop:disable Security/Eval end diff --git a/Gemfile.d/.rubocop.yml b/Gemfile.d/.rubocop.yml deleted file mode 100644 index 010906a7eba..00000000000 --- a/Gemfile.d/.rubocop.yml +++ /dev/null @@ -1,10 +0,0 @@ -inherit_from: ../.rubocop.yml - -Layout/LineLength: - Enabled: false -Layout/IndentationConsistency: - Enabled: false -Metrics/MethodLength: - Enabled: false -Metrics/ClassLength: - Enabled: false diff --git a/Gemfile.d/_before.rb b/Gemfile.d/_before.rb index a093b7d2305..f25f142dfb0 100644 --- a/Gemfile.d/_before.rb +++ b/Gemfile.d/_before.rb @@ -21,14 +21,14 @@ gem 'bundler', '>= 2.2.17', '<= 2.2.24' if Gem::Version.new(Bundler::VERSION) >= Gem::Version.new('1.14.0') && - Gem::Version.new(Gem::VERSION) < Gem::Version.new('2.6.9') + Gem::Version.new(Gem::VERSION) < Gem::Version.new('2.6.9') raise "Please run `gem update --system` to bring RubyGems to 2.6.9 or newer for use with Bundler 1.14 or newer." end if RUBY_ENGINE == 'truffleruby' - $stderr.puts "TruffleRuby support is experimental" unless ENV['SUPPRESS_RUBY_WARNING'] + warn "TruffleRuby support is experimental" unless ENV['SUPPRESS_RUBY_WARNING'] elsif RUBY_VERSION >= "3.0.0" && RUBY_VERSION < "3.1" - $stderr.puts "Ruby 3.0+ support is experimental" unless ENV['SUPPRESS_RUBY_WARNING'] + warn "Ruby 3.0+ support is experimental" unless ENV['SUPPRESS_RUBY_WARNING'] end ruby '>= 2.6.0', '< 3.1' @@ -45,10 +45,9 @@ unless CANVAS_RAILS6_0 end Bundler::Dsl.class_eval do - def to_definition(lockfile, unlock) + def to_definition(_lockfile, unlock) @sources << @rubygems_source if @sources.respond_to?(:include?) && !@sources.include?(@rubygems_source) Definition.new(Bundler.default_lockfile, @dependencies, @sources, unlock, @ruby_version) end end end - diff --git a/Gemfile.d/app.rb b/Gemfile.d/app.rb index e444082c232..0fcd4d49e99 100644 --- a/Gemfile.d/app.rb +++ b/Gemfile.d/app.rb @@ -17,7 +17,7 @@ # You should have received a copy of the GNU Affero General Public License along # with this program. If not, see . -# Note: Indented gems are meant to indicate transient dependencies of parent gems +# NOTE: Indented gems are meant to indicate transient dependencies of parent gems if CANVAS_RAILS6_0 gem 'rails', '6.0.4.1' @@ -38,7 +38,7 @@ end gem 'academic_benchmarks', '1.1.1', require: false gem 'active_model-better_errors', '1.6.7', require: 'active_model/better_errors' gem 'active_model_serializers', '0.9.0alpha1', - github: 'rails-api/active_model_serializers', ref: '61882e1e4127facfe92e49057aec71edbe981829' + github: 'rails-api/active_model_serializers', ref: '61882e1e4127facfe92e49057aec71edbe981829' gem 'activerecord-pg-extensions', '0.3.0' gem 'addressable', '2.7.0', require: false gem 'after_transaction_commit', '2.2.2' @@ -139,7 +139,7 @@ gem 'twilio-ruby', '5.36.0', require: false gem 'vault', '0.15.0', require: false gem 'vericite_api', '1.5.3' gem 'week_of_month', '1.2.5', - github: 'instructure/week-of-month', ref: 'b3013639e9474f302b5a6f27e4e45313e8d24902' + github: 'instructure/week-of-month', ref: 'b3013639e9474f302b5a6f27e4e45313e8d24902' gem 'will_paginate', '3.3.0', require: false # required for folio-pagination # needs pin to satisfy varying requirements of google_drive and another gem diff --git a/Gemfile.d/cassandra.rb b/Gemfile.d/cassandra.rb index 4142a29fd25..9f31b8c9c9d 100644 --- a/Gemfile.d/cassandra.rb +++ b/Gemfile.d/cassandra.rb @@ -18,10 +18,11 @@ # with this program. If not, see . group :cassandra do - gem 'cassandra-cql', '1.2.3', github: 'kreynolds/cassandra-cql', ref: '02b5abbe441a345c051a180327932566fd66bb36' #dependency of canvas_cassandra + gem 'cassandra-cql', '1.2.3', github: 'kreynolds/cassandra-cql', + ref: '02b5abbe441a345c051a180327932566fd66bb36' # dependency of canvas_cassandra gem 'simple_uuid', '0.4.0', require: false gem 'thrift', '0.9.3.0', require: false - gem 'thrift_client', '0.9.3', require: false, github: 'twitter/thrift_client', ref: '5c10d59881825cb8e26ab1aa8f1d2738e88c0e83' + gem 'thrift_client', '0.9.3', require: false, github: 'twitter/thrift_client', + ref: '5c10d59881825cb8e26ab1aa8f1d2738e88c0e83' gem "canvas_cassandra", path: "gems/canvas_cassandra" end - diff --git a/Gemfile.d/i18n_tools_and_rake_tasks.rb b/Gemfile.d/i18n_tools_and_rake_tasks.rb index 0bcaac0ab2a..fcc00c91639 100644 --- a/Gemfile.d/i18n_tools_and_rake_tasks.rb +++ b/Gemfile.d/i18n_tools_and_rake_tasks.rb @@ -19,10 +19,8 @@ group :i18n_tools do gem 'i18n_extraction', path: 'gems/i18n_extraction', require: false - end group :i18n_tools, :development do gem 'i18n_tasks', path: 'gems/i18n_tasks' end - diff --git a/Gemfile.d/pulsar.rb b/Gemfile.d/pulsar.rb index 3f38264d712..e9830166eb2 100644 --- a/Gemfile.d/pulsar.rb +++ b/Gemfile.d/pulsar.rb @@ -29,4 +29,4 @@ # be running `bundle install --without pulsar` to just skip it. group :pulsar do gem "pulsar-client", "2.6.1.pre.beta.2" -end \ No newline at end of file +end diff --git a/Gemfile.d/rubocop.rb b/Gemfile.d/rubocop.rb index 1f08dbe87ab..a3466bb0cdf 100644 --- a/Gemfile.d/rubocop.rb +++ b/Gemfile.d/rubocop.rb @@ -31,6 +31,7 @@ group :test do gem 'rubocop-canvas', require: false, path: "#{'../' if dedicated_gemfile}gems/rubocop-canvas" gem 'rainbow', '3.0.0', require: false gem 'rubocop-rails', '2.11.3', require: false + gem 'rubocop-rake', '0.6.0', require: false gem 'rubocop-rspec', '2.4.0', require: false gem 'rubocop-performance', '1.11.5', require: false end diff --git a/Gemfile.d/rubocop.rb.lock b/Gemfile.d/rubocop.rb.lock index ba4e690d399..c5f344f6eb8 100644 --- a/Gemfile.d/rubocop.rb.lock +++ b/Gemfile.d/rubocop.rb.lock @@ -63,6 +63,8 @@ GEM activesupport (>= 4.2.0) rack (>= 1.1) rubocop (>= 1.7.0, < 2.0) + rubocop-rake (0.6.0) + rubocop (~> 1.0) rubocop-rspec (2.4.0) rubocop (~> 1.0) rubocop-ast (>= 1.1.0) @@ -85,6 +87,7 @@ DEPENDENCIES rubocop-canvas! rubocop-performance (= 1.11.5) rubocop-rails (= 2.11.3) + rubocop-rake (= 0.6.0) rubocop-rspec (= 2.4.0) unicode-display_width (= 2.1.0) diff --git a/Gemfile.d/test.rb b/Gemfile.d/test.rb index fa31d29075a..e3f93f2a367 100644 --- a/Gemfile.d/test.rb +++ b/Gemfile.d/test.rb @@ -68,6 +68,6 @@ group :test do # axe-core* versions at or above 4.2 have difficulties with iframes. Keep these at 4.1.0 until fixes are investigated gem 'axe-core-selenium', '~> 4.1.0', require: false gem 'axe-core-rspec', '~> 4.1.0', require: false - gem 'axe-core-api', '~> 4.1.0', require:false + gem 'axe-core-api', '~> 4.1.0', require: false gem 'stormbreaker', '0.0.4', require: false end diff --git a/Gemfile.d/~after.rb b/Gemfile.d/~after.rb index 19467717b49..a68effc4513 100644 --- a/Gemfile.d/~after.rb +++ b/Gemfile.d/~after.rb @@ -21,5 +21,6 @@ # plugins. Dir[File.join(File.dirname(__FILE__), '../gems/plugins/*/Gemfile.d/*')].each do |g| next if g.end_with?('/_before.rb') - eval(File.read(g), nil, g) + + eval(File.read(g), nil, g) # rubocop:disable Security/Eval end diff --git a/Rakefile b/Rakefile index 98fe381101b..3afe2e4b839 100644 --- a/Rakefile +++ b/Rakefile @@ -3,7 +3,7 @@ # Add your own tasks in files placed in lib/tasks ending in .rake, # for example lib/tasks/capistrano.rake, and they will automatically be available to Rake. -require File.expand_path('../config/application', __FILE__) +require File.expand_path('config/application', __dir__) require 'rake' require 'rake/testtask' diff --git a/app/.rubocop.yml b/app/.rubocop.yml deleted file mode 100644 index 1c23f135e42..00000000000 --- a/app/.rubocop.yml +++ /dev/null @@ -1,4 +0,0 @@ -inherit_from: ../.rubocop.yml - -Migration/Delay: - Enabled: false diff --git a/build/new-jenkins/linters/run-gergich-linters.sh b/build/new-jenkins/linters/run-gergich-linters.sh index 039c21c45a7..cd1225060ae 100755 --- a/build/new-jenkins/linters/run-gergich-linters.sh +++ b/build/new-jenkins/linters/run-gergich-linters.sh @@ -27,7 +27,8 @@ gergich capture i18nliner 'rake i18n:check' ruby script/brakeman ruby script/tatl_tael ruby script/stylelint -ruby script/rlint --no-fail-on-offense +ruby script/rlint --optional --no-fail-on-offense +ruby script/rlint --boy-scout --no-fail-on-offense [ "${SKIP_ESLINT-}" != "true" ] && ruby script/eslint ruby script/lint_commit_message diff --git a/config.ru b/config.ru index 7ff0055a474..2ff7fc866d5 100644 --- a/config.ru +++ b/config.ru @@ -2,7 +2,7 @@ # This file is used by Rack-based servers to start the application. -require ::File.expand_path('../config/environment', __FILE__) +require ::File.expand_path('config/environment', __dir__) if defined?(CanvasRails) run CanvasRails::Application else diff --git a/db/migrate/.rubocop.yml b/db/migrate/.rubocop.yml deleted file mode 100644 index c989f78a4be..00000000000 --- a/db/migrate/.rubocop.yml +++ /dev/null @@ -1,12 +0,0 @@ -inherit_from: ../../.rubocop.yml - -Layout/LineLength: - Enabled: false -Layout/IndentationConsistency: - Enabled: false -Metrics/MethodLength: - Enabled: false -Metrics/ClassLength: - Enabled: false -Metrics/AbcSize: - Enabled: false diff --git a/hooks/pre-commit b/hooks/pre-commit index bda1c775a2d..59912f5c450 100755 --- a/hooks/pre-commit +++ b/hooks/pre-commit @@ -1,4 +1,5 @@ #!/bin/sh + if [ -f node_modules/.bin/lint-staged ]; then yarn run --silent lint:staged else @@ -7,4 +8,6 @@ else echo 'You should run `yarn` locally or check to make sure docker is running.' fi +script/rlint ${RLINT_ARGUMENTS:---auto-correct} + exit 0 diff --git a/script/linter.rb b/script/linter.rb index faa5b4ed5e7..556bc48b7e5 100644 --- a/script/linter.rb +++ b/script/linter.rb @@ -162,6 +162,11 @@ class Linter end if gerrit_patchset + if boyscout_mode + processed_comments.each do |comment| + comment[:severity] = :error + end + end publish_gergich_comments(processed_comments) else publish_local_comments(processed_comments) diff --git a/script/rlint b/script/rlint index 9959ba6c504..f64ea873510 100755 --- a/script/rlint +++ b/script/rlint @@ -10,7 +10,8 @@ linter_options = { linter_name: "Rubocop", file_regex: %r{(?:\.e?rb|\.rake|\.gemspec|/[^./]+)$}, format: "rubocop", - command: "bin/rubocop", + command: +"bin/rubocop", + auto_correct: false, campsite_mode: false, append_files_to_command: true, severe_levels: [], @@ -25,13 +26,47 @@ linter_options = { end } 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 } opts.on("--heavy") { linter_options[:heavy_mode] = true } - opts.on("--boy-scout") { linter_options[:boyscout_mode] = true } - opts.on("--plugin PLUGIN") { |v| linter_options[:plugin] = v } - opts.on("--no-fail-on-offense") { |v| no_fail = 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 + opts.on("--no-fail-on-offense", + <<~TEXT.tr("\n", " ")) do |_v| + Don't fail (exit code) if you find an offense. + Use if you're processing the output elsewise, like in Jenkins+Gergich. + TEXT + no_fail = true + end + opts.on("-a", "--auto-correct") do |_v| + linter_options[:auto_correct] = true + linter_options[:command] << " -a" + end + opts.on("-A", "--auto-correct-all") do |_v| + linter_options[:auto_correct] = true + linter_options[:command] << " -A" + end + opts.on("-x", "--fix-laout") do + linter_options[:auto_correct] = true + linter_options[:command] << " -x" + end + opts.on("-h", "--help", "Display this usage information") do + puts opts + exit 1 + 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. +unless optional + linter_options[:command] << " -c .rubocop.enforced.yml --force-exclusion" +end + rlint = Linter.new(linter_options) exit 1 if !rlint.run && !no_fail diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 6a18fe9cc2b..05e91af42f5 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,5 +1,4 @@ # frozen_string_literal: true - # # Copyright (C) 2011 - present Instructure, Inc. #