Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrating the Util.kt and KModifier from JVM to common #2006

Merged
merged 5 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import com.squareup.kotlinpoet.KModifier.INTERNAL
import com.squareup.kotlinpoet.KModifier.PRIVATE
import com.squareup.kotlinpoet.KModifier.PROTECTED
import com.squareup.kotlinpoet.KModifier.PUBLIC
import java.util.EnumSet

public enum class KModifier(
internal val keyword: String,
Expand Down Expand Up @@ -89,4 +88,4 @@ public enum class KModifier(
}
}

internal val VISIBILITY_MODIFIERS: Set<KModifier> = EnumSet.of(PUBLIC, INTERNAL, PROTECTED, PRIVATE)
internal val VISIBILITY_MODIFIERS: Set<KModifier> = enumSetOf(PUBLIC, INTERNAL, PROTECTED, PRIVATE)
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,17 @@
*/
package com.squareup.kotlinpoet

import com.squareup.kotlinpoet.CodeBlock.Companion.isPlaceholder
import java.util.Collections

internal object NullAppendable : Appendable {
override fun append(charSequence: CharSequence) = this
override fun append(charSequence: CharSequence, start: Int, end: Int) = this
override fun append(c: Char) = this
override fun append(value: CharSequence?) = this
override fun append(value: CharSequence?, startIndex: Int, endIndex: Int) = this
override fun append(value: Char) = this
}

internal fun <K, V> Map<K, V>.toImmutableMap(): Map<K, V> =
Collections.unmodifiableMap(LinkedHashMap(this))
internal expect fun <K, V> Map<K, V>.toImmutableMap(): Map<K, V>

internal fun <T> Collection<T>.toImmutableList(): List<T> =
Collections.unmodifiableList(ArrayList(this))
internal expect fun <T> Collection<T>.toImmutableList(): List<T>

internal fun <T> Collection<T>.toImmutableSet(): Set<T> =
Collections.unmodifiableSet(LinkedHashSet(this))
internal expect fun <T> Collection<T>.toImmutableSet(): Set<T>

internal inline fun <reified T : Enum<T>> Collection<T>.toEnumSet(): Set<T> =
enumValues<T>().filterTo(mutableSetOf(), this::contains)
Expand Down Expand Up @@ -63,10 +57,14 @@ internal fun characterLiteralWithoutSingleQuotes(c: Char) = when {
c == '\"' -> "\"" // \u0022: double quote (")
c == '\'' -> "\\'" // \u0027: single quote (')
c == '\\' -> "\\\\" // \u005c: backslash (\)
c.isIsoControl -> String.format("\\u%04x", c.code)
c.isIsoControl -> formatIsoControlCode(c.code)
else -> c.toString()
}

internal expect fun formatIsoControlCode(code: Int): String

internal expect fun Int.toHexStr(): String

internal fun escapeCharacterLiterals(s: String) = buildString {
for (c in s) append(characterLiteralWithoutSingleQuotes(c))
}
Expand Down Expand Up @@ -138,44 +136,36 @@ internal fun stringLiteralWithQuotes(
}
}

internal fun CodeBlock.ensureEndsWithNewLine() = trimTrailingNewLine('\n')

internal fun CodeBlock.trimTrailingNewLine(replaceWith: Char? = null) = if (isEmpty()) {
this
} else {
with(toBuilder()) {
val lastFormatPart = trim().formatParts.last()
if (lastFormatPart.isPlaceholder && args.isNotEmpty()) {
val lastArg = args.last()
if (lastArg is String) {
val trimmedArg = lastArg.trimEnd('\n')
args[args.size - 1] = if (replaceWith != null) {
trimmedArg + replaceWith
} else {
trimmedArg
}
}
} else {
formatParts[formatParts.lastIndexOf(lastFormatPart)] = lastFormatPart.trimEnd('\n')
if (replaceWith != null) {
formatParts += "$replaceWith"
}
}
return@with build()
}
}
// TODO Waiting for `CodeBlock` migration.
// internal fun CodeBlock.ensureEndsWithNewLine()

// TODO Waiting for `CodeBlock` migration.
// internal fun CodeBlock.trimTrailingNewLine(replaceWith: Char? = null)

private val IDENTIFIER_REGEX =
/**
* Will crash if used `IDENTIFIER_REGEX_VALUE.toRegex()` directly in WasmJs:
* `PatternSyntaxException: No such character class`.
*
* It works in JS and JVM.
*
* For now:
* - Keep the use of `Regex` in JVM and JS.
* - And use `RegExp` directly in WasmJs for matching,
* using it in a similar way as in JS.
*
* See also: [KT-71003](https://youtrack.jetbrains.com/issue/KT-71003)
*/
internal const val IDENTIFIER_REGEX_VALUE =
// language=regexp
(
"((\\p{gc=Lu}+|\\p{gc=Ll}+|\\p{gc=Lt}+|\\p{gc=Lm}+|\\p{gc=Lo}+|\\p{gc=Nl}+)+" +
"\\d*" +
"\\p{gc=Lu}*\\p{gc=Ll}*\\p{gc=Lt}*\\p{gc=Lm}*\\p{gc=Lo}*\\p{gc=Nl}*)" +
"|" +
"(`[^\n\r`]+`)"
)
.toRegex()

internal val String.isIdentifier get() = IDENTIFIER_REGEX.matches(this)
internal expect val String.isIdentifier: Boolean

// https://kotlinlang.org/docs/reference/keyword-reference.html
internal val KEYWORDS = setOf(
Expand Down Expand Up @@ -317,7 +307,7 @@ internal fun String.escapeAsAlias(validate: Boolean = true): String {

val newAlias = StringBuilder("")

if (!Character.isJavaIdentifierStart(first())) {
if (!first().isJavaIdentifierStart()) {
newAlias.append('_')
}

Expand All @@ -327,8 +317,8 @@ internal fun String.escapeAsAlias(validate: Boolean = true): String {
continue
}

if (!Character.isJavaIdentifierPart(ch)) {
newAlias.append("_U").append(Integer.toHexString(ch.code).padStart(4, '0'))
if (!ch.isJavaIdentifierPart()) {
newAlias.append("_U").append(ch.code.toHexStr().padStart(4, '0'))
continue
}

Expand All @@ -348,8 +338,8 @@ private fun String.escapeIfAllCharactersAreUnderscore() = if (allCharactersAreUn

private fun String.escapeIfNotJavaIdentifier(): String {
return if ((
!Character.isJavaIdentifierStart(first()) ||
drop(1).any { !Character.isJavaIdentifierPart(it) }
!first().isJavaIdentifierStart() ||
drop(1).any { !it.isJavaIdentifierPart() }
) &&
!alreadyEscaped()
) {
Expand All @@ -362,3 +352,9 @@ private fun String.escapeIfNotJavaIdentifier(): String {
internal fun String.escapeSegmentsIfNecessary(delimiter: Char = '.') = split(delimiter)
.filter { it.isNotEmpty() }
.joinToString(delimiter.toString()) { it.escapeIfNecessary() }

internal expect inline fun <reified E : Enum<E>> enumSetOf(vararg values: E): MutableSet<E>

internal expect fun Char.isJavaIdentifierStart(): Boolean

internal expect fun Char.isJavaIdentifierPart(): Boolean
20 changes: 20 additions & 0 deletions kotlinpoet/src/jsMain/kotlin/com/squareup/kotlinpoet/Util.js.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* Copyright (C) 2024 Square, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.squareup.kotlinpoet

private val IDENTIFIER_REGEX = IDENTIFIER_REGEX_VALUE.toRegex()

internal actual val String.isIdentifier: Boolean get() = IDENTIFIER_REGEX.matches(this)
87 changes: 87 additions & 0 deletions kotlinpoet/src/jvmMain/kotlin/com/squareup/kotlinpoet/Util.jvm.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/*
* Copyright (C) 2024 Square, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.squareup.kotlinpoet

import com.squareup.kotlinpoet.CodeBlock.Companion.isPlaceholder
import java.util.Collections
import java.util.EnumSet

internal actual fun <K, V> Map<K, V>.toImmutableMap(): Map<K, V> =
Collections.unmodifiableMap(LinkedHashMap(this))

internal actual fun <T> Collection<T>.toImmutableList(): List<T> =
Collections.unmodifiableList(ArrayList(this))

internal actual fun <T> Collection<T>.toImmutableSet(): Set<T> =
Collections.unmodifiableSet(LinkedHashSet(this))

internal actual fun formatIsoControlCode(code: Int): String =
String.format("\\u%04x", code)

internal actual fun Int.toHexStr(): String =
Integer.toHexString(this)

// TODO Waiting for `CodeBlock` migration.
internal fun CodeBlock.ensureEndsWithNewLine() = trimTrailingNewLine('\n')

// TODO Waiting for `CodeBlock` migration.
internal fun CodeBlock.trimTrailingNewLine(replaceWith: Char? = null) = if (isEmpty()) {
this
} else {
with(toBuilder()) {
val lastFormatPart = trim().formatParts.last()
if (lastFormatPart.isPlaceholder && args.isNotEmpty()) {
val lastArg = args.last()
if (lastArg is String) {
val trimmedArg = lastArg.trimEnd('\n')
args[args.size - 1] = if (replaceWith != null) {
trimmedArg + replaceWith
} else {
trimmedArg
}
}
} else {
formatParts[formatParts.lastIndexOf(lastFormatPart)] = lastFormatPart.trimEnd('\n')
if (replaceWith != null) {
formatParts += "$replaceWith"
}
}
return@with build()
}
}

private val IDENTIFIER_REGEX = IDENTIFIER_REGEX_VALUE.toRegex()

internal actual val String.isIdentifier: Boolean
get() = IDENTIFIER_REGEX.matches(this)

internal actual inline fun <reified E : Enum<E>> enumSetOf(vararg values: E): MutableSet<E> {
return when (values.size) {
0 -> EnumSet.noneOf(E::class.java)
1 -> EnumSet.of(values[0])
2 -> EnumSet.of(values[0], values[1])
3 -> EnumSet.of(values[0], values[1], values[2])
4 -> EnumSet.of(values[0], values[1], values[2], values[3])
5 -> EnumSet.of(values[0], values[1], values[2], values[3], values[4])
else -> EnumSet.copyOf(values.toSet())
}
}
Egorand marked this conversation as resolved.
Show resolved Hide resolved

internal actual fun Char.isJavaIdentifierStart(): Boolean =
Character.isJavaIdentifierStart(this)

internal actual fun Char.isJavaIdentifierPart(): Boolean =
Character.isJavaIdentifierPart(this)
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/*
* Copyright (C) 2024 Square, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.squareup.kotlinpoet

internal actual fun <K, V> Map<K, V>.toImmutableMap(): Map<K, V> =
toMap()

internal actual fun <T> Collection<T>.toImmutableList(): List<T> =
toList()

internal actual fun <T> Collection<T>.toImmutableSet(): Set<T> =
toSet()
Comment on lines +18 to +25
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're not going to put these in an unmodifiable wrapper on other targets, do we still need to do it on JVM? Are we really worried about Java callers mutating things? I don't think I am.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original implementation was copied when we forked from JavaPoet, for what it's worth.

Copy link
Contributor Author

@ForteScarlet ForteScarlet Oct 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, you have a point. If we no longer care about calls that mutate things on the JVM, do we keep toImmutable* in common and have its result just return a to* (like toSet), or do we simply remove them and use to* instead where appropriate?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's defending against something that is not a real problem in practice, and I would eliminate the concept entirely and use Kotlin's collection APIs directly everywhere. This should probably be done in its own PR rather than cluttering this one.


internal actual fun formatIsoControlCode(code: Int): String {
return buildString(6) {
append("\\u")
appendFormat04x(code)
}
}

@OptIn(ExperimentalStdlibApi::class)
private val HexFormatWithoutLeadingZeros = HexFormat {
number {
removeLeadingZeros = true
}
}

@OptIn(ExperimentalStdlibApi::class)
internal fun Appendable.appendFormat04x(code: Int) {
val hex = code.toHexString(HexFormatWithoutLeadingZeros)
if (hex.length < 4) {
repeat(4 - hex.length) { append('0') }
}
append(hex)
}
Egorand marked this conversation as resolved.
Show resolved Hide resolved

@OptIn(ExperimentalStdlibApi::class)
internal actual fun Int.toHexStr(): String =
toHexString(HexFormatWithoutLeadingZeros)
Egorand marked this conversation as resolved.
Show resolved Hide resolved

internal actual inline fun <reified E : Enum<E>> enumSetOf(vararg values: E): MutableSet<E> =
values.toMutableSet()

internal actual fun Char.isJavaIdentifierStart(): Boolean {
return isLetter() ||
this in CharCategory.LETTER_NUMBER ||
this == '$' ||
this == '_'
}

internal actual fun Char.isJavaIdentifierPart(): Boolean {
// TODO
// A character may be part of a Java identifier if any of the following conditions are true:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we maybe extract these checks into common to have consistent implementations on all platforms?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be done in later PRs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, I am dividing them because I am unsure if the current implementation of non-JVM is the same as the one in JVM.
It would be wonderful to merge them if we can confirm that they have the same effects!

// - it is a letter
// - it is a currency symbol (such as '$')
// - it is a connecting punctuation character (such as '_')
// - it is a digit
// - it is a numeric letter (such as a Roman numeral character)
// - it is a combining mark
// - it is a non-spacing mark
// isIdentifierIgnorable returns true for the character.
// Also missing here:
// - a combining mark
return isLetter() ||
isDigit() ||
this in CharCategory.LETTER_NUMBER ||
this in CharCategory.NON_SPACING_MARK ||
this == '_' ||
this == '$' ||
isIdentifierIgnorable()
//
}

internal fun Char.isIdentifierIgnorable(): Boolean {
// The following Unicode characters are ignorable in a Java identifier or a Unicode identifier:
// - ISO control characters that are not whitespace
// - '\u0000' through '\u0008'
// - '\u000E' through '\u001B'
// - '\u007F' through '\u009F'
// - all characters that have the FORMAT general category value
return (
isISOControl() && (
this in '\u0000'..'\u0008' ||
this in '\u000E'..'\u001B' ||
this in '\u007F'..'\u009F'
)
) || this in CharCategory.FORMAT
}
Loading