From 85dc423f7090da0a52c73eb66faf22ddb20efff9 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Sat, 19 Sep 2020 01:45:44 +0000 Subject: Add latest changes from gitlab-org/gitlab@13-4-stable-ee --- .../cop/gitlab/avoid_uploaded_file_from_params.rb | 51 ++++++++++++++++++++++ rubocop/cop/gitlab/bulk_insert.rb | 2 +- rubocop/cop/gitlab/except.rb | 23 ++++++++++ rubocop/cop/gitlab/intersect.rb | 23 ++++++++++ rubocop/cop/gitlab/rails_logger.rb | 35 +++++++-------- 5 files changed, 113 insertions(+), 21 deletions(-) create mode 100644 rubocop/cop/gitlab/avoid_uploaded_file_from_params.rb create mode 100644 rubocop/cop/gitlab/except.rb create mode 100644 rubocop/cop/gitlab/intersect.rb (limited to 'rubocop/cop/gitlab') diff --git a/rubocop/cop/gitlab/avoid_uploaded_file_from_params.rb b/rubocop/cop/gitlab/avoid_uploaded_file_from_params.rb new file mode 100644 index 00000000000..599371aa5a1 --- /dev/null +++ b/rubocop/cop/gitlab/avoid_uploaded_file_from_params.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Gitlab + # This cop checks for `UploadedFile.from_params` usage. + # See https://docs.gitlab.com/ee/development/uploads.html#how-to-add-a-new-upload-route + # + # @example + # + # # bad + # class MyAwfulApi < Grape::API::Instance + # params do + # optional 'file.path', type: String + # optional 'file.name', type: String + # optional 'file.type', type: String + # optional 'file.size', type: Integer + # optional 'file.md5', type: String + # optional 'file.sha1', type: String + # optional 'file.sha256', type: String + # end + # put '/files' do + # uploaded_file = UploadedFile.from_params(params, :file, FileUploader.workhorse_local_upload_path) + # end + # end + # + # # good + # class MyMuchBetterApi < Grape::API::Instance + # params do + # requires :file, type: ::API::Validations::Types::WorkhorseFile + # end + # put '/files' do + # uploaded_file = declared_params[:file] + # end + # end + class AvoidUploadedFileFromParams < RuboCop::Cop::Cop + MSG = 'Use the `UploadedFile` set by `multipart.rb` instead of calling `UploadedFile.from_params` directly. See https://docs.gitlab.com/ee/development/uploads.html#how-to-add-a-new-upload-route' + + def_node_matcher :calling_uploaded_file_from_params?, <<~PATTERN + (send (const nil? :UploadedFile) :from_params ...) + PATTERN + + def on_send(node) + return unless calling_uploaded_file_from_params?(node) + + add_offense(node, location: :expression) + end + end + end + end +end diff --git a/rubocop/cop/gitlab/bulk_insert.rb b/rubocop/cop/gitlab/bulk_insert.rb index c03ffbe0b2a..83d879ddf44 100644 --- a/rubocop/cop/gitlab/bulk_insert.rb +++ b/rubocop/cop/gitlab/bulk_insert.rb @@ -9,7 +9,7 @@ module RuboCop 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' def_node_matcher :raw_union?, <<~PATTERN - (send (const (const nil? :Gitlab) :Database) :bulk_insert ...) + (send (const (const _ :Gitlab) :Database) :bulk_insert ...) PATTERN def on_send(node) diff --git a/rubocop/cop/gitlab/except.rb b/rubocop/cop/gitlab/except.rb new file mode 100644 index 00000000000..24da6962457 --- /dev/null +++ b/rubocop/cop/gitlab/except.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Gitlab + # Cop that disallows the use of `Gitlab::SQL::Except`, in favour of using + # the `FromExcept` module. + class Except < RuboCop::Cop::Cop + MSG = 'Use the `FromExcept` concern, instead of using `Gitlab::SQL::Except` directly' + + def_node_matcher :raw_except?, <<~PATTERN + (send (const (const (const nil? :Gitlab) :SQL) :Except) :new ...) + PATTERN + + def on_send(node) + return unless raw_except?(node) + + add_offense(node, location: :expression) + end + end + end + end +end diff --git a/rubocop/cop/gitlab/intersect.rb b/rubocop/cop/gitlab/intersect.rb new file mode 100644 index 00000000000..4b61073b804 --- /dev/null +++ b/rubocop/cop/gitlab/intersect.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Gitlab + # Cop that disallows the use of `Gitlab::SQL::Intersect`, in favour of using + # the `FromIntersect` module. + class Intersect < RuboCop::Cop::Cop + MSG = 'Use the `FromIntersect` concern, instead of using `Gitlab::SQL::Intersect` directly' + + def_node_matcher :raw_intersect?, <<~PATTERN + (send (const (const (const nil? :Gitlab) :SQL) :Intersect) :new ...) + PATTERN + + def on_send(node) + return unless raw_intersect?(node) + + add_offense(node, location: :expression) + end + end + end + end +end diff --git a/rubocop/cop/gitlab/rails_logger.rb b/rubocop/cop/gitlab/rails_logger.rb index d1a06a9a100..ad35d2ccfbb 100644 --- a/rubocop/cop/gitlab/rails_logger.rb +++ b/rubocop/cop/gitlab/rails_logger.rb @@ -8,7 +8,7 @@ module RuboCop class RailsLogger < ::RuboCop::Cop::Cop include CodeReuseHelpers - # This cop checks for the Rails.logger in the codebase + # This cop checks for the Rails.logger log methods in the codebase # # @example # @@ -17,34 +17,29 @@ module RuboCop # # # good # Gitlab::AppLogger.error("Project %{project_path} could not be saved" % { project_path: project.full_path }) + # + # # OK + # Rails.logger.level MSG = 'Use a structured JSON logger instead of `Rails.logger`. ' \ 'https://docs.gitlab.com/ee/development/logging.html'.freeze - def_node_matcher :rails_logger?, <<~PATTERN - (send (const nil? :Rails) :logger ... ) - PATTERN + # See supported log methods: + # https://ruby-doc.org/stdlib-2.6.6/libdoc/logger/rdoc/Logger.html + LOG_METHODS = %i[debug error fatal info warn].freeze + LOG_METHODS_PATTERN = LOG_METHODS.map(&:inspect).join(' ').freeze - WHITELISTED_DIRECTORIES = %w[ - spec - ].freeze + def_node_matcher :rails_logger_log?, <<~PATTERN + (send + (send (const nil? :Rails) :logger) + {#{LOG_METHODS_PATTERN}} ... + ) + PATTERN def on_send(node) - return if in_whitelisted_directory?(node) - return unless rails_logger?(node) + return unless rails_logger_log?(node) add_offense(node, location: :expression) end - - def in_whitelisted_directory?(node) - path = file_path_for_node(node) - - WHITELISTED_DIRECTORIES.any? do |directory| - path.start_with?( - File.join(rails_root, directory), - File.join(rails_root, 'ee', directory) - ) - end - end end end end -- cgit v1.2.3