diff options
Diffstat (limited to 'rubocop/cop')
| -rw-r--r-- | rubocop/cop/gitlab/bulk_insert.rb | 6 | ||||
| -rw-r--r-- | rubocop/cop/gitlab/json.rb | 2 | ||||
| -rw-r--r-- | rubocop/cop/gitlab/mark_used_feature_flags.rb | 1 | ||||
| -rw-r--r-- | rubocop/cop/graphql/descriptions.rb | 34 | ||||
| -rw-r--r-- | rubocop/cop/graphql/id_type.rb | 4 | ||||
| -rw-r--r-- | rubocop/cop/graphql/json_type.rb | 2 | ||||
| -rw-r--r-- | rubocop/cop/graphql/old_types.rb | 44 | ||||
| -rw-r--r-- | rubocop/cop/graphql/resolver_type.rb | 4 | ||||
| -rw-r--r-- | rubocop/cop/ignored_columns.rb | 50 | ||||
| -rw-r--r-- | rubocop/cop/qa/selector_usage.rb | 39 |
10 files changed, 161 insertions, 25 deletions
diff --git a/rubocop/cop/gitlab/bulk_insert.rb b/rubocop/cop/gitlab/bulk_insert.rb index 83d879ddf44..4c8c232043f 100644 --- a/rubocop/cop/gitlab/bulk_insert.rb +++ b/rubocop/cop/gitlab/bulk_insert.rb @@ -3,13 +3,13 @@ module RuboCop module Cop module Gitlab - # Cop that disallows the use of `Gitlab::Database.bulk_insert`, in favour of using + # Cop that disallows the use of `Gitlab::Database.main.bulk_insert`, in favour of using # the `BulkInsertSafe` module. class BulkInsert < RuboCop::Cop::Cop - MSG = 'Use the `BulkInsertSafe` concern, instead of using `Gitlab::Database.bulk_insert`. See https://docs.gitlab.com/ee/development/insert_into_tables_in_batches.html' + MSG = 'Use the `BulkInsertSafe` concern, instead of using `Gitlab::Database.main.bulk_insert`. See https://docs.gitlab.com/ee/development/insert_into_tables_in_batches.html' def_node_matcher :raw_union?, <<~PATTERN - (send (const (const _ :Gitlab) :Database) :bulk_insert ...) + (send (send (const (const _ :Gitlab) :Database) :main) :bulk_insert ...) PATTERN def on_send(node) diff --git a/rubocop/cop/gitlab/json.rb b/rubocop/cop/gitlab/json.rb index 7cc719aca09..d2ba0012ca0 100644 --- a/rubocop/cop/gitlab/json.rb +++ b/rubocop/cop/gitlab/json.rb @@ -10,7 +10,7 @@ module RuboCop EOL def_node_matcher :json_node?, <<~PATTERN - (send (const nil? :JSON)...) + (send (const {nil? | (const nil? :ActiveSupport)} :JSON)...) PATTERN def on_send(node) diff --git a/rubocop/cop/gitlab/mark_used_feature_flags.rb b/rubocop/cop/gitlab/mark_used_feature_flags.rb index 2a020d6efb2..a0de43abe85 100644 --- a/rubocop/cop/gitlab/mark_used_feature_flags.rb +++ b/rubocop/cop/gitlab/mark_used_feature_flags.rb @@ -32,6 +32,7 @@ module RuboCop SELF_METHODS = %i[ push_frontend_feature_flag limit_feature_flag= + limit_feature_flag_for_override= ].freeze + EXPERIMENT_METHODS + RUGGED_METHODS + WORKER_METHODS RESTRICT_ON_SEND = FEATURE_METHODS + EXPERIMENTATION_METHODS + GRAPHQL_METHODS + SELF_METHODS diff --git a/rubocop/cop/graphql/descriptions.rb b/rubocop/cop/graphql/descriptions.rb index 520e34dcd16..0d69fd55931 100644 --- a/rubocop/cop/graphql/descriptions.rb +++ b/rubocop/cop/graphql/descriptions.rb @@ -7,16 +7,16 @@ # # # bad # class AwfulType -# field :some_field, GraphQL::STRING_TYPE +# field :some_field, GraphQL::Types::String # end # # class TerribleType -# argument :some_argument, GraphQL::STRING_TYPE +# argument :some_argument, GraphQL::Types::String # end # # class UngoodType # field :some_argument, -# GraphQL::STRING_TYPE, +# GraphQL::Types::String, # description: "A description that does not end in a period" # end # @@ -27,12 +27,12 @@ # # good # class GreatType # argument :some_field, -# GraphQL::STRING_TYPE, +# GraphQL::Types::String, # description: "Well described - a superb description." # # field :some_field, -# GraphQL::STRING_TYPE, -# description: "A thorough and compelling description." +# GraphQL::Types::String, +# description: "Thorough and compelling description." # end # # class GoodEnum @@ -43,8 +43,10 @@ module RuboCop module Cop module Graphql class Descriptions < RuboCop::Cop::Cop - MSG_NO_DESCRIPTION = 'Please add a `description` property.' - MSG_NO_PERIOD = '`description` strings must end with a `.`.' + MSG_STYLE_GUIDE_LINK = 'See the description style guide: https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#description-style-guide' + MSG_NO_DESCRIPTION = "Please add a `description` property. #{MSG_STYLE_GUIDE_LINK}" + MSG_NO_PERIOD = "`description` strings must end with a `.`. #{MSG_STYLE_GUIDE_LINK}" + MSG_BAD_START = "`description` strings should not start with \"A...\" or \"The...\". #{MSG_STYLE_GUIDE_LINK}" def_node_matcher :graphql_describable?, <<~PATTERN (send nil? {:field :argument :value} ...) @@ -75,6 +77,7 @@ module RuboCop return add_offense(node, location: :expression, message: MSG_NO_DESCRIPTION) unless description add_offense(node, location: :expression, message: MSG_NO_PERIOD) if no_period?(description) + add_offense(node, location: :expression, message: MSG_BAD_START) if bad_start?(description) end # Autocorrect missing periods at end of description. @@ -100,12 +103,19 @@ module RuboCop end def no_period?(description) - # Test that the description node is a `:str` (as opposed to - # a `#copy_field_description` call) before checking. - description.type == :str && !description.value.strip.end_with?('.') + string?(description) && !description.value.strip.end_with?('.') end - # Returns a Parser::Source::Range that ends just before the final String delimiter. + def bad_start?(description) + string?(description) && description.value.strip.downcase.start_with?('a ', 'the ') + end + + # Returns true if `description` node is a `:str` (as opposed to a `#copy_field_description` call) + def string?(description) + description.type == :str + end + + # Returns a `Parser::Source::Range` that ends just before the final `String` delimiter. def before_end_quote(string) return string.source_range.adjust(end_pos: -1) unless string.heredoc? diff --git a/rubocop/cop/graphql/id_type.rb b/rubocop/cop/graphql/id_type.rb index 0d2fb6ad852..ba973242efa 100644 --- a/rubocop/cop/graphql/id_type.rb +++ b/rubocop/cop/graphql/id_type.rb @@ -4,12 +4,12 @@ module RuboCop module Cop module Graphql class IDType < RuboCop::Cop::Cop - MSG = 'Do not use GraphQL::ID_TYPE, use a specific GlobalIDType instead' + MSG = 'Do not use GraphQL::Types::ID, use a specific GlobalIDType instead' WHITELISTED_ARGUMENTS = %i[iid full_path project_path group_path target_project_path namespace_path].freeze def_node_search :graphql_id_type?, <<~PATTERN - (send nil? :argument (_ #does_not_match?) (const (const nil? :GraphQL) :ID_TYPE) ...) + (send nil? :argument (_ #does_not_match?) (const (const (const nil? :GraphQL) :Types) :ID) ...) PATTERN def on_send(node) diff --git a/rubocop/cop/graphql/json_type.rb b/rubocop/cop/graphql/json_type.rb index a8c38358535..8518a771cbd 100644 --- a/rubocop/cop/graphql/json_type.rb +++ b/rubocop/cop/graphql/json_type.rb @@ -12,7 +12,7 @@ # # # good # class GreatClass -# field :some_field, GraphQL::STRING_TYPE +# field :some_field, GraphQL::Types::String # end module RuboCop diff --git a/rubocop/cop/graphql/old_types.rb b/rubocop/cop/graphql/old_types.rb new file mode 100644 index 00000000000..2df594c7016 --- /dev/null +++ b/rubocop/cop/graphql/old_types.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +# This cop checks for use of older GraphQL types in GraphQL fields +# and arguments. +# GraphQL::ID_TYPE, GraphQL::INT_TYPE, GraphQL::STRING_TYPE, GraphQL::BOOLEAN_TYPE +# +# @example +# +# # bad +# class AwfulClass +# field :some_field, GraphQL::STRING_TYPE +# end +# +# # good +# class GreatClass +# field :some_field, GraphQL::Types::String +# end + +module RuboCop + module Cop + module Graphql + class OldTypes < RuboCop::Cop::Cop + MSG_ID = 'Avoid using GraphQL::ID_TYPE. Use GraphQL::Types::ID instead' + MSG_INT = 'Avoid using GraphQL::INT_TYPE. Use GraphQL::Types::Int instead' + MSG_STRING = 'Avoid using GraphQL::STRING_TYPE. Use GraphQL::Types::String instead' + MSG_BOOLEAN = 'Avoid using GraphQL::BOOLEAN_TYPE. Use GraphQL::Types::Boolean instead' + + def_node_matcher :has_old_type?, <<~PATTERN + (send nil? {:field :argument} + (sym _) + (const (const nil? :GraphQL) ${:ID_TYPE :INT_TYPE :STRING_TYPE :BOOLEAN_TYPE}) + (...)?) + PATTERN + + def on_send(node) + old_constant = has_old_type?(node) + return unless old_constant + + add_offense(node, location: :expression, message: "#{self.class}::MSG_#{old_constant[0..-6]}".constantize) + end + end + end + end +end diff --git a/rubocop/cop/graphql/resolver_type.rb b/rubocop/cop/graphql/resolver_type.rb index 1209c5dbc6b..e9fa768fd3e 100644 --- a/rubocop/cop/graphql/resolver_type.rb +++ b/rubocop/cop/graphql/resolver_type.rb @@ -7,7 +7,7 @@ # # bad # module Resolvers # class NoTypeResolver < BaseResolver -# field :some_field, GraphQL::STRING_TYPE +# field :some_field, GraphQL::Types::String # end # end # @@ -16,7 +16,7 @@ # class WithTypeResolver < BaseResolver # type MyType, null: true # -# field :some_field, GraphQL::STRING_TYPE +# field :some_field, GraphQL::Types::String # end # end diff --git a/rubocop/cop/ignored_columns.rb b/rubocop/cop/ignored_columns.rb index 14bcfa04ae1..4a6f1e4f2d9 100644 --- a/rubocop/cop/ignored_columns.rb +++ b/rubocop/cop/ignored_columns.rb @@ -2,18 +2,60 @@ module RuboCop module Cop - # Cop that blacklists the usage of Group.public_or_visible_to_user + # Cop that blacklists the usage of `ActiveRecord::Base.ignored_columns=` directly class IgnoredColumns < RuboCop::Cop::Cop - MSG = 'Use `IgnoredColumns` concern instead of adding to `self.ignored_columns`.' + USE_CONCERN_MSG = 'Use `IgnoredColumns` concern instead of adding to `self.ignored_columns`.' + WRONG_MODEL_MSG = 'If the model exists in CE and EE, the column has to be ignored ' \ + 'in the CE model. If the model only exists in EE, then it has to be added there.' def_node_matcher :ignored_columns?, <<~PATTERN (send (self) :ignored_columns) PATTERN + def_node_matcher :ignore_columns?, <<~PATTERN + (send nil? :ignore_columns ...) + PATTERN + + def_node_matcher :ignore_column?, <<~PATTERN + (send nil? :ignore_column ...) + PATTERN + def on_send(node) - return unless ignored_columns?(node) + if ignored_columns?(node) + add_offense(node, location: :expression, message: USE_CONCERN_MSG) + end + + if using_ignore?(node) && used_in_wrong_model? + add_offense(node, location: :expression, message: WRONG_MODEL_MSG) + end + end + + private + + def using_ignore?(node) + ignore_columns?(node) || ignore_column?(node) + end + + def used_in_wrong_model? + file_path = processed_source.file_path + + ee_model?(file_path) && ce_model_exists?(file_path) + end + + def ee_model?(path) + path.include?(ee_directory) + end + + def ee_directory + File.join(rails_root, 'ee') + end + + def rails_root + File.expand_path('../..', __dir__) + end - add_offense(node, location: :expression) + def ce_model_exists?(path) + File.exist?(path.gsub(%r{/ee/}, '/')) end end end diff --git a/rubocop/cop/qa/selector_usage.rb b/rubocop/cop/qa/selector_usage.rb new file mode 100644 index 00000000000..568b1c30851 --- /dev/null +++ b/rubocop/cop/qa/selector_usage.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require_relative '../../qa_helpers' +require_relative '../../code_reuse_helpers' + +module RuboCop + module Cop + module QA + # This cop checks for the usage of data-qa-selectors or .qa-* classes in non-QA files + # + # @example + # # bad + # find('[data-qa-selector="the_selector"]') + # find('.qa-selector') + # + # # good + # find('[data-testid="the_selector"]') + # find('#selector') + class SelectorUsage < RuboCop::Cop::Cop + include QAHelpers + include CodeReuseHelpers + + SELECTORS = /\.qa-\w+|data-qa-\w+/.freeze + MESSAGE = %(Do not use `%s` as this is reserved for the end-to-end specs. Use a different selector or a data-testid instead.) + + def on_str(node) + return if in_qa_file?(node) + return unless in_spec?(node) + + add_offense(node, message: MESSAGE % node.value) if SELECTORS =~ node.value + rescue StandardError + # catch all errors and ignore them. + # without this catch-all rescue, rubocop will fail + # because of non-UTF-8 characters in some Strings + end + end + end + end +end |