[Proposal] Warn about implicit property access in own setter


(Marc Knaup) #1

Hello everyone,

I would like to propose yielding a warning when a property's getter is used
from within its setter implicitly without using "self.".

This can lead to programming errors which are very easy to miss and lead to
unexpected behavior at runtime.

Check out the full proposal here:
https://github.com/fluidsonic/swift-evolution/blob/master/proposals/NNNN-warn-about-implicit-property-access-in-own-setter.md

Thank you,
  Marc


(Dennis Lysenko) #2

Other possible solution: why not just remove the default "newValue" and
force the user to manually specify it instead?

···

On Tue, Dec 15, 2015, 6:02 PM Marc Knaup via swift-evolution < swift-evolution@swift.org> wrote:

Hello everyone,

I would like to propose yielding a warning when a property's getter is
used from within its setter implicitly without using "self.".

This can lead to programming errors which are very easy to miss and lead
to unexpected behavior at runtime.

Check out the full proposal here:

https://github.com/fluidsonic/swift-evolution/blob/master/proposals/NNNN-warn-about-implicit-property-access-in-own-setter.md

Thank you,
  Marc
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Colin Cornaby) #3

I’d be in favor of this. I was trying this out in a Playground and was trying to remember what the implicit keyword was to get the changed value. Migration wouldn’t be bad. newValue could just be added as an explicitly named parameter to setters that have none. I’m not a fan of these corners of Swift where things are so implicit. But this seems to me to be a problem with Swift where the behavior of the setter is non-obvious enough a developer could slip up. I’d be especially worried about users that are new to the language or new to development making this mistake.

I can understand the counter argument of wanting the shorthand to be present for reducing code cruft. But from my perspective it’s reducing visibility and readability. At the very least, the more compact usage doesn’t feel natural to me. But maybe that’s just me.

···

On Dec 15, 2015, at 6:39 PM, Dennis Lysenko via swift-evolution <swift-evolution@swift.org> wrote:

Other possible solution: why not just remove the default "newValue" and force the user to manually specify it instead?

On Tue, Dec 15, 2015, 6:02 PM Marc Knaup via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
Hello everyone,

I would like to propose yielding a warning when a property's getter is used from within its setter implicitly without using "self.".

This can lead to programming errors which are very easy to miss and lead to unexpected behavior at runtime.

Check out the full proposal here:
https://github.com/fluidsonic/swift-evolution/blob/master/proposals/NNNN-warn-about-implicit-property-access-in-own-setter.md

Thank you,
  Marc
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Marc Knaup) #4

That approach is mentioned in "Alternatives considered".

···

On Dec 16, 2015 03:39, "Dennis Lysenko" <dennis.s.lysenko@gmail.com> wrote:

Other possible solution: why not just remove the default "newValue" and
force the user to manually specify it instead?

On Tue, Dec 15, 2015, 6:02 PM Marc Knaup via swift-evolution < > swift-evolution@swift.org> wrote:

Hello everyone,

I would like to propose yielding a warning when a property's getter is
used from within its setter implicitly without using "self.".

This can lead to programming errors which are very easy to miss and lead
to unexpected behavior at runtime.

Check out the full proposal here:

https://github.com/fluidsonic/swift-evolution/blob/master/proposals/NNNN-warn-about-implicit-property-access-in-own-setter.md

Thank you,
  Marc
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Dennis Lysenko) #5

Caught red handed not having gone through the alternatives! I'm not sure
the listed negatives are too bad though. Impact on existing source, in
particular, why is that an issue? Code migration has solved every breaking
change so far. As for oldValue, yes, I think that should be changed as
well. Not sure why that is a con. Agreed it makes it more verbose but also
gives more clarity and the verbosity is negligible and I would say better
than hidden compiler magic.

The need for the proposed solution feels a bit like a monkey-patch on top
of a system that shouldn't need that monkey patch in the first place. That
said, the proposal, were it accepted as-is, would be a welcome change and
thanks for writing it up.

···

On Tue, Dec 15, 2015, 9:41 PM Marc Knaup <marc@knaup.koeln> wrote:

That approach is mentioned in "Alternatives considered".
On Dec 16, 2015 03:39, "Dennis Lysenko" <dennis.s.lysenko@gmail.com> > wrote:

Other possible solution: why not just remove the default "newValue" and
force the user to manually specify it instead?

On Tue, Dec 15, 2015, 6:02 PM Marc Knaup via swift-evolution < >> swift-evolution@swift.org> wrote:

Hello everyone,

I would like to propose yielding a warning when a property's getter is
used from within its setter implicitly without using "self.".

This can lead to programming errors which are very easy to miss and lead
to unexpected behavior at runtime.

Check out the full proposal here:

https://github.com/fluidsonic/swift-evolution/blob/master/proposals/NNNN-warn-about-implicit-property-access-in-own-setter.md

Thank you,
  Marc
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Marc Knaup) #6

Whether or not an explicit parameter name for property setters (and
observers) is enforced (instead of implicit newValue and oldValue) should
be a distinct proposal with its on discussion.
Even with explicit parameter names you can still accidentally refer to the
getter where you actually mean the new value if you don't give the
parameter exactly the same name.

When migrating existing code by adding an explicit newValue parameter name
then that code will still have this bug since the implicit access to the
getter is still neither fixed nor lamented.

I'll update the proposal accordingly.

···

On Wed, Dec 16, 2015 at 5:03 AM, Dennis Lysenko <dennis.s.lysenko@gmail.com> wrote:

Caught red handed not having gone through the alternatives! I'm not sure
the listed negatives are too bad though. Impact on existing source, in
particular, why is that an issue? Code migration has solved every breaking
change so far. As for oldValue, yes, I think that should be changed as
well. Not sure why that is a con. Agreed it makes it more verbose but also
gives more clarity and the verbosity is negligible and I would say better
than hidden compiler magic.

The need for the proposed solution feels a bit like a monkey-patch on top
of a system that shouldn't need that monkey patch in the first place. That
said, the proposal, were it accepted as-is, would be a welcome change and
thanks for writing it up.

On Tue, Dec 15, 2015, 9:41 PM Marc Knaup <marc@knaup.koeln> wrote:

That approach is mentioned in "Alternatives considered".
On Dec 16, 2015 03:39, "Dennis Lysenko" <dennis.s.lysenko@gmail.com> >> wrote:

Other possible solution: why not just remove the default "newValue" and
force the user to manually specify it instead?

On Tue, Dec 15, 2015, 6:02 PM Marc Knaup via swift-evolution < >>> swift-evolution@swift.org> wrote:

Hello everyone,

I would like to propose yielding a warning when a property's getter is
used from within its setter implicitly without using "self.".

This can lead to programming errors which are very easy to miss and
lead to unexpected behavior at runtime.

Check out the full proposal here:

https://github.com/fluidsonic/swift-evolution/blob/master/proposals/NNNN-warn-about-implicit-property-access-in-own-setter.md

Thank you,
  Marc
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution