Fix or report all current small Swiftlint warnings in xcode #5103

rablador · 2023-09-05T09:34:39Z

This PR fixes all smaller lint warnings we have in the project.

This change is

linear · 2023-09-05T09:34:53Z

IOS-296 Fix Swiftlint warnings in Xcode

Go through the existing warnings and either fix them or disable + report them (in Linear).

rablador

Reviewable status: 0 of 34 files reviewed, 5 unresolved discussions

ios/MullvadVPN/AppDelegate.swift line 408 at r1 (raw file):

        let initTunnelManagerOperation = AsyncBlockOperation(dispatchQueue: .main) { finish in
            self.tunnelManager.loadConfiguration { error in
                // TODO: avoid throwing fatal error and show the problem report UI instead.

Reported separately in Linear.

ios/MullvadVPN/View controllers/Preferences/PreferencesViewController.swift line 120 at r1 (raw file):

    }

    // swiftlint:disable:next function_body_length

Having a hard time shaving off more code from this function. It's certainly doable by eg. moving all localizable string creations outside, but there seems little point in doing so.

ios/MullvadVPN/View controllers/ProblemReport/ProblemReportInteractor.swift line 20 at r1 (raw file):

    private lazy var consolidatedLog: ConsolidatedApplicationLog = {
        let securityGroupIdentifier = ApplicationConfiguration.securityGroupIdentifier

Seems we redact all 16-digit numbers, so I'm assuming this is already taken care of.

ios/MullvadVPN/View controllers/Tunnel/ConnectionPanelView.swift line 78 at r1 (raw file):

        outAddressRow.translatesAutoresizingMaskIntoConstraints = false

        // Remove this line when we have out address

Reported to Linear. Kept this line to make it easy to find when fixing.

ios/MullvadVPN/Views/AppButton.swift line 289 at r1 (raw file):

    }

    // swiftlint:disable:next cyclomatic_complexity

The switch is what's causing the warning. I think it's fine to keep it as-is and disable the warning.

buggmagnet

Reviewed 29 of 34 files at r1, 7 of 7 files at r2, all commit messages.
Reviewable status: 36 of 38 files reviewed, 15 unresolved discussions (waiting on @rablador)

ios/MullvadVPN/AppDelegate.swift line 495 at r2 (raw file):

    }

    // swiftlint:disable:next file_length

We should definitely have a Linear issue for this file (if we don't already)

ios/MullvadVPN/Classes/ConsolidatedApplicationLog.swift line 179 at r2 (raw file):

    private static func redactAccountNumber(string: String) -> String {
        redact(
            // swiftlint:disable:next force_try

Instead of manually using regexes to redact stuff, we should entirely get rid of this system and instead use
the OSLogPrivacy feature of the logging system to not log sensitive information in the first place.

Now that we've dropped iOS 13, we can use it everywhere without having to backport the API.

I will create a Linear issue about this

ios/MullvadVPN/Containers/Root/RootContainerViewController.swift line 844 at r2 (raw file):

    }

    // swiftlint:disable:next file_length

We should have a Linear issue about this.

ios/MullvadVPN/Coordinators/ApplicationCoordinator.swift line 983 at r2 (raw file):

    }

    // swiftlint:disable:next file_length

We should have a Linear issue about this

ios/MullvadVPN/Notifications/NotificationManager.swift line 210 at r2 (raw file):

        }

        invalidateInAppNotification(notificationProvider)

Simple, but efficient, love it !

ios/MullvadVPN/SimulatorTunnelProvider/SimulatorTunnelProvider.swift line 426 at r2 (raw file):

}

// swiftlint:disable:next file_length

There are a lot of classes in this file that merit being in their own files, and we should do that instead.
This should get rid of this warning.

ios/MullvadVPN/TunnelManager/TunnelManager.swift line 757 at r2 (raw file):

                }

            case .invalid:

Why was this TODO removed ? Do we not want to handle this anymore ?

ios/MullvadVPN/View controllers/Preferences/PreferencesViewController.swift line 120 at r1 (raw file):

Previously, rablador (Jon Petersson) wrote…

Having a hard time shaving off more code from this function. It's certainly doable by eg. moving all localizable string creations outside, but there seems little point in doing so.

The Localisation of the app has been a pet peeve of mine for a long while, we should definitely spend some time working on that, this adds so much visual noise for no good reason.

ios/MullvadVPN/Views/AppButton.swift line 289 at r1 (raw file):

Previously, rablador (Jon Petersson) wrote…

The switch is what's causing the warning. I think it's fine to keep it as-is and disable the warning.

I think still think this could be rewritten to remove this omega switch.
But ain't nobody go'time fo'dat

ios/MullvadVPNTests/DeviceCheckOperationTests.swift line 571 at r2 (raw file):

    }

    // swiftlint:disable:next file_length

Lots of non test code in this file that should be moved somewhere else.
I will create a Linear issue about this

ios/Operations/AsyncOperation.swift line 427 at r2 (raw file):

    }

    // swiftlint:disable:next file_length

If we instead move the OperationBlockObserverSupport definition and extension to its own file, this should get rid of the warning

ios/PacketTunnel/PacketTunnelProvider.swift line 767 at r2 (raw file):

    }

    // swiftlint:disable:next file_length

I think we already have a Linear issue about this, but I will create one if there isn't

pronebird

Reviewed 1 of 34 files at r1.
Reviewable status: 37 of 38 files reviewed, 14 unresolved discussions (waiting on @buggmagnet and @rablador)

ios/MullvadVPN/Views/AppButton.swift line 289 at r1 (raw file):