"Inline Method" fails with type params

[DanTup]

void test<T>(T a, T b) {
  print(a);
  print(b);
}
void f() {
  test(1, 2);
}

Trying to inline this method results in "No argument for the parameter "a"., type: 1". Removing <T> "fixes" it.

[DanTup]

@bwilkerson I'm investigating why the Inline Method refactor doesn't work correctly when the method has type args (No argument for the parameter "a".). It seems to fail because code in the refactor doesn't match up the arguments with the parameters correctly.

The code compares arg.staticParameterElement == parameter (see highlighted line below), but when there are type parameters these do not match (arg.staticParameterElement is a ParameterMember and not the ParameterElement).

ParameterMember has a .declaration which looks similar (ParameterElementImpl (T a)), but it's not the same instance. The one in parameter is fully populated, but the one in the ParameterMember is not (it's synthetic and looks a bit like a placeholder.. it has -1 for nameOffset for example).

I'm not sure whether a) my code is bad and something is not resolving correctly, b) this is a bug and that parameter declaration should be non-synthetic and match, or c) this is expected and the refactor should match the parameters in a different way (for example comparing the names?).

Some tests and comments are here if it helps:

https://dart-review.googlesource.com/c/sdk/+/260580/

[bwilkerson]

@scheglov

[scheglov]

Yes, we have issues around the way generic method invocation resolution is implemented. IIRC we get the generic FunctionType, eventually instantiate it, and use ParameterElements of this new instantiated FunctionType to bind arguments. This causes a few issues, and a few exceptions.

Maybe we should do what we did for RecordType, and FunctionType should not pretend that it has ParameterElements, it should have positional and named formal parameter... types?

This will not fix this immediate issue though.

We probably should instantiate corresponding ExecutableElement, with its ParameterElements, and use these for resolution. Relatedly, calling these MethodMember and FunctionMember might be not the best. FunctionElement is not a member of a ClassElement. And, for this issue, it does not even have MapSubstitution substitution, only bool isLegacy.

[DanTup]

I don't understand this well enough to know what the scale of that work is - is it significant/breaking? (and/or something I can do without significant hand-holding?).

Out of curiosity, I did try changing it to just compare based on name, and it passes the tests:

https://dart-review.googlesource.com/c/sdk/+/260580/2/pkg/analysis_server/lib/src/services/refactoring/legacy/inline_method.dart

I can't think of any reasons why this wouldn't be sound, but I'm not certain there aren't any :-)

[scheglov]

Yes, this looks as a relatively big piece of work.

Your solution looks good, I left a couple of comments.

[DanTup]

Fixed by dart-lang/sdk@813aa69.