Swiftlint should consider excluded paths in config file

[absolute-heike]

Right now changes in e.g. Pods folder are also checked by Swiftlint, even though they are excluded in the local swiftlint.yml config file. This bug was fixed 8 days ago in realm/SwiftLint#2574 (version 0.30.1).

This PR changes the handling to pass the filelist to swiftlint as an array and set --force-exclude so the config exludes are adhered to.

Calls to Swiftlint should look like this: SCRIPT_INPUT_FILE_COUNT=2 SCRIPT_INPUT_FILE_0="Source/File1.swift" SCRIPT_INPUT_FILE_1="Source/File2.swift" swiftlint lint --quiet --use-script-input-files --force-exclude --reporter json

[f-meloni]

There are some tests that needs to be updated and maybe created (I would like to keep the swiftlint code well tested), but in general looks good 👍
This should also make it quite faster when you have Swiftlint on you Package.swift! 🎉

[absolute-heike]

Yeah, I will fix the tests for Swiftlint 👍

Gonna be interesting, because the tests heavily rely on the command just being swiftlint (or the swiftlintPath). But I suffixed the command with the Environment variables 😬

I'm thinking of extending the ShellExecutor protocol to support environment variables, as this would ease up a lot of the issues that we have with the tests 😉

This should also make it quite faster when you have Swiftlint on you Package.swift! 🎉

You mean, because only one instance of Swiftlint is started, instead of one per file? 😄

[f-meloni]

You mean, because only one instance of Swiftlint is started, instead of one per file? 😄

Yes :)

[f-meloni]

@absolute-heike if i understood your problem correctly I suppose you can use contains instead of == to find the command and then check that the final string is prefixed with the correct variables, something like

command == "SCRIPT_INPUT_FILE_COUNT=2 SCRIPT_INPUT_FILE_0=\"expectedFile0\" SCRIPT_INPUT_FILE_0=\"expectedFile1\" swiftlint"

[absolute-heike]

Yes I could do that. Do you prefer this approach?

The change to ShellExecutor would look kinda like this:

func execute(_ command: String, arguments: [String] = [], environmentVariables: [String] = []) -> String {
        let script = [environmentVariables.joined(separator: " "),
                      command,
                      arguments.joined(separator: " ")].filter { !$0.isEmpty }.joined(separator: " ")

[f-meloni]

Yep, cool :)

[DangerCI]

	Warnings
⚠️	Sources/Danger-Swift/Fake.swift#L1 - Files should have a single trailing newline. (trailing_newline)
⚠️	Sources/Danger/Plugins/SwiftLint/ShellExecutor.swift#L3 - TODOs should be resolved (Get a logger into here this is...). (todo)
⚠️	Sources/Danger/Plugins/SwiftLint/ShellExecutor.swift#L65 - Line should be 120 characters or less: currently 128 characters (line_length)
⚠️	Sources/Danger/Plugins/SwiftLint/SwiftLint.swift#L33 - Function should have complexity 10 or less: currently complexity equals 11 (cyclomatic_complexity)

Generated by 🚫 dangerJS against fdf904f

[f-meloni]

One minor comment, but looks good 👍

[absolute-heike]

Found a bug. When there is no file to lint (e.g. you didn't change a swift file), Swiftlint is still executed and prints No lintable files found at paths: '', hence danger-swift is failing cause it cannot parse this message as JSON.

• do not execute swiftlint, if no files should be linted
• test this behavior

[f-meloni]

I would suggest to not execute it in that case, we run on CI then we aim to be as fast as possible.
👍

[f-meloni]

Just removing the approval because of the bug :)

[f-meloni]

Nice!

[f-meloni]

merge on green