diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-04-21 02:50:22 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-04-21 02:50:22 +0300 |
commit | 9dc93a4519d9d5d7be48ff274127136236a3adb3 (patch) | |
tree | 70467ae3692a0e35e5ea56bcb803eb512a10bedb /rubocop/cop/gitlab | |
parent | 4b0f34b6d759d6299322b3a54453e930c6121ff0 (diff) |
Add latest changes from gitlab-org/gitlab@13-11-stable-eev13.11.0-rc43
Diffstat (limited to 'rubocop/cop/gitlab')
-rw-r--r-- | rubocop/cop/gitlab/change_timzone.rb | 2 | ||||
-rw-r--r-- | rubocop/cop/gitlab/delegate_predicate_methods.rb | 45 | ||||
-rw-r--r-- | rubocop/cop/gitlab/feature_available_usage.rb | 99 | ||||
-rw-r--r-- | rubocop/cop/gitlab/rails_logger.rb | 2 |
4 files changed, 146 insertions, 2 deletions
diff --git a/rubocop/cop/gitlab/change_timzone.rb b/rubocop/cop/gitlab/change_timzone.rb index 63e6dd411f3..c30a057d51c 100644 --- a/rubocop/cop/gitlab/change_timzone.rb +++ b/rubocop/cop/gitlab/change_timzone.rb @@ -5,7 +5,7 @@ module RuboCop module Gitlab class ChangeTimezone < RuboCop::Cop::Cop MSG = "Do not change timezone in the runtime (application or rspec), " \ - "it could result in silently modifying other behavior.".freeze + "it could result in silently modifying other behavior." def_node_matcher :changing_timezone?, <<~PATTERN (send (const nil? :Time) :zone= ...) diff --git a/rubocop/cop/gitlab/delegate_predicate_methods.rb b/rubocop/cop/gitlab/delegate_predicate_methods.rb new file mode 100644 index 00000000000..43b5184faab --- /dev/null +++ b/rubocop/cop/gitlab/delegate_predicate_methods.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Gitlab + # This cop looks for delegations to predicate methods with `allow_nil: true` option. + # This construct results in three possible results: true, false and nil. + # In other words, it does not preserve the strict Boolean nature of predicate method return value. + # This cop suggests creating a method to handle `nil` delegator and ensure only Boolean type is returned. + # + # @example + # # bad + # delegate :is_foo?, to: :bar, allow_nil: true + # + # # good + # def is_foo? + # return false unless bar + # bar.is_foo? + # end + # + # def is_foo? + # !!bar&.is_foo? + # end + class DelegatePredicateMethods < RuboCop::Cop::Cop + MSG = "Using `delegate` with `allow_nil` on the following predicate methods is discouraged: %s." + RESTRICT_ON_SEND = %i[delegate].freeze + def_node_matcher :predicate_allow_nil_option, <<~PATTERN + (send nil? :delegate + (sym $_)* + (hash <$(pair (sym :allow_nil) true) ...>) + ) + PATTERN + + def on_send(node) + predicate_allow_nil_option(node) do |delegated_methods, _options| + offensive_methods = delegated_methods.select { |method| method.end_with?('?') } + next if offensive_methods.empty? + + add_offense(node, message: format(MSG, offensive_methods.join(', '))) + end + end + end + end + end +end diff --git a/rubocop/cop/gitlab/feature_available_usage.rb b/rubocop/cop/gitlab/feature_available_usage.rb new file mode 100644 index 00000000000..b50bdd8ca43 --- /dev/null +++ b/rubocop/cop/gitlab/feature_available_usage.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Gitlab + # Cop that checks for correct calling of #feature_available? + class FeatureAvailableUsage < RuboCop::Cop::Cop + OBSERVED_METHOD = :feature_available? + LICENSED_FEATURE_LITERAL_ARG_MSG = '`feature_available?` should not be called for features that can be licensed (`%s` given), use `licensed_feature_available?(feature)` instead.' + LICENSED_FEATURE_DYNAMIC_ARG_MSG = "`feature_available?` should not be called for features that can be licensed (`%s` isn't a literal so we cannot say if it's legit or not), using `licensed_feature_available?(feature)` may be more appropriate." + NOT_ENOUGH_ARGS_MSG = '`feature_available?` should be called with two arguments: `feature` and `user`.' + FEATURES = %i[ + issues + forking + merge_requests + wiki + snippets + builds + repository + pages + metrics_dashboard + analytics + operations + security_and_compliance + container_registry + ].freeze + EE_FEATURES = %i[requirements].freeze + ALL_FEATURES = (FEATURES + EE_FEATURES).freeze + SPECIAL_CLASS = %w[License].freeze + + def on_send(node) + return unless method_name(node) == OBSERVED_METHOD + return if caller_is_special_case?(node) + return if feature_name(node).nil? + return if ALL_FEATURES.include?(feature_name(node)) && args_count(node) == 2 + + if !ALL_FEATURES.include?(feature_name(node)) + add_offense(node, location: :expression, message: licensed_feature_message(node)) + elsif args_count(node) < 2 + add_offense(node, location: :expression, message: NOT_ENOUGH_ARGS_MSG) + end + end + + def licensed_feature_message(node) + message_template = dynamic_feature?(node) ? LICENSED_FEATURE_DYNAMIC_ARG_MSG : LICENSED_FEATURE_LITERAL_ARG_MSG + + message_template % feature_arg_name(node) + end + + private + + def method_name(node) + node.children[1] + end + + def class_caller(node) + node.children[0]&.const_name.to_s + end + + def caller_is_special_case?(node) + SPECIAL_CLASS.include?(class_caller(node)) + end + + def feature_arg(node) + node.children[2] + end + + def dynamic_feature?(node) + feature = feature_arg(node) + return false unless feature + + !feature.literal? + end + + def feature_name(node) + feature = feature_arg(node) + return unless feature + return feature.children.compact.join('.') if dynamic_feature?(node) + return feature.value if feature.respond_to?(:value) + + feature.type + end + + def feature_arg_name(node) + feature = feature_arg(node) + return unless feature + return feature.children.compact.join('.') if dynamic_feature?(node) + return feature.children[0].inspect if feature.literal? + + feature.type + end + + def args_count(node) + node.children[2..].size + end + end + end + end +end diff --git a/rubocop/cop/gitlab/rails_logger.rb b/rubocop/cop/gitlab/rails_logger.rb index ad35d2ccfbb..5a1695ce56e 100644 --- a/rubocop/cop/gitlab/rails_logger.rb +++ b/rubocop/cop/gitlab/rails_logger.rb @@ -21,7 +21,7 @@ module RuboCop # # OK # Rails.logger.level MSG = 'Use a structured JSON logger instead of `Rails.logger`. ' \ - 'https://docs.gitlab.com/ee/development/logging.html'.freeze + 'https://docs.gitlab.com/ee/development/logging.html' # See supported log methods: # https://ruby-doc.org/stdlib-2.6.6/libdoc/logger/rdoc/Logger.html |