Skip to content

Commit

Permalink
Add api_type_parameters cop (#146)
Browse files Browse the repository at this point in the history
  • Loading branch information
timdavies authored Jun 25, 2024
1 parent 02efde4 commit b1abb4d
Show file tree
Hide file tree
Showing 6 changed files with 151 additions and 2 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
65 changes: 65 additions & 0 deletions lib/rubocop/cop/boxt/api_type_parameters.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Boxt
# This cop ensures that each parameter in a Grape API has a type specified.
#
# @example
# # bad
# requires :name
# optional :age
#
# # good
# requires :name, type: String
# optional :age, type: Integer
#
class ApiTypeParameters < Cop
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 :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
next unless grape_api_class?(node) || grape_entity_class?(node)

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)
grape_parent_class?(node, "Grape::API")
end

def grape_entity_class?(node)
grape_parent_class?(node, "Grape::Entity")
end

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
end
end
end
end
1 change: 1 addition & 0 deletions lib/rubocop/cop/boxt_cops.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# frozen_string_literal: true

require_relative "boxt/api_path_format"
require_relative "boxt/api_type_parameters"
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
76 changes: 76 additions & 0 deletions spec/rubocop/cop/boxt/api_type_parameters_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Boxt::ApiTypeParameters, :config do
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
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
end
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`.
end
end
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

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
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
expect_no_offenses(<<~RUBY)
class Test
params do
requires :name
end
end
RUBY
end
end

0 comments on commit b1abb4d

Please sign in to comment.