diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-11-19 08:27:35 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-11-19 08:27:35 +0000 |
commit | 7e9c479f7de77702622631cff2628a9c8dcbc627 (patch) | |
tree | c8f718a08e110ad7e1894510980d2155a6549197 /rubocop | |
parent | e852b0ae16db4052c1c567d9efa4facc81146e88 (diff) | |
download | gitlab-ce-7e9c479f7de77702622631cff2628a9c8dcbc627.tar.gz |
Add latest changes from gitlab-org/gitlab@13-6-stable-eev13.6.0-rc42
Diffstat (limited to 'rubocop')
-rw-r--r-- | rubocop/cop/code_reuse/active_record.rb | 168 | ||||
-rw-r--r-- | rubocop/cop/graphql/resolver_type.rb | 46 | ||||
-rw-r--r-- | rubocop/cop/line_break_around_conditional_block.rb | 132 | ||||
-rw-r--r-- | rubocop/qa_helpers.rb | 2 | ||||
-rw-r--r-- | rubocop/rubocop-code_reuse.yml | 41 | ||||
-rw-r--r-- | rubocop/rubocop-migrations.yml | 1 | ||||
-rw-r--r-- | rubocop/rubocop-usage-data.yml | 1 |
7 files changed, 90 insertions, 301 deletions
diff --git a/rubocop/cop/code_reuse/active_record.rb b/rubocop/cop/code_reuse/active_record.rb deleted file mode 100644 index f6b5d66647d..00000000000 --- a/rubocop/cop/code_reuse/active_record.rb +++ /dev/null @@ -1,168 +0,0 @@ -# frozen_string_literal: true - -require_relative '../../code_reuse_helpers' - -module RuboCop - module Cop - module CodeReuse - # Cop that denies the use of ActiveRecord methods outside of models. - class ActiveRecord < RuboCop::Cop::Cop - include CodeReuseHelpers - - MSG = 'This method can only be used inside an ActiveRecord model: ' \ - 'https://gitlab.com/gitlab-org/gitlab-foss/issues/49653' - - # Various methods from ActiveRecord::Querying that are denied. We - # exclude some generic ones such as `any?` and `first`, as these may - # lead to too many false positives, since `Array` also supports these - # methods. - # - # The keys of this Hash are the denied method names. The values are - # booleans that indicate if the method should only be denied if any - # arguments are provided. - NOT_ALLOWED = { - average: true, - calculate: true, - count_by_sql: true, - create_with: true, - distinct: false, - eager_load: true, - exists?: true, - find_by: true, - find_by!: true, - find_by_sql: true, - find_each: true, - find_in_batches: true, - find_or_create_by: true, - find_or_create_by!: true, - find_or_initialize_by: true, - first!: false, - first_or_create: true, - first_or_create!: true, - first_or_initialize: true, - from: true, - group: true, - having: true, - ids: false, - includes: true, - joins: true, - limit: true, - lock: false, - many?: false, - offset: true, - order: true, - pluck: true, - preload: true, - readonly: false, - references: true, - reorder: true, - rewhere: true, - take: false, - take!: false, - unscope: false, - where: false, - with: true - }.freeze - - # Directories that allow the use of the denied methods. These - # directories are checked relative to both . and ee/ - ALLOWED_DIRECTORIES = %w[ - app/models - config - danger - db - lib/backup - lib/banzai - lib/gitlab/background_migration - lib/gitlab/cycle_analytics - lib/gitlab/database - lib/gitlab/import_export - lib/gitlab/project_authorizations - lib/gitlab/sql - lib/system_check - lib/tasks - qa - rubocop - spec - ].freeze - - def on_send(node) - return if in_allowed_directory?(node) - - receiver = node.children[0] - send_name = node.children[1] - first_arg = node.children[2] - - if receiver && NOT_ALLOWED.key?(send_name) - # If the rule requires an argument to be given, but none are - # provided, we won't register an offense. This prevents us from - # adding offenses for `project.group`, while still covering - # `Project.group(:name)`. - return if NOT_ALLOWED[send_name] && !first_arg - - add_offense(node, location: :selector) - end - end - - # Returns true if the node resides in one of the allowed - # directories. - def in_allowed_directory?(node) - path = file_path_for_node(node) - - ALLOWED_DIRECTORIES.any? do |directory| - path.start_with?( - File.join(rails_root, directory), - File.join(rails_root, 'ee', directory) - ) - end - end - - # We can not auto correct code like this, as it requires manual - # refactoring. Instead, we'll just allow the surrounding scope. - # - # Despite this method's presence, you should not use it. This method - # exists to make it possible to allow large chunks of offenses we - # can't fix in the short term. If you are writing new code, follow the - # code reuse guidelines, instead of allowing any new offenses. - def autocorrect(node) - scope = surrounding_scope_of(node) - indent = indentation_of(scope) - - lambda do |corrector| - # This prevents us from inserting the same enable/disable comment - # for a method or block that has multiple offenses. - next if allowed_scopes.include?(scope) - - corrector.insert_before( - scope.source_range, - "# rubocop: disable #{cop_name}\n#{indent}" - ) - - corrector.insert_after( - scope.source_range, - "\n#{indent}# rubocop: enable #{cop_name}" - ) - - allowed_scopes << scope - end - end - - def indentation_of(node) - ' ' * node.loc.expression.source_line[/\A */].length - end - - def surrounding_scope_of(node) - %i[def defs block begin].each do |type| - if (found = node.each_ancestor(type).first) - return found - end - end - end - - def allowed_scopes - @allowed_scopes ||= Set.new - end - end - end - end -end diff --git a/rubocop/cop/graphql/resolver_type.rb b/rubocop/cop/graphql/resolver_type.rb new file mode 100644 index 00000000000..1209c5dbc6b --- /dev/null +++ b/rubocop/cop/graphql/resolver_type.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +# This cop checks for missing GraphQL type annotations on resolvers +# +# @example +# +# # bad +# module Resolvers +# class NoTypeResolver < BaseResolver +# field :some_field, GraphQL::STRING_TYPE +# end +# end +# +# # good +# module Resolvers +# class WithTypeResolver < BaseResolver +# type MyType, null: true +# +# field :some_field, GraphQL::STRING_TYPE +# end +# end + +module RuboCop + module Cop + module Graphql + class ResolverType < RuboCop::Cop::Cop + MSG = 'Missing type annotation: Please add `type` DSL method call. ' \ + 'e.g: type UserType.connection_type, null: true' + + def_node_matcher :typed?, <<~PATTERN + (... (begin <(send nil? :type ...) ...>)) + PATTERN + + def on_class(node) + add_offense(node, location: :expression) if resolver?(node) && !typed?(node) + end + + private + + def resolver?(node) + node.loc.name.source.end_with?('Resolver') + end + end + end + end +end diff --git a/rubocop/cop/line_break_around_conditional_block.rb b/rubocop/cop/line_break_around_conditional_block.rb deleted file mode 100644 index 2523cc162f3..00000000000 --- a/rubocop/cop/line_break_around_conditional_block.rb +++ /dev/null @@ -1,132 +0,0 @@ -# frozen_string_literal: true - -module RuboCop - module Cop - # Ensures a line break around conditional blocks. - # - # @example - # # bad - # do_something - # if condition - # do_extra_stuff - # end - # do_something_more - # - # # good - # do_something - # - # if condition - # do_extra_stuff - # end - # - # do_something_more - # - # # bad - # do_something - # unless condition - # do_extra_stuff - # end - # - # do_something_more - # - # # good - # def a_method - # if condition - # do_something - # end - # end - # - # # good - # on_block do - # if condition - # do_something - # end - # end - class LineBreakAroundConditionalBlock < RuboCop::Cop::Cop - include RangeHelp - - MSG = 'Add a line break around conditional blocks' - - def on_if(node) - # This cop causes errors in haml files, so let's skip those - return if in_haml?(node) - return if node.single_line? - return unless node.if? || node.unless? - - add_offense(node, location: :expression, message: MSG) unless previous_line_valid?(node) - add_offense(node, location: :expression, message: MSG) unless last_line_valid?(node) - end - - def autocorrect(node) - lambda do |corrector| - line = range_by_whole_lines(node.source_range) - unless previous_line_valid?(node) - corrector.insert_before(line, "\n") - end - - unless last_line_valid?(node) - corrector.insert_after(line, "\n") - end - end - end - - private - - def previous_line_valid?(node) - previous_line(node).empty? || - start_clause_line?(previous_line(node)) || - block_start?(previous_line(node)) || - begin_line?(previous_line(node)) || - assignment_line?(previous_line(node)) || - rescue_line?(previous_line(node)) - end - - def last_line_valid?(node) - last_line(node).empty? || - end_line?(last_line(node)) || - end_clause_line?(last_line(node)) - end - - def previous_line(node) - processed_source[node.loc.line - 2] - end - - def last_line(node) - processed_source[node.loc.last_line] - end - - def start_clause_line?(line) - line =~ /^\s*(def|=end|#|module|class|if|unless|else|elsif|ensure|when)/ - end - - def end_clause_line?(line) - line =~ /^\s*(#|rescue|else|elsif|when)/ - end - - def begin_line?(line) - # an assignment followed by a begin or ust a begin - line =~ /^\s*(@?(\w|\|+|=|\[|\]|\s)+begin|begin)/ - end - - def assignment_line?(line) - line =~ /^\s*.*=/ - end - - def rescue_line?(line) - line =~ /^\s*rescue/ - end - - def block_start?(line) - line.match(/ (do|{)( \|.*?\|)?\s?(#.+)?\z/) - end - - def end_line?(line) - line =~ /^\s*(end|})/ - end - - def in_haml?(node) - node.location.expression.source_buffer.name.end_with?('.haml.rb') - end - end - end -end diff --git a/rubocop/qa_helpers.rb b/rubocop/qa_helpers.rb index 95875d64727..f4adf7f4e9f 100644 --- a/rubocop/qa_helpers.rb +++ b/rubocop/qa_helpers.rb @@ -5,7 +5,7 @@ module RuboCop def in_qa_file?(node) path = node.location.expression.source_buffer.name - path.start_with?(File.join(RuboCop::PathUtil.pwd, 'qa')) + path.start_with?(File.join(Dir.pwd, 'qa')) end end end diff --git a/rubocop/rubocop-code_reuse.yml b/rubocop/rubocop-code_reuse.yml new file mode 100644 index 00000000000..64e51c859f4 --- /dev/null +++ b/rubocop/rubocop-code_reuse.yml @@ -0,0 +1,41 @@ +# Denies the use of ActiveRecord methods outside of configured +# excluded directories +# Directories that allow the use of the denied methods. +# To start we provide a default configuration that matches +# a standard Rails app and enable. +# The default configuration can be overridden by +# providing your own Exclusion list as follows: +# CodeReuse/ActiveRecord: +# Enabled: true +# Exclude: +# - app/models/**/*.rb +# - config/**/*.rb +# - db/**/*.rb +# - lib/tasks/**/*.rb +# - spec/**/*.rb +# - lib/gitlab/**/*.rb +CodeReuse/ActiveRecord: + Exclude: + - app/models/**/*.rb + - config/**/*.rb + - db/**/*.rb + - lib/tasks/**/*.rake + - spec/**/*.rb + - danger/**/*.rb + - lib/backup/**/*.rb + - lib/banzai/**/*.rb + - lib/gitlab/background_migration/**/*.rb + - lib/gitlab/cycle_analytics/**/*.rb + - lib/gitlab/database/**/*.rb + - lib/gitlab/database_importers/common_metrics/importer.rb + - lib/gitlab/import_export/**/*.rb + - lib/gitlab/project_authorizations.rb + - lib/gitlab/sql/**/*.rb + - lib/system_check/**/*.rb + - qa/**/*.rb + - rubocop/**/*.rb + - ee/app/models/**/*.rb + - ee/spec/**/*.rb + - ee/db/fixtures/**/*.rb + - ee/lib/tasks/**/*.rake + - ee/lib/ee/gitlab/background_migration/**/*.rb diff --git a/rubocop/rubocop-migrations.yml b/rubocop/rubocop-migrations.yml index f8ff6e005f9..e1772deee9b 100644 --- a/rubocop/rubocop-migrations.yml +++ b/rubocop/rubocop-migrations.yml @@ -29,6 +29,7 @@ Migration/UpdateLargeTable: - :push_event_payloads - :resource_label_events - :routes + - :security_findings - :sent_notifications - :system_note_metadata - :taggings diff --git a/rubocop/rubocop-usage-data.yml b/rubocop/rubocop-usage-data.yml index fdfe2a5e1aa..51ad3ed0bef 100644 --- a/rubocop/rubocop-usage-data.yml +++ b/rubocop/rubocop-usage-data.yml @@ -50,3 +50,4 @@ UsageData/DistinctCountByLargeForeignKey: - 'owner_id' - 'project_id' - 'user_id' + - 'resource_owner_id' |