egonSchiele / contracts.ruby

Contracts for Ruby.
http://egonschiele.github.com/contracts.ruby
BSD 2-Clause "Simplified" License
1.44k stars 83 forks source link

Contracts polluting rails console #18

Open mcandre opened 11 years ago

mcandre commented 11 years ago

I'm trying to integrate a robust contract-ified gem into a new Rails app.

I added my gem as a dependency in the Rails app's Gemspec. I generated a scaffold for Employees. I can successfully create new employees using the Rails web interface. But when I try to use rails console, somehow Contracts interferes. When I remove the contract-ified gem as a dependency in the Rails app's Gemspec, the error goes away. And strangely, the error only presents in the rails console interface. Everything working okay in the Rails web interface.

Trace:

$ rails console
Loading development environment (Rails 3.2.13)
irb(main):001:0> Employee.create :name => "Andrew Pennebaker, :github_username => "mcandre"
RuntimeError: Couldn't find decorator for method Class:qualified_const_defined?.
Does this method look correct to you? If you are using contracts from rspec, rspec wraps classes in
it's own class.
Look at the specs for contracts.ruby as an example of how to write contracts in this case.
from c:/Ruby193/lib/ruby/gems/1.9.1/gems/contracts-0.2.1/lib/decorators.rb:57:in `qualified_const_defined?'
from c:/Ruby193/lib/ruby/gems/1.9.1/gems/activesupport-.2.13/lib/active_support/dependencies.rb:378:in `qualified_const_defined?'
from c:/Ruby193/lib/ruby/gems/1.9.1/gems/activesupport-.2.13/lib/active_support/dependencies.rb:490:in `load_missing_constant'
from c:/Ruby193/lib/ruby/gems/1.9.1/gems/activesupport-.2.13/lib/active_support/dependencies.rb:192:in `block in const_missing'
from c:/Ruby193/lib/ruby/gems/1.9.1/gems/activesupport-.2.13/lib/active_support/dependencies.rb:190:in `each'
from c:/Ruby193/lib/ruby/gems/1.9.1/gems/activesupport-.2.13/lib/active_support/dependencies.rb:190:in `const_missing'
from (irb):1
from c:/Ruby193/lib/ruby/gems/1.9.1/gems/railties-.2.13/lib/rails/commands/console.rb:47:in
`start'
from c:/Ruby193/lib/ruby/gems/1.9.1/gems/railties-.2.13/lib/rails/commands/console.rb:8:in
`start'
from c:/Ruby193/lib/ruby/gems/1.9.1/gems/railties-3.2.13/lib/rails/commands.rb:41:in `<top (required)>'
from script/rails:6:in `require'
from script/rails:6:in `<main>'

System:

$ specs rails ruby os
Specs:

specs 0.2
https://github.com/mcandre/specs#readme

rails --version
Rails 3.2.13

gem --version
2.0.3

ruby --version
ruby 1.9.3p374 (2013-01-15) [i386-mingw32]

systeminfo | findstr /B /C:"OS Name" /C:"OS Version"
OS Name:                   Microsoft Windows XP Professional
OS Version:                5.1.2600 Service Pack 3 Build 2600

I suspect that some of the toplevel include Contract magic is colliding with Rails.

mcandre commented 11 years ago

Update: Moving include Contracts out of toplevel mitigates this issue. But then you have to prefix Bool, Or, ArrayOf, etc., with TheModuleYouIncludeContractsIn::.

... is there a safer way to do Contracts magic that doesn't require pushing the include Contracts declaration down into submodules in user code?

egonSchiele commented 11 years ago

Hmm, this is really weird. I'll take a look.

mcfilib commented 11 years ago

@egonSchiele Did anything come of further investigation?

egonSchiele commented 11 years ago

Nope, I haven't had a chance to take a look. Are you having the same issue?

waterlink commented 9 years ago

Usually when I use contracts.ruby I do include Contracts only at the target classes where I actually need it (instead of some namespace or even global namespace). This way you get your contracts code without need to specify Contracts:: or YourOwnNamespace:: everywhere and you are not polluting global scope (actually Object class).

To my opinion, if it wasn't a breaking change I would actually tend to restrict users from including it in global namespace. Or at least issue a warning, that it pollutes Object class and could cause problems in different libraries/frameworks, with some kind of link to recommended usage.

WDYT @egonSchiele @mcandre @filib ?

mcfilib commented 9 years ago

I'm afraid the nuts and bolts of this are lost in the sands of time for me.

My current usage is pretty much as you described, i.e. only including Contracts inside target classes. To me this seems like the most sensible usage but I'm happy to be convinced otherwise.

waterlink commented 9 years ago

@filib Me convincing including Contracts globally - highly unlikely. To me it seems the most reasonable way to use it (ie including it only in target classes/modules).

I thought about actually adding a warning/deprecation for include Contract in global scope (Object). And raise error in 1.0 release.

WDYT?

mcfilib commented 9 years ago

@waterlink Would this mean that doing include Contracts in an irb session would show the deprecation (and eventually raise an error in 1.0)?

waterlink commented 9 years ago

@filib Yes, unless we do something about it. Like having an integration with irb. I think it is easy to detect if you are running under any kind of irb-like thing.

And again, for me personally I just create class right away and include contracts inside of it. But probably from irb it is way simpler to just define a bunch of methods.

mcfilib commented 9 years ago

@waterlink Only speaking as a user of the library here and not a maintainer.

I think it'd be a shame to lose the ability to easily open an irb session and include Contracts. If, as you say, that behaviour could be preserved and we gradually nudge users away from including at the top-level then I think that'd be pretty great.

waterlink commented 9 years ago

@filib Makes sense to me too. I think I will research if it is possible to do it in acceptable way.

egonSchiele commented 9 years ago

A bit late, but I like the idea of having a deprecation warning for top-level contracts. It's pretty easy to check if the user is using irb: http://stackoverflow.com/questions/2226979/how-can-a-ruby-script-detect-that-it-is-running-in-irb