Add ArgumentMatcher for matching call arguments to function parameters#3292
Add ArgumentMatcher for matching call arguments to function parameters#3292loveucifer wants to merge 4 commits intomainfrom
Conversation
This adds a new ArgumentMatcher utility for matching the arguments of a function call to the parameters of a function declaration Phase 1 covers: - Labeled arguments (matched by external label) - Positional arguments (matched to `_` parameters by position) - External vs. internal parameter names - Default parameters accepted (omission doesn't throw; synthesis deferred)
loveucifer
requested review from
ahoppen,
hamishknight,
hborla and
rintaro
as code owners
|
@hamishknight @rintaro Would one of you be able to review this PR? |
|
its still in works as im reading through rust lsp for this and i think this will take a while but probably worth it |
rintaro
left a comment
rintaro
left a comment
There was a problem hiding this comment.
FYI, the compiler implementation lives here https://github.com/swiftlang/swift/blob/e25af44da87f1899a00dd6fa71fe26669cffb3bc/lib/Sema/CSSimplify.cpp#L999
We don't need to implement everything from the start, but we should eventually support the full functionality. With that in mind, it would be good to model the data types carefully upfront.
Sorry I don't have a concrete proposal yet, but please keep this in mind as you think through the design.
|
|
||
| /// The expression at the call site that should be substituted for uses of | ||
| /// ``internalName`` in the function body. | ||
| public let argument: ExprSyntax |
There was a problem hiding this comment.
Overall, I'm not sure this should be modeled using Syntax types.
In many cases the element doesn't correspond to written syntax (e.g. default argument values or synthesized functions) in general. It may be simpler to represent this using dedicated data types instead.
| call: FunctionCallExprSyntax, | ||
| parameters: FunctionParameterListSyntax | ||
| ) throws -> [ArgumentMatch] { | ||
| let arguments = Array(call.arguments) |
There was a problem hiding this comment.
We should take trailing closures into account
| /// matched to the parameter list. | ||
| public static func match( | ||
| call: FunctionCallExprSyntax, | ||
| parameters: FunctionParameterListSyntax |
There was a problem hiding this comment.
We'd also need to support subscript and calling closures eventually.
|
|
||
| /// The expression at the call site that should be substituted for uses of | ||
| /// ``internalName`` in the function body. | ||
| public let argument: ExprSyntax |
There was a problem hiding this comment.
Considering variadic parameters, the relationship between a parameter and arguments is not always 1:1. It can be 1:n.
|
this was quite long , i will keep checking if thee is something i can fix though |
3 participants
This adds a new ArgumentMatcher utility for matching the arguments of a function call to the parameters of a function declaration
This is just part of Phase 1 and it covers:
_parameters by position)This is ultimately part of inline refactoring features discussed with @ahoppen over swift forums ( implementation plan can be found hereNotion