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

Matcher DSL deprecations for 2.99 #337

Merged
merged 3 commits into from Oct 15, 2013

Conversation

myronmarston
Copy link
Member

I can't tell if I'm proud of this code or embarrassed by it. It's simultaneously just about the worse hack I've ever used, but also the most meta metaprogramming I've ever done.

I wouldn't want code like this in master, but it serves a useful purpose to provide a deprecation warning in 2.99 and won't be a maintenance burden given that we won't continue 2.x development.

Deprecate calling helper methods from a custom matcher with the wrong scope.

  • def self.foo / extend Helper can be used to add macro methods
    (e.g. methods that call the custom matcher DSL methods), but should
    not be used to define helper methods called from within the DSL
    blocks.
  • def foo / include Helper is the opposite: it's for helper methods
    callable from within a DSL block, but not for defining macros.

In 2.x, there wasn't a clear separation here because self in define
was the same as self wtihin the DSL blocks -- the matcher instance.
In 3.0, the self in define will be a class and self in the DSL
blocks will be an instance of that class -- so thi separation is
necessary for helper methods to work properly on 3.0.

This code is VERY hacky but it's the only way I could find to make it
work. We have to evaluate the block using class_eval in order
to differentiate methods defined using def foo vs def self.foo;
instance_eval treats them the same.

The string eval stuff is also quite hacky, but is necessary because
we still semi-support 1.8.6 in RSpec 2.99, and 1.8.6 can't parse
blocks that accept blocks using define_method :(.

… scope.

- `def self.foo` / `extend Helper` can be used to add macro methods
  (e.g. methods that call the custom matcher DSL methods), but should
  not be used to define helper methods called from within the DSL
  blocks.
- `def foo` / `include Helper` is the opposite: it's for helper methods
  callable from within a DSL block, but not for defining macros.

In 2.x, there wasn't a clear separation here because `self` in `define`
was the same as `self` wtihin the DSL blocks -- the matcher instance.
In 3.0, the `self` in `define` will be a class and `self` in the DSL
blocks will be an instance of that class -- so thi separation is
necessary for helper methods to work properly on 3.0.

This code is VERY hacky but it's the only way I could find to make it
work.  We have to evaluate the block using `class_eval` in order
to differentiate methods defined using `def foo` vs `def self.foo`;
`instance_eval` treats them the same.

The string eval stuff is also quite hacky, but is necessary because
we still semi-support 1.8.6 in RSpec 2.99, and 1.8.6 can't parse
blocks that accept blocks using `define_method` :(.
1.8.7 treats splat args different, apparently.
@JonRowe
Copy link
Member

JonRowe commented Oct 14, 2013

After a first pass this looks good to me, but I want to do another pass tomorrow to make sure I'm not missing anything :)


ignore_macro_methods

capture_added_methods(singletons_singleton_class, singleton_methods)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could just be me, but singletons_singleton_class reads funny... not sure of a better name though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it's the singleton class of the singleton class...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe singleton_class_of_singleton would have read better...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really don't care either way. Feel free to open a PR changing that if you do.

This code is temporary so other things seem more important than the naming of that method.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More just musing really, I also don't care enough to change it since this is already merged ;)

@JonRowe
Copy link
Member

JonRowe commented Oct 15, 2013

I'm pretty happy with this, given it's going into only 2.99 I'm not sure what else you could do. RTM

myronmarston added a commit that referenced this pull request Oct 15, 2013
@myronmarston myronmarston merged commit 043359d into 2-99-maintenance Oct 15, 2013
@myronmarston myronmarston deleted the matcher-dsl-deprecations-2-99 branch October 15, 2013 14:48
yujinakayama pushed a commit to yujinakayama/rspec-monorepo that referenced this pull request Oct 6, 2021
…spec/matcher-dsl-deprecations-2-99

Matcher DSL deprecations for 2.99

---
This commit was imported from rspec/rspec-expectations@043359d.
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.

None yet

2 participants