[MiscDiagnostics] Emit a deprecation warning for some writes through …

[rudkx]

…literal keypaths.

We incorrectly allowed some keypaths to be inferred as writable
keypaths in Swift 3/4 modes. This no longer happens when
-swift-version 5 is specified.

This warning is a limited attempt at providing some advanced notice of
code that will break, only in the cases where the keypath is a direct
argument to a keypath subscript write.

Fixes: rdar://problem/40068274

[rudkx]

@swift-ci Please smoke test

[jrose-apple]

How are you supposed to fix this one to be read-only, though?

[rudkx]

You can make it read-only with an explicit type, but in retrospect this warning should really be tracking the keypaths that would be formed as read-only in -swift-version 5 that are used in key path applications that write.

I'll take a look at updating the PR to do that instead.

[jckarter]

In Swift 5 mode, this infers as a read-only KeyPath, right?

[rudkx]

Yes, that’s correct.

[CodaFi]

Why not diagnose both? You'll probably need to build a cache of incorrectly-inferred declrefs to catch stuff like this

class Foo {
  private(set) var foo: String = ""

  init() {}
}

let f = Foo();
let kp = \Foo.foo // forming a writable keypath to property...
f[keyPath: kp] = "asdf" // write to writeable key path that is read-only in this context is deprecated...

[jrose-apple]

I'm unhappy with giving people a warning on code that doesn't do anything wrong.

let f = Foo()
let kp = \Foo.foo // happens to be writable...
print(f[keyPath: kp]) // ...but I only use it to read

[rudkx]

@CodaFi Detecting the write in your example is what I meant when I said "...this warning should really be tracking the keypaths that would be formed as read-only in -swift-version 5 that are used in key path applications that write.".

As Jordan points out, we don't want to warn about things that we happen to infer today as writable but are never used that way.

Anything we do in Sema is going to be necessarily approximate because doing this properly requires data flow analysis.

I'm not sure how much value we'll actually get from doing something approximate, either. I don't want to end up with any false positives, and the only way to do that is to clear out the cache of values we believe hold writable keypaths that would have been inferred as readable any time we exit a control structure.

This may ultimately not be worth doing.

[rudkx]

I ended up changing this around so that we will now emit a warning on key path subscript writes with literal key paths that would be inferred to be read-only under -swift-version 5.

[rudkx]

@swift-ci Please smoke test

[rudkx]

@swift-ci Please test source compatibility

[rudkx]

I grabbed the logs from the source compatibility run to ensure that this warning is not firing spuriously anywhere. Everything looks good on that front.