When no lintable files are found, serialization fails

[thedavidharris]

The output when the only changed files are ignored by the SwiftLint config seems to be:

No lintable files found at paths: ''

which then fails linting.

Should we just try and catch that message directly before serializing?

Output from the messaging is:

{
  "messages" : [
  ],
  "markdowns" : [
  ],
  "meta" : {
    "runtimeHref" : "https:\/\/danger.systems\/swift",
    "runtimeName" : "Danger Swift"
  },
  "fails" : [
    {
      "message" : "Error deserializing SwiftLint JSON response (): dataCorrupted(Swift.DecodingError.Context(codingPath: [], debugDescription: \"The given data was not valid JSON.\", underlyingError: Optional(Error Domain=NSCocoaErrorDomain Code=3840 \"No value.\" UserInfo={NSDebugDescription=No value.})))"
    }
  ],
  "warnings" : [
  ]
}

[orta]

How do you end up in this position? passing all modified files into the swiftlint api and it has no swift files?

[thedavidharris]

If there is a modified file, and you run swiftlint but that modified file is covered by the “excluded” section of the config, it seems to trigger this.

[f-meloni]

I think is related to this change #180.
We have arguments.append("--use-script-input-files"), but if there are no file the script input files is empty

EDIT: actually not because there is

guard !files.isEmpty else {
  return []
}

Then yes is probably related to the excluded files as @thedavidharris was suggesting

[f-meloni]

Looking at swiftlint code it looks exactly that https://github.com/realm/SwiftLint/blob/a97487f3e1540be97bbfef0d491f28302413d7c6/Source/swiftlint/Extensions/Configuration%2BCommandLine.swift

[f-meloni]

    fileprivate func getFiles(with visitor: LintableFilesVisitor) -> Result<[File], CommandantError<()>> {
        if visitor.useSTDIN {
            let stdinData = FileHandle.standardInput.readDataToEndOfFile()
            if let stdinString = String(data: stdinData, encoding: .utf8) {
                return .success([File(contents: stdinString)])
            }
            return .failure(.usageError(description: "stdin isn't a UTF8-encoded string"))
        } else if visitor.useScriptInputFiles {
            return scriptInputFiles()
                .map { files in
                    guard visitor.forceExclude else {
                        return files
                    }

                    let scriptInputPaths = files.compactMap { $0.path }
                    return filterExcludedPaths(in: scriptInputPaths)
                            .map(File.init(pathDeferringReading:))
                }
        }
        if !visitor.quiet {
            let filesInfo: String
            if visitor.paths.isEmpty {
                filesInfo = "in current working directory"
            } else {
                filesInfo = "at paths \(visitor.paths.joined(separator: ", "))"
            }

            queuedPrintError("\(visitor.action) Swift files \(filesInfo)")
        }
        return .success(visitor.paths.flatMap {
            self.lintableFiles(inPath: $0, forceExclude: visitor.forceExclude)
        })
    }

        if files.isEmpty {
            let errorMessage = "No lintable files found at paths: '\(visitor.paths.joined(separator: ", "))'"
            return .failure(.usageError(description: errorMessage))
        }

[thedavidharris]

We could naively just look for No lintable files found at paths found in the stdout and do something with it, but if we want something more general purpose I'd be happy to try to get a PR up

[f-meloni]

PRs are always welcome :)
I think that looking for No lintable files found at paths could be a problem, if they change the string danger just breaks

[f-meloni]

I did find a solution, but I'm not happy with it... Avoid to read the stderror works, but means that if swiftlint fails we just ignore it

[thedavidharris]

Feels like there's a good longer term solution of importing the SwiftLint framework directly instead of depending on stdin/stdout.

I feel like on the Swiftlint side that shouldn't go to stderr though... if you pass ignored files it should just send back an empty success? Not like there are any linting violations

[f-meloni]

I agree, I think is like that because their use case is probably a little bit different (is meant to be used on the shell build task).
Importing Swiftlint could be a problem, because is a big dependency, and adding it for a non core functionality means that everyone needs to download and compile it even if they don't use swiftlint

[thedavidharris]

Ah true, forgot it's not bundled with it here.

Cursory look through swiftlint looks like it only outputs to stderr if the linting scenario is invalid, which should be guarded by this wrapper right?

[thedavidharris]

Yeah looked through the SwiftLint source a bit more, and the stderr outputs seem to only occur on usage errors; unfortunately, all of them are typically reasons to actually fail (like bad flags, etc.) except for this one, where the only changes are in the ignored files

[f-meloni]

That is today, we can not make assumption on what they use the standard error for.
Also still there could be something relevant there.
An alternative could be generate a warning with the error content like:
"Swiftlint return with exit code (exitCode), reason: (reason)"

[thedavidharris]

True. Definitely better than the alternative, but it doesn't capture what should be the correct scenario here which is that when you only change files in the .swiftlint.yml's ignored section, Danger should pass without issue

[f-meloni]

You are right, the thing is more that we can not know what is going on on swiftlint, best thing for us would be fix it on swiftlint itself

[thedavidharris]

Conveniently: realm/SwiftLint#2608

[ecerney]

@thedavidharris @f-meloni did you ever figure out a workaround (without having to change swiftlint directly)? I don't believe I can even put a fix into my dangerfile to look for that error message and ignore it right? I can check the output of lint but and post an additional message, but I believe swiftlint will still fail the step right?

[f-meloni]

@ecerney sorry for the really big delay, I totally missed your message, I believe this issue is been fixed on realm/SwiftLint#2732

[thedavidharris]

That's correct, just noticing this as well, so I'll close it