diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-11-17 11:33:21 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-11-17 11:33:21 +0000 |
commit | 7021455bd1ed7b125c55eb1b33c5a01f2bc55ee0 (patch) | |
tree | 5bdc2229f5198d516781f8d24eace62fc7e589e9 /rubocop | |
parent | 185b095e93520f96e9cfc31d9c3e69b498cdab7c (diff) | |
download | gitlab-ce-7021455bd1ed7b125c55eb1b33c5a01f2bc55ee0.tar.gz |
Add latest changes from gitlab-org/gitlab@15-6-stable-eev15.6.0-rc42
Diffstat (limited to 'rubocop')
-rw-r--r-- | rubocop/cop/api/ensure_string_detail.rb | 51 | ||||
-rw-r--r-- | rubocop/cop/gitlab/feature_available_usage.rb | 1 | ||||
-rw-r--r-- | rubocop/cop/gitlab/json.rb | 33 | ||||
-rw-r--r-- | rubocop/cop/gitlab/mark_used_feature_flags.rb | 2 | ||||
-rw-r--r-- | rubocop/cop/gitlab/rspec/avoid_setup.rb | 82 | ||||
-rw-r--r-- | rubocop/cop/graphql/enum_names.rb | 87 | ||||
-rw-r--r-- | rubocop/cop/graphql/enum_values.rb | 79 | ||||
-rw-r--r-- | rubocop/cop/migration/schema_addition_methods_no_post.rb | 35 | ||||
-rw-r--r-- | rubocop/cop/rake/require.rb | 84 | ||||
-rw-r--r-- | rubocop/cop/rspec/duplicate_spec_location.rb (renamed from rubocop/cop/gitlab/duplicate_spec_location.rb) | 8 | ||||
-rw-r--r-- | rubocop/cop/rspec/factory_bot/strategy_in_callback.rb | 45 | ||||
-rw-r--r-- | rubocop/migration_helpers.rb | 12 |
12 files changed, 507 insertions, 12 deletions
diff --git a/rubocop/cop/api/ensure_string_detail.rb b/rubocop/cop/api/ensure_string_detail.rb new file mode 100644 index 00000000000..bc999525055 --- /dev/null +++ b/rubocop/cop/api/ensure_string_detail.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require_relative '../../code_reuse_helpers' + +module RuboCop + module Cop + module API + class EnsureStringDetail < RuboCop::Cop::Base + include CodeReuseHelpers + + # This cop checks that API detail entries use Strings + # + # https://gitlab.com/gitlab-org/gitlab/-/issues/379037 + # + # @example + # + # # bad + # detail ['Foo bar baz bat', 'http://example.com'] + # + # # good + # detail 'Foo bar baz bat. http://example.com' + # + # end + # + MSG = 'Only String objects are permitted in API detail field.' + + def_node_matcher :detail_in_desc, <<~PATTERN + (block + (send nil? :desc ...) + _args + `(send nil? :detail $_ ...) + ) + PATTERN + + RESTRICT_ON_SEND = %i[detail].freeze + + def on_send(node) + return unless in_api?(node) + + parent = node.each_ancestor(:block).first + detail_arg = detail_in_desc(parent) + + return unless detail_arg + return if [:str, :dstr].include?(detail_arg.type) + + add_offense(node) + end + end + end + end +end diff --git a/rubocop/cop/gitlab/feature_available_usage.rb b/rubocop/cop/gitlab/feature_available_usage.rb index 4dba4baf1e7..fcf4992a19d 100644 --- a/rubocop/cop/gitlab/feature_available_usage.rb +++ b/rubocop/cop/gitlab/feature_available_usage.rb @@ -27,6 +27,7 @@ module RuboCop environments feature_flags releases + infrastructure ].freeze EE_FEATURES = %i[requirements].freeze ALL_FEATURES = (FEATURES + EE_FEATURES).freeze diff --git a/rubocop/cop/gitlab/json.rb b/rubocop/cop/gitlab/json.rb index 56846e3c276..cf2ed0ba536 100644 --- a/rubocop/cop/gitlab/json.rb +++ b/rubocop/cop/gitlab/json.rb @@ -7,25 +7,44 @@ module RuboCop extend RuboCop::Cop::AutoCorrector MSG = <<~EOL - Avoid calling `JSON` directly. Instead, use the `Gitlab::Json` - wrapper. This allows us to alter the JSON parser being used. + Prefer `Gitlab::Json` over calling `JSON` directly. See https://docs.gitlab.com/ee/development/json.html EOL + AVAILABLE_METHODS = %i[parse parse! load decode dump generate encode pretty_generate].to_set.freeze + def_node_matcher :json_node?, <<~PATTERN - (send (const {nil? | (const nil? :ActiveSupport)} :JSON)...) + (send (const {nil? | (const nil? :ActiveSupport)} :JSON) $_ $...) PATTERN def on_send(node) - return unless json_node?(node) + method_name, arg_source = match_node(node) + return unless method_name add_offense(node) do |corrector| - _, method_name, *arg_nodes = *node - - replacement = "Gitlab::Json.#{method_name}(#{arg_nodes.map(&:source).join(', ')})" + replacement = "#{cbased(node)}Gitlab::Json.#{method_name}(#{arg_source})" corrector.replace(node.source_range, replacement) end end + + private + + def match_node(node) + method_name, arg_nodes = json_node?(node) + + # Only match if the method is implemented by Gitlab::Json + if method_name && AVAILABLE_METHODS.include?(method_name) + return [method_name, arg_nodes.map(&:source).join(", ")] + end + + nil + end + + def cbased(node) + return unless %r{/ee/}.match?(node.location.expression.source_buffer.name) + + "::" + end end end end diff --git a/rubocop/cop/gitlab/mark_used_feature_flags.rb b/rubocop/cop/gitlab/mark_used_feature_flags.rb index 23de0644385..d1722a47c8a 100644 --- a/rubocop/cop/gitlab/mark_used_feature_flags.rb +++ b/rubocop/cop/gitlab/mark_used_feature_flags.rb @@ -59,7 +59,7 @@ module RuboCop def on_casgn(node) _, lhs_name, rhs = *node - save_used_feature_flag(rhs.value) if lhs_name == :FEATURE_FLAG + save_used_feature_flag(rhs.value) if lhs_name.to_s.end_with?('FEATURE_FLAG') end def on_send(node) diff --git a/rubocop/cop/gitlab/rspec/avoid_setup.rb b/rubocop/cop/gitlab/rspec/avoid_setup.rb new file mode 100644 index 00000000000..fd2ed3b7e34 --- /dev/null +++ b/rubocop/cop/gitlab/rspec/avoid_setup.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +require 'rubocop-rspec' + +module RuboCop + module Cop + module Gitlab + module RSpec + # This cop checks for use of constructs that may lead to deterioration in readability + # in specs. + # + # @example + # + # # bad + # before do + # enforce_terms + # end + # + # it 'auto accepts terms and redirects to the group path' do + # visit sso_group_saml_providers_path(group, token: group.saml_discovery_token) + # + # click_link 'Sign in' + # + # expect(page).to have_content('Signed in with SAML') + # end + # + # # good + # it 'auto accepts terms and redirects to the group path' do + # enforce_terms + # + # visit sso_group_saml_providers_path(group, token: group.saml_discovery_token) + # + # click_link 'Sign in' + # + # expect(page).to have_content('Signed in with SAML') + # end + # + # # good + # it 'registers the user and starts to import a project' do + # user_signs_up + # + # expect_to_see_account_confirmation_page + # + # confirm_account + # + # user_signs_in + # + # expect_to_see_welcome_form + # + # fills_in_welcome_form + # click_on 'Continue' + # + # expect_to_see_group_and_project_creation_form + # + # click_on 'Import' + # + # expect_to_see_import_form + # + # fills_in_import_form + # click_on 'GitHub' + # + # expect_to_be_in_import_process + # end + # + class AvoidSetup < RuboCop::Cop::Base + MSG = 'Avoid the use of `%{name}` to keep this area as readable as possible. ' \ + 'See https://gitlab.com/gitlab-org/gitlab/-/issues/373194' + + NOT_ALLOWED = %i[let_it_be let_it_be_with_refind let_it_be_with_reload let let! + before after around it_behaves_like shared_examples shared_examples_for + shared_context include_context subject].freeze + + RESTRICT_ON_SEND = NOT_ALLOWED + + def on_send(node) + add_offense(node, message: format(MSG, name: node.children[1])) + end + end + end + end + end +end diff --git a/rubocop/cop/graphql/enum_names.rb b/rubocop/cop/graphql/enum_names.rb new file mode 100644 index 00000000000..74847cb8d17 --- /dev/null +++ b/rubocop/cop/graphql/enum_names.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +# This cop enforces the enum naming conventions from the enum style guide: +# https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#enums +# +# @example +# +# # bad +# class FooBar < BaseEnum +# value 'FOO' +# end +# +# class SubparEnum < BaseEnum +# end +# +# class UngoodEnum < BaseEnum +# graphql_name 'UngoodEnum' +# end +# +# # good +# +# class GreatEnum < BaseEnum +# graphql_name 'Great' +# +# value 'BAR' +# end +# +# class NiceEnum < BaseEnum +# declarative_enum NiceDeclarativeEnum +# end + +module RuboCop + module Cop + module Graphql + class EnumNames < RuboCop::Cop::Base + SEE_SG_MSG = "See https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#enums" + CLASS_NAME_SUFFIX_MSG = "Enum class names must end with `Enum`. #{SEE_SG_MSG}" + GRAPHQL_NAME_MISSING_MSG = "A `graphql_name` must be defined for a GraphQL enum. #{SEE_SG_MSG}" + GRAPHQL_NAME_WITH_ENUM_MSG = "The `graphql_name` must not contain the string \"Enum\". #{SEE_SG_MSG}" + + def_node_matcher :enum_subclass, <<~PATTERN + (class $(const nil? _) (const {nil? cbase} /.*Enum$/) ...) + PATTERN + + def_node_search :find_graphql_name, <<~PATTERN + (... `(send nil? :graphql_name $(...)) ...) + PATTERN + + def_node_search :declarative_enum?, <<~PATTERN + (... (send nil? :declarative_enum ...) ...) + PATTERN + + def on_class(node) + const_node = enum_subclass(node) + return unless const_node + + check_class_name(const_node) + check_graphql_name(node) + end + + private + + def check_class_name(const_node) + return unless const_node&.const_name + return if const_node.const_name.end_with?('Enum') + + add_offense(const_node, message: CLASS_NAME_SUFFIX_MSG) + end + + def check_graphql_name(node) + graphql_name_node = find_graphql_name(node)&.first + + if graphql_name_node + return unless graphql_name_node&.str_content + return unless graphql_name_node.str_content.downcase.include?('enum') + + add_offense(graphql_name_node, message: GRAPHQL_NAME_WITH_ENUM_MSG) + else + return if declarative_enum?(node) + + add_offense(node, message: GRAPHQL_NAME_MISSING_MSG) + end + end + end + end + end +end diff --git a/rubocop/cop/graphql/enum_values.rb b/rubocop/cop/graphql/enum_values.rb new file mode 100644 index 00000000000..71c42596334 --- /dev/null +++ b/rubocop/cop/graphql/enum_values.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +# This cop enforces the enum value conventions from the enum style guide: +# https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#enums +# +# @example +# +# # bad +# class BadEnum < BaseEnum +# graphql_name 'Bad' +# +# value 'foo' +# end +# +# class UngoodEnum < BaseEnum +# graphql_name 'Ungood' +# +# ['bar'].each do |val| +# value val +# end +# end +# +# # good +# class GoodEnum < BaseEnum +# graphql_name 'Good' +# +# value 'FOO' +# end +# +# class GreatEnum < BaseEnum +# graphql_name 'Great' +# +# ['bar'].each do |val| +# value val.upcase +# end +# end + +module RuboCop + module Cop + module Graphql + class EnumValues < RuboCop::Cop::Base + MSG = "Enum values must either be an uppercase string literal or uppercased with the `upcase` method. " \ + "See https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#enums" + + def_node_matcher :enum_value, <<~PATTERN + (send nil? :value $_ $...) + PATTERN + + def_node_search :deprecated?, <<~PATTERN + (hash <(pair (sym :deprecated) _) ...>) + PATTERN + + def_node_matcher :upcase_literal?, <<~PATTERN + (str #upcase?) + PATTERN + + def_node_matcher :upcase_method?, <<~PATTERN + `(send _ :upcase) + PATTERN + + def on_send(node) + value_node, params = enum_value(node) + + return unless value_node + return if params.any? { deprecated?(_1) } + return if upcase_literal?(value_node) || upcase_method?(value_node) + + add_offense(value_node) + end + + private + + def upcase?(str) + str == str.upcase + end + end + end + end +end diff --git a/rubocop/cop/migration/schema_addition_methods_no_post.rb b/rubocop/cop/migration/schema_addition_methods_no_post.rb new file mode 100644 index 00000000000..5bb5bb63f0c --- /dev/null +++ b/rubocop/cop/migration/schema_addition_methods_no_post.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require_relative '../../migration_helpers' + +module RuboCop + module Cop + module Migration + # Cop that checks that no background batched migration helpers are called by regular migrations. + class SchemaAdditionMethodsNoPost < RuboCop::Cop::Base + include MigrationHelpers + + MSG = "This method may not be used in post migrations. Please see documentation here: https://docs.gitlab.com/ee/development/migration_style_guide.html#choose-an-appropriate-migration-type" + + FORBIDDEN_METHODS = %w[ + add_column + create_table + ].freeze + + SYMBOLIZED_MATCHER = FORBIDDEN_METHODS.map { |w| ":#{w}" }.join(' | ') + + def_node_matcher :on_forbidden_method, <<~PATTERN + (send nil? {#{SYMBOLIZED_MATCHER}} ...) + PATTERN + + def on_send(node) + return unless time_enforced?(node) + + on_forbidden_method(node) do + add_offense(node, message: MSG) + end + end + end + end + end +end diff --git a/rubocop/cop/rake/require.rb b/rubocop/cop/rake/require.rb new file mode 100644 index 00000000000..e3e1696943d --- /dev/null +++ b/rubocop/cop/rake/require.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Rake + # Flag global `require`s or `require_relative`s in rake files. + # + # Load dependencies lazily in `task` definitions if possible. + # + # @example + # # bad + # + # require_relative 'gitlab/json' + # require 'json' + # + # task :parse_json do + # Gitlab::Json.parse(...) + # end + # + # # good + # + # task :parse_json do + # require_relative 'gitlab/json' + # require 'json' + # + # Gitlab::Json.parse(...) + # end + # + # RSpec::Core::RakeTask.new(:parse_json) do |t, args| + # require_relative 'gitlab/json' + # require 'json' + # + # Gitlab::Json.parse(...) + # end + # + # # Requiring files which contain the word `task` is allowed. + # require 'some_gem/rake_task' + # require 'some_gem/rake_tasks' + # + # SomeGem.define_tasks + # + # # Loading in method definition as well. + # def load_deps + # require 'json' + # end + # + # task :parse_json + # load_deps + # end + # + class Require < RuboCop::Cop::Base + MSG = 'Load dependencies inside `task` definitions if possible.' + + METHODS = %i[require require_relative].freeze + RESTRICT_ON_SEND = METHODS + + def_node_matcher :require_method, <<~PATTERN + (send nil? ${#{METHODS.map(&:inspect).join(' ')}} $_) + PATTERN + + def on_send(node) + method, file = require_method(node) + return unless method + + return if requires_task?(file) + return if inside_block_or_method?(node) + + add_offense(node) + end + + private + + # Allow `require "foo/rake_task"` + def requires_task?(file) + file.source.include?('task') + end + + def inside_block_or_method?(node) + node.each_ancestor(:block, :def).any? + end + end + end + end +end diff --git a/rubocop/cop/gitlab/duplicate_spec_location.rb b/rubocop/cop/rspec/duplicate_spec_location.rb index f8c19caf351..c920104c8c0 100644 --- a/rubocop/cop/gitlab/duplicate_spec_location.rb +++ b/rubocop/cop/rspec/duplicate_spec_location.rb @@ -5,7 +5,7 @@ require 'rubocop/cop/rspec/mixin/top_level_group' module RuboCop module Cop - module Gitlab + module RSpec # Cop that detects duplicate EE spec files # # There should not be files in both ee/spec/*/ee/my_spec.rb and ee/spec/*/my_spec.rb @@ -29,9 +29,9 @@ module RuboCop path = file_path_for_node(node.send_node).sub(%r{\A#{rails_root}/}, '') duplicate_path = find_duplicate_path(path) - if duplicate_path && File.exist?(File.join(rails_root, duplicate_path)) - add_offense(node.send_node, message: format(MSG, path: duplicate_path)) - end + return unless duplicate_path && File.exist?(File.join(rails_root, duplicate_path)) + + add_offense(node.send_node, message: format(MSG, path: duplicate_path)) end private diff --git a/rubocop/cop/rspec/factory_bot/strategy_in_callback.rb b/rubocop/cop/rspec/factory_bot/strategy_in_callback.rb new file mode 100644 index 00000000000..3153d54887e --- /dev/null +++ b/rubocop/cop/rspec/factory_bot/strategy_in_callback.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'rubocop-rspec' + +module RuboCop + module Cop + module RSpec + module FactoryBot + class StrategyInCallback < RuboCop::Cop::Base + include RuboCop::RSpec::FactoryBot::Language + + MSG = 'Prefer inline `association` over `%{type}`. ' \ + 'See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#factories' + + FORBIDDEN_METHODS = %i[build build_list build_stubbed build_stubbed_list create create_list].freeze + + def_node_matcher :forbidden_factory_usage, <<~PATTERN + (block + (send nil? { :after :before } (sym _strategy)) + _args + ` # in all descandents + (send + { nil? #factory_bot? } + ${ #{FORBIDDEN_METHODS.map(&:inspect).join(' ')} } + (sym _factory_name) + ... + ) + ) + PATTERN + + RESTRICT_ON_SEND = FORBIDDEN_METHODS + + def on_send(node) + parent = node.each_ancestor(:block).first + + strategy = forbidden_factory_usage(parent) + return unless strategy + + add_offense(node, message: format(MSG, type: strategy)) + end + end + end + end + end +end diff --git a/rubocop/migration_helpers.rb b/rubocop/migration_helpers.rb index 16a9aa53cd3..22f3931be73 100644 --- a/rubocop/migration_helpers.rb +++ b/rubocop/migration_helpers.rb @@ -49,6 +49,14 @@ module RuboCop dirname(node).end_with?('db/post_migrate', 'db/geo/post_migrate') end + # Returns true if we've defined an 'EnforcedSince' variable in rubocop.yml and the migration version + # is greater. + def time_enforced?(node) + return false unless enforced_since + + version(node) > enforced_since + end + def version(node) File.basename(node.location.expression.source_buffer.name).split('_').first.to_i end @@ -80,5 +88,9 @@ module RuboCop def rubocop_path File.expand_path(__dir__) end + + def enforced_since + @enforced_since ||= config.for_cop(name)['EnforcedSince'] + end end end |