Skip to content

Commit

Permalink
Extend cop to cover Grape entities as well
Browse files Browse the repository at this point in the history
  • Loading branch information
timdavies committed Jun 25, 2024
1 parent 2446088 commit 66cb7c1
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 52 deletions.
5 changes: 5 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,8 @@ AllCops:

Rails/RakeEnvironment:
Enabled: false

# Disabling this as, due to how we test cops (i.e. with large code snippets),
# the individual tests are always going to be a bit long.
RSpec/ExampleLength:
Enabled: false
4 changes: 4 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ Boxt/ApiPathFormat:
Description: 'Ensure that the API path uses kebab case'
Enabled: false

Boxt/ApiTypeParameters:
Description: 'Ensure that API parameters are typed'
Enabled: false

Layout/ClassStructure:
Enabled: true

Expand Down
42 changes: 26 additions & 16 deletions lib/rubocop/cop/boxt/api_type_parameters.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,37 +15,47 @@ module Boxt
# optional :age, type: Integer
#
class ApiTypeParameters < Cop
MSG = "Ensure each parameter has a type specified, e.g., `type: String`."
API_MESSAGE = "Ensure each parameter has a type specified, e.g., `type: String`."
ENTITY_MESSAGE = "Ensure each parameter has a type specified, e.g., `documentation: { type: String }`."

def_node_matcher :param_declaration, <<-PATTERN
(send nil? {:optional :requires} _ $...)
(send nil? {:optional :requires :expose} _ $...)
PATTERN

def_node_search :param_with_type, <<-PATTERN
(send nil? {:optional :requires} _ (hash <(pair (sym :type) $_)>))
PATTERN

def_node_search :entity_with_type_documentation, <<-PATTERN
(send nil? :expose _ (hash <(pair (sym :documentation) (hash <(pair (sym :type) $_)>)) ...>))
PATTERN

def on_send(node)
param_declaration(node) do |args|
next unless grape_api_class?(node)
next if type_specified?(args)
param_declaration(node) do
next unless grape_api_class?(node) || grape_entity_class?(node)

add_offense(node, message: MSG)
if grape_api_class?(node) && param_with_type(node).none?
add_offense(node, message: API_MESSAGE)
elsif grape_entity_class?(node) && entity_with_type_documentation(node).none?
add_offense(node, message: ENTITY_MESSAGE)
end
end
end

private

def grape_api_class?(node)
node.each_ancestor(:class).any? do |ancestor|
ancestor.children.any? do |child|
child&.source == "Grape::API"
end
end
grape_parent_class?(node, "Grape::API")
end

def type_specified?(args)
return false if args.empty?
def grape_entity_class?(node)
grape_parent_class?(node, "Grape::Entity")
end

args.any? do |arg|
arg.type == :hash && arg.children.any? do |pair|
pair.type == :pair && pair.children[0].source == "type"
def grape_parent_class?(node, class_name)
node.each_ancestor(:class).any? do |ancestor|
ancestor.children.any? do |child|
child&.source == class_name
end
end
end
Expand Down
2 changes: 0 additions & 2 deletions spec/rubocop/cop/boxt/api_path_format_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ class Test < Grape::API
RUBY
end

# rubocop:disable RSpec/ExampleLength
it "registers an offense when using get with a path that contains underscores" do
expect_offense(<<~RUBY)
class Test < Grape::API
Expand Down Expand Up @@ -69,7 +68,6 @@ class Test < Grape::API
end
RUBY
end
# rubocop:enable RSpec/ExampleLength

it "does not register an offense when there is an underscore in the path parameter" do
expect_no_offenses(<<~RUBY)
Expand Down
87 changes: 53 additions & 34 deletions spec/rubocop/cop/boxt/api_type_parameters_spec.rb
Original file line number Diff line number Diff line change
@@ -1,47 +1,67 @@
# frozen_string_literal: true

# rubocop:disable RSpec/ExampleLength
RSpec.describe RuboCop::Cop::Boxt::ApiTypeParameters, :config do
it "does not register an offense when required parameter has type set" do
expect_no_offenses(<<~RUBY)
class Test < Grape::API
params do
requires :name, type: String
context "when checking Grape::API parameters" do
it "does not register an offense when required parameter has type set" do
expect_no_offenses(<<~RUBY)
class Test < Grape::API
params do
requires :name, type: String
end
end
end
RUBY
end
RUBY
end

it "does not register an offense when optional parameter has type set" do
expect_no_offenses(<<~RUBY)
class Test < Grape::API
params do
optional :age, type: Integer
it "does not register an offense when optional parameter has type set" do
expect_no_offenses(<<~RUBY)
class Test < Grape::API
params do
optional :age, type: Integer
end
end
end
RUBY
end
RUBY
end

it "registers an offense when required parameter has no type set" do
expect_offense(<<~RUBY)
class Test < Grape::API
params do
requires :name
^^^^^^^^^^^^^^ Ensure each parameter has a type specified, e.g., `type: String`.
it "registers an offense when required parameter has no type set" do
expect_offense(<<~RUBY)
class Test < Grape::API
params do
requires :name
^^^^^^^^^^^^^^ Ensure each parameter has a type specified, e.g., `type: String`.
end
end
end
RUBY
RUBY
end

it "registers an offense when optional parameter has no type set" do
expect_offense(<<~RUBY)
class Test < Grape::API
params do
optional :age
^^^^^^^^^^^^^ Ensure each parameter has a type specified, e.g., `type: String`.
end
end
RUBY
end
end

it "registers an offense when optional parameter has no type set" do
expect_offense(<<~RUBY)
class Test < Grape::API
params do
optional :age
^^^^^^^^^^^^^ Ensure each parameter has a type specified, e.g., `type: String`.
context "when checking Grape::Entity parameters" do
it "does not register an offense when parameter has type set" do
expect_no_offenses(<<~RUBY)
class Test < Grape::Entity
expose :name, documentation: { type: String }
end
end
RUBY
RUBY
end

it "registers an offense when parameter has no type set" do
expect_offense(<<~RUBY)
class Test < Grape::Entity
expose :name
^^^^^^^^^^^^ Ensure each parameter has a type specified, e.g., `documentation: { type: String }`.
end
RUBY
end
end

it "does not register an offense when the class isn't a Grape API" do
Expand All @@ -54,4 +74,3 @@ class Test
RUBY
end
end
# rubocop:enable RSpec/ExampleLength

0 comments on commit 66cb7c1

Please sign in to comment.