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

Disallow overriding of runtimeType #4283

Open
jakemac53 opened this issue Mar 4, 2025 · 13 comments
Open

Disallow overriding of runtimeType #4283

jakemac53 opened this issue Mar 4, 2025 · 13 comments
Labels
feature Proposed language feature that solves one or more problems

Comments

@jakemac53
Copy link
Contributor

We should consider blocking overrides of runtimeType, as a language versioned change so it can be considered non-breaking. Existing code will work until the language version they are currently on is no longer supported, which is anyways breaking.

Is there any valid use case for overriding runtimeType?

There was some previous discussion here #3357, but it feels deserving of its own issue.

@jakemac53 jakemac53 added the feature Proposed language feature that solves one or more problems label Mar 4, 2025
@rrousselGit
Copy link

I once investigating overriding runtimeType for the sake of controlling Flutter's InheritedWidgets ; because they are a Map<Type, Object?>.

I didn't go with it in the end though.

@tatumizer
Copy link

tatumizer commented Mar 4, 2025

The original discussion dates back to 2013: https://groups.google.com/u/1/a/dartlang.org/g/misc/c/XZbhI1N9Ads/m/HQtmawfJDB0J
Quote:

The reason you need to be able to override runtimeType is so that you can abstract from implementation details. If you define a proxy for another object, you'd like to make it behave as close to the proxied object as possible. You make sure it forwards all methods to its target, you make sure its runtimeType is a proxy as well, you make sure it implements the desired interface and you make sure you do not expose the Proxy type itself, so you can't use type tests or casts to discover it is a proxy. Hopefully we have not missed anything. Of course, if you have access to reflection, you can still bypass all this - but you really have to go out of your way.

@jakemac53
Copy link
Contributor Author

... yeah I only think we should disallow this more now 🤣

@mateusfccp
Copy link
Contributor

mateusfccp commented Mar 4, 2025

I had this case in a Flutter project which required overriding Flutter's Widget.canUpdate.

I opened an issue (flutter/flutter#146504), but it was rejected, so I still rely on overriding runtimeType.

@lrhn
Copy link
Member

lrhn commented Mar 4, 2025

It's used internally to make Smi return int and OneByteString return String.

Other classes can choose to hide implementation classes in the same way.

But why?
What is the purpose of disallowing overriding runtimeType?

That only makes sense if someone uses runtimeType for something which requires it to be precise, but that's also exactly the reason why someone else will want to control it.

If we could require runtimeType to only return a supertype of the actual runtime type, then it might still be both trustworthy enough and flexible enough to cover the current use cases and be used for whatever use would demand not overriding it.
But when all a Type object can do is ==, it's still too simplistic to actually solve any problem properly in a language based on subtype substitutability.

Personally I'd rather remove runtimeType entirely than double down on making it look more useful than it actually is. (I maintain that any actual use of runtimeType is a design mistake. It's a hack that fails to properly achieve what it actually wants.)

@feinstein
Copy link

Maybe make it a lint warning instead and let people be able to do it if they can't find another way to solve their issue at hand?

I also feel this is ugly, but if it's not breaking anything in the language, or is a big known source of bugs, then I would advise to only have it as a lint and add this lint to the main lint package that ship with Flutter.

@jakemac53
Copy link
Contributor Author

It just seems massively hacky to do anything to this other than return the actual runtime type of the object. I can't possibly see any actual good coming from overriding it?

At best it might hack around something that would be better solved another way... and its going to create some very surprising bugs in who knows what other code.

@rrousselGit
Copy link

Not overriding it certainly leads to better debuggability IMO.

I personally quite enjoy how obj.runtimeType tells me which private subclass of an interface is used.
I've used it to dig inside analyzer's or the Dart SDK's source.

@tatumizer
Copy link

github search shows quite a few overrides of runtimeType in dart sdk, Pretty common in js_runtime.
https://github.com/search?q=repo%3Adart-lang%2Fsdk%20%22runtimeType%20%3D%3E%22&type=code
(I searched for "runtimeType =>").
None is found in flutter. (the search is not precise though).

@lrhn
Copy link
Member

lrhn commented Mar 5, 2025

It just seems massively hacky to do anything to this other than return the actual runtime type of the object. I can't possibly see any actual good coming from overriding it?

And yet we already do so ourselves. Something must be flawed in that conclusion.

Can anything bad come from overriding it?
(Only if you use Type objects for anything other than debug printing, which you shouldn't if you're not using dart:mirrors. And stil only if someone is being deliberately obstructive about it.)

In our own case the good that does come out of overriding it is to hide a private (and platform-specific) subclass from users.

Without that int x = ...; assert(x.runtimeType == (x+1).runtimeType); could fail.
And if (s.runtimeType == int) (or == String) would never be true, because int and String are abstract interfaces, not concrete classes, and the actual implementation classes are platform dependent and totally private, so you can't say s.runtimeType == _Smi.)

If the SDK couldn't do that, that would make the runtimeType more useless, not less.

So there is a use for overriding. And the SDK isn't the only libraries having hidden implementation types.
That doesn't mean that you should just have any class return int as runtimeType.
The actually "useful" cases are those where you return a supertype of the real runtime type, a type which the object still implements, even if it's not its actual runtime type.

@jakemac53
Copy link
Contributor Author

And yet we already do so ourselves. Something must be flawed in that conclusion.

The existence of code has no bearing on its quality. Are there examples that you would defend as good code?

Without that int x = ...; assert(x.runtimeType == (x+1).runtimeType); could fail.

And that is perfectly fine, what is the point of that code if not to compare the actual runtime type? The assertion failing here would be a feature.

(Only if you use Type objects for anything other than debug printing, which you shouldn't if you're not using dart:mirrors. And stil only if someone is being deliberately obstructive about it.)

It is also harmful for debug printing, it makes it hard to figure out what a thing actually is.

@jakemac53
Copy link
Contributor Author

And if (s.runtimeType == int) (or == String) would never be true, because int and String are abstract interfaces, not concrete classes, and the actual implementation classes are platform dependent and totally private, so you can't say s.runtimeType == _Smi.)

And that is why type checks exist and should be used for this purpose.

@lrhn
Copy link
Member

lrhn commented Mar 6, 2025

And that is why type checks exist and should be used for this purpose.

What purpose should runtimeType be used for?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Proposed language feature that solves one or more problems
Projects
None yet
Development

No branches or pull requests

6 participants