Commit 50295e65 authored by Sean McGivern's avatar Sean McGivern Committed by Stan Hu

Require all API resources to inherit from a common base class

This makes changing the base class in future easier.
parent dcc29126
...@@ -335,7 +335,7 @@ Gitlab/Union: ...@@ -335,7 +335,7 @@ Gitlab/Union:
- 'spec/**/*' - 'spec/**/*'
- 'ee/spec/**/*' - 'ee/spec/**/*'
API/GrapeAPIInstance: API/Base:
Enabled: true Enabled: true
Include: Include:
- 'lib/**/api/**/*.rb' - 'lib/**/api/**/*.rb'
......
# frozen_string_literal: true
module API
class Base < Grape::API::Instance # rubocop:disable API/Base
end
end
...@@ -3,8 +3,8 @@ ...@@ -3,8 +3,8 @@
module RuboCop module RuboCop
module Cop module Cop
module API module API
class GrapeAPIInstance < RuboCop::Cop::Cop class Base < RuboCop::Cop::Cop
# This cop checks that APIs subclass Grape::API::Instance with Grape v1.3+. # This cop checks that APIs subclass API::Base.
# #
# @example # @example
# #
...@@ -14,19 +14,24 @@ module RuboCop ...@@ -14,19 +14,24 @@ module RuboCop
# end # end
# end # end
# #
# # good
# module API # module API
# class Projects < Grape::API::Instance # class Projects < Grape::API::Instance
# end # end
# end # end
MSG = 'Inherit from Grape::API::Instance instead of Grape::API. ' \ #
'For more details check the https://gitlab.com/gitlab-org/gitlab/-/issues/215230.' # # good
# module API
# class Projects < ::API::Base
# end
# end
MSG = 'Inherit from ::API::Base instead of Grape::API::Instance or Grape::API. ' \
'For more details check https://gitlab.com/gitlab-org/gitlab/-/issues/215230.'
def_node_matcher :grape_api, '(const (const {nil? (cbase)} :Grape) :API)'
def_node_matcher :grape_api_definition, <<~PATTERN def_node_matcher :grape_api_definition, <<~PATTERN
(class (class
(const _ _) (const _ _)
(const {#grape_api (const #grape_api :Instance)}
(const nil? :Grape) :API)
... ...
) )
PATTERN PATTERN
...@@ -36,6 +41,12 @@ module RuboCop ...@@ -36,6 +41,12 @@ module RuboCop
add_offense(node.children[1]) add_offense(node.children[1])
end end
end end
def autocorrect(node)
lambda do |corrector|
corrector.replace(node, '::API::Base')
end
end
end end
end end
end end
......
# frozen_string_literal: true
require 'fast_spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/api/base'
RSpec.describe RuboCop::Cop::API::Base, type: :rubocop do
include CopHelper
subject(:cop) { described_class.new }
let(:corrected) do
<<~CORRECTED
class SomeAPI < ::API::Base
end
CORRECTED
end
['Grape::API', '::Grape::API', 'Grape::API::Instance', '::Grape::API::Instance'].each do |offense|
it "adds an offense when inheriting from #{offense}" do
expect_offense(<<~CODE)
class SomeAPI < #{offense}
#{'^' * offense.length} #{described_class::MSG}
end
CODE
expect_correction(corrected)
end
end
it 'does not add an offense when inheriting from BaseAPI' do
expect_no_offenses(corrected)
end
end
# frozen_string_literal: true
require 'fast_spec_helper'
require 'rubocop'
require_relative '../../../../rubocop/cop/api/grape_api_instance'
RSpec.describe RuboCop::Cop::API::GrapeAPIInstance do
include CopHelper
subject(:cop) { described_class.new }
it 'adds an offense when inheriting from Grape::API' do
inspect_source(<<~CODE)
class SomeAPI < Grape::API
end
CODE
expect(cop.offenses.size).to eq(1)
end
it 'does not add an offense when inheriting from Grape::API::Instance' do
inspect_source(<<~CODE)
class SomeAPI < Grape::API::Instance
end
CODE
expect(cop.offenses.size).to be_zero
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment