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

Use #public_send to disallow hitting private methods #33

Closed
wants to merge 1 commit into from

Conversation

jamesottaway
Copy link
Contributor

Fixes #28.

If the subject looks like this:

class Person < Struct.new(:name)
  private
  def inner_name
    "Inner #{name}"
  end
end

then its shouldn't be able to hit the inner_name method. The following test:

describe Person do
  subject { Person.new('John') }
  describe '#inner_name' do
    its(:inner_name} { is_expected.to eq 'Inner John' }
  end
end

should raise a NoMethodError.

@jamesottaway jamesottaway force-pushed the public_send branch 2 times, most recently from 3b6a170 to 5a89793 Compare December 11, 2014 06:05
@jamesottaway jamesottaway force-pushed the public_send branch 2 times, most recently from 3d801ae to f68e4c3 Compare March 26, 2015 11:23
@jamesottaway
Copy link
Contributor Author

I finally remembered to come back to this and fix the broken build.

The easiest solution seemed to be to not run the broken spec on MRI 1.8.7

@jamesottaway
Copy link
Contributor Author

@palfvin any idea when this could be merged?

There's also the conversation over in #28 about whether to drop support for 1.8.7 and bump the major version number.

@jamesottaway
Copy link
Contributor Author

Bump.

@magni-
Copy link

magni- commented Sep 27, 2018

Bump... maybe this could be part of a 1.3.0 release ? Had something break in production recently because we were unaware that its could call private methods 😞

@JonRowe
Copy link
Member

JonRowe commented Jan 10, 2019

This would require a 2.0.0 release as it's a breaking change, we could do this if theres enough interest however.

@JonRowe JonRowe changed the base branch from master to main August 2, 2020 01:28
@JonRowe JonRowe mentioned this pull request Nov 4, 2024
JonRowe added a commit that referenced this pull request Nov 4, 2024
@JonRowe
Copy link
Member

JonRowe commented Nov 4, 2024

Whats that they say about better late than never? Theres been a few bugfixes recently so I'm launching a very minimal 2.0.0 and well as promised, this is included. Thank you.

@jamesottaway
Copy link
Contributor Author

I love that this finally landed Jon, seriously thank you, but I'll admit that I'm a little sad I didn't get to celebrate this PR's 10th birthday. I had the date marked in my calendar, and it was only a month-and-a-bit away 😭

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should use Object#public_send instead of Object#send.
3 participants