review: feature: Substitute string literals

[pvojtechovsky]

implements #1337

[monperrus]

what is the parameter (in parameters) driving this replacement?

[pvojtechovsky]

It is the parameter implicitly added by this code

class Substitution {
...
	public static <T> CtType<T> createTypeFromTemplate(String qualifiedTypeName, CtType<T> templateOfType, Map<String, Object> templateParameters) {
		final Factory f = templateOfType.getFactory();
		CtTypeReference<T> typeRef = f.Type().createReference(qualifiedTypeName);
		CtPackage targetPackage = f.Package().getOrCreate(typeRef.getPackage().getSimpleName());
		final Map<String, Object> extendedParams = new HashMap<String, Object>(templateParameters);
//--------Here is the implicit parameter added----------------
		extendedParams.put(templateOfType.getSimpleName(), typeRef);
		List<CtType<T>> generated = new SubstitutionVisitor(f, extendedParams).substitute(templateOfType.clone());

[monperrus]

Implicit is worse than explicit. What if we move this code in the test? ie this responsibility to the caller?

I guess that string substitution should work outside createTypeFromTemplate, consequently, could you add a test with a template dedicated to string substitution?

thanks

[pvojtechovsky]

OK, you are right.
I have added explicit tests of several combinations of string literal substitution and parameter value type.
There is also tested how behaves invocation replacement - it is tested in same test, to easily see the difference in behavior.
It is ready for merge. WDYT?

[monperrus]

That's good thanks.

What I miss in SubstituteLiteralTemplate is that the template parameter are implicit, we don't explicitly define a template parameter related to String and annotated with @parameter. What do you think?

[pvojtechovsky]

The Template parameter is declared explicitly now. OK? ;-)

[monperrus]

thanks Pavel