[SR-3401] Invalid fixit for mutating subscript

[swift-ci]

Previous ID	SR-3401
Radar	None
Original Reporter	KingOfBrian (JIRA User)
Type	Bug
Status	Resolved
Resolution	Done

Additional Detail from JIRA

| | | |------------------|-----------------| |Votes | 0 | |Component/s | Compiler | |Labels | Bug, DiagnosticsQoI | |Assignee | None | |Priority | Medium | md5: 8a73de6e503f5d9f8296c09c37c83310

Issue Description:

Just noticed a very low priority bug with the mutating fixit.

struct Y4 {
    var x : Int

    subscript(i: Int) -> Int {
        x = 10 // expected-error{{error: cannot assign to property: 'self' is immutable}}
        return x
    }
}

The error above is correct, but the fixit is not valid. The fixit if applied will generate:

subscript(i: Int) -> Int mutating {
    x = 10
    return x
}

[swift-ci]

Comment by Brian (JIRA)

I got my test cycle a bit faster after some tips from https://speakerdeck.com/ayanonagon/contributing-to-swift, and I was able to fix this by adding the following:

    // If this is a subscript getter, don't suggest adding mutating.
    if (FD->getAccessorStorageDecl()->getGetter() == FD) return;

It will discard the fixit in the valid case of an explicit `get {}` block however. Having a mutating `get {}` block is probably strange enough that not having a fixit wouldn't be a big deal though.

So this has been fun, but I'm not sure it's worth bothering the development team with a PR. I'm going to scan through the bug list and see if there are any other fixit issues I can resolve and if I can get something that seems interesting I'll make a PR.

[belkadan]

That's no bother, although we will ask you to write a test case

(Yes, the language supports mutating getters—although I bet we still have some bugs with them—but even if we lose the fix-it there it still seems like an incremental improvement. Alternately, there might already be a place to ask for the location of the "get" keyword, which would distinguish between the implicit and explicit cases.)

[swift-ci]

Comment by Brian (JIRA)

Thanks @belkadan! https://github.com/apple/swift/pull/6245