Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

False positive / conflicting cops: Rails/Delegate and Rails/LexicallyScopedActionFilter? #1447

Closed
tagliala opened this issue Feb 17, 2025 · 0 comments · Fixed by #1450
Closed

Comments

@tagliala
Copy link
Contributor

Hello,

I don't know if this is actually an issue, but I will report it the same

#912 introduced the ability to delegate to instance variables

In controllers, however, this may conflict with Rails/LexicallyScopedActionFilter, and the developer should take action.

For the moment, I'm undecided on the workaround. I can either:

  • disable the delegate on the offending line; or
  • explicitly add head :no_content to the controller (but it should be redundant, since it's an API only responding to json and the 204 is the default

Expected behavior

Can't tell

Actual behavior

Fixing the Rails/Delegate offense introduces a Rails/LexicallyScopedActionFilter that in order to be fixed should reintroduce the Rails/Delegate offense

Steps to reproduce the problem

Gemfile

# frozen_string_literal: true

source 'https://rubygems.org'

gem 'rubocop', '1.72.1'
gem 'rubocop-rails', '2.30.1'

rubocop.yml

plugins:
  - rubocop-rails

AllCops:
  NewCops: disable

Style/Documentation:
  Enabled: false

app/controllers/posts_controller.rb

# frozen_string_literal: true

class PostsController < ApplicationController
  before_action :set_post, only: %i[destroy]

  def destroy
    @post.destroy
  end

  private

  def set_post
    @post = Post.find(params[:id])
  end
end

First run:

$ rubocop app/controllers/posts_controller.rb 
Inspecting 1 file
C

Offenses:

app/controllers/posts_controller.rb:6:3: C: [Correctable] Rails/Delegate: Use delegate to define delegations.
  def destroy
  ^^^

1 file inspected, 1 offense detected, 1 offense autocorrectable

Second run (with autocorrect)

$ rubocop app/controllers/posts_controller.rb -a
Inspecting 1 file
C

Offenses:

app/controllers/posts_controller.rb:4:3: C: Rails/LexicallyScopedActionFilter: destroy is not explicitly defined on the class.
  before_action :set_post, only: %i[destroy]
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
app/controllers/posts_controller.rb:6:3: C: [Corrected] Rails/Delegate: Use delegate to define delegations.
  def destroy
  ^^^

1 file inspected, 2 offenses detected, 1 offense corrected

Third run

$ rubocop app/controllers/posts_controller.rb
Inspecting 1 file
C

Offenses:

app/controllers/posts_controller.rb:4:3: C: Rails/LexicallyScopedActionFilter: destroy is not explicitly defined on the class.
  before_action :set_post, only: %i[destroy]
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected

RuboCop version

$ bundle exec rubocop -V
1.72.1 (using Parser 3.3.7.1, rubocop-ast 1.38.0, analyzing as Ruby 2.7, running on ruby 3.4.1) [arm64-darwin24]
  - rubocop-rails 2.30.1
vlad-pisanov added a commit to vlad-pisanov/rubocop-rails that referenced this issue Feb 18, 2025
…nFilter` when action methods are delegated
@koic koic closed this as completed in 7b92670 Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant