Consistent Code Style

[Patrick-Kladek]

Description

As discussed in #33 we want to ensure a consistent code style. Therefore this PR adds SwiftLint and also SwiftFormat.

Tasks

• Add Swiftlint Plugin (this is a precompiled binary to reduce build time)
• Add SwiftFormat, and let it auto configure and save results
• Run SwiftFormat automatically on commit + add Documentation how to enable it.

Infos for Reviewer

SwiftLint requires iOS13 and above, if that's a deal breaker I'll remove SwiftLint

Had to disable the following default rules as they were producing a lot of errors which need some re-architecture and while doing so might break something:

• force_cast
• function_body_length
• identifier_name
• cyclomatic_complexity
• type_body_length
• file_length
• force_try

Now that SwiftLint runs again, how should we deal with the TODO annotations in the codebase that SwiftLint exposes?

[Patrick-Kladek]

@StephanPartzsch @boldtrn As discussed earlier this PR reformats the whole codebase. Can you please review it

[boldtrn]

It's a bit unfortunate that we need this now, but OK, I guess this is required for swiftlint

[Patrick-Kladek]

I've used SwiftLintPlugin which only contains the binary. We can of course always use SwiftLint repo and build it from source, I think this would eliminate this option, however it would significantly increase build time on our CI and also add various unrelated packages (like SourceKitten, ...) to users of this package

[boldtrn]

Sorry for being a bit hesitant here. Could we maybe step one step back and discuss if we need the SwiftLintPlugin. I am still not 100% sure we need this plugin.

For CI we could install SwiftLint through brew or we could install it through SPM. Would this change that much about the CI build time?

While looking at other repos that use SwiftLint I stumbled over the repo from @ianthetechie here: https://github.com/stadiamaps/maplibre-swiftui-dsl-playground

They seem to use the brew install method. I am open to discussing different options, but if we can use widely used standard tools in favour of a fork of a plugin, that requires also to change our build settings, then I am leaning towards going down the standard route.

[ianthetechie]

Actually, funny that you notice that @boldtrn. A few comments...

1. We actually use swiftformat rather than swiftlint. These are quite similar projects. I don't actually have much of an opinion on preferring one or the other (or both) for a project. We just happen to use swiftformat.
2. Aside: swiftformat is actually installed in the base GitHub runners. The main reason we run brew install is because we have no idea when GitHub will update it 😂 So to simplify things, we just have an internal policy of running the latest and using brew install in Actions.
3. I agree I'd prefer to minimize use of plugins and (this is purely personal preference) tend to go for CI checks over commit hooks as they are significantly more robust. Mostly this is because it's usually simpler, more robust, and has fewer potential security risks. Just my opinion.

[Patrick-Kladek]

I have worked in multiple companies now and it was de-facto standard to use SwiftLint. The reason I've added SwiftLint to Xcode and not actions was, that you see the warnings right away, instead of waiting for the CI to finish.

SwiftFormat can work in tandem with SwiftLint which is even encouraged, as it can automatically fix issues, the more complex ones needs to be fixed by the programmer.

If we move this to CI, how are we communicating back the warnings it found? Thats an unsolved issue right now, you would need to manually go to the checks tab and then click on summary to see the warnings - they don't show up in the PR.

Of course we can fail the build if a warning is detected, but for this we first need to resolve the existing ones, where we also don't have a clear path. I've already silenced quite a lot of warnings SwiftLint found as they need quite big changes which we want to do in separate PRs anyway.

[boldtrn]

Do we need the platform change from iOS 12 to 13?

I didn't check every line of the change to be honest. The parts I checked look good to me 👍

[Patrick-Kladek]

SwiftLint Package requires iOS13, maybe we can raise an Issue there https://github.com/lukepistrol/SwiftLintPlugin?

[StephanPartzsch]

Speaking of packages and SPM. Do we have in mind to add a line to the readme that SPM is now an additional option and not only carthage?

[boldtrn]

@StephanPartzsch, Carthage has been removed actually.

[boldtrn]

SwiftLint Package requires iOS13, maybe we can raise an Issue there https://github.com/lukepistrol/SwiftLintPlugin?

I general I think going to iOS13, if we have to is no problem. Doing this for tooling, I am not sure? Do we get enough benefit from SwiftLint that we believe this is worth it? Or should just run the auto format for now?

So SwiftLint introduces two possible issues so far:

• The build command with -skipPackagePluginValidation
• Raise iOS to 13

[Patrick-Kladek]

@boldtrn I've submitted a PR which drops the min version for SwiftLint, till this is merged we can use my fork to keep iOS12 supported.

I guess in September when Xcode 16 becomes mandatory, iOS13 becomes the last supported version, till then we can go ahead with my fork. Even if my changes are not accepted for half a year I can maintain the fork

[Patrick-Kladek]

SwiftLint Plugin was removed and is run on CI now. But we still don't have a solution on how to communicate back the results

[boldtrn]

Thank you very much for this awesome PR 👍 - the changes look good to me, let's get this merged :)