Do not require importing contract class names into client classes

[robnormal]

On a large project, there may be a large number of contracts you will want to name, and these names will then be included in every class that includes the Contracts module. The potential for naming collisions could grow very quickly. (I am already having this problem - I have a class called "Maybe".)

One possible fix would be an alternative include, in which the Contract method takes a block instead of a hash.

class Alpha
  include ContractsSafe
  Contract { ArrayOf[Num] => Maybe[Or[This, That]] }
  # ...
end

The block could then be instance_eval-ed elsewhere, isolating the contract names.

[nixpulvis]

Related #101 and #105.

[alex-fedorov]

This code is not a block now, but normal hash, probably you want something:

Contract { [ ArrayOf[Num] => Maybe[Or[This, That]] ] }

[robnormal]

Actually, it's a syntax error! The equivalent would be

Contract { { ArrayOf[Num] => Maybe[Or[This, That]] } }

which I have to admit is pretty ugly. Another possibility entirely - which would solve some other issues, including the load-order problem - would be to use a string to specify the contract. This could be eval-ed, or, better yet, parsed. I realize this is a pretty different direction than the project is currently going in, though.

[waterlink]

Hm, not really:

2.2.1 :001 > def Contract(*, &blk)
2.2.1 :002?>   end
 => :Contract 
2.2.1 :003 > Contract { [ String => String ] }
 => nil 

[waterlink]

You are wrong here, it is still possible to shape this project if it is worth it. We had long conversation on gitter some time ago about this and arrived at conclusion that we want to have block/eval/whatever there, because it allows to have more advanced type system.

[nixpulvis]

The trick is going to be moving forward with this while maintaining functionality.

[waterlink]

Yeah, of course =) Not really a big of a problem. And remember, we can always break an API, especially if we find some better API than current one (I doubt so, but everything is possible), if we provide nice deprecation-removal curve.

EDIT: and of course, don't forget, we are at 0.* version still, so that means that public API is still not frozen

[waterlink]

Supporting current API and block-based one, not really a big deal, since it is totally different calls and may even result in instantiating totally different decorator classes for instance.

[nixpulvis]

True, but I'd expect (might not be a huge deal to ensure this) Contract A ... => B === Contract { A ... => B } ignoring the syntax issues.

[waterlink]

How about Contract { A >= B } ? :) This is a valid ruby code, and I see how to implement it. Even so it looks strange

[waterlink]

Heh. Ended up doing some black magic:

Contract { A } >-> { B }

EDIT: btw, valid ruby: x >-> ... = x.>(-> ...)

[waterlink]

Allows to be more haskelly:

Contract { A } >-> { B } >-> { C } >-> ...

[nixpulvis]

Before I spend too much time thinking about what you're doing here. I'd be very cautious of the notion of currying here. Since Ruby is not implicitly curried.

[waterlink]

If you haven't yet, look at this stuff: https://github.com/txus/kleisli

[nixpulvis]

Very cool library, but I would avoid doing this here. Ruby is not Haskell and contracts are perfectly well defined in both contexts. It is true that Haskell lends itself much better to these kinds of ideas, but we should not depart to far from Ruby syntax.

This is such a perversion of Ruby syntax that Matz would probably condemn this

I agree.

[waterlink]

I know, I am not going to, it is just that the one who wants to find a way, will always find it.

Still, what about this one?

Contract { [A, B => C] }

[nixpulvis]

That's still probably one of the best options. Has anyone considered parsing our own contract language before? Is this something we are 100% opposed to?

[waterlink]

No, not 100% at least. But that would mean writing proper lexer and parser to get what we want. It would mean parsing free-form ruby + stuff we want to add on top. The only way is to cut features to end up with something manageable.

For example, this probably makes sense to be cut: Contract "lambda { |x| x.age > 12 }" - because it is something that can have free form ruby inside.

[waterlink]

Or we might just do eval-ing to ruby: Contract Contracts::LateBinding[MyContracts, "A, B => C"] will evaluate the string once, but in different context.

[waterlink]

This might create difficulties for tracing errors, when you have typo in that string..

[waterlink]

Other thing, that is dangling in my mind is to allow very verbose syntax like that:

Contract do
  inside(MyContracts) do
    argument A
    argument B
    keyword_argument D, default: D.default_value
    result C
  end
end

[nixpulvis]

The beauty of parsing our own language would be the ability to avoid all the namespacing and syntax restrictions.

To @robnormal's point we would be smart to evaluate contracts within a namespace with the contract classes defined inside, to avoid global pollution.

In terms of errors yes a potential point for errors, but a smart parser could handle this. I've used kschiess/parslet a little bit before, with great results.

Regardless of parsing or not, I think the main point of this issue should not be an opt-in thing. It should be default behavior.

[waterlink]

By default, you mean this: include Contracts will add only Contract method and nothing else. Everything else is up to user. ?

[nixpulvis]

Well, and that in addition to only adding the Contract method, all contracts are evaluated in their own namespace. This would be exceptionally helpful for defining "types" (classes with a valid? method) for other classes with the same name. I've personally wanted this a number of times.

[waterlink]

Can you provide small example? I can't understand what do you mean by "all contracts are evaluated in their own namespace".

[waterlink]

Do I understand it right, something like that:

Contract(App::Engine::Contracts, Contracts::Builtin) { [A, B => Not[C]] }

EDIT: this might be noisy to my taste, don't know.

[nixpulvis]

class A
  # Bunch of application logic.
end

define_contracts do
  class A
    def valid?
      # ...
    end
  end
end

class B
  Contract "A => Num"  # using a string to show that this isn't necessarily correct syntax.
  def foo(a)
    # ...
  end
end

This way there are two As, one for the actual class with application logic, and one for the type of A in the case where I want to do more than the default logic for a class.

define_contracts could especially be a way to add constants to another namespace. This way the global namespace is not cluttered with "types".

UPDATE: Contract form changed.

[waterlink]

Don't really see, how it will work without a block or a string/eval. The closest will be Contract { [A => Num] } again..

[nixpulvis]

This is nice too because it would still work if users defined constants on the global namespace, however it provides a way to avoid that.

Oh and, I should have used some other notation for the contract, yes. We'll need to figure out the block contract form for this. Updated above.

[waterlink]

Now I get with that example. So all contract definitions will be in one namespace isolated from user codebase. Not bad idea.

[alex-fedorov]

@egonSchiele @sfcgeorge @robnormal What do you think about the last idea from @nixpulvis?

[sfcgeorge]

The idea is kinda nice, isolating namespace, being able to have a contract with the same name as the class you're validating, etc.

I don't like using a string at all. Another project already does that anyway. I like Contracts looking like part of Ruby and being syntax highlighted etc.

[alex-fedorov]

Then { [A, B => C] } ? Or anything else you can see?

[sfcgeorge]

Yeah I can't see any way of making a Block without curly brackets. So then either a hash or array inside that.

Contract { [A, B => C] }

Contract {{ A, B => C }} # I slightly prefer this, quicker to type too

[robnormal]

I also prefer {{ X => Y }} to {[ X => Y ]}.

The only downside of @nixpulvis's idea is, you would have to write a contract class for every class you wanted to type-check for. I suppose we could use constant_missing to default to the current behavior.

[nixpulvis]

Not necessarily. We could make it so it simply executes within a namespace of the current context with the base contracts included. That way the classes and variables where a contract is attached are available.

[egonSchiele]

I like define_contracts too, seems like an easy way to put Contracts in a hidden namespace. I don't like the idea of eval-ing strings though.

[waterlink]

I like this. But: the question is, will this approach with hidden namespace and contracts with the same name create any confusion for readers of the code? What about autoloading that is done by certain libraries out there?

For example in my code, if I want to have my own custom contract (implements .valid?), but I want still to express that this argument should be of certain class I just append Contract suffix for the name of the contract, ie:

class Person
end

class A
  include Contracts

  Contract NamedPersonContract => String
  def fancy_name_for(person)
    # ...
  end
end

NamedPersonContract = Contracts::RespondTo[:first_name, :last_name, :nickname]

---

Or other thing I considered using in this case: Contract Person::NamedContract => String

[nixpulvis]

Adding Contract to the end of every contract seems overly verbose and likely to not be done, causing consistency problems. The ability to define a contract with any name (even one used in the application logic) and not collide seems positive, but there shouldn't be anything stopping you from not putting it in the special namespace, and avoiding collisions on your own as well.

[alem0lars]

I would prefer parsing the string instead of doing evaluation because of the possible security problems.. Also consider this code goes to production and it's only controlled by a environment variable which can be easily manipulated by an attacker... Also, it's never encouraged to use eval in almost every language..

Parsing is a much better option but we also should keep in mind it has to NOT slow down too much.. e.g. parsing can be done when evaluating class code and not every time the method is called, etc...

[waterlink]

So.

I want to see something like include Contracts::Core in the wild, that will not pollute current namespace with anything at all. No builtin contracts, nothing. include Contracts will still work as it worked, because we probably don't want to break people' code.

Examples:

# old way, with polluting of namespace:
class X
  include Contracts

  Contract Num, Maybe[Num] => Float
  def fetch(a, b)
    # ...
  end
end

# new way, without namespace polluting:
class X
  include Contracts::Core

  Contract Contracts::Num, Contracts::Maybe[Contracts::Num] => Float
  def fetch(a, b)
    # ...
  end
end

# block form, shorter, don't need to spell out `Contracts::` for builtin contracts:
class X
  include Contracts::Core

  Contract {{ Num, Maybe[Num] => Float }}
  def fetch(a, b)
    # ...
  end
end

At some point, probably 1.0.0 release, we can make include Contracts behave like include Contracts::Core as described above. And if we want to leave old polluting behavior at all, then something like include Contracts::Full or something.

This is to stop polluting people' namespaces and deal with name clashes with different gems. Definition of your own contracts in separate hidden namespace is probably next feature after that.

WDYT?

[nixpulvis]

Seems like a classic deprecation issue. We should defiantly deprecate the include Contracts behavior. I'd like to allow for that to stay be the main method of including, without a need for another include Contracts::Core. I think we should also deprecate non blocked contracts, since many features could make good use of contracts being blocks. Before we can say if this is a good or bad idea though, we'll want to have some benchmarks on performance, and strategies for pre-evaluating the blocks.

Overall I'm not a fan of include Contracts::Core, but I do think we should add it in the mean time while we deprecate the functionality of include Contracts. Then at some point remove it.

[robnormal]

Thinking more about it, I don't see any way we can defeat the problem of the lexical scoping of constants here, except to use strings for the contracts; if we want to avoid class load-order issues, that is.

[sfcgeorge]

No strings please, there are several other similar libraries that use parsed strings so there's no point stepping on their toes. There's currently choice which is good, Contracts using native ruby syntax, or the others using parsed strings. Use whichever you prefer :)

I like @waterlink's idea above. Version 1.0 removing old behaviour and swapping Contracts::Core for Contracts seems like a good upgrade path. I'm not sure why you'd want to use the verbose syntax, I'd ultimately be in favour of just having the double brace syntax and nothing else if it makes Contracts' internal code simpler.

I don't think making scoping work is a problem, instance_eval should do it as Ruby lets you leave off namespaces you're already inside.

[robnormal]

I'm afraid instance_eval is no help here. Constants are lexically scoped, meaning the Alpha in Contract { Alpha } is bound to the class where this statement occurs, not the class that evaluates the block. I don't think Ruby has a facility for circumventing this.

[nixpulvis]

Well you see, it is possible. Though I'm not 100% sure we want to be doing it.

def do_it(&block)
  new_block = proc do |s|
    old = s::Foo.send(:remove_const, 'VAL')
    s::Foo.const_set('VAL', 1)
    block.call(s)
    s::Foo.send(:remove_const, 'VAL')
    s::Foo.const_set('VAL', old)
  end
  class_eval(&new_block)
end

module A
  class Foo
    VAL = 1
  end
end

class B
  class Foo
    VAL = 2
  end

  do_it { puts Foo::VAL }
  puts Foo::VAL
end

Just a proof of concept.

[sfcgeorge]

I've seen something that crazy with changing the binding of a proc but it only works for variables and methods—not constants—because you can't really change the binding, only the value of self to proxy method calls (which variables act like but constants don't).

const_missing is another dead end as in the case of a naming collision it obviously won't be missing.

@robnormal yeah sorry I was wrong. Forgot that instance_eval changes self which is only good for proxying method calls and variables, not constants which are lexically scoped as you say. Here's a method of proxying variables and method calls (not constants) that's interesting nonetheless: http://stackoverflow.com/a/10059209

Nice @nixpulvis . I'm ok with magic (especially if it's optional) but I think @egonSchiele isn't.

[nixpulvis]

It's either magic, strings, or explicit namespaces. Choose one.

[nixpulvis]

Another huge point against the code I posted is thread safety. Which unless Ruby does something fancy I don't know about, will defiantly be broken in a horrific way by this.

I posted more or less just as a means of showing the only way I could think to do it.

[alem0lars]

I vote for doing a DSL with it's own parser. It's the most clean way to accomplish this without dirty hacks and being thread safe. Also, it gives us full control over both syntax and semantics, which is great for the future..