You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
SR-6354 Compiler should warn if a setter does not use newValue
Issue Description:
I think we should add a warning if a computed property defines a setter that doesn't reference the new value. I just discovered a bug in my app today that was cause by code that looked like
varsomeProp: Foo {
get {
varprop: Foo!
dispatch_sync(queue, { prop = _inner.prop })
returnprop
}
set {
letvalue = someProp.copy() as! Foodispatch_barrier_async(queue, { _inner.prop = value })
}
}
Note how the setter is accessing the someProp getter instead of the implicit newValue parameter. And this isn't the first time I've made this mistake. I think the source of the problem is that I usually write didSet observers, where you do access the getter, and so it's easy to slip into using the getter when writing a set.
I considered having a warning if a set accesses the getter, but there are legitimate reasons to want to do that. But warning if the setter never accesses the newValue parameter seems like a good approach. I'm unlikely to use both newValue and the computed getter in the same setter by accident, and if I really do want to ignore the new value I can suppress the warning with _ = newValue.
The text was updated successfully, but these errors were encountered:
Ugh, sorry about this, it got lost in the shuffle. I had the fix based to the 4.1 release branch but it got closed and... life, tooling and disk space got in the way.
I just built and tested on a branch off master, and things should be good to go. Thanks for re-opening this @marcelofabri and sorry to swift users everywhere for letting it sit for so long.
Additional Detail from JIRA
md5: 63ff5a5ab70edfee28c9a22f818da6f7
is duplicated by:
Issue Description:
I think we should add a warning if a computed property defines a setter that doesn't reference the new value. I just discovered a bug in my app today that was cause by code that looked like
Note how the setter is accessing the
someProp
getter instead of the implicitnewValue
parameter. And this isn't the first time I've made this mistake. I think the source of the problem is that I usually writedidSet
observers, where you do access the getter, and so it's easy to slip into using the getter when writing aset
.I considered having a warning if a
set
accesses the getter, but there are legitimate reasons to want to do that. But warning if the setter never accesses thenewValue
parameter seems like a good approach. I'm unlikely to use bothnewValue
and the computed getter in the same setter by accident, and if I really do want to ignore the new value I can suppress the warning with_ = newValue
.The text was updated successfully, but these errors were encountered: