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

Consider not using runtimeType #85

Open
jonasfj opened this issue Jun 10, 2022 · 2 comments
Open

Consider not using runtimeType #85

jonasfj opened this issue Jun 10, 2022 · 2 comments
Labels
contributions-welcome Contributions welcome to help resolve this (the resolution is expected to be clear from the issue) type-enhancement A request for a change that isn't a bug

Comments

@jonasfj
Copy link
Member

jonasfj commented Jun 10, 2022

This causes code-size warnings when compiling to js:

Hint: Using '.runtimeType.toString()' causes the compiler to generate more code because it needs to preserve type arguments on generic classes, even if they are not necessary elsewhere.
If used only for debugging, consider using option --lax-runtime-type-to-string to reduce the code size impact.

And we only use it in:

@override
String toString() => '<$runtimeType: from $start to $end "$text">';

Maybe we can just change it to:

String toString() => '<SourceSpan: from $start to $end "$text">';

And have subclasses overwrite toString to ensure that they keep their name.

@jonasfj jonasfj added the type-enhancement A request for a change that isn't a bug label Jun 10, 2022
@devoncarew
Copy link
Member

This sounds reasonable to me; I doubt it's critical that the toString() method identify the exact subclass of the sourcespan.

@devoncarew devoncarew added the contributions-welcome Contributions welcome to help resolve this (the resolution is expected to be clear from the issue) label Jun 10, 2022
@devoncarew
Copy link
Member

For posterity, the lint we want to enable to catch violations here is no_runtimeType_toString (that lint does have an issue where it doesn't report all violations: dart-lang/linter#3635).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributions-welcome Contributions welcome to help resolve this (the resolution is expected to be clear from the issue) type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants