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

ActiveSupport: Fixing issue when delegating to methods named "block", "args", or "arg" #21302

Merged
merged 1 commit into from Oct 20, 2015

Conversation

Projects
None yet
3 participants
@theunraveler
Contributor

theunraveler commented Aug 19, 2015

When using delegate with to: :block (for example), the method calls the block argument to the method, not the method on the delegating object. For example, the following will fail even if the Assignment has a block:

class Assignment
  has_one :block 
  delegate :class, to: :block 
end
Assignment.new.class

The delegate method already prefixes certain reserved Ruby keywords with self., so this patch just does the same for block, args, and arg. I've also included a test case that should fail without the patch applied.

Thanks!

@theunraveler theunraveler changed the title from Fixing issue when delegating to methods named "block", "args", or "arg" to ActiveSupport: Fixing issue when delegating to methods named "block", "args", or "arg" Aug 19, 2015

@theunraveler

This comment has been minimized.

Contributor

theunraveler commented Aug 19, 2015

Duplicate of #11173, but perhaps with clearer code and test.

@matthewd

This comment has been minimized.

Member

matthewd commented Aug 19, 2015

Can we just add them to the existing constant?

We should include _ too.

@theunraveler

This comment has been minimized.

Contributor

theunraveler commented Aug 19, 2015

I thought of that, but it seemed a little odd to me. block, args, and arg aren't exactly reserved keywords--they just happen to be significant in this context.

@matthewd

This comment has been minimized.

Member

matthewd commented Aug 19, 2015

Then feel free to rename the constant; its only purpose is to provide a blacklist for that conditional.

@theunraveler

This comment has been minimized.

Contributor

theunraveler commented Aug 19, 2015

In that case, updated.

And while I'm thinking about it, is there any reason not to just always prefix that call with self.? Why only do it for certain special method names?

sgrif added a commit that referenced this pull request Oct 20, 2015

Merge pull request #21302 from theunraveler/delegate_reserved_argumen…
…t_names

ActiveSupport: Fixing issue when delegating to methods named "block", "args", or "arg"

@sgrif sgrif merged commit 7b92798 into rails:master Oct 20, 2015

@theunraveler theunraveler deleted the theunraveler:delegate_reserved_argument_names branch Oct 20, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment