[SwiftRefactor] Fix add-target-dependency matching nested target references

[mit112]

Summary

swift package add-target-dependency can produce a Package.swift that no longer compiles when the target being modified is also referenced as a dependency of another target. The root cause is in SwiftRefactor, so fixing it here.

Reported as swiftlang/swift-package-manager#10122.

Details

findManifestTargetCall(targetName:) locates the target to modify with a recursive FunctionCallExprSyntax.findFirst(in: targetArray, ...). Because that walks the whole subtree, given a manifest like:

targets: [
    .target(
        name: "MyLib",
        dependencies: [
            .target(name: "TargetOne"),   // <- matched first
        ]
    ),
    .target(name: "TargetOne"),           // <- the real definition
]

it matches the nested .target(name: "TargetOne") reference before the actual top-level definition, then appends a dependencies: argument to that reference, yielding malformed output such as:

.target(name: "TargetOne",dependencies: [
    .target(name: "TargetThree"),]),

which fails to compile (extra argument 'dependencies' in call).

Fix

Match only the top-level elements of the targets array rather than recursively descending into nested calls, so the target definition is always the node that gets modified.

Test

Added testAddTargetDependencyToTargetReferencedByAnotherTarget to ManifestEditTests, which reproduces the case above. It fails before the change and passes after; the full ManifestEditTests suite (19 tests) passes.

[mit112]

Gentle ping on this one — it's been open about three weeks with no review and no CI run yet.

For context, this fixes swiftlang/swift-package-manager#10122: swift package add-target-dependency writes a Package.swift that no longer compiles when the target being edited is also referenced in another target's dependencies array. The root cause is here rather than in SwiftPM — findManifestTargetCall in SwiftRefactor/PackageManifest/SyntaxEditUtils.swift searched the targets array with a recursive findFirst, so it could match a nested .target(name:) reference before reaching the actual target definition, and then mutate the reference. The fix matches only the top-level elements of the targets array. There's a regression test in ManifestEditTests (verified red → green).

@ahoppen @bnbarham — you've both reviewed recent SwiftRefactor/PackageManifest changes, so tagging you rather than guessing: would either of you be able to take a look, or trigger CI? Happy to rebase if it's gone stale in the meantime.

[ahoppen]

Ben and I are no longer code owners of swift-syntax. @rintaro @hamishknight would be the points of contact to review this PR.

[mit112]

Thanks @ahoppen, appreciate the redirect.

@rintaro @hamishknight — would one of you be able to take a look at this, or kick off CI?

Short version: swift package add-target-dependency can write a Package.swift that no longer compiles (swiftlang/swift-package-manager#10122). findManifestTargetCall in SwiftRefactor/PackageManifest/SyntaxEditUtils.swift searched the targets array with a recursive findFirst, so when the target being edited is also referenced inside another target's dependencies array, it could match that nested .target(name:) reference first and mutate it instead of the real definition. The fix matches only top-level elements of the targets array. There's a regression test in ManifestEditTests, verified red before / green after.

One thing worth surfacing: #3374 by @krishnapermi predates this and fixes the same root cause in the same two files — I only spotted it after opening this. No preference from me on which one you take; happy to close this in favour of theirs if that's simpler. Just flagging it so you don't review the same bug twice.