Skip to content

Commit 95f0651

Browse files
theblixguytheblixguy
authored
[Diagnostics] Emit a warning when an immutable decodable property has an initial value (#30218)
* [Diagnostics] Emit a warning when an immutable decodable property has an initial value * [Sema] Use Decl::diagnose instead of Diags.diagnose * [AST] Remove property name from 'decodable_property_will_not_be_decoded' diagnostic * [Test] Update tests * [Test] Update existing codable tests
1 parent efa1a01 commit 95f0651

8 files changed

+194
-2
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2758,6 +2758,19 @@ NOTE(decodable_suggest_overriding_init_here,none,
27582758
NOTE(codable_suggest_overriding_init_here,none,
27592759
"did you mean to override 'init(from:)' and 'encode(to:)'?", ())
27602760

2761+
WARNING(decodable_property_will_not_be_decoded, none,
2762+
"immutable property will not be decoded because it is declared with "
2763+
"an initial value which cannot be overwritten", ())
2764+
NOTE(decodable_property_init_or_codingkeys_implicit, none,
2765+
"set the initial value via the initializer or explicitly define a "
2766+
"CodingKeys enum %select{including|without}0 a %1 case to silence "
2767+
"this warning", (unsigned, DeclName))
2768+
NOTE(decodable_property_init_or_codingkeys_explicit, none,
2769+
"set the initial value via the initializer or remove the %0 case from "
2770+
"the CodingKeys enum to silence this warning", (DeclName))
2771+
NOTE(decodable_make_property_mutable, none,
2772+
"make the property mutable instead", ())
2773+
27612774
NOTE(missing_member_type_conformance_prevents_synthesis, none,
27622775
"%select{associated value|stored property}0 type %1 does not conform to "
27632776
"protocol %2, preventing synthesized conformance "

‎lib/Sema/DerivedConformanceCodable.cpp

Lines changed: 60 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -830,9 +830,67 @@ deriveBodyDecodable_init(AbstractFunctionDecl *initDecl, void *) {
830830
std::tie(varDecl, varType, useIfPresentVariant) =
831831
lookupVarDeclForCodingKeysCase(conformanceDC, elt, targetDecl);
832832

833-
// Don't output a decode statement for a var let with a default value.
834-
if (varDecl->isLet() && varDecl->isParentInitialized())
833+
// Don't output a decode statement for a let with an initial value.
834+
if (varDecl->isLet() && varDecl->isParentInitialized()) {
835+
// But emit a warning to let the user know that it won't be decoded.
836+
auto lookupResult =
837+
codingKeysEnum->lookupDirect(varDecl->getBaseName());
838+
auto keyExistsInCodingKeys =
839+
llvm::any_of(lookupResult, [&](ValueDecl *VD) {
840+
if (isa<EnumElementDecl>(VD)) {
841+
return VD->getBaseName() == varDecl->getBaseName();
842+
}
843+
return false;
844+
});
845+
auto *encodableProto = C.getProtocol(KnownProtocolKind::Encodable);
846+
bool conformsToEncodable =
847+
TypeChecker::conformsToProtocol(
848+
targetDecl->getDeclaredInterfaceType(), encodableProto,
849+
conformanceDC,
850+
ConformanceCheckFlags::SkipConditionalRequirements) != nullptr;
851+
852+
// Strategy to use for CodingKeys enum diagnostic part - this is to
853+
// make the behaviour more explicit:
854+
//
855+
// 1. If we have an *implicit* CodingKeys enum:
856+
// (a) If the type is Decodable only, explicitly define the enum and
857+
// remove the key from it. This makes it explicit that the key
858+
// will not be decoded.
859+
// (b) If the type is Codable, explicitly define the enum and keep the
860+
// key in it. This is because removing the key will break encoding
861+
// which is mostly likely not what the user expects.
862+
//
863+
// 2. If we have an *explicit* CodingKeys enum:
864+
// (a) If the type is Decodable only and the key exists in the enum,
865+
// then explicitly remove the key from the enum. This makes it
866+
// explicit that the key will not be decoded.
867+
// (b) If the type is Decodable only and the key does not exist in
868+
// the enum, do nothing. This is because the user has explicitly
869+
// made it clear that that they don't want the key to be decoded.
870+
// (c) If the type is Codable, do nothing. This is because removing
871+
// the key will break encoding which is most likely not what the
872+
// user expects.
873+
if (!codingKeysEnum->isImplicit()) {
874+
if (conformsToEncodable || !keyExistsInCodingKeys) {
875+
continue;
876+
}
877+
}
878+
879+
varDecl->diagnose(diag::decodable_property_will_not_be_decoded);
880+
if (codingKeysEnum->isImplicit()) {
881+
varDecl->diagnose(
882+
diag::decodable_property_init_or_codingkeys_implicit,
883+
conformsToEncodable ? 0 : 1, varDecl->getName());
884+
} else {
885+
varDecl->diagnose(
886+
diag::decodable_property_init_or_codingkeys_explicit,
887+
varDecl->getName());
888+
}
889+
varDecl->diagnose(diag::decodable_make_property_mutable)
890+
.fixItReplace(varDecl->getAttributeInsertionLoc(true), "var");
891+
835892
continue;
893+
}
836894

837895
auto methodName =
838896
useIfPresentVariant ? C.Id_decodeIfPresent : C.Id_decode;

‎test/decl/protocol/special/coding/class_codable_member_lookup.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22

33
class SynthesizedSuperclass : Codable {
44
let superValue: Double = .pi
5+
// expected-warning@-1 {{immutable property will not be decoded because it is declared with an initial value which cannot be overwritten}}
6+
// expected-note@-2 {{set the initial value via the initializer or explicitly define a CodingKeys enum including a 'superValue' case to silence this warning}}
7+
// expected-note@-3 {{make the property mutable instead}}{{3-6=var}}
58
}
69

710
// Classes which subclass something Codable should be able to override their

‎test/decl/protocol/special/coding/class_codable_member_type_lookup.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ public enum CodingKeys : String, CodingKey {
1111
// CodingKey enums during member type lookup.
1212
struct SynthesizedClass : Codable {
1313
let value: String = "foo"
14+
// expected-warning@-1 {{immutable property will not be decoded because it is declared with an initial value which cannot be overwritten}}
15+
// expected-note@-2 {{set the initial value via the initializer or explicitly define a CodingKeys enum including a 'value' case to silence this warning}}
16+
// expected-note@-3 {{make the property mutable instead}}{{3-6=var}}
1417

1518
// Qualified type lookup should always be unambiguous.
1619
public func qualifiedFoo(_ key: SynthesizedClass.CodingKeys) {} // expected-error {{method cannot be declared public because its parameter uses a private type}}

‎test/decl/protocol/special/coding/class_codable_nonconforming_property.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ struct NonCodable : Hashable {
1010

1111
struct CodableGeneric<T> : Codable {
1212
let value: Int = 5
13+
// expected-warning@-1 {{immutable property will not be decoded because it is declared with an initial value which cannot be overwritten}}
14+
// expected-note@-2 {{set the initial value via the initializer or explicitly define a CodingKeys enum including a 'value' case to silence this warning}}
15+
// expected-note@-3 {{make the property mutable instead}}{{5-8=var}}
1316
}
1417

1518
// Classes whose properties are not all Codable should fail to synthesize

‎test/decl/protocol/special/coding/codable_immutable_property_with_initial_value.swift

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
// RUN: %target-typecheck-verify-swift -verify-ignore-unknown
2+
3+
// SR-11996 - Warning when a immutable decodable property has an initial value
4+
5+
// Implicit CodingKeys enum //
6+
7+
struct Foo1: Codable {
8+
let bar: Int = 1
9+
// expected-warning@-1 {{immutable property will not be decoded because it is declared with an initial value which cannot be overwritten}}
10+
// expected-note@-2 {{set the initial value via the initializer or explicitly define a CodingKeys enum including a 'bar' case to silence this warning}}
11+
// expected-note@-3 {{make the property mutable instead}}{{3-6=var}}
12+
let baz1: String
13+
var baz2 = 8
14+
}
15+
16+
struct Foo2: Decodable {
17+
let bar: Int = 1
18+
// expected-warning@-1 {{immutable property will not be decoded because it is declared with an initial value which cannot be overwritten}}
19+
// expected-note@-2 {{set the initial value via the initializer or explicitly define a CodingKeys enum without a 'bar' case to silence this warning}}
20+
// expected-note@-3 {{make the property mutable instead}}{{3-6=var}}
21+
let baz1: String
22+
var baz2 = 8
23+
}
24+
25+
struct Foo3: Encodable {
26+
let bar: Int = 1 // Ok, since 'Foo3' isn't decodable
27+
let baz1: String
28+
var baz2 = 8
29+
}
30+
31+
// Explicit CodingKeys enum //
32+
33+
struct Foo4: Codable {
34+
let bar: Int = 1 // Ok, since removing 'bar' from 'CodingKeys' will break encoding
35+
let baz1: String
36+
var baz2 = 8
37+
38+
private enum CodingKeys: String, CodingKey {
39+
case bar
40+
case baz1
41+
case baz2
42+
}
43+
}
44+
45+
struct Foo5: Decodable {
46+
let bar: Int = 1
47+
// expected-warning@-1 {{immutable property will not be decoded because it is declared with an initial value which cannot be overwritten}}
48+
// expected-note@-2 {{set the initial value via the initializer or remove the 'bar' case from the CodingKeys enum to silence this warning}}
49+
// expected-note@-3 {{make the property mutable instead}}{{3-6=var}}
50+
let baz1: String
51+
var baz2 = 8
52+
53+
private enum CodingKeys: String, CodingKey {
54+
case bar
55+
case baz1
56+
case baz2
57+
}
58+
}
59+
60+
struct Foo6: Encodable {
61+
let bar: Int = 1 // Ok, since 'Foo6' isn't decodable
62+
let baz1: String
63+
var baz2 = 8
64+
65+
private enum CodingKeys: String, CodingKey {
66+
case bar
67+
case baz1
68+
case baz2
69+
}
70+
}
71+
72+
struct Foo7: Codable {
73+
let bar: Int = 1 // Ok, since 'bar' doesn't exist in CodingKeys enum
74+
let baz1: String
75+
var baz2 = 8
76+
77+
private enum CodingKeys: String, CodingKey {
78+
case baz1
79+
case baz2
80+
}
81+
}
82+
83+
struct Foo8: Decodable {
84+
let bar: Int = 1 // Ok, since 'bar' does not exist in CodingKeys enum
85+
let baz1: String
86+
var baz2 = 8
87+
88+
private enum CodingKeys: String, CodingKey {
89+
case baz1
90+
case baz2
91+
}
92+
}
93+
94+
struct Foo9: Encodable {
95+
let bar: Int = 1 // Ok, since 'Foo9' isn't decodable (and 'bar' doesn't exist in CodingKeys enum anyway)
96+
let baz1: String
97+
var baz2 = 8
98+
99+
private enum CodingKeys: String, CodingKey {
100+
case baz1
101+
case baz2
102+
}
103+
}

‎test/decl/protocol/special/coding/struct_codable_member_type_lookup.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ public enum CodingKeys : String, CodingKey {
1111
// CodingKey enums during member type lookup.
1212
struct SynthesizedStruct : Codable {
1313
let value: String = "foo"
14+
// expected-warning@-1 {{immutable property will not be decoded because it is declared with an initial value which cannot be overwritten}}
15+
// expected-note@-2 {{set the initial value via the initializer or explicitly define a CodingKeys enum including a 'value' case to silence this warning}}
16+
// expected-note@-3 {{make the property mutable instead}}{{3-6=var}}
1417

1518
// Qualified type lookup should always be unambiguous.
1619
public func qualifiedFoo(_ key: SynthesizedStruct.CodingKeys) {} // expected-error {{method cannot be declared public because its parameter uses a private type}}
@@ -660,4 +663,7 @@ struct sr6886 {
660663
struct StaticInstanceNameDisambiguation : Codable {
661664
static let version: Float = 0.42
662665
let version: Int = 42
666+
// expected-warning@-1 {{immutable property will not be decoded because it is declared with an initial value which cannot be overwritten}}
667+
// expected-note@-2 {{set the initial value via the initializer or explicitly define a CodingKeys enum including a 'version' case to silence this warning}}
668+
// expected-note@-3 {{make the property mutable instead}}{{3-6=var}}
663669
}

‎test/decl/protocol/special/coding/struct_codable_nonconforming_property.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ struct NonCodable : Hashable {
1010

1111
struct CodableGeneric<T> : Codable {
1212
let value: Int = 5
13+
// expected-warning@-1 {{immutable property will not be decoded because it is declared with an initial value which cannot be overwritten}}
14+
// expected-note@-2 {{set the initial value via the initializer or explicitly define a CodingKeys enum including a 'value' case to silence this warning}}
15+
// expected-note@-3 {{make the property mutable instead}}{{5-8=var}}
1316
}
1417

1518
// Structs whose properties are not all Codable should fail to synthesize

0 commit comments

Comments
 (0)