Please ensure that your changes compile before you send a pull request.

@RX14: They do, in the context of a shard anyway, as i exported the OAuth namespace into an OAuthmp namespace in my project, it compiles and runs, though i've had to make other changes to adjust for the slightly non-standard OAuth service in use. Compiling Crystal itself on Arch with grsecurity and a bunch of other changes can get nasty - PAX needs to be off or carefully exempted.
Far as the cause of failure - by the time @scheme is compared to the string "https," it should already be instantiated. If the constructor is unable to handle this, we can monkey patch the PR to move that comparison into the method body, but this approach works in Ruby and other languages, meaning we probably want have the constructor assess all static call params, then proceed to evaluate the dynamic ones after that first pass.

@sempervictus You shouldn't need the compile crystal itself to run the standard library test suite. Just use make std_spec with the latest compiler release and it should be fine.

As for the compile error, it does look weird. It might be a compiler bug. You'll probably have to extract it out into a field in the constructor, like tls : Bool | OpenSSL::SSL::Context::Client | Nil = nil then do @tls = tls.nil? ? @scheme == "https" : tls with a note about why.

@sempervictus crystal play is good for an interactive playground, gdb/lldb can replace pry for debugging purposes but they're not quite the same. It likely never will be.

Sure you can use travis but it's easier for most people to run the specs locally if they have crystal already checked out. A lot of people don't bother to check the travis status after they make their PR and need a nudge.

@RX14: So i'm thinking this is actually a compiler bug:

Error: instance variable '@scheme' was used before it was initialized in one of the 'initialize' methods, rendering it nilable

where @scheme, only capable of being a String, and having a default value provided, or taking any other potential value passed, would still be a String, is compared to the String "https." In my limited understanding of compiler safety, this should be a "safe" operation since we must be comparing the same data types, even if they're empty.

What i find a bit more troubling, is that by setting up @tls as a union of Nil | Bool | OpenSSL::SSL::Context::Client during initialization of the Consumer as discussed above, we're breaking a later call passing @tls to HTTP::Client.new as seen in:

 make std_spec
Using /usr/bin/llvm-config [version=4.0.0]
./bin/crystal build  -o .build/std_spec spec/std_spec.cr
Using compiled compiler at .build/crystal
Error in spec/std_spec.cr:2: while requiring "./std/**"

require "./std/**"
^

in spec/std/oauth/consumer_spec.cr:47: instantiating 'OAuth::Consumer#get_request_token()'

    consumer.get_request_token(oauth_callback: "foo.bar.baz")
             ^~~~~~~~~~~~~~~~~

in src/oauth/consumer.cr:74: instantiating 'post(Nil, Nil, Hash(String, String), String)'

    post(nil, nil, {"oauth_callback" => oauth_callback}, @request_token_uri) do |response|
    ^~~~

in src/oauth/consumer.cr:141: argument 'tls' already specified

      client = HTTP::Client.new @host, @port, tls: @tls
                            ^~~

make: *** [Makefile:106: .build/std_spec] Error 1

Smells (to me anyway) of the instance variable @tls being a different type of union than the one expected by the HTTP::Client.

The following code compiles and runs the spec just fine:

 class OAuth::Consumer
                  @request_token_uri : String = "/oauth/request_token",
                  @authorize_uri : String = "/oauth/authorize",
                  @access_token_uri : String = "/oauth/access_token",
                 @tls : Bool | OpenSSL::SSL::Context::Client = false )
    @tls = @scheme == "https" unless @tls

though i'm assuming a bit of Ruby-ish inferential behavior in that snippet, of which i'm not yet sure. Specifically the notion that @tls being a Union is "truthy," or will evaluate to True in the case of being passed true or an OpenSSL::SSL::Context::Client object, but not false. The actual commit i just pushed takes the approach of using #is_a?(OpenSSL::SSL::Context::Client), which seems less "elegant" than the snippet above.

Your code doesn't allow passing tls: false when scheme is https. I'm not sure why you'd want to do that but I think it should be possible. Simply don't use the @tls shorthand in the constructor arguments and pass it as a normal argument, and set it's default only if it's nil.

@sempervictus You absolutely can if you remove nil from the union before it's set to @tls. Which is exactly what tls.nil? ? @scheme == "https" : tls should do, the type of that statement should be OpenSSL::SSL::Context::Client | Bool, which doesn't change the type of @tls at all.

Just noticed the one character difference... local variables and instance variables can share names. Grand. Still prevents passing a boolean option for the tls setting, which is counter to the normal logic, but probably the lesser of evils available to us.

@sempervictus if you just add Bool to the type of the constructor argument, passing booleans should work...

Sorry for the lag, got lost in the churn :(. Implemented as you requested @RX14. Thanks for prodding me through this.
Rebased on master and squashed, tests show:

Pending:
  Math Functions for computing quotient and remainder
  Thread::ConditionVariable signals
  Thread::ConditionVariable broadcasts
  Thread::ConditionVariable waits and send signal

Finished in 21.75 seconds
5081 examples, 0 failures, 0 errors, 4 pending

Looks like the build is failing because of formatting. Would run crystal tool format to get that fixed?

CI failure on the last commit is unrelated - OSX seems to not have been able to acquire LLVM:

Error: Failed to download resource "llvm"

@RX14: seems to be passing CI, any blockers remaining?

There are couple of things from my point of view:

1. It itches me quite a bit that up to now the API of the consumer was agnostic from OpenSSL, and now we're tying the implementation to a very specific OpenSSL class. Sounds like we need a little more design to not expose that implementation detail.
2. This PR is adding functionality but not tests. Even if we agree on the design, I'd still expect at least one spec checking the consumer with an OpenSSL::SSL::Context::Client.

@mverzilli We do a similar thing in HTTP::Client so the change to accept a TLS context isn't without precedent. Personally, I think we should design some sort of openssl-agnostic TLS api and only expose that in the stdlib. We then implement a openssl-backed version of that API and extract the old openssl module to a shard which people who need advanced features can use. Then, everywhere we use TLS, we can have a nice crystally options class which people can pass in. Obviously beyond the scope of this PR.

Agreed. Still we would need at least one test to exercise the new option (even more important considering that eventually we'll have to break backwards compatibility to "agnosticize" the design).

Windows's built in SSL would probably be a good test of that. Windows support will likely be here in some fashion by the time fiddling with SSL apis is at the top of the todo list.

Wholly agreed on abstraction of TLS functions to a shim lib for interface with the multiple back ends available (win, libre, etc).
I'll get a test pushed here in the meantime as doing the middle ware is way out of scope and I'm not yet familiar with the core devs' coding preferences for such bodies of work (hell, still getting some of the rubyisms out of the way).

@sempervictus are you still working on the specs? ;)
if not, we should either adopt or close this PR for the sake of cleaning.