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

Eliminates the useless type property from ViewEnvironmentKey. #999

Merged
merged 2 commits into from
May 16, 2023
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -31,7 +31,7 @@ enum class OverviewDetailConfig {
Single;

@OptIn(WorkflowUiExperimentalApi::class)
companion object : ViewEnvironmentKey<OverviewDetailConfig>(OverviewDetailConfig::class) {
companion object : ViewEnvironmentKey<OverviewDetailConfig>() {
override val default = None
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ internal class LegacyPreviewViewFactoryTest {
)
}

object TestEnvironmentKey : ViewEnvironmentKey<String>(String::class) {
object TestEnvironmentKey : ViewEnvironmentKey<String>() {
override val default: String get() = error("Not specified")
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ internal class PreviewViewFactoryTest {
)
}

object TestEnvironmentKey : ViewEnvironmentKey<String>(String::class) {
object TestEnvironmentKey : ViewEnvironmentKey<String>() {
override val default: String get() = error("Not specified")
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ internal class ComposeViewFactoryTest {
}

@Test fun getsViewEnvironmentUpdates() {
val testEnvironmentKey = object : ViewEnvironmentKey<String>(String::class) {
val testEnvironmentKey = object : ViewEnvironmentKey<String>() {
override val default: String get() = error("No default")
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ internal class LegacyComposeViewFactoryTest {
}

@Test fun getsViewEnvironmentUpdates() {
val testEnvironmentKey = object : ViewEnvironmentKey<String>(String::class) {
val testEnvironmentKey = object : ViewEnvironmentKey<String>() {
override val default: String get() = error("No default")
}

Expand Down
18 changes: 0 additions & 18 deletions workflow-ui/core-android/api/core-android.api
Original file line number Diff line number Diff line change
Expand Up @@ -334,24 +334,6 @@ public final class com/squareup/workflow1/ui/container/BackButtonScreen : com/sq
public fun map (Lkotlin/jvm/functions/Function1;)Lcom/squareup/workflow1/ui/container/BackButtonScreen;
}

public final class com/squareup/workflow1/ui/container/BackStackConfig : java/lang/Enum {
public static final field Companion Lcom/squareup/workflow1/ui/container/BackStackConfig$Companion;
public static final field First Lcom/squareup/workflow1/ui/container/BackStackConfig;
public static final field None Lcom/squareup/workflow1/ui/container/BackStackConfig;
public static final field Other Lcom/squareup/workflow1/ui/container/BackStackConfig;
public static fun valueOf (Ljava/lang/String;)Lcom/squareup/workflow1/ui/container/BackStackConfig;
public static fun values ()[Lcom/squareup/workflow1/ui/container/BackStackConfig;
}

public final class com/squareup/workflow1/ui/container/BackStackConfig$Companion : com/squareup/workflow1/ui/ViewEnvironmentKey {
public fun getDefault ()Lcom/squareup/workflow1/ui/container/BackStackConfig;
public synthetic fun getDefault ()Ljava/lang/Object;
}

public final class com/squareup/workflow1/ui/container/BackStackConfigKt {
public static final fun plus (Lcom/squareup/workflow1/ui/ViewEnvironment;Lcom/squareup/workflow1/ui/container/BackStackConfig;)Lcom/squareup/workflow1/ui/ViewEnvironment;
}

public class com/squareup/workflow1/ui/container/BackStackContainer : android/widget/FrameLayout {
public fun <init> (Landroid/content/Context;)V
public fun <init> (Landroid/content/Context;Landroid/util/AttributeSet;)V
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ internal class DecorativeViewFactoryTest {
}
}

val envString = object : ViewEnvironmentKey<String>(String::class) {
val envString = object : ViewEnvironmentKey<String>() {
override val default: String get() = "Not set"
}

Expand Down Expand Up @@ -120,7 +120,7 @@ internal class DecorativeViewFactoryTest {
}
}

val envString = object : ViewEnvironmentKey<String>(String::class) {
val envString = object : ViewEnvironmentKey<String>() {
override val default: String get() = "Not set"
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,7 @@ public interface ScreenViewFactoryFinder {
)
}

public companion object : ViewEnvironmentKey<ScreenViewFactoryFinder>(
ScreenViewFactoryFinder::class
) {
public companion object : ViewEnvironmentKey<ScreenViewFactoryFinder>() {
override val default: ScreenViewFactoryFinder
get() = object : ScreenViewFactoryFinder {}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@ import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
* for the `@StyleRes themeResId: Int` argument of `AlertDialog.Builder()`.
*/
@WorkflowUiExperimentalApi
public object AlertDialogThemeResId : ViewEnvironmentKey<Int>(type = Int::class) {
public object AlertDialogThemeResId : ViewEnvironmentKey<Int>() {
override val default: Int = 0
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@ import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
* https://stackoverflow.com/questions/2886407/dealing-with-rapid-tapping-on-buttons
*/
@WorkflowUiExperimentalApi
internal object CoveredByModal : ViewEnvironmentKey<Boolean>(type = Boolean::class) {
internal object CoveredByModal : ViewEnvironmentKey<Boolean>() {
override val default: Boolean = false
}
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ internal class DialogCollator {
")"
}

companion object : ViewEnvironmentKey<DialogCollator>(DialogCollator::class) {
companion object : ViewEnvironmentKey<DialogCollator>() {
override val default: DialogCollator
get() = error("Call ViewEnvironment.establishDialogCollator first.")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import kotlinx.coroutines.flow.StateFlow
internal class OverlayArea(
val bounds: StateFlow<Rect>
) {
companion object : ViewEnvironmentKey<OverlayArea>(type = OverlayArea::class) {
companion object : ViewEnvironmentKey<OverlayArea>() {
override val default: OverlayArea = OverlayArea(MutableStateFlow(Rect()))
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,7 @@ public interface OverlayDialogFactoryFinder {
)
}

public companion object : ViewEnvironmentKey<OverlayDialogFactoryFinder>(
OverlayDialogFactoryFinder::class
) {
public companion object : ViewEnvironmentKey<OverlayDialogFactoryFinder>() {
override val default: OverlayDialogFactoryFinder = object : OverlayDialogFactoryFinder {}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ internal fun mockView(): View {
}
}

internal object SomeEnvValue : ViewEnvironmentKey<String>(String::class) {
internal object SomeEnvValue : ViewEnvironmentKey<String>() {
override val default: String get() = error("Unset")
}

Expand Down
20 changes: 19 additions & 1 deletion workflow-ui/core-common/api/core-common.api
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,12 @@ public final class com/squareup/workflow1/ui/ViewEnvironment$Companion {
}

public abstract class com/squareup/workflow1/ui/ViewEnvironmentKey {
public fun <init> ()V
public fun <init> (Lkotlin/reflect/KClass;)V
public fun combine (Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;
public final fun equals (Ljava/lang/Object;)Z
public abstract fun getDefault ()Ljava/lang/Object;
public final fun hashCode ()I
public final fun toString ()Ljava/lang/String;
}

public abstract interface class com/squareup/workflow1/ui/ViewRegistry {
Expand Down Expand Up @@ -194,6 +194,24 @@ public final class com/squareup/workflow1/ui/container/AlertOverlay$Event$Cancel
public static final field INSTANCE Lcom/squareup/workflow1/ui/container/AlertOverlay$Event$Canceled;
}

public final class com/squareup/workflow1/ui/container/BackStackConfig : java/lang/Enum {
public static final field Companion Lcom/squareup/workflow1/ui/container/BackStackConfig$Companion;
public static final field First Lcom/squareup/workflow1/ui/container/BackStackConfig;
public static final field None Lcom/squareup/workflow1/ui/container/BackStackConfig;
public static final field Other Lcom/squareup/workflow1/ui/container/BackStackConfig;
public static fun valueOf (Ljava/lang/String;)Lcom/squareup/workflow1/ui/container/BackStackConfig;
public static fun values ()[Lcom/squareup/workflow1/ui/container/BackStackConfig;
}

public final class com/squareup/workflow1/ui/container/BackStackConfig$Companion : com/squareup/workflow1/ui/ViewEnvironmentKey {
public fun getDefault ()Lcom/squareup/workflow1/ui/container/BackStackConfig;
public synthetic fun getDefault ()Ljava/lang/Object;
}

public final class com/squareup/workflow1/ui/container/BackStackConfigKt {
public static final fun plus (Lcom/squareup/workflow1/ui/ViewEnvironment;Lcom/squareup/workflow1/ui/container/BackStackConfig;)Lcom/squareup/workflow1/ui/ViewEnvironment;
}

public final class com/squareup/workflow1/ui/container/BackStackScreen : com/squareup/workflow1/ui/Container, com/squareup/workflow1/ui/Screen {
public fun <init> (Lcom/squareup/workflow1/ui/Screen;Ljava/util/List;)V
public fun <init> (Lcom/squareup/workflow1/ui/Screen;[Lcom/squareup/workflow1/ui/Screen;)V
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,23 @@ constructor(
}

/**
* Defines a value that can be provided by a [ViewEnvironment] map, specifying its [type]
* and [default] value.
* Defines a value type [T] that can be provided by a [ViewEnvironment] map,
* and specifies its [default] value.
*
* It is hard to imagine a useful implementation of this that is not a Kotlin `object`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we make a lint rule to enforce this convention?

* Preferred use is to have the `companion object` of [T] extend this class. See
* [BackStackConfig.Companion][com.squareup.workflow1.ui.container.BackStackConfig.Companion]
* for an example.
*/
@WorkflowUiExperimentalApi
public abstract class ViewEnvironmentKey<T : Any>(
private val type: KClass<T>
) {
public abstract class ViewEnvironmentKey<T : Any>() {
@Deprecated("Use no args constructor", ReplaceWith("ViewEnvironmentKey<T>()"))
public constructor(@Suppress("UNUSED_PARAMETER") type: KClass<T>) : this()
0legg marked this conversation as resolved.
Show resolved Hide resolved

/**
* Defines the default value for this key. It is a grievous error for this value to be
* dynamic in any way.
*/
public abstract val default: T

/**
Expand All @@ -80,15 +90,9 @@ public abstract class ViewEnvironmentKey<T : Any>(
right: T
): T = right

final override fun equals(other: Any?): Boolean = when {
this === other -> true
other != null && this::class != other::class -> false
else -> type == (other as ViewEnvironmentKey<*>).type
final override fun equals(other: Any?): Boolean {
return this === other || (other != null && this::class == other::class)
}

final override fun hashCode(): Int = type.hashCode()

final override fun toString(): String {
return "${this::class.simpleName}(${type.simpleName})"
}
final override fun hashCode(): Int = this::class.hashCode()
}
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public interface ViewRegistry {
renderingType: KClass<out RenderingT>
): Entry<RenderingT>?

public companion object : ViewEnvironmentKey<ViewRegistry>(ViewRegistry::class) {
public companion object : ViewEnvironmentKey<ViewRegistry>() {
override val default: ViewRegistry get() = ViewRegistry()
override fun combine(
left: ViewRegistry,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ import com.squareup.workflow1.ui.container.BackStackConfig.First
import com.squareup.workflow1.ui.container.BackStackConfig.Other

/**
* Informs views whether they're children of a [BackStackContainer],
* Informs views whether they're children of a [BackStackScreen],
* and if so whether they're the [first frame][First] or [not][Other].
*/
@WorkflowUiExperimentalApi
public enum class BackStackConfig {
/**
* There is no [BackStackContainer] above here.
* There is no [BackStackScreen] above here.
*/
None,

Expand All @@ -29,7 +29,7 @@ public enum class BackStackConfig {
*/
Other;

public companion object : ViewEnvironmentKey<BackStackConfig>(BackStackConfig::class) {
public companion object : ViewEnvironmentKey<BackStackConfig>() {
override val default: BackStackConfig = None
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,19 @@ import org.junit.Test

@OptIn(WorkflowUiExperimentalApi::class)
internal class ViewEnvironmentTest {
private object StringHint : ViewEnvironmentKey<String>(String::class) {
private object StringHint : ViewEnvironmentKey<String>() {
override val default = ""
}

private object OtherStringHint : ViewEnvironmentKey<String>(String::class) {
private object OtherStringHint : ViewEnvironmentKey<String>() {
override val default = ""
}

private data class DataHint(
val int: Int = -1,
val string: String = ""
) {
companion object : ViewEnvironmentKey<DataHint>(DataHint::class) {
companion object : ViewEnvironmentKey<DataHint>() {
override val default = DataHint()
}
}
Expand Down Expand Up @@ -101,7 +101,7 @@ internal class ViewEnvironmentTest {
}

@Test fun `honors combine`() {
val combiningHint = object : ViewEnvironmentKey<String>(String::class) {
val combiningHint = object : ViewEnvironmentKey<String>() {
override val default: String
get() = error("")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ internal class EnvironmentScreenTest {
) : ViewRegistry.Entry<T>

private data class TestValue(val value: String) {
companion object : ViewEnvironmentKey<TestValue>(TestValue::class) {
companion object : ViewEnvironmentKey<TestValue>() {
override val default: TestValue get() = error("Set a default")
}
}
Expand Down