[Dart Fix] Proposal: expose the static type of a `fragment`

[LongCatIsLooong]

I'm writing a dart fix that migrates a constructor argument from a double? to a TextScaler?, the migrated code is hard to read in some cases.

Original Code:

Text(textScaleFactor: scaleFactor) where scaleFactor is an expression that can either be a double?, double, or null.

Desired migrated code:

if scaleFactor's static type is double? => Text(textScaler: switch (scaleFactor) { null => null, final factor => TextScaler.linear(factor), }) if scaleFactor's static type is double => Text(textScaler: TextScaler.linear(scaleFactor) if scaleFactor's static type is null => Text()

Current migrated code:

Text(textScaler: switch (scaleFactor) { null => null, final factor => TextScaler.linear(factor), })

When scaleFactor is a double the current code is a lot harder to read than necessary, e.g.:

Text(textScaler: switch (2.0) { null => null, final factor => TextScaler.linear(factor), })

[bwilkerson]

Supporting arbitrary type checks would require performing resolution of code snippets (type annotations) taken from the fix_data.yaml file. I'm not sure how feasible that is.

However, I doubt very much that you're likely to encounter an expression of type Null except when the expression is the null literal, and I believe we can check that case easily enough today.

That just leaves the question of whether the type is nullable, which seems like an easier thing to support.

The question then is whether transform authors need full control when the value is nullable or whether they just need to direct the tool how to deal with null values and let the tool generate the boilerplate to handle them. My preference would be the latter, so that if, in a future version of Dart, we get a better syntax for dealing with this kind of situation, we can change the tool and transform authors don't have to update their transforms.

[LongCatIsLooong]

whether the type is nullable which seems like an easier thing to support.

Yeah that works for my use case. Out of curiosity, would it be as easy to support if the argument's type is a sealed class instead of double?

[bwilkerson]

I don't think sealed would be a factor, but I might be missing something.

Let me explain my thoughts more to see whether my conclusion makes sense. I'm guessing that the transform would have a variable like scaleFactor whose value is a fragment representing the value of the argument named textScaleFactor and then we'd allow a condition of the form scaleFactor.type == 'double'. That would kind of work, but relies on the textual representation of a type being unique, which isn't a valid assumption. (It's probably fine for double because very few developers are likely to define a type with that name, but there have been plenty of other name clashes in code.) In order to do better we'd have to know the scope in which to look up the name, and that gets hard. Having the class be sealed wouldn't really make the lookup any easier.

[LongCatIsLooong]

Ah thanks for the explanation. What I meant was if one day someone would like to write a transform that migrate an API from a sealed class to something else, they would have to do this: switch(fragment) { Square(..) =>, Circle(...), }, and that's a bit too verbose if the static type of the fragment is Square?

[bwilkerson]

I agree; I wouldn't want to see the longer form when it isn't necessary (and it would probably produce code with warnings because some of the branches are dead code). It's just a question of how feasible it is to support this kind of change.

I think there will always be transforms that can't reasonably be encoded in the fix_data.yaml file because of their complexity. I also think that it's perfectly reasonable for us to use a more appropriate mechanism to provide those fixes when we need to do so.