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

False positive implicit_getter violation when mentioning word "set" in a comment in custom getter of a read-write var #3149

Closed
2 tasks done
yas375 opened this issue Mar 25, 2020 · 17 comments · Fixed by #3151
Labels
bug Unexpected and reproducible misbehavior.

Comments

@yas375
Copy link

yas375 commented Mar 25, 2020

New Issue Checklist

Describe the bug

Mentioning word "set" (without parentheses) in a comment in a get of a read-write var with both get and set defined triggers SwiftLint to think that this is a read-only property.

image

Complete output when running SwiftLint, including the stack trace and command used
$ swiftlint lint --path Test.swift --no-cache --enable-all-rules
Loading configuration from '.swiftlint.yml'
Linting Swift files at paths Test.swift
Linting 'Test.swift' (1/1)
/Users/yas/code/caremobile/Test.swift:3:9: warning: Implicit Getter Violation: Computed read-only properties should avoid using the get keyword. (implicit_getter)
Done linting! Found 1 violation, 0 serious in 1 file.

Environment

  • SwiftLint 0.39.1
  • Installation method used: Homebrew
  • Paste your configuration file:
excluded:
  - Pods

opt_in_rules:
  - array_init
  - contains_over_first_not_nil
  - collection_alignment
  - contains_over_filter_count
  - empty_count
  - empty_collection_literal
  - overridden_super_call
  - sorted_first_last
  - yoda_condition

disabled_rules:
  - cyclomatic_complexity
  - file_length
  - force_cast
  - force_try
  - function_body_length
  - function_parameter_count
  - identifier_name
  - large_tuple
  - line_length
  - multiple_closures_with_trailing_closure
  - nesting # consider
  - notification_center_detachment
  - operator_whitespace
  - todo # fix
  - type_body_length
  - type_name # consider
  - weak_delegate # fix

statement_position:
  statement_mode: uncuddled_else
  • Are you using nested configurations?No

  • Which Xcode version are you using (check xcodebuild -version)?
    Xcode 11.4
    Build version 11E146

  • Do you have a sample that shows the issue?
    Yes:

Test.swift

extension Test {
    var foo: Bool {
        get {
            bar?.boolValue ?? true // Comment mentioning word set which triggers violation
        }
        set {
            bar = NSNumber(value: newValue as Bool)
        }
    }
}

If I don't mention word "set" in the comment then it seem to work fine.
I don't recall seeing this issues prior to updating to Xcode 11.4.

Hope it helps.

@freak4pc
Copy link
Contributor

Pretty sure this fixes it?
#3099

@yas375
Copy link
Author

yas375 commented Mar 26, 2020

Oh, thanks. I somehow missed that PR during my search for existing reports.
Based on the description I guess it is indeed supposed to fix this issue... But I'm running current SwiftLint version (0.39.1), which includes that fix, and I'm still getting the issue. So perhaps there is something else needs to be done. Maybe something around swift version check? Not sure...

@freak4pc
Copy link
Contributor

It doesn’t include that fix IMO

@freak4pc
Copy link
Contributor

0.39.1 is almost two months old,
I imagine they’ll accumulate some content and release 0.40 soon

@yas375
Copy link
Author

yas375 commented Mar 26, 2020

@p4checo
Copy link

p4checo commented Mar 26, 2020

It's also triggering a false positive if we have both a get and a nonmutating set:

extension Reactive where Base: UITapGestureRecognizer {

    var tapped: CocoaAction<Base>? {
        get {
            return associatedAction.withValue { $0.flatMap { $0.action } }
        }

        nonmutating set {
            setAction(newValue)
        }
    }
}

@mkj-is
Copy link

mkj-is commented Mar 26, 2020

I found another false-positive. When the set is before the get you get warning. If it is the other way round then everything is all right. I believe another rule could be created to formalize the order of get-set.

extension Float {
    var clamped: Float {
        set {
            self = min(1, max(0, newValue))
        }
        get {
            min(1, max(0, self))
        }
    }
}

Command:

swiftlint

Output:

Linting Swift files at paths 
Linting 'lint.swift' (1/1)
/Users/kaspar/Downloads/swiftlint-issue/lint.swift:6:9: warning: Implicit Getter Violation: Computed read-only properties should avoid using the get keyword. (implicit_getter)
Done linting! Found 1 violation, 0 serious in 1 file.

@yas375
Copy link
Author

yas375 commented Mar 27, 2020

@marcelofabri thanks a lot!

@marcelofabri
Copy link
Collaborator

thanks for everyone reporting different minimal examples! this has been very helpful to fix the issue quickly

pull bot pushed a commit to scope-demo/SwiftLint that referenced this issue Mar 27, 2020
@freak4pc
Copy link
Contributor

Not sure if this fits in the same fix:

image

@tahirmt
Copy link

tahirmt commented Mar 28, 2020

Which release is this going to and when?

@jpsim
Copy link
Collaborator

jpsim commented Mar 28, 2020

The fix will go in the next release, either 0.39.2 or 0.40.0, depending on how we choose to version it. That’ll be in the next few days probably. Depends on if we think we can get more Swift 5.2 fixes in time.

@jpsim
Copy link
Collaborator

jpsim commented Mar 28, 2020

@freak4pc can you see if the latest commit from master fixes that for you?

@nicolas-miari
Copy link

nicolas-miari commented Mar 29, 2020

Looking forward to the fixing release. My code below still triggers this false positive on 0.39.1:

import Foundation

extension UserDefaults {
    var didShowWelcomeWindow: Bool {
        set {
            self.set(newValue, forKey: "didShowWelcomeWindow")
        }
        get {
            return bool(forKey: "didShowWelcomeWindow")
        }
    }
}

Update: it resolves if I move the getter above the setter:

var didShowWelcomeWindow: Bool {
    get {
        return bool(forKey: "didShowWelcomeWindow”)
    }
    set {
        self.set(newValue, forKey: "didShowWelcomeWindow”)
    }    
}

@ahmedsafadii
Copy link

no fix ?

@freak4pc
Copy link
Contributor

@jpsim my issues are fixed on the last master

@kb100824
Copy link

u need find file : .swiftlint.yml , then just do it

disabled_rules: 
  - implicit_getter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unexpected and reproducible misbehavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants