Allow unused_setter_value for overrides #3653

grosem · 2021-06-08T00:20:16Z

Fixes #2585. Happy to make changes if necessary 😎

SwiftLintBot · 2021-06-08T00:28:06Z

	1 Warning
⚠️	This PR may need tests.

	21 Messages
📖	Linting Aerial with this PR took 0.24s vs 0.23s on master (4% slower)
📖	Linting Alamofire with this PR took 0.27s vs 0.27s on master (0% slower)
📖	Linting Firefox with this PR took 1.08s vs 1.07s on master (0% slower)
📖	Linting Kickstarter with this PR took 1.83s vs 1.83s on master (0% slower)
📖	Linting Moya with this PR took 0.12s vs 0.12s on master (0% slower)
📖	Linting Nimble with this PR took 0.13s vs 0.13s on master (0% slower)
📖	Linting Quick with this PR took 0.07s vs 0.07s on master (0% slower)
📖	Linting Realm with this PR took 0.79s vs 0.79s on master (0% slower)
📖	Linting SourceKitten with this PR took 0.11s vs 0.11s on master (0% slower)
📖	Linting Sourcery with this PR took 0.38s vs 0.38s on master (0% slower)
📖	Linting Swift with this PR took 1.18s vs 1.17s on master (0% slower)
📖	Linting WordPress with this PR took 1.98s vs 1.98s on master (0% slower)
📖	This PR fixed a violation in Firefox: /Client/Frontend/Login Management/LoginDetailTableViewCell.swift:80:9: warning: Unused Setter Value Violation: Setter value is not used. (unused_setter_value)
📖	This PR fixed a violation in Firefox: /Client/Frontend/Settings/AppSettingsOptions.swift:177:9: warning: Unused Setter Value Violation: Setter value is not used. (unused_setter_value)
📖	This PR fixed a violation in WordPress: /WordPress/Classes/Utility/WPMediaEditor.swift:41:9: warning: Unused Setter Value Violation: Setter value is not used. (unused_setter_value)
📖	This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/Post/AuthorFilterViewController.swift:63:9: warning: Unused Setter Value Violation: Setter value is not used. (unused_setter_value)
📖	This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/Post/Preview/PreviewDeviceSelectionViewController.swift:87:9: warning: Unused Setter Value Violation: Setter value is not used. (unused_setter_value)
📖	This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/Reader/Detail/ReaderDetailViewController.swift:128:9: warning: Unused Setter Value Violation: Setter value is not used. (unused_setter_value)
📖	This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/Stats/Insights/Posting Activity/PostingActivityCell.swift:48:9: warning: Unused Setter Value Violation: Setter value is not used. (unused_setter_value)
📖	This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/Stats/Insights/Posting Activity/PostingActivityMonth.swift:180:9: warning: Unused Setter Value Violation: Setter value is not used. (unused_setter_value)
📖	This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/Stats/Insights/Posting Activity/PostingActivityMonth.swift:40:9: warning: Unused Setter Value Violation: Setter value is not used. (unused_setter_value)

Generated by 🚫 Danger

marcelofabri · 2021-06-17T18:09:04Z

@grosem what do you think about checking that the set body is empty? I feel like this change as is could end up introducing issues on overrides where the author would want to use newValue but made a mistake.

grosem · 2021-06-25T22:19:34Z

Makes sense, I'll try to add that 👍

grosem · 2021-07-10T08:20:09Z

@marcelofabri I tried to find the easiest way to accomplish that, what do you think?
The checks still run into an error here which I cannot reproduce locally - not really sure what the problem is.

zvonicek · 2021-11-08T20:20:14Z

Hi, thanks for this PR! I'm seeing a similar problem in my codebase and would like to see the PR merged. Can you try to revive this PR @grosem?

Current problem I'm seeing in the checks on this PR – Closure expressions should have a single space inside each brace. – seem to be because you are missing a space characters inside of the contains(where:) closure in UnusedSetterValueRule.swift line 159.

Instead of contains(where: {!$0.isCommentLike}) try to write it as contains(where: { !$0.isCommentLike }).

grosem · 2021-11-08T21:36:14Z

Hi, thanks for this PR! I'm seeing a similar problem in my codebase and would like to see the PR merged. Can you try to revive this PR @grosem?

Current problem I'm seeing in the checks on this PR – Closure expressions should have a single space inside each brace. – seem to be because you are missing a space characters inside of the contains(where:) closure in UnusedSetterValueRule.swift line 159.

Instead of contains(where: {!$0.isCommentLike}) try to write it as contains(where: { !$0.isCommentLike }).

Sure thing, thank you for your comment. In my local version I had that already fixed but didn't commit it for a reason I cannot remember... if there is anything else to adjust, please let me know!

zvonicek · 2021-11-10T09:21:31Z

Thanks for the fix! The PR now seem to pass, hope we can get someone to review and merge this.

jpsim · 2022-01-20T22:24:19Z

Thanks for the contribution!

amomchilov · 2022-02-13T01:08:11Z

Source/SwiftLintFramework/Rules/Lint/UnusedSetterValueRule.swift

+             get {
+                 return Persister.shared.aValue
+             }
+             set() { }


These examples get automatically run as tests, right?

This example fails for me, on v0.46.2

echo 'private var abc: Int { get { 123 } set() { } }' | swiftlint lint --no-cache --use-stdin --enable-all-rules

<nopath>:1:36: warning: Unused Setter Value Violation: Setter value is not used. (unused_setter_value) Done linting! Found 1 violation, 0 serious in 1 file.

Can you reproduce?

Yes these are run as tests, please open a new issue with full steps to reproduce, what you're seeing and what you're expecting.

Opened #3863 :)

Fixes realm#2585

grosem marked this pull request as draft July 6, 2021 23:34

grosem marked this pull request as ready for review July 6, 2021 23:34

grosem marked this pull request as draft July 6, 2021 23:35

grosem marked this pull request as ready for review July 10, 2021 08:20

grosem and others added 5 commits January 20, 2022 17:18

Allow unused_setter_value for overrides

d6226b9

Allow overrides only when body is empty

456378d

Refactor to stay under 40 lines

15d9bcb

Fix whitespace

a8bb552

Move changelog section

4274887

jpsim force-pushed the allow-unused-setter-for-overrides branch from 674f480 to 4274887 Compare January 20, 2022 22:22

jpsim enabled auto-merge (squash) January 20, 2022 22:24

jpsim merged commit c90c329 into realm:master Jan 20, 2022

amomchilov reviewed Feb 13, 2022

View reviewed changes

rcole34 pushed a commit to rcole34/SwiftLint that referenced this pull request Feb 22, 2022

Allow unused_setter_value for overrides (realm#3653)

523db3f

Fixes realm#2585

Coeur mentioned this pull request Apr 4, 2022

unused_setter_value incorrectly triggers for overridden properties. #2585

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow unused_setter_value for overrides #3653

Allow unused_setter_value for overrides #3653

grosem commented Jun 8, 2021

SwiftLintBot commented Jun 8, 2021 •

edited

Loading

marcelofabri commented Jun 17, 2021

grosem commented Jun 25, 2021

grosem commented Jul 10, 2021

zvonicek commented Nov 8, 2021

grosem commented Nov 8, 2021

zvonicek commented Nov 10, 2021

jpsim commented Jan 20, 2022

amomchilov Feb 13, 2022

jpsim Feb 17, 2022

amomchilov Feb 24, 2022

Allow unused_setter_value for overrides #3653

Allow unused_setter_value for overrides #3653

Conversation

grosem commented Jun 8, 2021

SwiftLintBot commented Jun 8, 2021 • edited Loading

marcelofabri commented Jun 17, 2021

grosem commented Jun 25, 2021

grosem commented Jul 10, 2021

zvonicek commented Nov 8, 2021

grosem commented Nov 8, 2021

zvonicek commented Nov 10, 2021

jpsim commented Jan 20, 2022

amomchilov Feb 13, 2022

Choose a reason for hiding this comment

jpsim Feb 17, 2022

Choose a reason for hiding this comment

amomchilov Feb 24, 2022

Choose a reason for hiding this comment

SwiftLintBot commented Jun 8, 2021 •

edited

Loading