Skip to content

[5.1][SourceKit] Fix placeholder expansion not working inside #if #26040

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

nathawes
Copy link
Contributor

@nathawes nathawes commented Jul 9, 2019

Cherry-pick of #26036 from master

  • Issue: rdar://problem/51995648

  • Explanation:
    All editor placeholder expansions inside #if blocks were failing. For example, with the below placeholder produced by code completion:

    #if DEBUG
    func test2() {
        [1,2,3].filter(<#T##isIncluded: (Int) throws -> Bool##(Int) throws -> Bool#>)
    }
    #endif
    

    the expansion was failing and Xcode would simply insert the placeholder content verbatim:

    [1,2,3].filter(#T##isIncluded: (Int) throws -> Bool##(Int) throws -> Bool#)
    

    rather than expanding successfully as it does outside of #if blocks:

    [1,2,3].filter { (<#Int#>) -> Bool in
        <#code#>
    }
    

    This was failing because the PlaceholderFinder ASTWalker wasn't walking into the clauses of IfConfigDecls, so didn't find them. Additionally the CallExprFinder walker (used to determine if expansions should use trailing closure syntax) wasn't configured to walk into inactive if-config clauses, so expansions weren't using trailing closure syntax in inactive regions.

  • Scope: This issue affects all editor placeholder expansions inside of #if blocks

  • Risk: Low. The fix is limited to SourceKit's placeholder expansion request, and only affects the behavior on placeholders within #if blocks.

  • Origination: Placeholder expansion never worked in inactive #if blocks, but used to work in active #if blocks prior to Xcode 10.2. The offending change was that #if conditions are no longer evaluated when parsing to produce an AST for SourceKit's purely syntactic requests (including placeholder expansion) so there aren't any active clauses anymore.

  • Testing: Added regression tests and manually checked it behaves correctly in Xcode

  • Reviewer: Xi Ge (in the master PR)

Resolves rdar://problem/51995648

Update the PlaceholderFinder ASTWalker to walk into the clauses of
IfConfigDecls. It wasn't previously, resulting in any placeholders there not
being expanded.

Also update CallExprFinder (used to determine if expansions should use trailing
closure syntax) to walk into inactive if-config clauses. Previously it only
walked into active regions, so expansions never used trailing closure syntax in
inactive regions.

Resolves rdar://problem/51995648
@nathawes
Copy link
Contributor Author

nathawes commented Jul 9, 2019

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - f723c8e

@nathawes
Copy link
Contributor Author

@swift-ci please test Linux

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - f723c8e

@nathawes
Copy link
Contributor Author

@swift-ci please test Linux

@nathawes nathawes requested a review from akyrtzi July 11, 2019 00:19
@nathawes nathawes marked this pull request as ready for review July 11, 2019 00:19
@nathawes
Copy link
Contributor Author

@swift-ci please nominate

@nathawes nathawes merged commit 37b0ce8 into swiftlang:swift-5.1-branch Jul 11, 2019
@nathawes nathawes deleted the r51995648-placeholder-not-working-inside-pound-if-5.1 branch July 11, 2019 00:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants