IssueTracker

Hi, I recently did a cleanup for Text related classes. Can you please ping me if you find a Text related public data class? b/172252363

There is only one remaining data class in text module, AnnotatedString.Range<T> (AnnotatedString.kt line 222). The 45 others are elsewhere in Compose.

Are data classes something we never want to use in public API? Is it worth filing a feature request for Metalava to throw an error if you try and use one?

Not sure other people's opinion, but I'm thinking it probably would make sense to simply lint/metalava-ban it, yes. In some rare cases there is a deliberate intention to support destructuring as a recommended API usage, such as the old val (value, setValue) = state { 0 } API. But if so, that can be done using explicit component1, component2 functions instead (that's clearer about API intent and also doesn't bring in the equals implementation which might not be needed).

I get the impression the majority of the time people use "data" keyword basically just for the explanatory value of "this is Plain Old Data like a C struct: I intend to keep this simple and to make copies of it" which is a fine enough attitude in an internal/private class, but not appropriate for a public API.

Hmm. I notice when attempting to remove data keyword from ProgressBarRangeInfo, one of the ProgressSemanticsTest fails because the test matchers use ==. When data is removed, then == silently has the same meaning as === in Kotlin.

So it is hard to statically determine whether there are any uses of == with a data class, and a systematic removal may introduce bugs which may not all be caught by tests. I guess the safe way to remove data is to make sure we also add equals to all of them at the same time. (As I understand it, the other methods: hashCode, toString, componentN, and copy would all cause a compile error if something relies on them, so we don't need to worry about those as much.)