Process path/result so it's paths have same case as excludedPaths

[alexbbrown]

TL;DR;

When swiftLint is invoked like this:

swiftLint --config <config> /path/to/SwiftCode

on a folder

/path/to/SWIFTCODE/

where config includes an exclusion

excluded:
  - subPathToExclude

excludePaths can fail to actually exclude if <path> contains path elements whose case does not exactly match the filesystem.

This is because path and excludePaths are processed slightly differently, so that excludePaths gets case-corrected to the filesystem version but <path> does not.

This can occur in real situations because when checking out a git repo, the client can write the name of the git repo in ANY case. Scripts written in sibling repos may refer to it using their preferred case.

This patch fixes the case processing and makes them consistent.

Summary

Recompute the absolute path supplied on the command line as relative, so that it correctly resolves path case consistent with the exclude and include values.

To address this issue: #4415 - where a trivial use of excludes can fail if the (root) path supplied to lint is not case-identical to the path on the filesystem.

Detailed Issue

In detail: currently the included and excluded paths are always computed as relative paths, like so:

        includedPaths = includedPaths.map {
            $0.bridge().absolutePathRepresentation(rootDirectory: previousBasePath).path(relativeTo: newBasePath)
        }

        excludedPaths = excludedPaths.map {
            $0.bridge().absolutePathRepresentation(rootDirectory: previousBasePath).path(relativeTo: newBasePath)
        }
    }

while the rootPath is just taken verbatim, and is an absolute path.

The includedPaths, excludedPaths and the rootPath (aka path) are searched to find all lintable files

    let pathsForPath = includedPaths.isEmpty
            ? fileManager.filesToLint(inPath: path, rootDirectory: nil)
            : []

       let includedPaths = self.includedPaths
            .flatMap(Glob.resolveGlob)
            .parallelFlatMap { fileManager.filesToLint(inPath: $0, rootDirectory: rootDirectory) }

        return excludeByPrefix
            ? filterExcludedPathsByPrefix(in: pathsForPath, includedPaths)
            : filterExcludedPaths(fileManager: fileManager, in: pathsForPath, includedPaths)

In filterExcludedPathsByPrefix the set of lintable files reachable from the excludedPaths is subtracted from the set reachable from includedPaths plus rootPath.

let excludedPaths = self.excludedPaths(fileManager: fileManager)
result.minusSet(Set(excludedPaths))

This subtraction can fail if rootPath case does not exactly match the actual case of files on disk, leading to excluded paths being unexpectedly linted.

This is because rootPath is the only absolute path, and case errors in its path are not corrected.

However, the Include and Exclude paths are computed as paths relative to the CWD, and when they are converted back to absolute paths, any case errors are corrected.

Resolution

Compute the path as a path relative to CWD, and then turn it back to absolute.

Now the exact same processing that happens to the include/exclude paths is also applied to the rootPath, which means it has the same case.

Alternatives considered:

Alternative: change SourceKitten

This change could also be applied to sourceKitten's absolutePathRepresentation(rootDirectory: String = FileManager.default.currentDirectoryPath) to make its response to the different values more consistent. However that might affect other clients of that Package in unexpected ways, so instead I think it makes sense for SwiftLint to just use that API in a more consistent way.

Alternative: Case Insensitive Compare when excluding files

Instead of making the case consistent for the path/result and excludePaths path sets, we could just change it so it does a case insensitive match - of the whole URL or just the relative portion. However, it's possible some clients of swiftLint are using truly case insensitive file systems or rely on that for other reasons. Since I can't know that, I'm skipping that option.

Alternative: fix excludePaths so it doesn't adjust its case

Change excludePaths and includePaths so they don't rewrite the case of the base portions of their URLs.
That would make them consistent with the path/result URL compute. However, I don't know why that design decision was originally made: it might have an unexpected impact, so I don't want to change that. Someone with more insight may understand if that's appropriate.

[SwiftLintBot]

	1 Warning
⚠️	If this is a user-facing change, please include a CHANGELOG entry to credit yourself! You can find it at CHANGELOG.md.

	14 Messages
📖	Linting Aerial with this PR took 1.5s vs 1.5s on main (0% slower)
📖	Linting Alamofire with this PR took 2.06s vs 2.08s on main (0% faster)
📖	Linting DuckDuckGo with this PR took 4.27s vs 4.25s on main (0% slower)
📖	Linting Firefox with this PR took 9.33s vs 9.33s on main (0% slower)
📖	Linting Kickstarter with this PR took 15.09s vs 14.98s on main (0% slower)
📖	Linting Moya with this PR took 0.99s vs 0.99s on main (0% slower)
📖	Linting Nimble with this PR took 0.89s vs 0.88s on main (1% slower)
📖	Linting PocketCasts with this PR took 15.27s vs 15.46s on main (1% faster)
📖	Linting Quick with this PR took 0.31s vs 0.31s on main (0% slower)
📖	Linting Realm with this PR took 16.82s vs 16.77s on main (0% slower)
📖	Linting SourceKitten with this PR took 0.59s vs 0.59s on main (0% slower)
📖	Linting Sourcery with this PR took 3.7s vs 3.73s on main (0% faster)
📖	Linting Swift with this PR took 6.61s vs 6.63s on main (0% faster)
📖	Linting WordPress with this PR took 14.72s vs 14.68s on main (0% slower)

Here's an example of your CHANGELOG entry:

* Recompute the absolute path supplied on the command line as relative,….  
  [alexbbrown](https://github.com/alexbbrown)
  [#issue_number](https://github.com/realm/SwiftLint/issues/issue_number)

note: There are two invisible spaces after the entry's text.

Generated by 🚫 Danger

[alexbbrown]

Closing while I fix a couple issues

[alexbbrown]

It's hard to write a test for this without actually creating files on the fileSystem, because the behaviour that changes the case of the paths actually comes from NSURL (not fileManager) reading the disk

[alexbbrown]

I have re-opened this pull request. If anyone reviews this, I would appreciate suggestions on how to write a test that constructs a sample set of folders and swift files in a temporary directory (see the issue for a simple example) so I can write a test.

I can see only one example - in sandbox test code - where this is done so far in the SwiftLint tests.