diff options
Diffstat (limited to 'rubocop/cop')
-rw-r--r-- | rubocop/cop/api/grape_api_instance.rb | 42 | ||||
-rw-r--r-- | rubocop/cop/api/grape_array_missing_coerce.rb | 83 | ||||
-rw-r--r-- | rubocop/cop/graphql/authorize_types.rb | 21 | ||||
-rw-r--r-- | rubocop/cop/migration/drop_table.rb | 5 | ||||
-rw-r--r-- | rubocop/cop/migration/with_lock_retries_disallowed_method.rb | 1 |
5 files changed, 137 insertions, 15 deletions
diff --git a/rubocop/cop/api/grape_api_instance.rb b/rubocop/cop/api/grape_api_instance.rb new file mode 100644 index 00000000000..de11b9ef3f6 --- /dev/null +++ b/rubocop/cop/api/grape_api_instance.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module API + class GrapeAPIInstance < RuboCop::Cop::Cop + # This cop checks that APIs subclass Grape::API::Instance with Grape v1.3+. + # + # @example + # + # # bad + # module API + # class Projects < Grape::API + # end + # end + # + # # good + # module API + # class Projects < Grape::API::Instance + # end + # end + MSG = 'Inherit from Grape::API::Instance instead of Grape::API. ' \ + 'For more details check the https://gitlab.com/gitlab-org/gitlab/-/issues/215230.' + + def_node_matcher :grape_api_definition, <<~PATTERN + (class + (const _ _) + (const + (const nil? :Grape) :API) + ... + ) + PATTERN + + def on_class(node) + grape_api_definition(node) do + add_offense(node.children[1]) + end + end + end + end + end +end diff --git a/rubocop/cop/api/grape_array_missing_coerce.rb b/rubocop/cop/api/grape_array_missing_coerce.rb new file mode 100644 index 00000000000..3d7a6a72d81 --- /dev/null +++ b/rubocop/cop/api/grape_array_missing_coerce.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module API + class GrapeArrayMissingCoerce < RuboCop::Cop::Cop + # This cop checks that Grape API parameters using an Array type + # implement a coerce_with method: + # + # https://github.com/ruby-grape/grape/blob/master/UPGRADING.md#ensure-that-array-types-have-explicit-coercions + # + # @example + # + # # bad + # requires :values, type: Array[String] + # + # # good + # requires :values, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce + # + # end + MSG = 'This Grape parameter defines an Array but is missing a coerce_with definition. ' \ + 'For more details, see https://github.com/ruby-grape/grape/blob/master/UPGRADING.md#ensure-that-array-types-have-explicit-coercions' + + def_node_matcher :grape_api_instance?, <<~PATTERN + (class + (const _ _) + (const + (const + (const nil? :Grape) :API) :Instance) + ... + ) + PATTERN + + def_node_matcher :grape_api_param_block?, <<~PATTERN + (send _ {:requires :optional} + (sym _) + $_) + PATTERN + + def_node_matcher :grape_type_def?, <<~PATTERN + (sym :type) + PATTERN + + def_node_matcher :grape_array_type?, <<~PATTERN + (send + (const nil? :Array) :[] + (const nil? _)) + PATTERN + + def_node_matcher :grape_coerce_with?, <<~PATTERN + (sym :coerce_with) + PATTERN + + def on_class(node) + @grape_api ||= grape_api_instance?(node) + end + + def on_send(node) + return unless @grape_api + + match = grape_api_param_block?(node) + + return unless match.is_a?(RuboCop::AST::HashNode) + + is_array_type = false + has_coerce_method = false + + match.each_pair do |first, second| + has_coerce_method ||= grape_coerce_with?(first) + + if grape_type_def?(first) && grape_array_type?(second) + is_array_type = true + end + end + + if is_array_type && !has_coerce_method + add_offense(node) + end + end + end + end + end +end diff --git a/rubocop/cop/graphql/authorize_types.rb b/rubocop/cop/graphql/authorize_types.rb index 7aaa9299362..1dba719cdff 100644 --- a/rubocop/cop/graphql/authorize_types.rb +++ b/rubocop/cop/graphql/authorize_types.rb @@ -7,39 +7,30 @@ module RuboCop MSG = 'Add an `authorize :ability` call to the type: '\ 'https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#type-authorization' - TYPES_DIR = 'app/graphql/types' - # We want to exclude our own basetypes and scalars - WHITELISTED_TYPES = %w[BaseEnum BaseScalar BasePermissionType MutationType - QueryType GraphQL::Schema BaseUnion].freeze + ALLOWED_TYPES = %w[BaseEnum BaseScalar BasePermissionType MutationType + QueryType GraphQL::Schema BaseUnion].freeze def_node_search :authorize?, <<~PATTERN (send nil? :authorize ...) PATTERN def on_class(node) - return unless in_type?(node) - return if whitelisted?(class_constant(node)) - return if whitelisted?(superclass_constant(node)) + return if allowed?(class_constant(node)) + return if allowed?(superclass_constant(node)) add_offense(node, location: :expression) unless authorize?(node) end private - def in_type?(node) - path = node.location.expression.source_buffer.name - - path.include? TYPES_DIR - end - - def whitelisted?(class_node) + def allowed?(class_node) class_const = class_node&.const_name return false unless class_const return true if class_const.end_with?('Enum') - WHITELISTED_TYPES.any? { |whitelisted| class_node.const_name.include?(whitelisted) } + ALLOWED_TYPES.any? { |allowed| class_node.const_name.include?(allowed) } end def class_constant(node) diff --git a/rubocop/cop/migration/drop_table.rb b/rubocop/cop/migration/drop_table.rb index 2a0f57c0c13..531cbb14021 100644 --- a/rubocop/cop/migration/drop_table.rb +++ b/rubocop/cop/migration/drop_table.rb @@ -17,6 +17,7 @@ module RuboCop def on_def(node) return unless in_deployment_migration?(node) + return if down_method?(node) node.each_descendant(:send) do |send_node| next unless offensible?(send_node) @@ -27,6 +28,10 @@ module RuboCop private + def down_method?(node) + node.method?(:down) + end + def offensible?(node) drop_table?(node) || drop_table_in_execute?(node) end diff --git a/rubocop/cop/migration/with_lock_retries_disallowed_method.rb b/rubocop/cop/migration/with_lock_retries_disallowed_method.rb index 309ddcc9746..9bf81e7db0c 100644 --- a/rubocop/cop/migration/with_lock_retries_disallowed_method.rb +++ b/rubocop/cop/migration/with_lock_retries_disallowed_method.rb @@ -10,6 +10,7 @@ module RuboCop ALLOWED_MIGRATION_METHODS = %i[ create_table + create_hash_partitions drop_table add_foreign_key remove_foreign_key |