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

Different output on different systems (Error: where clauses are preferred) #3990

Closed
2 tasks done
0xWDG opened this issue Jun 4, 2022 · 12 comments
Closed
2 tasks done

Comments

@0xWDG
Copy link

0xWDG commented Jun 4, 2022

New Issue Checklist

Describe the bug

On 2 different systems with (almost) the same configuration I got different messages

Complete output when running SwiftLint, including the stack trace and command used

System 1 (MacOS Local)

wesley@Wesleys-MacBook-Air iOS % swiftlint --version
0.47.1
wesley@Wesleys-MacBook-Air iOS % swiftlint --strict                             
Linting Swift files in current working directory
...
Linting 'ObservableGeocoder.swift' (99/138)
...
Done linting! Found 0 violations, 0 serious in 138 files.

System 2 (GitHub Worker)

swiftlint --version
0.47.1

swiftlint --reporter github-actions-logging --strict
Linting Swift files in current working directory
...
Linting 'ObservableGeocoder.swift' (99/138)
Error: `where` clauses are preferred over a single `if` inside a `for`. (for_where)
...
Done linting! Found 0 violations, 0 serious in 138 files.

Environment

  • SwiftLint version (run swiftlint version to be sure)?
  • Installation method used (Homebrew, CocoaPods, building from source, etc)?
  • Paste your configuration file:
disabled_rules:
- redundant_discardable_let # to enable let _ in SwiftUI
- todo # Enable // TODO:
  • Are you using nested configurations?
    If so, paste their relative paths and respective contents.
  • Which Xcode version are you using (check xcodebuild -version)?
  • Do you have a sample that shows the issue? Run echo "[string here]" | swiftlint lint --no-cache --use-stdin --enable-all-rules
    to quickly test if your example is really demonstrating the issue. If your example is more
    complex, you can use swiftlint lint --path [file here] --no-cache --enable-all-rules.
// This triggers a violation:
let foo = try! bar()

===

Code

import Foundation
import CoreLocation

class Geocoder: ObservableObject {
    /// Placemark
    @Published public var placeMark: CLPlacemark?
    /// Location
    @Published public var location: CLLocation?
    /// The name of the placemark.
    @Published public var name: String?
    /// The abbreviated country or region name.
    @Published public var iso: String?
    /// The name of the country or region associated with the placemark.
    @Published public var country: String?
    /// The postal code associated with the placemark.
    @Published public var postalCode: String?
    /// The state or province associated with the placemark.
    @Published public var state: String?
    /// Additional administrative area information for the placemark.
    @Published public var subState: String?
    /// The city associated with the placemark.
    @Published public var city: String?
    /// Additional city-level information for the placemark.
    @Published public var subCity: String?
    /// The street address associated with the placemark.
    @Published public var street: String?
    /// Additional street-level information for the placemark.
    @Published public var subStreet: String?
    /// The geographic region associated with the placemark.
    @Published public var region: CLRegion?
    /// The time zone associated with the placemark.
    @Published public var timeZone: TimeZone?
    /// The name of the inland water body associated with the placemark.
    @Published public var inlandWater: String?
    /// The name of the ocean associated with the placemark.
    @Published public var ocean: String?
    /// The relevant areas of interest associated with the placemark.
    @Published public var areasOfInterest: [String]?

    private let geoCoder = CLGeocoder()

    init () { }
    func update (to location: CLLocation) {
        geoCoder.reverseGeocodeLocation(
            location,
            completionHandler: { (placemarks, _) -> Void in
                if let placeMark = placemarks?[0] {
                    self.placeMark = placeMark
                    self.location = placeMark.location
                    self.name = placeMark.name
                    self.iso = placeMark.isoCountryCode
                    self.country = placeMark.country
                    self.postalCode = placeMark.postalCode
                    self.state = placeMark.administrativeArea
                    self.subState = placeMark.subAdministrativeArea
                    self.city = placeMark.locality
                    self.subCity = placeMark.subLocality
                    self.street = placeMark.thoroughfare
                    self.subStreet = placeMark.subThoroughfare
                    self.region = placeMark.region
                    self.timeZone = placeMark.timeZone
                    self.inlandWater = placeMark.inlandWater
                    self.ocean = placeMark.ocean
                    self.areasOfInterest = placeMark.areasOfInterest
                    self.objectWillChange.send()
                }
            }
        )
    }
}
@markst
Copy link

markst commented Jun 6, 2022

@wdg different toolchains?

try on each machine:

xcrun swift -version

@marcelofabri
Copy link
Collaborator

This is likely caused by different Swift versions - see #3975

@0xWDG
Copy link
Author

0xWDG commented Jun 11, 2022

@wdg different toolchains?

try on each machine:

xcrun swift -version

Hi,

Sorry for the delay.

GitHub:

Run swift -version
Apple Swift version 5.5.2 (swiftlang-1300.0.47.5 clang-1300.0.29.30)
Target: x86_64-apple-macosx11.0
swift-driver version: 1.26.21

I accedently upgraded my Mac, I missed a step, so I'm running the beta software now (unfortunately).

wesley@Wesleys-MacBook-Air iOS % swift -version       
swift-driver version: 1.45.2 Apple Swift version 5.6.1 (swiftlang-5.6.0.323.66 clang-1316.0.20.12)
Target: arm64-apple-macosx13.0

@markst
Copy link

markst commented Jun 11, 2022

@schlagelk
Copy link

schlagelk commented Jun 19, 2022

I'm seeing this too - one of our scenarios is using the docker (latest) image on an Ubuntu runner (this is the one flagging several new for_where errors as of last week) and the other is using the latest SwiftLint release (0.47.1) on developer macOS machines (Xcode 13.4.1), both are using swift 5.6.1. I wonder if it's because #3979 was recently merged and a new docker image was pushed (but the last release on GitHub was in April before this)?

Edit: confirmed that going back and pinning the docker image to 0.47.1 instead of latest does not flag these new for_where violations we're seeing.

@SimplyDanny
Copy link
Collaborator

@wdg: Where in you example does the rule trigger. I do not manage to trigger the warning with any of the mentioned Swift versions in this example.

@schlagelk: You say for_where shows up more often. Are these warnings false-positives or are they correctly shown? If you think they should not occur, could you please post an example for such a case?

@schlagelk
Copy link

@wdg: Where in you example does the rule trigger. I do not manage to trigger the warning with any of the mentioned Swift versions in this example.

@schlagelk: You say for_where shows up more often. Are these warnings false-positives or are they correctly shown? If you think they should not occur, could you please post an example for such a case?

@SimplyDanny They seem like legit violations of this rule which were not previously reported

@SimplyDanny SimplyDanny added the repro-needed Issues that cannot be reproduced or miss proper descriptive examples. label Jun 20, 2022
@0xWDG
Copy link
Author

0xWDG commented Jun 21, 2022

@wdg: Where in you example does the rule trigger. I do not manage to trigger the warning with any of the mentioned Swift versions in this example.

@schlagelk: You say for_where shows up more often. Are these warnings false-positives or are they correctly shown? If you think they should not occur, could you please post an example for such a case?

Weird, on a / the test repo it doesn't happen...
https://github.com/wdg/test-lint/runs/6979647122?check_suite_focus=true

@0xWDG
Copy link
Author

0xWDG commented Jun 21, 2022

update: with further testing, the problem seems to be in a differtent file, the github reporter was pointing to a different file.

/Users/runner/work/-iOS/-iOS/*/Views/Notifications/NotificationsService.swift:48:17: error: For Where Violation: where clauses are preferred over a single if inside a for. (for_where)

which contained

            for notification in self.notification {
                if !(notification.isRead ?? false) {
                    DispatchQueue.main.async {
                        self.unread += 1
                    }
                }
            }

Still weird, that it triggered an error on the GitHub runner and not on my local machine.

@SimplyDanny
Copy link
Collaborator

The for_where rule in SwiftLint 0.47.1 triggers in Xcode 13.2 but not in Xcodes >13.2 for some code snippets. I think, this is exactly the constellation you are dealing with.

This issue has been addressed in the context of #3975. Thus, with the current HEAD version of SwiftLint or its next coming release, the differences are expected to vanish.

@schlagelk
Copy link

schlagelk commented Jun 21, 2022

The for_where rule in SwiftLint 0.47.1 triggers in Xcode 13.2 but not in Xcodes >13.2 for some code snippets. I think, this is exactly the constellation you are dealing with.

This issue has been addressed in the context of #3975. Thus, with the current HEAD version of SwiftLint or its next coming release, the differences are expected to vanish.

Thanks for confirming @SimplyDanny. Just to add another level of confidence to this diagnosis I went ahead and compiled the project from master and ran it on my codebase, and it indeed flagged these other violations not currently being flagged in 0.47.1.

@SimplyDanny
Copy link
Collaborator

Thank you for checking that out and the confirmation of the theory! 👍

@SimplyDanny SimplyDanny removed the repro-needed Issues that cannot be reproduced or miss proper descriptive examples. label Jun 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants