From b1abb4dfa2288e7357ec6a3e16b159d1b16534a9 Mon Sep 17 00:00:00 2001 From: Tim Davies Date: Tue, 25 Jun 2024 11:42:20 +0100 Subject: [PATCH] Add api_type_parameters cop (#146) --- .rubocop.yml | 5 ++ config/default.yml | 4 + lib/rubocop/cop/boxt/api_type_parameters.rb | 65 ++++++++++++++++ lib/rubocop/cop/boxt_cops.rb | 1 + spec/rubocop/cop/boxt/api_path_format_spec.rb | 2 - .../cop/boxt/api_type_parameters_spec.rb | 76 +++++++++++++++++++ 6 files changed, 151 insertions(+), 2 deletions(-) create mode 100644 lib/rubocop/cop/boxt/api_type_parameters.rb create mode 100644 spec/rubocop/cop/boxt/api_type_parameters_spec.rb diff --git a/.rubocop.yml b/.rubocop.yml index 600cc02..0d65131 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -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 diff --git a/config/default.yml b/config/default.yml index 6d6e11d..83423c6 100644 --- a/config/default.yml +++ b/config/default.yml @@ -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 diff --git a/lib/rubocop/cop/boxt/api_type_parameters.rb b/lib/rubocop/cop/boxt/api_type_parameters.rb new file mode 100644 index 0000000..eefe50c --- /dev/null +++ b/lib/rubocop/cop/boxt/api_type_parameters.rb @@ -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 diff --git a/lib/rubocop/cop/boxt_cops.rb b/lib/rubocop/cop/boxt_cops.rb index 573bbb2..a72c889 100644 --- a/lib/rubocop/cop/boxt_cops.rb +++ b/lib/rubocop/cop/boxt_cops.rb @@ -1,3 +1,4 @@ # frozen_string_literal: true require_relative "boxt/api_path_format" +require_relative "boxt/api_type_parameters" diff --git a/spec/rubocop/cop/boxt/api_path_format_spec.rb b/spec/rubocop/cop/boxt/api_path_format_spec.rb index 38bde66..4463d88 100644 --- a/spec/rubocop/cop/boxt/api_path_format_spec.rb +++ b/spec/rubocop/cop/boxt/api_path_format_spec.rb @@ -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 @@ -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) diff --git a/spec/rubocop/cop/boxt/api_type_parameters_spec.rb b/spec/rubocop/cop/boxt/api_type_parameters_spec.rb new file mode 100644 index 0000000..420edbe --- /dev/null +++ b/spec/rubocop/cop/boxt/api_type_parameters_spec.rb @@ -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