Fix Array[Object] documentation type parsing (#86)

Fixes incorrect Swagger schema generation when using custom array
documentation syntax like `type: 'Array[Object]'`.

### Problem

When using `type: 'Array[Object]'` with an entity, the Swagger schema
was not correctly generated as an array type with `$ref` items.

### Solution

Add `array_type?` method that detects array notations:
- `is_array: true` - explicit flag (existing behavior)
- `type: 'Array[Object]'` - new syntax for entity arrays

### Usage Example

```ruby
class Report < Grape::Entity
  expose :items,
         using: ItemEntity,
         documentation: {
           type: 'Array[Object]',
           desc: 'List of items'
         }
end
```

Generates correct schema:
```json
{
  "type": "array",
  "description": "List of items",
  "items": { "$ref": "#/definitions/ItemEntity" }
}
```

### Other Changes

- Refactor `parse_grape_entity_params` into smaller, focused methods for
readability
- Add nil guards to prevent potential crashes
- Fix indentation in `parse_nested` method

### Backward Compatibility

- `is_array: true` continues to work as before
- `is_array: false` is respected when explicitly set
- Bare `type: 'Array'` is not double-wrapped

### Note

Tests for empty entity handling require grape-swagger >= 2.1.3
([ruby-grape/grape-swagger#963](ruby-grape/grape-swagger#963))
and are skipped for older versions.

Fixes
[ruby-grape/grape-swagger#962](ruby-grape/grape-swagger#962)

Author: numbata

.rspec

@@ -1,2 +1,3 @@
 --format documentation
 --color
+--require "spec_helper"

.rubocop_todo.yml

@@ -6,12 +6,13 @@
 # Note that changes in the inspected code, or installation of new
 # versions of RuboCop, may require this file to be generated again.
 
-# Offense count: 3
+# Offense count: 7
 # Configuration parameters: AllowedMethods.
 # AllowedMethods: enums
 Lint/ConstantDefinitionInBlock:
   Exclude:
     - 'spec/grape-swagger/entities/response_model_spec.rb'
+    - 'spec/issues/962_polymorphic_entity_with_custom_documentation_spec.rb'
     - 'spec/support/shared_contexts/inheritance_api.rb'
     - 'spec/support/shared_contexts/this_api.rb'
 
@@ -21,6 +22,11 @@ Lint/EmptyBlock:
   Exclude:
     - 'spec/grape-swagger/entity/attribute_parser_spec.rb'
 
+# Offense count: 1
+Lint/EmptyClass:
+  Exclude:
+    - 'spec/support/shared_contexts/custom_type_parser.rb'
+
 # Offense count: 2
 # This cop supports unsafe autocorrection (--autocorrect-all).
 # Configuration parameters: AllowedMethods.
@@ -37,7 +43,7 @@ Metrics/AbcSize:
 # Offense count: 2
 # Configuration parameters: CountComments, CountAsOne.
 Metrics/ClassLength:
-  Max: 120
+  Max: 135
 
 # Offense count: 2
 # Configuration parameters: AllowedMethods, AllowedPatterns.
@@ -79,11 +85,12 @@ RSpec/ContextWording:
     - 'spec/support/shared_contexts/inheritance_api.rb'
     - 'spec/support/shared_contexts/this_api.rb'
 
-# Offense count: 2
+# Offense count: 3
 # Configuration parameters: IgnoredMetadata.
 RSpec/DescribeClass:
   Exclude:
     - '**/spec/features/**/*'
+    - '**/spec/issues/**/*'
     - '**/spec/requests/**/*'
     - '**/spec/routing/**/*'
     - '**/spec/system/**/*'
@@ -95,10 +102,11 @@ RSpec/DescribeClass:
 RSpec/ExampleLength:
   Max: 213
 
-# Offense count: 26
+# Offense count: 30
 RSpec/LeakyConstantDeclaration:
   Exclude:
     - 'spec/grape-swagger/entities/response_model_spec.rb'
+    - '**/spec/issues/**/*'
     - 'spec/support/shared_contexts/inheritance_api.rb'
     - 'spec/support/shared_contexts/this_api.rb'
 

CHANGELOG.md

@@ -7,6 +7,7 @@
 #### Fixes
 
 * Your contribution here.
+* [#86](https://github.com/ruby-grape/grape-swagger-entity/pull/86): Fix Array[Object] documentation type parsing - [@numbata](https://github.com/numbata).
 * [#87](https://github.com/ruby-grape/grape-swagger-entity/pull/87): Remove hidden attributes from required - [@bogdan](https://github.com/bogdan).
 * [#88](https://github.com/ruby-grape/grape-swagger-entity/pull/88): Update danger workflows - [@numbata](https://github.com/numbata).
 * [#89](https://github.com/ruby-grape/grape-swagger-entity/pull/89): Remove ruby-grape-danger - [@dblock](https://github.com/dblock).

lib/grape-swagger/entity/attribute_parser.rb

@@ -20,7 +20,7 @@ def call(entity_options)
         documentation = entity_options[:documentation]
         return param if documentation.nil?
 
-        add_array_documentation(param, documentation) if documentation[:is_array]
+        add_array_documentation(param, documentation) if array_type?(documentation)
 
         add_attribute_sample(param, documentation, :default)
         add_attribute_sample(param, documentation, :example)
@@ -37,11 +37,26 @@ def call(entity_options)
       def model_from(entity_options)
         model = entity_options[:using] if entity_options[:using].present?
 
-        model ||= entity_options[:documentation][:type] if could_it_be_a_model?(entity_options[:documentation])
+        documentation = entity_options[:documentation]
+        model ||= documentation[:type] if could_it_be_a_model?(documentation)
 
         model
       end
 
+      # Checks if documentation indicates an array type that needs items wrapping.
+      # Returns true for:
+      #   - is_array: true (explicit flag)
+      #   - type: 'Array[Something]' (array with element type)
+      # Returns false for:
+      #   - type: 'Array' (bare array - already an array type, no wrapping needed)
+      def array_type?(documentation)
+        return false if documentation.nil?
+        return documentation[:is_array] if documentation.key?(:is_array)
+
+        # Only match Array[ElementType] syntax, not bare 'Array'
+        documentation[:type].to_s.match?(/\Aarray\[.+\]\z/i)
+      end
+
       def could_it_be_a_model?(value)
         return false if value.nil?
 
@@ -57,6 +72,8 @@ def ambiguous_model_type?(type)
       end
 
       def primitive_type?(type)
+        return false if type.nil?
+
         data_type = GrapeSwagger::DocMethods::DataType.call(type)
         GrapeSwagger::DocMethods::DataType.request_primitive?(data_type)
       end
@@ -69,7 +86,7 @@ def data_type_from(entity_options)
 
         documented_data_type = document_data_type(documentation, data_type)
 
-        if documentation[:is_array]
+        if array_type?(documentation)
           {
             type: :array,
             items: documented_data_type
@@ -97,7 +114,9 @@ def document_data_type(documentation, data_type)
       end
 
       def entity_model_type(name, entity_options)
-        if entity_options[:documentation] && entity_options[:documentation][:is_array]
+        documentation = entity_options[:documentation]
+
+        if array_type?(documentation)
           {
             'type' => 'array',
             'items' => {

lib/grape-swagger/entity/parser.rb

@@ -43,37 +43,57 @@ def extract_params(exposure)
       def parse_grape_entity_params(params, parent_model = nil)
         return unless params
 
-        parsed = params.each_with_object({}) do |(entity_name, entity_options), memo|
-          documentation_options = entity_options.fetch(:documentation, {})
-          in_option = documentation_options.fetch(:in, nil).to_s
-          hidden_option = documentation_options.fetch(:hidden, nil)
-          next if in_option == 'header' || hidden_option == true
-
-          entity_name = entity_name.original if entity_name.is_a?(Alias)
-          final_entity_name = entity_options.fetch(:as, entity_name)
-          documentation = entity_options[:documentation]
-
-          memo[final_entity_name] = if entity_options[:nesting]
-                                      parse_nested(entity_name, entity_options, parent_model)
-                                    else
-                                      attribute_parser.call(entity_options)
-                                    end
-
-          next unless documentation
-
-          memo[final_entity_name][:readOnly] = documentation[:read_only].to_s == 'true' if documentation[:read_only]
-          memo[final_entity_name][:description] = documentation[:desc] if documentation[:desc]
+        required = required_params(params)
+        parsed_params = parse_params(params, parent_model)
+
+        handle_discriminator(parsed_params, required)
+      end
+
+      def parse_params(params, parent_model)
+        params.each_with_object({}) do |(entity_name, entity_options), memo|
+          next if skip_param?(entity_options)
+
+          original_entity_name = entity_name.is_a?(Alias) ? entity_name.original : entity_name
+          final_entity_name = entity_options.fetch(:as, original_entity_name)
+
+          memo[final_entity_name] = parse_entity_options(entity_options, original_entity_name, parent_model)
+          add_documentation_to_memo(memo[final_entity_name], entity_options[:documentation])
+        end
+      end
+
+      def skip_param?(entity_options)
+        documentation_options = entity_options.fetch(:documentation, {})
+        in_option = documentation_options.fetch(:in, nil).to_s
+        hidden_option = documentation_options.fetch(:hidden, nil)
+
+        in_option == 'header' || hidden_option == true
+      end
+
+      def parse_entity_options(entity_options, entity_name, parent_model)
+        if entity_options[:nesting]
+          parse_nested(entity_name, entity_options, parent_model)
+        else
+          attribute_parser.call(entity_options)
         end
+      end
+
+      def add_documentation_to_memo(memo_entry, documentation)
+        return unless documentation
+
+        memo_entry[:readOnly] = documentation[:read_only].to_s == 'true' if documentation[:read_only]
+        memo_entry[:description] = documentation[:desc] if documentation[:desc]
+      end
 
+      def handle_discriminator(parsed, required)
         discriminator = GrapeSwagger::Entity::Helper.discriminator(model)
         if discriminator
-          respond_with_all_of(parsed, params, discriminator)
+          respond_with_all_of(parsed, required, discriminator)
         else
-          [parsed, required_params(params)]
+          [parsed, required]
         end
       end
 
-      def respond_with_all_of(parsed, params, discriminator)
+      def respond_with_all_of(parsed, required, discriminator)
         parent_name = GrapeSwagger::Entity::Helper.model_name(model.superclass, endpoint)
 
         {
@@ -83,7 +103,7 @@ def respond_with_all_of(parsed, params, discriminator)
             },
             [
               add_discriminator(parsed, discriminator),
-              required_params(params).push(discriminator.attribute)
+              required.push(discriminator.attribute)
             ]
           ]
         }
@@ -111,7 +131,7 @@ def parse_nested(entity_name, entity_options, parent_model = nil)
                  .map(&:nested_exposures)
                  .flatten
                  .each_with_object({}) do |value, memo|
-                   memo[value.attribute] = value.send(:options)
+          memo[value.attribute] = value.send(:options)
         end
 
         properties, required = parse_grape_entity_params(params, nested_entities.last)

spec/grape-swagger/entities/response_model_spec.rb

@@ -102,6 +102,8 @@ def app
     JSON.parse(last_response.body)['definitions']
   end
 
+  include_context 'this api'
+
   before :all do
     module TheseApi
       module Entities

spec/grape-swagger/entity/attribute_parser_spec.rb

@@ -218,6 +218,29 @@ def self.to_s
         it { is_expected.to include(type: :array) }
         it { is_expected.to include(items: { type: 'string' }) }
 
+        context 'when using Array[Type] syntax' do
+          # Array[Type] syntax for primitives preserves the type string as-is.
+          # For entity models, use the `using:` option with Array[Object] syntax.
+          let(:entity_options) { { documentation: { type: 'Array[String]', desc: 'Colors' } } }
+
+          it { is_expected.to include(type: :array) }
+          it { is_expected.to include(items: { type: 'Array[String]' }) }
+        end
+
+        context 'when using lowercase array[type] syntax' do
+          let(:entity_options) { { documentation: { type: 'array[integer]', desc: 'Numbers' } } }
+
+          it { is_expected.to include(type: :array) }
+          it { is_expected.to include(items: { type: 'array[integer]' }) }
+        end
+
+        context 'when is_array is explicitly false' do
+          let(:entity_options) { { documentation: { type: 'string', desc: 'Single value', is_array: false } } }
+
+          it { is_expected.to include(type: 'string') }
+          it { is_expected.not_to include(:items) }
+        end
+
         context 'when it contains example' do
           let(:entity_options) do
             { documentation: { type: 'string', desc: 'Colors', is_array: true, example: %w[green blue] } }
@@ -262,6 +285,13 @@ def self.to_s
         it { is_expected.not_to include('$ref') }
       end
 
+      context 'when using bare Array type' do
+        let(:entity_options) { { documentation: { type: 'Array', desc: 'Generic array' } } }
+
+        it { is_expected.to include(type: 'array') }
+        it { is_expected.not_to include(:items) }
+      end
+
       context 'when it is exposed as a Boolean class' do
         let(:entity_options) do
           { documentation: { type: Grape::API::Boolean, example: example_value, default: example_value } }