Implemented package mapping for Java/Kotlin code generation #499
Conversation
|
@bioball - I've started the package mapping implementation, just wanted to show the overall direction before I start implementing it for Kotlin. What do you think? |
| * When this option is set, the mapping will be used to translate package names derived from | ||
| * module names to the specified package names. | ||
| */ | ||
| val packageMapping: String? = null |
There was a problem hiding this comment.
This should accept a structured Map<String, String>; trace how externalProperties and environmentVariables get populated from the CLI args parser.
There was a problem hiding this comment.
Thanks for the pointer, I hoped that there is something like that!
|
|
||
| private fun mapModuleName(name: String): kotlin.Pair<String, String> { | ||
| val packageName = mapPackageName(name.substringBeforeLast('.', "")) | ||
| val className = name.substringAfterLast('.').replaceFirstChar { it.titlecaseChar() } |
There was a problem hiding this comment.
Should maybe accept the name as-is?
| val className = name.substringAfterLast('.').replaceFirstChar { it.titlecaseChar() } | |
| val className = name.substringAfterLast('.') |
There was a problem hiding this comment.
This change would result in differences from the current behavior:
// current:
module foo.bar.baz -> package foo.bar; class Baz {}
// proposed:
module foo.bar.baz -> package foo.bar; class baz {}
That is, the class name would be lowercase, which is against Java naming conventions. This, in turn, would require users to always use a module name like foo.bar.Baz if they want to get conventional Java names.
Ultimately, I think that even if we decide to do this, this should happen as a separate change, because it is only tangentially related to the original purpose of this PR.
There was a problem hiding this comment.
Canonically, such Pkl code should also have module foo.bar.Baz if it's meant to be a class.
At the very least, though, if a package mapping is provided, we should probably not alter it.
There was a problem hiding this comment.
Canonically, such Pkl code should also have module foo.bar.Baz if it's meant to be a class.
This is the first time I hear about it :( I don't believe it is documented anywhere, and it is definitely not followed by many Pkl modules in the wild.
Different behavior when mapping is specified or not would be very confusing, IMO. Also, the mapping is specifically about packages, not module names.
I would prefer to limit the scope of the change and avoid doing unexpected changes in the existing behavior. But if you insist I can change it here.
There was a problem hiding this comment.
It's part of our style guide actually; see filename and module name
See #456 for motivation.
Closes #456