From 85f5b78f5a3ad469239c8fe6239a6bfb648101d6 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Sat, 12 Apr 2025 14:56:47 +0200 Subject: [PATCH 1/9] Add linting feature through Grape.config and `lint_api!` method Add spec All specs are now linted --- lib/grape.rb | 1 + lib/grape/dsl/routing.rb | 4 ++++ lib/grape/endpoint.rb | 5 +++++ spec/grape/api_spec.rb | 21 +++++++++++++++++++++ spec/spec_helper.rb | 1 + 5 files changed, 32 insertions(+) diff --git a/lib/grape.rb b/lib/grape.rb index a1e6f511c..6415c16f5 100644 --- a/lib/grape.rb +++ b/lib/grape.rb @@ -75,6 +75,7 @@ def self.deprecator configure do |config| config.param_builder = :hash_with_indifferent_access + config.lint_api = false config.compile_methods! end end diff --git a/lib/grape/dsl/routing.rb b/lib/grape/dsl/routing.rb index 8c32db364..d73977119 100644 --- a/lib/grape/dsl/routing.rb +++ b/lib/grape/dsl/routing.rb @@ -81,6 +81,10 @@ def do_not_route_options! namespace_inheritable(:do_not_route_options, true) end + def lint_api! + namespace_inheritable(:lint_api, true) + end + def do_not_document! namespace_inheritable(:do_not_document, true) end diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index 2e5136fda..0b7725055 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -358,6 +358,7 @@ def build_stack(helpers) format = namespace_inheritable(:format) stack.use Rack::Head + stack.use Rack::Lint if lint_api? stack.use Class.new(Grape::Middleware::Error), helpers: helpers, format: format, @@ -408,5 +409,9 @@ def build_response_cookies Rack::Utils.set_cookie_header! header, name, cookie_value end end + + def lint_api? + namespace_inheritable(:lint_api) || Grape.config.lint_api + end end end diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index f16b31ee2..b45fcee5a 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -4738,4 +4738,25 @@ def uniqe_id_route expect(last_response.body).to eq('unknown params_builder: unknown') end end + + describe '.lint_api!' do + let(:app) do + Class.new(described_class) do + lint_api! + get '/' do + status 42 + end + end + end + + around do |example| + Grape.config.lint_api = false + example.run + Grape.config.lint_api = true + end + + it 'raises a Rack::Lint error' do + expect { get '/' }.to raise_error(Rack::Lint::LintError, 'Status must be an Integer >=100') + end + end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 06cb282a0..2df1bdd54 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -13,6 +13,7 @@ end end +Grape.config.lint_api = true # lint all apis by default Grape::Util::Registry.include(Deregister) # issue with ruby 2.7 with ^. We need to extend it again Grape::Validations.extend(Grape::Util::Registry) if Gem::Version.new(RUBY_VERSION).release < Gem::Version.new('3.0') From e0d1d10cc09194fac8a81385c7ca62c2ba1c1834 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Sat, 12 Apr 2025 15:10:22 +0200 Subject: [PATCH 2/9] Remove error message for Lint::Error --- spec/grape/api_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index b45fcee5a..f85c94f22 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -4756,7 +4756,8 @@ def uniqe_id_route end it 'raises a Rack::Lint error' do - expect { get '/' }.to raise_error(Rack::Lint::LintError, 'Status must be an Integer >=100') + # Status must be an Integer >= 100 + expect { get '/' }.to raise_error(Rack::Lint::LintError) end end end From e4d5c35c8eef8ec09c13d2fb8246b05df7b018a1 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Sat, 12 Apr 2025 18:27:15 +0200 Subject: [PATCH 3/9] Add ruby 3.1 to 3.2 to spec integrations Add `yaml-dev` in Dockerfile --- .github/workflows/test.yml | 69 +++++++++++++++++++++++++ docker/Dockerfile | 2 +- spec/integration/rails/mounting_spec.rb | 7 +-- 3 files changed, 74 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 14a663818..87e057d65 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -36,6 +36,51 @@ jobs: - ruby: '2.7' gemfile: gemfiles/rack_3_0.gemfile specs: 'spec/integration/rack_3_0' + - ruby: '3.1' + gemfile: gemfiles/grape_entity.gemfile + specs: 'spec/integration/grape_entity' + - ruby: '3.1' + gemfile: gemfiles/hashie.gemfile + specs: 'spec/integration/hashie' + - ruby: '3.1' + gemfile: gemfiles/dry_validation.gemfile + specs: 'spec/integration/dry_validation' + - ruby: '3.1' + gemfile: gemfiles/rails_6_1.gemfile + specs: 'spec/integration/rails' + - ruby: '3.1' + gemfile: gemfiles/rails_7_0.gemfile + specs: 'spec/integration/rails' + - ruby: '3.1' + gemfile: gemfiles/rails_7_1.gemfile + specs: 'spec/integration/rails' + - ruby: '3.1' + gemfile: gemfiles/rails_7_2.gemfile + specs: 'spec/integration/rails' + - ruby: '3.2' + gemfile: gemfiles/grape_entity.gemfile + specs: 'spec/integration/grape_entity' + - ruby: '3.2' + gemfile: gemfiles/hashie.gemfile + specs: 'spec/integration/hashie' + - ruby: '3.2' + gemfile: gemfiles/dry_validation.gemfile + specs: 'spec/integration/dry_validation' + - ruby: '3.2' + gemfile: gemfiles/rails_6_1.gemfile + specs: 'spec/integration/rails' + - ruby: '3.2' + gemfile: gemfiles/rails_7_0.gemfile + specs: 'spec/integration/rails' + - ruby: '3.2' + gemfile: gemfiles/rails_7_1.gemfile + specs: 'spec/integration/rails' + - ruby: '3.2' + gemfile: gemfiles/rails_7_2.gemfile + specs: 'spec/integration/rails' + - ruby: '3.2' + gemfile: gemfiles/rails_8_0.gemfile + specs: 'spec/integration/rails' - ruby: '3.3' gemfile: gemfiles/grape_entity.gemfile specs: 'spec/integration/grape_entity' @@ -60,6 +105,30 @@ jobs: - ruby: '3.3' gemfile: gemfiles/rails_8_0.gemfile specs: 'spec/integration/rails' + - ruby: '3.4' + gemfile: gemfiles/grape_entity.gemfile + specs: 'spec/integration/grape_entity' + - ruby: '3.4' + gemfile: gemfiles/hashie.gemfile + specs: 'spec/integration/hashie' + - ruby: '3.4' + gemfile: gemfiles/dry_validation.gemfile + specs: 'spec/integration/dry_validation' + - ruby: '3.4' + gemfile: gemfiles/rails_6_1.gemfile + specs: 'spec/integration/rails' + - ruby: '3.4' + gemfile: gemfiles/rails_7_0.gemfile + specs: 'spec/integration/rails' + - ruby: '3.4' + gemfile: gemfiles/rails_7_1.gemfile + specs: 'spec/integration/rails' + - ruby: '3.4' + gemfile: gemfiles/rails_7_2.gemfile + specs: 'spec/integration/rails' + - ruby: '3.4' + gemfile: gemfiles/rails_8_0.gemfile + specs: 'spec/integration/rails' exclude: - ruby: '2.7' gemfile: gemfiles/rails_7_2.gemfile diff --git a/docker/Dockerfile b/docker/Dockerfile index 2bc484ecf..5cca3aa5b 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -7,7 +7,7 @@ ENV RUBYOPT --enable-frozen-string-literal --yjit ENV LD_PRELOAD libjemalloc.so.2 ENV MALLOC_CONF dirty_decay_ms:1000,narenas:2,background_thread:true -RUN apk add --update --no-cache make gcc git libc-dev gcompat jemalloc && \ +RUN apk add --update --no-cache make gcc git libc-dev yaml-dev gcompat jemalloc && \ gem update --system && gem install bundler WORKDIR $LIB_PATH diff --git a/spec/integration/rails/mounting_spec.rb b/spec/integration/rails/mounting_spec.rb index 3d8288b93..eef8e9ad2 100644 --- a/spec/integration/rails/mounting_spec.rb +++ b/spec/integration/rails/mounting_spec.rb @@ -17,7 +17,7 @@ ActiveSupport::Dependencies.autoload_paths = [] ActiveSupport::Dependencies.autoload_once_paths = [] - Class.new(Rails::Application) do + app = Class.new(Rails::Application) do config.eager_load = false config.load_defaults "#{Rails::VERSION::MAJOR}.#{Rails::VERSION::MINOR}" config.api_only = true @@ -28,15 +28,16 @@ mount GrapeApi => '/' get 'up', to: lambda { |_env| - ['200', {}, ['hello world']] + [200, {}, ['hello world']] } end end + app.initialize! + Rack::Lint.new(app) end before do stub_const('GrapeApi', api) - app.initialize! end it 'cascades' do From abfdaea282cb9bf7e86555d36d05a1970c6001ca Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Sun, 13 Apr 2025 09:31:19 +0200 Subject: [PATCH 4/9] Rename `lint_api!` to `lint!` --- lib/grape.rb | 2 +- lib/grape/dsl/routing.rb | 4 ++-- lib/grape/endpoint.rb | 6 +++--- spec/grape/api_spec.rb | 8 ++++---- spec/spec_helper.rb | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/grape.rb b/lib/grape.rb index 6415c16f5..4dfffc257 100644 --- a/lib/grape.rb +++ b/lib/grape.rb @@ -75,7 +75,7 @@ def self.deprecator configure do |config| config.param_builder = :hash_with_indifferent_access - config.lint_api = false + config.lint = false config.compile_methods! end end diff --git a/lib/grape/dsl/routing.rb b/lib/grape/dsl/routing.rb index d73977119..4db89e5d3 100644 --- a/lib/grape/dsl/routing.rb +++ b/lib/grape/dsl/routing.rb @@ -81,8 +81,8 @@ def do_not_route_options! namespace_inheritable(:do_not_route_options, true) end - def lint_api! - namespace_inheritable(:lint_api, true) + def lint! + namespace_inheritable(:lint, true) end def do_not_document! diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index 0b7725055..e3bd6b61f 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -358,7 +358,7 @@ def build_stack(helpers) format = namespace_inheritable(:format) stack.use Rack::Head - stack.use Rack::Lint if lint_api? + stack.use Rack::Lint if lint? stack.use Class.new(Grape::Middleware::Error), helpers: helpers, format: format, @@ -410,8 +410,8 @@ def build_response_cookies end end - def lint_api? - namespace_inheritable(:lint_api) || Grape.config.lint_api + def lint? + namespace_inheritable(:lint) || Grape.config.lint end end end diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index f85c94f22..b530a3deb 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -4739,10 +4739,10 @@ def uniqe_id_route end end - describe '.lint_api!' do + describe '.lint!' do let(:app) do Class.new(described_class) do - lint_api! + lint! get '/' do status 42 end @@ -4750,9 +4750,9 @@ def uniqe_id_route end around do |example| - Grape.config.lint_api = false + Grape.config.lint = false example.run - Grape.config.lint_api = true + Grape.config.lint = true end it 'raises a Rack::Lint error' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 2df1bdd54..4a8e2f1b0 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -13,7 +13,7 @@ end end -Grape.config.lint_api = true # lint all apis by default +Grape.config.lint = true # lint all apis by default Grape::Util::Registry.include(Deregister) # issue with ruby 2.7 with ^. We need to extend it again Grape::Validations.extend(Grape::Util::Registry) if Gem::Version.new(RUBY_VERSION).release < Gem::Version.new('3.0') From 2caf6da01605c154fe3d8272055a3c86f8e63334 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Sun, 13 Apr 2025 09:33:43 +0200 Subject: [PATCH 5/9] Revert test.yml --- .github/workflows/test.yml | 69 -------------------------------------- 1 file changed, 69 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 87e057d65..14a663818 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -36,51 +36,6 @@ jobs: - ruby: '2.7' gemfile: gemfiles/rack_3_0.gemfile specs: 'spec/integration/rack_3_0' - - ruby: '3.1' - gemfile: gemfiles/grape_entity.gemfile - specs: 'spec/integration/grape_entity' - - ruby: '3.1' - gemfile: gemfiles/hashie.gemfile - specs: 'spec/integration/hashie' - - ruby: '3.1' - gemfile: gemfiles/dry_validation.gemfile - specs: 'spec/integration/dry_validation' - - ruby: '3.1' - gemfile: gemfiles/rails_6_1.gemfile - specs: 'spec/integration/rails' - - ruby: '3.1' - gemfile: gemfiles/rails_7_0.gemfile - specs: 'spec/integration/rails' - - ruby: '3.1' - gemfile: gemfiles/rails_7_1.gemfile - specs: 'spec/integration/rails' - - ruby: '3.1' - gemfile: gemfiles/rails_7_2.gemfile - specs: 'spec/integration/rails' - - ruby: '3.2' - gemfile: gemfiles/grape_entity.gemfile - specs: 'spec/integration/grape_entity' - - ruby: '3.2' - gemfile: gemfiles/hashie.gemfile - specs: 'spec/integration/hashie' - - ruby: '3.2' - gemfile: gemfiles/dry_validation.gemfile - specs: 'spec/integration/dry_validation' - - ruby: '3.2' - gemfile: gemfiles/rails_6_1.gemfile - specs: 'spec/integration/rails' - - ruby: '3.2' - gemfile: gemfiles/rails_7_0.gemfile - specs: 'spec/integration/rails' - - ruby: '3.2' - gemfile: gemfiles/rails_7_1.gemfile - specs: 'spec/integration/rails' - - ruby: '3.2' - gemfile: gemfiles/rails_7_2.gemfile - specs: 'spec/integration/rails' - - ruby: '3.2' - gemfile: gemfiles/rails_8_0.gemfile - specs: 'spec/integration/rails' - ruby: '3.3' gemfile: gemfiles/grape_entity.gemfile specs: 'spec/integration/grape_entity' @@ -105,30 +60,6 @@ jobs: - ruby: '3.3' gemfile: gemfiles/rails_8_0.gemfile specs: 'spec/integration/rails' - - ruby: '3.4' - gemfile: gemfiles/grape_entity.gemfile - specs: 'spec/integration/grape_entity' - - ruby: '3.4' - gemfile: gemfiles/hashie.gemfile - specs: 'spec/integration/hashie' - - ruby: '3.4' - gemfile: gemfiles/dry_validation.gemfile - specs: 'spec/integration/dry_validation' - - ruby: '3.4' - gemfile: gemfiles/rails_6_1.gemfile - specs: 'spec/integration/rails' - - ruby: '3.4' - gemfile: gemfiles/rails_7_0.gemfile - specs: 'spec/integration/rails' - - ruby: '3.4' - gemfile: gemfiles/rails_7_1.gemfile - specs: 'spec/integration/rails' - - ruby: '3.4' - gemfile: gemfiles/rails_7_2.gemfile - specs: 'spec/integration/rails' - - ruby: '3.4' - gemfile: gemfiles/rails_8_0.gemfile - specs: 'spec/integration/rails' exclude: - ruby: '2.7' gemfile: gemfiles/rails_7_2.gemfile From ed493d09bd3080f697a6076e147290416d2d16bb Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Thu, 1 May 2025 19:25:43 +0200 Subject: [PATCH 6/9] Add README.md and CHANGELOG.md Fix typo in UPGRADING.md --- CHANGELOG.md | 1 + README.md | 21 +++++++++++++++++++++ UPGRADING.md | 2 +- 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e7ef011ac..0740742f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ * [#2549](https://github.com/ruby-grape/grape/pull/2549): Delegate cookies management to `Grape::Request` - [@ericproulx](https://github.com/ericproulx). * [#2554](https://github.com/ruby-grape/grape/pull/2554): Remove `Grape::Http::Headers` and `Grape::Util::Lazy::Object` - [@ericproulx](https://github.com/ericproulx). * [#2556](https://github.com/ruby-grape/grape/pull/2556): Remove unused `Grape::Request::DEFAULT_PARAMS_BUILDER` constant - [@eriklovmo](https://github.com/eriklovmo). +* [#2557](https://github.com/ruby-grape/grape/pull/2557): Add lint! - [@ericproulx](https://github.com/ericproulx). * Your contribution here. #### Fixes diff --git a/README.md b/README.md index 7fe411835..28a7c1526 100644 --- a/README.md +++ b/README.md @@ -28,6 +28,8 @@ - [Header](#header) - [Accept-Version Header](#accept-version-header) - [Param](#param) +- [Linting](#linting) + - [Bug in Rack::ETag under Rack 3.X](#bug-in-racketag-under-rack-3x) - [Describing Methods](#describing-methods) - [Configuration](#configuration) - [Parameters](#parameters) @@ -650,6 +652,25 @@ version 'v1', using: :param, parameter: 'v' curl http://localhost:9292/statuses/public_timeline?v=v1 +## Linting + +You can check if your API is in conformance with [Rack's specification](https://github.com/rack/rack/blob/main/SPEC.rdoc) by calling `lint!` at the API level or through [configuration](#configuration): +```ruby +# API Level +class Api < Grape::API + lint! +end +# OR through configuration +Grape.configure do |config| + config.lint = true +end +# OR +Grape.config.lint = true +``` + +### Bug in Rack::ETag under Rack 3.X +If you're using Rack 3.X and the `Rack::Etag` middleware (used by [Rails](https://guides.rubyonrails.org/rails_on_rack.html#inspecting-middleware-stack)), a [bug](https://github.com/rack/rack/pull/2324) related to linting has been fixed in [3.1.13](https://github.com/rack/rack/blob/v3.1.13/CHANGELOG.md#3113---2025-04-13) and [3.0.15](https://github.com/rack/rack/blob/v3.1.13/CHANGELOG.md#3015---2025-04-13) respectively. + ## Describing Methods You can add a description to API methods and namespaces. The description would be used by [grape-swagger][grape-swagger] to generate swagger compliant documentation. diff --git a/UPGRADING.md b/UPGRADING.md index 2ace6622d..8c1744959 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -130,7 +130,7 @@ When using together with `Grape::Extensions::Hash::ParamBuilder`, `route_param` This was a regression introduced by [#2326](https://github.com/ruby-grape/grape/pull/2326) in Grape v1.8.0. ```ruby -grape.configure do |config| +Grape.configure do |config| config.param_builder = Grape::Extensions::Hash::ParamBuilder end From 7bde62c06d463b1df94f2042833c36567e905825 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Sat, 3 May 2025 17:10:05 +0200 Subject: [PATCH 7/9] Fix CHANGELOG --- README.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 28a7c1526..863a8d3f3 100644 --- a/README.md +++ b/README.md @@ -654,17 +654,19 @@ version 'v1', using: :param, parameter: 'v' ## Linting -You can check if your API is in conformance with [Rack's specification](https://github.com/rack/rack/blob/main/SPEC.rdoc) by calling `lint!` at the API level or through [configuration](#configuration): +You can check whether your API is in conformance with the [Rack's specification](https://github.com/rack/rack/blob/main/SPEC.rdoc) by calling `lint!` at the API level or through [configuration](#configuration). + ```ruby -# API Level class Api < Grape::API lint! end -# OR through configuration +``` +```ruby Grape.configure do |config| config.lint = true end -# OR +``` +```ruby Grape.config.lint = true ``` From 2439121a8176271228e104a5529e693a75b0f21b Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Sat, 3 May 2025 17:11:58 +0200 Subject: [PATCH 8/9] Use `lint!` in rails spec --- spec/integration/rails/mounting_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/integration/rails/mounting_spec.rb b/spec/integration/rails/mounting_spec.rb index eef8e9ad2..39d31c761 100644 --- a/spec/integration/rails/mounting_spec.rb +++ b/spec/integration/rails/mounting_spec.rb @@ -4,6 +4,7 @@ context 'rails mounted' do let(:api) do Class.new(Grape::API) do + lint! get('/test_grape') { 'rails mounted' } end end @@ -17,7 +18,7 @@ ActiveSupport::Dependencies.autoload_paths = [] ActiveSupport::Dependencies.autoload_once_paths = [] - app = Class.new(Rails::Application) do + Class.new(Rails::Application) do config.eager_load = false config.load_defaults "#{Rails::VERSION::MAJOR}.#{Rails::VERSION::MINOR}" config.api_only = true @@ -32,12 +33,11 @@ } end end - app.initialize! - Rack::Lint.new(app) end before do stub_const('GrapeApi', api) + app.initialize! end it 'cascades' do From 3347801dfe06327ad935aa8fcf81d5cc747227d1 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Sun, 4 May 2025 15:34:32 +0200 Subject: [PATCH 9/9] Fix api_spec lint with ensure --- spec/grape/api_spec.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index b530a3deb..b2fa81e75 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -4750,9 +4750,11 @@ def uniqe_id_route end around do |example| + @lint = Grape.config.lint Grape.config.lint = false example.run - Grape.config.lint = true + ensure + Grape.config.lint = @lint end it 'raises a Rack::Lint error' do