Hey @kszczek I think where you have 'null' in the fix yaml, it should be ''. In dart fix I don't think it will evaluate these arguments as null, they will just be blank.

Also, I am not sure some of this is written in the way we expect the code to be

 - if: "height == 'null'"
        changes:
          - kind: "removeParameter"
            name: "height"

Does this mean if no height is provided, we should remove height? I think maybe the if here should be height != '', to remove it if it is provided in the users code.
Although, this would also be incorrect since is it oneOf. That would mean the fix would apply only one if and then ignore the rest.

Hi, thanks for taking a look!

I think where you have 'null' in the fix yaml, it should be ''. In dart fix I don't think it will evaluate these arguments as null, they will just be blank.

Yes, when the height parameter is missing then it will be reported as blank.

Does this mean if no height is provided, we should remove height? I think maybe the if here should be height != '', to remove it if it is provided in the users code.
Although, this would also be incorrect since is it oneOf. That would mean the fix would apply only one if and then ignore the rest.

There are three scenarios in which we want to take action:

1. if: "height == 'null'" - this condition is not meant to represent "if height was not provided", but rather "user explicitly placed a height: null parameter". This is quite unlikely though, as null is the default value anyway so setting it to null explicitly is basically a no-op, but in case someone has this dead code then let's clean up by simply removing it.

Now the ordering of those fixes is important, because if the first condition was not met, then in the remaining two conditionals we can assume that height is either not present or not null.

2. if: "constraints == '' && height != ''" - constraints are not provided, but height is and we know it's not null. In this case we simply add constraints and remove height.
3. if: "constraints == 'null' && height != ''" - user explicitly set constraints: null, and set some non-null height. In this case, we first remove the constraints: null, add constraints with the height's value, and remove height.

Let me know if I unnecessarily overcomplicated this fix 😄

It turns out I forgot to mark the height parameter of the TooltipThemeData.copyWith as deprecated and that is the reason why dart fix wouldn't make the substitution.

This is now properly ready for review.

Also - a few of the existing tests use the height parameter. Should I refactor those tests in this PR along with the deprecation, or is this something that should be done when the parameter is going to be removed?

Also - a few of the existing tests use the height parameter. Should I refactor those tests in this PR along with the deprecation, or is this something that should be done when the parameter is going to be removed?

No, having them in place during the deprecation period ensures we do not break anyone during the deprecation period. :)

@Piinks @chunhtai Seems like comments have been addressed. Can you take a look?