Processing of command line paths is unintuitive and confusing

mildm8nnered commented 1 year ago

New Issue Checklist

[X] Updated SwiftLint to the latest version
[X] I searched for existing GitHub issues

Describe the bug

Consider the case where I have a .swiftlint.yml in my project, which specifies some included paths (like SwiftLint's own configuration), and I'm running swiftlint on the command line from the top level directory.

% head .swiftlint.yml
included:
  - Plugins
  - Source
  - Tests
excluded:
  - Tests/SwiftLintFrameworkTests/Resources
analyzer_rules:
  - unused_declaration
  - unused_import
opt_in_rules:

(excluded: can be ignored for the moment).

Some of the invocations of swiftlint do not work as expected for me. All examples using SwiftLint itself as the project.

1) swiftlint

Expected: Should run swiftlint using the configuration file.

Works as expected (Linting Swift files in current working directory could perhaps be clearer, because it's actually linting the files specified in the config).

% swiftlint                      
Linting Swift files in current working directory
Linting 'SwiftLintPlugin.swift' (1/544)
Linting 'Path+Helpers.swift' (2/544)
Linting 'BodyLengthRuleVisitor.swift' (3/544)
Linting 'CommandVisitor.swift' (4/544)

2) swiftlint Source/SwiftLintFramework/Reporters/CSVReporter.swift (single file(s))

Expected: Should run swiftlint using the configuration file, but only on the specified file(s).

Works as expected:

 % swiftlint Source/SwiftLintFramework/Reporters/CSVReporter.swift 
Linting Swift files at paths Source/SwiftLintFramework/Reporters/CSVReporter.swift
Linting 'CSVReporter.swift' (1/1)
Done linting! Found 0 violations, 0 serious in 1 file.

3) swiftlint Source/SwiftLintFramework/Reporters/CSVReporter.swift a.swift (single files, where some do not exist)

Expected: Should run swiftlint using the configuration file, but only on the specified file(s), and should report any files that do not exist.

Observed: SwiftLint lints Source/SwiftLintFramework/Reporters/CSVReporter.swift, and then lints all 544 files defined in the configuration (so presumably CSVReporter gets linted twice).

And there is an obvious mismatch between Linting Swift files at paths Source/SwiftLintFramework/Reporters/CSVReporter.swift, a.swift and what actually happens.

% swiftlint Source/SwiftLintFramework/Reporters/CSVReporter.swift a.swift
Linting Swift files at paths Source/SwiftLintFramework/Reporters/CSVReporter.swift, a.swift
Linting 'SwiftLintPlugin.swift' (1/545)
Linting 'Path+Helpers.swift' (2/545)
Linting 'Source/SwiftLintFramework/Reporters/CSVReporter.swift' (3/545)
Linting 'BodyLengthRuleVisitor.swift' (4/545)

4) swiftlint Source - directory or directories

Expected: any swift files under the specified directory or directories will get linted.

Observed: The supplied directories will be ignored, and the files specified in the configuration will be used instead even though swiftlint says Linting Swift files at paths Source

% swiftlint Source
Linting Swift files at paths Source
Linting 'SwiftLintPlugin.swift' (1/544)
Linting 'Path+Helpers.swift' (2/544)
Linting 'BodyLengthRuleVisitor.swift' (3/544)

But, if the included entries in .swiftlint.yml are commented out, then the directories specified on the command line will be respected:

% swiftlint Source    
Linting Swift files at paths Source
Linting 'BodyLengthRuleVisitor.swift' (1/446)
Linting 'RuleListDocumentation.swift' (2/446)
Linting 'RuleDocumentation.swift' (4/446)

I think what would be most natural here is that if any paths are supplied on the command line, swiftlint should ignore included from the configuration file, and report any paths that don't actually exist, in a similar way to, for example

% ls -l Gemfile Gemfile.nope
ls: Gemfile.nope: No such file or directory
-rw-r--r--  1 martin.redington  staff  72 16 Oct 17:50 Gemfile

Complete output when running SwiftLint, including the stack trace and command used

See above

Environment

SwiftLint version (run swiftlint version to be sure)? 0.50.3
Installation method used (Homebrew, CocoaPods, building from source, etc)? homebrew
Paste your configuration file:

SwiftLint's own .swiftlint.yml file

Are you using nested configurations? No
Which Xcode version are you using (check xcodebuild -version)?

Xcode 14.2
Build version 14C18

Do you have a sample that shows the issue?

See description above.

mildm8nnered commented 1 year ago

I think the relevant logic is here, but maybe there are good reasons for the current behaviour

    internal func lintablePaths(
        inPath path: String,
        forceExclude: Bool,
        excludeByPrefix: Bool = false,
        fileManager: LintableFileManager = FileManager.default
    ) -> [String] {
        if path.isFile {
            if forceExclude {
                return excludeByPrefix
                    ? filterExcludedPathsByPrefix(in: [path.absolutePathStandardized()])
                    : filterExcludedPaths(fileManager: fileManager, in: [path.absolutePathStandardized()])
            }
            // If path is a file and we're not forcing excludes, skip filtering with excluded/included paths
            return [path]
        }

        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)
    }

AlexKobachiJP commented 1 year ago

Am running into this as well.

Please ignore below, that seems to be fixed in main. I had run 0.50.3 locally when I investigated but just built and tested current main and cannot reproduce it.

In addition: the interplay with the --force-exclude flag doesn't work as expected. Continuing the example 2 above, running swiftlint Source/SwiftLintFramework/Reporters/CSVReporter.swift will lint just that one file (as expected), but

~~5. swiftlint --force-exclude Source/SwiftLintFramework/Reporters/CSVReporter.swift (adding --force-exclude flag)~~

~~Expected: Forcing excludes should only ever decrease the number of files matched. Observed: The path in the argument is ignored and the whole repo is linted. Note that Source is an included directory and the file is not explicitly excluded.~~

realm / SwiftLint