Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unused_setter_value incorrectly triggers for overridden properties. #2585

Closed
2 tasks done
Coeur opened this issue Jan 24, 2019 · 8 comments · Fixed by #3653
Closed
2 tasks done

unused_setter_value incorrectly triggers for overridden properties. #2585

Coeur opened this issue Jan 24, 2019 · 8 comments · Fixed by #3653
Labels
enhancement Ideas for improvements of existing features and rules. wontfix Issues that became stale and were auto-closed by a bot.

Comments

@Coeur
Copy link

Coeur commented Jan 24, 2019

New Issue Checklist

Describe the bug

unused_setter_value incorrectly triggers for overridden properties.

I use the following code that I gave as a Stack Overflow answer:

/// sub-class to disallow selection.
class UnselectableTappableTextView: UITextView {
    // required to prevent blue background selection from any situation
    override var selectedTextRange: UITextRange? {
        get { return nil }
        // This triggers a SwiftLint violation:
        set {}
    }
}

Having an empty setter is the desired behavior when overriding.

Environment

  • SwiftLint version 0.30.0
  • Xcode 10.1
  • Swift 4.2
@jpsim
Copy link
Collaborator

jpsim commented Jan 24, 2019

Yeah I agree we should allow no-op sets for overridden variables.

@jpsim jpsim added the enhancement Ideas for improvements of existing features and rules. label Jan 24, 2019
@jgavris
Copy link

jgavris commented Jan 25, 2019

+1, Similarly for @available(*, unavailable) properties.

screen shot 2019-01-25 at 2 23 31 pm

@marcelofabri
Copy link
Collaborator

Note: you can silence the warning:

override var selectedTextRange: UITextRange? {
        get { return nil }
        set { _ = newValue }
    }

@jpsim
Copy link
Collaborator

jpsim commented Jan 26, 2019

That's a good workaround for now @marcelofabri although I think we should also implement this change because users shouldn't adapt their code just to satisfy SwiftLint unless it's also better for other reasons. I think in this case, an empty set sufficiently conveys the author's intention to not do anything with newValue.

@marcelofabri
Copy link
Collaborator

yeah, but this will be a warning on Swift too soon: swiftlang/swift#21983 and https://bugs.swift.org/browse/SR-964

I don't mind changing the behavior though, so PRs are welcome!

@stale
Copy link

stale bot commented Nov 8, 2020

This issue has been automatically marked as stale because it has not had any recent activity. Please comment to prevent this issue from being closed. Thank you for your contributions!

@stale stale bot added the wontfix Issues that became stale and were auto-closed by a bot. label Nov 8, 2020
@stale stale bot closed this as completed Nov 15, 2020
jpsim pushed a commit that referenced this issue Jan 20, 2022
@username0x0a
Copy link

username0x0a commented Jan 23, 2022

This change appears nice, but it would be good to have an option (attribute?) to allow enforcing this rule on overrides as well as that might be the actual rule decision our team took for example – we're used to use the _ = newValue workaround when it's really desired, yet it can be still useful on overrides wrapping something else on inherited instances.

rcole34 pushed a commit to rcole34/SwiftLint that referenced this issue Feb 22, 2022
@Coeur
Copy link
Author

Coeur commented Apr 4, 2022

For readers, to clarify, even though it's labeled as "wontfix", it got fixed on 21 Jan 2022 with #3653, three years after the issue was opened. 🎉
It got released in SwiftLint 0.46.1 the next day.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Ideas for improvements of existing features and rules. wontfix Issues that became stale and were auto-closed by a bot.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants