diff options
Diffstat (limited to 'rubocop')
| -rw-r--r-- | rubocop/cop/gitlab/policy_rule_boolean.rb | 56 | ||||
| -rw-r--r-- | rubocop/cop/graphql/descriptions.rb | 60 | ||||
| -rw-r--r-- | rubocop/cop/graphql/id_type.rb | 2 | ||||
| -rw-r--r-- | rubocop/cop/lint/last_keyword_argument.rb | 71 | ||||
| -rw-r--r-- | rubocop/cop/rspec/httparty_basic_auth.rb | 49 | ||||
| -rw-r--r-- | rubocop/rubocop-migrations.yml | 3 | ||||
| -rw-r--r-- | rubocop/rubocop-usage-data.yml | 2 |
7 files changed, 228 insertions, 15 deletions
diff --git a/rubocop/cop/gitlab/policy_rule_boolean.rb b/rubocop/cop/gitlab/policy_rule_boolean.rb new file mode 100644 index 00000000000..ca69eebab6e --- /dev/null +++ b/rubocop/cop/gitlab/policy_rule_boolean.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Gitlab + # This cop checks for usage of boolean operators in rule blocks, which + # does not work because conditions are objects, not booleans. + # + # @example + # + # # bad, `conducts_electricity` returns a Rule object, not a boolean! + # rule { conducts_electricity && batteries }.enable :light_bulb + # + # # good + # rule { conducts_electricity & batteries }.enable :light_bulb + # + # @example + # + # # bad, `conducts_electricity` returns a Rule object, so the ternary is always going to be true + # rule { conducts_electricity ? can?(:magnetize) : batteries }.enable :motor + # + # # good + # rule { conducts_electricity & can?(:magnetize) }.enable :motor + # rule { ~conducts_electricity & batteries }.enable :motor + class PolicyRuleBoolean < RuboCop::Cop::Cop + def_node_search :has_and_operator?, <<~PATTERN + (and ...) + PATTERN + + def_node_search :has_or_operator?, <<~PATTERN + (or ...) + PATTERN + + def_node_search :has_if?, <<~PATTERN + (if ...) + PATTERN + + def on_block(node) + return unless node.method_name == :rule + + if has_and_operator?(node) + add_offense(node, message: '&& is not allowed within a rule block. Did you mean to use `&`?') + end + + if has_or_operator?(node) + add_offense(node, message: '|| is not allowed within a rule block. Did you mean to use `|`?') + end + + if has_if?(node) + add_offense(node, message: 'if and ternary operators are not allowed within a rule block.') + end + end + end + end + end +end diff --git a/rubocop/cop/graphql/descriptions.rb b/rubocop/cop/graphql/descriptions.rb index b4e00dfe336..1585e5c9788 100644 --- a/rubocop/cop/graphql/descriptions.rb +++ b/rubocop/cop/graphql/descriptions.rb @@ -13,42 +13,74 @@ # argument :some_argument, GraphQL::STRING_TYPE # end # +# class UngoodClass +# field :some_argument, +# GraphQL::STRING_TYPE, +# description: "A description that does not end in a period" +# end +# # # good # class GreatClass # argument :some_field, # GraphQL::STRING_TYPE, -# description: "Well described - a superb description" +# description: "Well described - a superb description." # # field :some_field, # GraphQL::STRING_TYPE, -# description: "A thorough and compelling description" +# description: "A thorough and compelling description." # end module RuboCop module Cop module Graphql class Descriptions < RuboCop::Cop::Cop - MSG = 'Please add a `description` property.' + MSG_NO_DESCRIPTION = 'Please add a `description` property.' + MSG_NO_PERIOD = '`description` strings must end with a `.`.' # ability_field and permission_field set a default description. - def_node_matcher :fields, <<~PATTERN - (send nil? :field $...) - PATTERN - - def_node_matcher :arguments, <<~PATTERN - (send nil? :argument $...) + def_node_matcher :field_or_argument?, <<~PATTERN + (send nil? {:field :argument} ...) PATTERN - def_node_matcher :has_description?, <<~PATTERN - (hash <(pair (sym :description) _) ...>) + def_node_matcher :description, <<~PATTERN + (... (hash <(pair (sym :description) $_) ...>)) PATTERN def on_send(node) - matches = fields(node) || arguments(node) + return unless field_or_argument?(node) + + description = description(node) + + 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) + end + + # Autocorrect missing periods at end of description. + def autocorrect(node) + lambda do |corrector| + description = description(node) + next unless description + + corrector.insert_after(before_end_quote(description), '.') + end + end + + private + + 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?('.') + end - return if matches.nil? + # 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? - add_offense(node, location: :expression) unless has_description?(matches.last) + heredoc_source = string.location.heredoc_body.source + adjust = heredoc_source.index(/\s+\Z/) - heredoc_source.length + string.location.heredoc_body.adjust(end_pos: adjust) end end end diff --git a/rubocop/cop/graphql/id_type.rb b/rubocop/cop/graphql/id_type.rb index 96f90ac136a..0d2fb6ad852 100644 --- a/rubocop/cop/graphql/id_type.rb +++ b/rubocop/cop/graphql/id_type.rb @@ -6,7 +6,7 @@ module RuboCop class IDType < RuboCop::Cop::Cop MSG = 'Do not use GraphQL::ID_TYPE, use a specific GlobalIDType instead' - WHITELISTED_ARGUMENTS = %i[iid full_path project_path group_path target_project_path].freeze + 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) ...) diff --git a/rubocop/cop/lint/last_keyword_argument.rb b/rubocop/cop/lint/last_keyword_argument.rb new file mode 100644 index 00000000000..430ea66e9a1 --- /dev/null +++ b/rubocop/cop/lint/last_keyword_argument.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Lint + # This cop only works if there are files from deprecation_toolkit. You can + # generate these files by: + # + # 1. Running specs with RECORD_DEPRECATIONS=1 + # 1. Downloading the complete set of deprecations/ files from a CI + # pipeline (see https://gitlab.com/gitlab-org/gitlab/-/merge_requests/47720) + class LastKeywordArgument < Cop + MSG = 'Using the last argument as keyword parameters is deprecated'.freeze + + DEPRECATIONS_GLOB = File.expand_path('../../../deprecations/**/*.yml', __dir__) + KEYWORD_DEPRECATION_STR = 'maybe ** should be added to the call' + + def on_send(node) + arg = node.arguments.last + return unless arg + + return unless known_match?(processed_source.file_path, node.first_line, node.method_name.to_s) + + return if arg.children.first.respond_to?(:kwsplat_type?) && arg.children.first&.kwsplat_type? + + # parser thinks `a: :b, c: :d` is hash type, it's actually kwargs + return if arg.hash_type? && !arg.source.match(/\A{/) + + add_offense(arg) + end + + def autocorrect(arg) + lambda do |corrector| + if arg.hash_type? + kwarg = arg.source.sub(/\A{\s*/, '').sub(/\s*}\z/, '') + corrector.replace(arg, kwarg) + elsif arg.splat_type? + corrector.insert_before(arg, '*') + else + corrector.insert_before(arg, '**') + end + end + end + + private + + def known_match?(file_path, line_number, method_name) + file_path_from_root = file_path.sub(File.expand_path('../../..', __dir__), '') + + method_name = 'initialize' if method_name == 'new' + + self.class.keyword_warnings.any? do |warning| + warning.include?("#{file_path_from_root}:#{line_number}") && warning.include?("called method `#{method_name}'") + end + end + + def self.keyword_warnings + @keyword_warnings ||= keywords_list + end + + def self.keywords_list + hash = Dir.glob(DEPRECATIONS_GLOB).each_with_object({}) do |file, hash| + hash.merge!(YAML.safe_load(File.read(file))) + end + + hash.values.flatten.select { |str| str.include?(KEYWORD_DEPRECATION_STR) }.uniq + end + end + end + end +end diff --git a/rubocop/cop/rspec/httparty_basic_auth.rb b/rubocop/cop/rspec/httparty_basic_auth.rb new file mode 100644 index 00000000000..529a5808662 --- /dev/null +++ b/rubocop/cop/rspec/httparty_basic_auth.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module RSpec + # This cop checks for invalid credentials passed to HTTParty + # + # @example + # + # # bad + # HTTParty.get(url, basic_auth: { user: 'foo' }) + # + # # good + # HTTParty.get(url, basic_auth: { username: 'foo' }) + class HTTPartyBasicAuth < RuboCop::Cop::Cop + MESSAGE = "`basic_auth: { user: ... }` does not work - replace `user:` with `username:`".freeze + + RESTRICT_ON_SEND = %i(get put post delete).freeze + + def_node_matcher :httparty_basic_auth?, <<~PATTERN + (send + (const _ :HTTParty) + {#{RESTRICT_ON_SEND.map(&:inspect).join(' ')}} + <(hash + <(pair + (sym :basic_auth) + (hash + <(pair $(sym :user) _) ...> + ) + ) ...> + ) ...> + ) + PATTERN + + def on_send(node) + return unless m = httparty_basic_auth?(node) + + add_offense(m, location: :expression, message: MESSAGE) + end + + def autocorrect(node) + lambda do |corrector| + corrector.replace(node.loc.expression, 'username') + end + end + end + end + end +end diff --git a/rubocop/rubocop-migrations.yml b/rubocop/rubocop-migrations.yml index e1772deee9b..41bd2a4ce7d 100644 --- a/rubocop/rubocop-migrations.yml +++ b/rubocop/rubocop-migrations.yml @@ -25,6 +25,7 @@ Migration/UpdateLargeTable: - :project_authorizations - :projects - :project_ci_cd_settings + - :project_settings - :project_features - :push_event_payloads - :resource_label_events @@ -35,6 +36,8 @@ Migration/UpdateLargeTable: - :taggings - :todos - :users + - :user_preferences + - :user_details - :web_hook_logs DeniedMethods: - :change_column_type_concurrently diff --git a/rubocop/rubocop-usage-data.yml b/rubocop/rubocop-usage-data.yml index 51ad3ed0bef..0e40a5971ee 100644 --- a/rubocop/rubocop-usage-data.yml +++ b/rubocop/rubocop-usage-data.yml @@ -20,6 +20,7 @@ UsageData/LargeTable: - :Gitlab::Runtime - :Gitaly::Server - :Gitlab::UsageData + - :Gitlab::UsageDataCounters - :License - :Rails - :Time @@ -51,3 +52,4 @@ UsageData/DistinctCountByLargeForeignKey: - 'project_id' - 'user_id' - 'resource_owner_id' + - 'requirement_id' |