Template parameters can be deprecated

[mkurz]

Right now it's not possible to deprecate the generated render, apply and f methods. So we have no choice of breaking the API as soon the args passed via @(...) change. This bit as before and bites us again in playframework/playframework#9447.

Therefore I propose to once and for all fix this problem by adding a @deprecatedParams tag, which you can use to overload the render and apply method with deprecated ones. There is just one small problem: We can not overload the f method, because they just differ in the return type... Therefore to make this work, we need to rename that f method. Therefore I will add a @templateFunctionName("...") tag which just sets the name of the f method.

And this is how I suggest it will look like. Given you have the following template:

@*********************
 * This is a comment *
 *********************@
@import com.typsafe....
@this()
@(one: Int, two: Int)
...Template content here...

And you want to deprecate the arguments and replace them with String equivalents:

@*********************
 * This is a comment *
 *********************@
@import com.typsafe....
@this()
@templateFunctionName("fn")
@deprecatedParams("2.8.0", "f")(one: Int, two: Int) = @{
   val newOne = one + 1
   val newTwo = two - 1
   apply(newOne.toString, newTwo.toString)
}
@(one: String, two: String)
...Template content here...

We pass the @deprecatedParams the version since when we want the methods to be deprecate and the name of the f function which should be deprecated (here it's f because that is the default name). (the generated annotation will be like @deprecated(since="2.8.0", message="Use new method fn instead") for the f method and @deprecated(since="2.8.0", message="""Use arguments (one: String, two: String) instead""") for apply and render). Finally we pass the arguments for the deprecated render, apply and return type of f.

Now in the next version we can just remove the @deprecatedParams tag, but keep the @templateFunctionName (because that's the method's name now):

@*********************
 * This is a comment *
 *********************@
@import com.typsafe....
@this()
@templateFunctionName("fn")
@(one: String, two: String)
...Template content here...

Now in case we want to again deprecate these arguments you can now just remove the @templateFunctionName (resulting in the generating the method f with its default name), but tell @deprecatedParams("2.10.0", "fn")(... that the method to deprecate is fn.

TODO's:

• @templateFunctionName("...")
• Docs for @templateFunctionName
• @deprecatedParams
• Docs for @deprecatedParams
• Tests

(Docs need to be added to Java and Scala API always)

[mkurz]

One more thing:
Because in playframework/playframework#9488 we have lots of args: (Symbol,Any)* only changes, it's a good idea to pass (implicit d: DummyImplicit) to the "new" generated apply and render methods (but only if the @deprecatedParams is used) to avoid

[error] double definition:
[error] def render(foome: String, args: Array[(String, Any)]): play.twirl.api.HtmlFormat.Appendable at line 53 and
[error] def render(foome: String, args: Array[(Symbol, Any)]): play.twirl.api.HtmlFormat.Appendable at line 57
[error] have same type after erasure: (foome: String, args: Array[Tuple2])play.twirl.api.Html

Still this would maybe not work 100% all of the time, because Java users would have to pass a DummyImplicit (DummyImplicit.dummyImplicit()) explicitly when calling such a new render/apply method directly from Java code

However, because the template helpers we want to deprecate are usually called from within templates anyway, this is very unlikely to happen.
Also when removing the @deprecatedParams later on, means the implicit DummyImplicit would be removed as well, which means this probably isn't 100% binary backwards compatible. However I think it's good enough.

[mkurz]

More notes:
When using @deprecatedParams without @templateFunctionName we can then just automatically add (implicit d: DummyImplicit) to the new f method (meaning we just have to look if there are two f methods with the same name and if so add the implict to the new one). One could think we can just always solve the f problem like this, however if you have templates you call from Java code that is maybe not so nice and renaming would be a better choice. We could also make the function name param in @deprecatedParams (second param) optional.
Therefore we always have to distinguish between templates that will 1) very likely not be used by Java code (ones that will just be called from other twirl templates) were we can make use of DummyImplicit and 2) templates that will directly be called from controllers (and therefore possibly also from Java code).

Because in playframework/playframework#9488 we have lots of args: (Symbol,Any)* only changes, it's a good idea to pass (implicit d: DummyImplicit) to the "new" generated apply and render methods

Adding (implicit d: DummyImplicit) could be done by the user, not automatically. We just have to document it:

@deprecatedParams("2.8.0")(args: (Symbol,Any)*) = @{
   apply(args.map(t => (t._1, t._2))
}
@(args: (String,Any)*)(implicit d: DummyImplicit)
...Template content here...

However we must take care now what happens with the f methods signature. I guess in theory the (implicit d: DummyImplicit) would be added to its return type - is this necessary? Because the f method would have be f()(implicit d: DummyImplicit) already anyway (or it would be renamed already if using @templateFunctionName, so I guess we would not need it in the return type - but how remove it?

[marcospereira]

An empty comment so that this will not be part of my "review requests" list.

Sorry for the noise, @mkurz.

[BillyAutrey]

An empty comment so that this will not be part of my "review requests" list.

Sorry for the noise, @mkurz.

Hey Marcos, been a while! Hope all is well.

I typically just click on the notification, and then hit "unsubscribe" at the top. That works well enough for the notification spam for a particular PR, for me.

[marcospereira]

Hey Billy, how are you? Yeah, it's been a while, and I hope everything is well with you too.

I typically just click on the notification, and then hit "unsubscribe" at the top. That works well enough for the notification spam for a particular PR, for me.

Oh, that is not a notification, but instead, the list of review requests that appear when you access https://github.com/pulls/review-requested.