Skip to content

Commit

Permalink
proposal to update name generation
Browse files Browse the repository at this point in the history
This commit updates the name generation to always add a trailing underscore for
the following reasons:

1. There are some missing keywords - for example, a custom type named `Null` will
   crash motif since it will try to name the function `null()`, it seems that adding
   to the list is not as safe as always append
2. Motif append a `2` (or `3` and so on) to a function name to avoid collision, so a
   type whose name ends with a `2` e.g. `Function2` will have its name `Function22`
   (or `Function23` and so on) - it's not wrong, but reads a bit funny
  • Loading branch information
davissuber committed Mar 28, 2024
1 parent 64eb967 commit 34c49be
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 75 deletions.
64 changes: 5 additions & 59 deletions compiler/src/main/kotlin/motif/compiler/Names.kt
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ package motif.compiler

import androidx.room.compiler.processing.XAnnotation
import androidx.room.compiler.processing.XType
import com.google.common.annotations.VisibleForTesting
import java.util.Locale
import motif.ast.compiler.CompilerAnnotation
import motif.ast.compiler.CompilerType
import motif.models.Type
Expand Down Expand Up @@ -49,14 +51,11 @@ private class UniqueNameSet(blacklist: Iterable<String>) {
object Names {

@JvmStatic
@VisibleForTesting
fun safeName(typeMirror: XType, annotation: XAnnotation?): String {
var name = XNameVisitor.visit(typeMirror)
val name = XNameVisitor.visit(typeMirror)
val annotationString = annotationString(annotation)
name = "$annotationString$name".decapitalize()
if (name in KEYWORDS) {
name += "_"
}
return name
return "$annotationString${name}_".replaceFirstChar { it.lowercase(Locale.ENGLISH) }
}

private fun annotationString(annotation: XAnnotation?): String {
Expand All @@ -67,56 +66,3 @@ object Names {
}
}
}

private val KEYWORDS =
setOf(
"abstract",
"continue",
"for",
"new",
"switch",
"assert",
"default",
"goto",
"package",
"synchronized",
"boolean",
"do",
"if",
"private",
"this",
"break",
"double",
"implements",
"protected",
"throw",
"byte",
"else",
"import",
"public",
"throws",
"case",
"enum",
"instanceof",
"return",
"transient",
"catch",
"extends",
"int",
"short",
"try",
"char",
"final",
"interface",
"static",
"void",
"class",
"finally",
"long",
"strictfp",
"volatile",
"const",
"float",
"native",
"super",
"while")
32 changes: 17 additions & 15 deletions compiler/src/test/java/motif/compiler/NamesTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import org.junit.runners.Parameterized

@RunWith(Parameterized::class)
@OptIn(ExperimentalProcessingApi::class)
class XNamesTest(private val processorType: ProcessorType, private val srcLang: SourceLanguage) {
class NamesTest(private val processorType: ProcessorType, private val srcLang: SourceLanguage) {

companion object {
@JvmStatic
Expand All @@ -49,64 +49,66 @@ class XNamesTest(private val processorType: ProcessorType, private val srcLang:

@Test
fun primitive() {
assertSafeName("int", "kotlin.Int", "integer")
assertSafeName("int", "kotlin.Int", "integer_")
}

@Test
fun boxed() {
assertSafeName("java.lang.Integer", "kotlin.Int?", "integer")
assertSafeName("java.lang.Integer", "kotlin.Int?", "integer_")
}

@Test
fun raw() {
assertSafeName("java.util.HashMap", "java.util.HashMap<*, *>", "hashMap")
assertSafeName("java.util.HashMap", "java.util.HashMap<*, *>", "hashMap_")
}

@Test
fun typeArgument() {
assertSafeName(
"java.util.HashMap<String, Integer>",
"java.util.HashMap<String, Integer>",
"stringIntegerHashMap")
"stringIntegerHashMap_")
}

@Test
fun wildcard() {
assertSafeName(
"java.util.HashMap<? extends String, ? super Integer>",
"java.util.HashMap<out String, out Integer>",
"stringIntegerHashMap")
"stringIntegerHashMap_")
}

@Test
fun wildcardUnbounded() {
assertSafeName("java.util.HashMap<?, ?>", "java.util.HashMap<*, *>", "hashMap")
assertSafeName("java.util.HashMap<?, ?>", "java.util.HashMap<*, *>", "hashMap_")
}

@Test
fun typeVariable() {
assertSafeName("java.util.HashMap<String, A>", "java.util.HashMap<String, A>", "stringAHashMap")
assertSafeName(
"java.util.HashMap<String, A>", "java.util.HashMap<String, A>", "stringAHashMap_")
}

@Test
fun typeVariableUnbounded() {
assertSafeName("java.util.HashMap<String, B>", "java.util.HashMap<String, B>", "stringBHashMap")
assertSafeName(
"java.util.HashMap<String, B>", "java.util.HashMap<String, B>", "stringBHashMap_")
}

@Test
fun nested() {
assertSafeName(
"java.util.HashMap<java.util.HashMap<String, Integer>, Integer>",
"java.util.HashMap<java.util.HashMap<String, Integer>, Integer>",
"stringIntegerHashMapIntegerHashMap")
"stringIntegerHashMapIntegerHashMap_")
}

@Test
fun innerClass() {
assertSafeName(
"java.util.Map.Entry<String, Integer>",
"java.util.Map.Entry<String, Integer>",
"stringIntegerMapEntry")
"stringIntegerMapEntry_")
}

@Test
Expand All @@ -116,22 +118,22 @@ class XNamesTest(private val processorType: ProcessorType, private val srcLang:

@Test
fun named() {
assertSafeName("String", "String", "fooString", "@javax.inject.Named(\"Foo\")")
assertSafeName("String", "String", "fooString_", "@javax.inject.Named(\"Foo\")")
}

@Test
fun customQualifier() {
assertSafeName("String", "String", "fooString", qualifierString = "@Foo")
assertSafeName("String", "String", "fooString_", qualifierString = "@Foo")
}

@Test
fun array() {
assertSafeName("String[]", "Array<String>", "stringArray")
assertSafeName("String[]", "Array<String>", "stringArray_")
}

@Test
fun enum() {
assertSafeName("LogLevel", "LogLevel", "logLevel")
assertSafeName("LogLevel", "LogLevel", "logLevel_")
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
---- String | Objects.string ----
[ Required ]
[ Consumed By ]
* Scope | Scope.string()
* Scope | Scope.string_()

---- Scope | implicit ----
[ Required ]
Expand Down

0 comments on commit 34c49be

Please sign in to comment.