Add API to match function parameters to arguments at call sites

[Steffeeen]

Resolves #2047

API Design

The API currently looks like this:

let call: FunctionCallExprSyntax = ...
let parameters: FunctionParameterListSyntax = ...
let matchResult = call.matchArguments(to: parameters)
guard let matchResult else { return }

let parameterMatch: ParameterMatch = matchResult.match(for: parameters[0])
let parameterMatchByIndex: ParameterMatch? = matchResult.matchForParameter(at: 3)
let parameterMatchByName: ParameterMatch? = matchResult.matchForParameter(named: "a")

let arguments: [any SyntaxProtocol] = parameterMatch.arguments

I'm looking for feedback on the API. I'm especially interested in feedback on the following points:

• The API currently expects the user to provide the correct function declaration that matches the call. Otherwise, the matching may fail. However, this is not explicitly checked. It would be better to fail fast in case the function declaration does not match, but I'm not sure that's possible without attempting to match. We could accept the FunctionDeclSyntax instead of the FunctionParameterListSyntax and verify that the name matches the name of the call, but that does not guarantee that the matching will succeed.

• The matching only works correctly if the call is actually valid. However, we can only check that the call and declaration are syntactically valid but not if they are semantically valid. The API again expects the user to only provide valid combinations of declarations and calls.

• I don't like that the arguments array in ParameterMatch has the type [any SyntaxProtocol]. The main problem with making this nicer is that the arguments to variadic closure parameters may be split across the "normal" arguments and the trailing closure. Example:

func test(_ cs: () -> Int...) {
    for c in cs {
        print(c())
    }
}

test({ 1 }) { 2 }

• The matchForParameter(named) method expects the name to be the parameter name rather than the argument label. This is only communicated in the documentation of the method.

• I'm also not sure if the API should be located in SwiftRefactor as the original issue explicitly mentions using it in macros.

Validating correctness

I added extensive unit tests to validate that the matching works correctly. However, the main tool used for verifying the correctness is ArgumentMatchingExhaustiveTests.swift. It generates an exhaustive list of function declarations together with function calls. It generates different variations based on the number of parameters, whether a parameter is labeled/unlabeled, whether it is variadic, whether it has a default value, etc. An example for a generated function declaration with its calls:

func decl_closure_144(a p0: () -> Int = { -1000 }, a p1: (() -> Int)..., a p2: () -> Int)  {}
decl_closure_144(a: ({ 1000 }))
decl_closure_144() { 1000 }
decl_closure_144() { 1000 } a: { 1001 }
decl_closure_144(a: ({ 1000 }), a: ({ 1001 }))
decl_closure_144(a: ({ 1000 })) { 1001 }
decl_closure_144(a: ({ 1000 })) { 1001 } a: { 1002 }
decl_closure_144(a: ({ 1000 }), ({ 1001 }), a: ({ 1002 }))
decl_closure_144(a: ({ 1000 }), ({ 1001 })) { 1002 }
decl_closure_144(a: ({ 1000 }), ({ 1001 })) { 1002 } a: { 1003 }
decl_closure_144(a: ({ 1000 }), a: ({ 1001 }))
decl_closure_144(a: ({ 1000 })) { 1001 }
decl_closure_144(a: ({ 1000 })) { 1001 } a: { 1002 }
decl_closure_144(a: ({ 1000 }), a: ({ 1001 }), a: ({ 1002 }))
decl_closure_144(a: ({ 1000 }), a: ({ 1001 })) { 1002 }
decl_closure_144(a: ({ 1000 }), a: ({ 1001 })) { 1002 } a: { 1003 }
decl_closure_144(a: ({ 1000 }), a: ({ 1001 }), ({ 1002 }), a: ({ 1003 }))
decl_closure_144(a: ({ 1000 }), a: ({ 1001 }), ({ 1002 })) { 1003 }
decl_closure_144(a: ({ 1000 }), a: ({ 1001 }), ({ 1002 })) { 1003 } a: { 1004 }
decl_closure_144() { 1000 } a: { 1001 }
decl_closure_144(a: ({ 1001 })) { 1000 } a: { 1002 }
decl_closure_144(a: ({ 1001 }), ({ 1002 })) { 1000 } a: { 1003 }

These are then written to a file and typechecked by calling swiftc. The results of typechecking them are used to filter out calls that are semantically invalid. After this step only valid calls remain for each generated declaration. These are again written to a file together with a body for the declaration to automatically evaluate the matching of the arguments. The generated body for declaration from the example above looks like this:

func decl_closure_144(a p0: () -> Int = { -1000 }, a p1: (() -> Int)..., a p2: () -> Int)  {
  let m_p0: [Int] = p0() == -1000 ? [] : [p0()];
  let m_p1: [Int] = p1.map { $0() };
  let m_p2: [Int] = [p2()];
  let m_p0_mapped: String = m_p0.map(String.init).joined(separator: ",");
  let m_p1_mapped: String = m_p1.map(String.init).joined(separator: ",");
  let m_p2_mapped: String = m_p2.map(String.init).joined(separator: ",");
  let matches: [String] = [m_p0_mapped, m_p1_mapped, m_p2_mapped];
  let matchString = matches.joined(separator: ";");
  print("MATCH|decl_closure_144|" + matchString)
}

The generated file is compiled and run. The argument matching can then be extracted from the output of running the compiled executable. This creates a baseline for the correct behavior. For each generated declaration the results of the baseline are then compared with the results of using the argument matching logic. In total these tests validated that 9294 declarations with a total of 160066 calls were matched correctly by the argument matching implementation.

I used these tests extensively to improve the implementation. Most of the test cases in the argument matching unit tests were also originally found by these generated test cases. Ideally, we could include these generated test cases into the CI, but I think that would slow down the CI too much. I have already optimized them to split everything into multiple files and typecheck/compile them in parallel depending on the number of available cores. I have also adjusted the generated method bodies to use more explicit type annotations to reduce the time spent on typechecking them. However, running the exhaustive tests still takes around 8 min on my M4 Pro. I don't know what kind of runners the CI runs on, but I imagine that it would take significantly longer to run them there. I have included the code in a separate commit for now, but I'm not sure if it should be included in the final merge into main. The code is also not really cleaned up for now.

RFC

I assume this would be considered part of the public API and thus needs an RFC post. I can create one, but it probably makes sense to decide on the final API first.