Added annotatedWithName to ScalaModule

[michalkowol]

Added BindingProxies.annotatedWithName(name: String).

Example:

bind[String].annotatedWithName("first").toInstance("first")
bind[String].annotatedWithName("second").toInstance("second")

Added test for annotatedWithName UseCase.

Cleaned up imports.

[nbauernfeind]

From a style perspective, I'd prefer the trait definition to be on a single line. There are no other multi-line definitions in this project, and there are definitely longer lines (in this file even) than the ones created by squashing lines 109-110 and 118-119. So I suggest:

trait ScalaAnnotatedBindingBuilder[T] extends ScalaLinkedBindingBuilder[T] with AnnotatedBindingBuilderProxy[T] {
  outer =>

  def annotatedWith[...

[nbauernfeind]

Otherwise, this seems good to me; and thanks for the test! I hadn't realized we're inconsistently switching between javax imports and guice imports.

[nbauernfeind]

Ahh I didn't see your other issue / pull request. That makes even more sense (w.r.t. javax vs not).

[michalkowol]

Should be ok now. I added annotatedWithName to ScalaAnnotatedConstantBindingBuilder to allow:

bindConstant().annotatedWithName("name").to("constant")

[nbauernfeind]

There's another annotatedWith method on KeyExtensions if you're looking for complete consistency =).

[michalkowol]

Should be consistent now :). Unfortunately I dont' know how to thest this method.

[nbauernfeind]

LGTM. I don't have merge permissions; so you'll have to wait for Thomas' approval.

[tsuckow]

So, looking through this I noticed that ScalaAnnotatedConstantBindingBuilder has annotatedWithType and KeyExtensions has annotatedWith that appears to do the same thing. This wasn't changed in this pull request but am I crazy or should those be the same?

On that same note, should annotatedWithName just be annotatedWith? I can't think of a reason the method call would ever be ambiguous even if we could annotate with something new in the future.

Lastly, we need references to these in the README.

I may or may not get around to building this on my own system this weekend. In the mean time, discuss the above.

Also, Nate, you make reviewing things so much easier. ^_^

[tsuckow]

Any thoughts on what I mentioned. I never did get around to building this. I am also hoping Guice4 will finalize this century.

[nbauernfeind]

I think what you're talking about is reasonable, but I don't think it can work. You can create the method 'annotatedWith[T: Manifest]' on ScalaAnnotatedConstantBindingBuilder, but you can't invoke it. I think the problem is that there are methods that exist on AnnotatedConstantBindingBuilder named 'annotatedWith' that accept a single parameter. So I think the compiler has a hard time realizing that you want to call the overloaded method.

For example if I try to call (obviously this is a bad example to use Named, but it is an annotation which is useful for a quick test):

        binder.bindType[A].annotatedWith[Named].toType[B]

Then I get this error:

[error] /Users/nbauernfeind/code/external/scala-guice/src/test/scala/net/codingwell/scalaguice/BindingExtensionsSpec.scala:37: overloaded method value annotatedWith with alternatives:
[error]   (java.lang.annotation.Annotation)com.google.inject.binder.LinkedBindingBuilder[net.codingwell.scalaguice.A] <and>
[error]   (java.lang.Class[_ <: java.lang.annotation.Annotation])com.google.inject.binder.LinkedBindingBuilder[net.codingwell.scalaguice.A]
[error]  does not take type parameters
[error]         binder.bindType[A].annotatedWith[Named].toType[B]
[error]                            ^
[error] one error found

But if the currently named method 'annotatedWithType' is called, then it compiles just fine.

The reason that KeyExtensions gets away with this is because it's not actually enriching any guice objcet. So if anything then we should rename KeyExtensions.

So then the question I haven't looked into is the can 'annotatedWithName' be called 'annotatedWith'... But, I would wonder if there are any types that have implicit conversions to String that would complicate the experience for what would otherwise be a compiler error. But it could be a completely unwarranted worry.

[tsuckow]

Please look over https://github.com/codingwell/scala-guice/tree/release/4.0.0-beta4 and verify this appears to be merged correctly. I am going to move forward with these changes and address the previous concerns at a later date, this has been waiting long enough.

[michalkowol]

Sure! I will take a look.
I didn't have time, because I had to finish my master thesis. Now, I will be more available.

[michalkowol]

It looks good for me.