Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error severity for every rule using .swiftlint.yml #5220

Closed
juanuribeo13 opened this issue Sep 13, 2023 · 14 comments · Fixed by #5226
Closed

Error severity for every rule using .swiftlint.yml #5220

juanuribeo13 opened this issue Sep 13, 2023 · 14 comments · Fixed by #5226
Labels
enhancement Ideas for improvements of existing features and rules. good first issue Issue to be taken up by new contributors yet unfamiliar with the project.

Comments

@juanuribeo13
Copy link

Would it be possible to have a configuration in .swiftlint.yml that treats all violations as errors? It'll be nice to have the option to enable what the --strict flag does but from the .swiftlint.yml config file. I'd like to run swiftlint on strict mode but I'm using SPM to add it into my project and I can't find a way to pass the --strict flag to the plugin command. I'm thinking on forking the repository and updating the plugin code on my side, but that solution isn't ideal since I'd have to keep updating that fork to get updates.

@SimplyDanny
Copy link
Collaborator

That's a reasonable request. In the meantime, until it's implemented, you can set the severity of every enabled rule manually to error in your configuration.

@SimplyDanny SimplyDanny added enhancement Ideas for improvements of existing features and rules. good first issue Issue to be taken up by new contributors yet unfamiliar with the project. labels Sep 17, 2023
@mildm8nnered
Copy link
Collaborator

I have a branch that implements this - will try to put it up later today

@mildm8nnered
Copy link
Collaborator

See #5226

@juanuribeo13
Copy link
Author

Thanks for the quick turnaround!

I just updated my project to point to the commit with this change (f783b0279a92c6fef29560453848df4899f89172), if I use the command swift run swiftlint, I see the strict: true configuration taking effect and failing as expected when there's a violation.

But when I open Xcode and use the plugin, I'm getting a warning that says Configuration contains invalid keys strict. and the violations are still treated as warnings and not errors.

@mildm8nnered
Copy link
Collaborator

ok - are you sure that your plugin is picking up the right version of SwiftLint? Because I can see where that error would be thrown from, and looking at the list of keys it should accept as valid, strict should definitely be in there.

If it wasn't, you would also see that warning when running swiftlint directly ...

@juanuribeo13
Copy link
Author

I think it is grabbing the correct version. This is my Package.swift file

let package = Package(
    name: "******",
    platforms: [.iOS(.v15), .macOS(.v12)],
    products: [
        // Products define the executables and libraries a package produces, and make them visible to other packages.
        .library(
            name: "******",
            targets: ["******"]
        )
    ],
    dependencies: [
        // Dependencies declare other packages that this package depends on.
        .package(path: "******"),
        .package(url: "https://github.com/realm/SwiftLint", branch: "main")
    ],
    targets: [
        // Targets are the basic building blocks of a package. A target can define a module or a test suite.
        // Targets can depend on other targets in this package, and on products in packages this package depends on.
        .target(
            name: "******",
            dependencies: ["******"],
            plugins: [
                .plugin(name: "SwiftLintPlugin", package: "SwiftLint")
            ]
        ),
        .testTarget(
            name: "******",
            dependencies: ["******"]
        )
    ]
)

And here's the relevant piece for the Package.resolved

    {
      "identity" : "swiftlint",
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/realm/SwiftLint",
      "state" : {
        "branch" : "main",
        "revision" : "f783b0279a92c6fef29560453848df4899f89172"
      }
    },

Here are the warnings I see on Xcode
Screenshot 2023-09-19 at 9 41 08 AM

And this is the output when I run swift run swiftlint

Building for debugging...
Build complete! (0.20s)
Linting Swift files in current working directory
Linting 'TestExample.swift' (1/3)
Linting 'Package.swift' (2/3)
Linting 'GreetingView.swift' (3/3)
/Users/***/***/GreetingView.swift:14:1: error: Trailing Whitespace Violation: Lines should not have trailing whitespace (trailing_whitespace)
/Users/***/***/GreetingView.swift:15:1: error: Vertical Whitespace Violation: Limit vertical whitespace to a single empty line; currently 2 (vertical_whitespace)
Done linting! Found 2 violations, 2 serious in 3 files.

As you can see I'm only getting the warning on Xcode and doesn't seem to apply the strict mode.

@mildm8nnered
Copy link
Collaborator

Hmmm. So that does look right, but looking back at the source, I don't see how you could be getting those errors :-(

The error is only thrown from here:

    private static func warnAboutInvalidKeys(configurationDictionary dict: [String: Any], ruleList: RuleList) {
        // Log an error when supplying invalid keys in the configuration dictionary
        let invalidKeys = Set(dict.keys).subtracting(validKeys(ruleList: ruleList))
        if invalidKeys.isNotEmpty {
            Issue.invalidConfigurationKeys(invalidKeys.sorted()).print()
        }
    }

where validKeys is defined as:

    private static func validKeys(ruleList: RuleList) -> Set<String> {
        return validGlobalKeys.union(ruleList.allValidIdentifiers())
    }

and validGlobalKeys is:

    private static let validGlobalKeys: Set<String> = Set(Key.allCases.map { $0.rawValue })

and Key is defined as:

    internal enum Key: String, CaseIterable {
        case cachePath = "cache_path"
        case disabledRules = "disabled_rules"
        case enabledRules = "enabled_rules" // deprecated in favor of optInRules
        case excluded = "excluded"
        case included = "included"
        case optInRules = "opt_in_rules"
        case reporter = "reporter"
        case swiftlintVersion = "swiftlint_version"
        case warningThreshold = "warning_threshold"
        case onlyRules = "only_rules"
        case indentation = "indentation"
        case analyzerRules = "analyzer_rules"
        case allowZeroLintableFiles = "allow_zero_lintable_files"
        case strict = "strict"
        case childConfig = "child_config"
        case parentConfig = "parent_config"
        case remoteConfigTimeout = "remote_timeout"
        case remoteConfigTimeoutIfCached = "remote_timeout_if_cached"
    }

I really think something must be going on on your side, despite what your Package.resolved says, otherwise you'd get the same results in both contexts.

If you search the SwiftLint package, does the string let strict = (strict && !options.lenient) || options.strict occur in LintOrAnalyzeCommand.swift? That line was introduced in the strict configuration PR.

The only other thing that occurs is the usual "Resetting Package Caches" / nuke Derived Data etc.

@juanuribeo13
Copy link
Author

Hey @mildm8nnered, I searched for let strict = (strict && !options.lenient) || options.strict and I see it on the SwiftLint package.

I also created a new package for testing this out and I'm getting the same results, the package is published here https://github.com/juanuribeo13/SwiftlintSPM in case you want to try it out.

@mildm8nnered
Copy link
Collaborator

I also created a new package for testing this out and I'm getting the same results, the package is published here https://github.com/juanuribeo13/SwiftlintSPM in case you want to try it out.

Thank you so much - I did try to get plugins to work for me locally a while ago to look at another issue, but I was doing something wrong somewhere.

I get package 'swiftlintspm' is using Swift tools version 5.9.0 but the installed version is 5.7.1 on my main (Monterey/Xcode 14.2) machine, but I've got another machine with Ventura/later versions of Xcode that I'll try to play around with in the next few days.

@juanuribeo13
Copy link
Author

I updated the Swift tools version to 5.7.1 so you can run it in your main machine. I'm now on Xcode 15 but it was also happening earlier this week when I had Xcode 14.

Thanks for all the help!

@mildm8nnered
Copy link
Collaborator

So ... getting closer

I'm on Monterey 12.6.6 (21G646), Xcode 14.2 (14C18)

Initially the build failed with error: The package product 'SwiftLintPlugin' requires minimum platform version 12.0 for the macOS platform, but this target supports 10.13 (in target 'SwiftlintSPM' from project 'SwiftlintSPM')

Adding

    platforms: [
        .macOS(.v12)
    ],

Fixed that.

Then I get the build succeeding, but it looks like it can't find swiftlint:

Apply build tool plug-in "SwiftLintPlugin" to "SwiftlintSPM" in package "swiftlintspm"

working directory:
   /Users/martin.redington/Documents/Source/SwiftlintSPM
tool mapping:
   swiftlint: /${BUILD_DIR}/${CONFIGURATION}/swiftlint
tool search paths:
   /Applications/Xcode-14.2.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
   /Applications/Xcode-14.2.app/Contents/Developer/usr/bin
   /bin
   /sbin
   /usr/bin
   /usr/sbin

SwiftLint

/usr/bin/sandbox-exec -p "(version 1)
(deny default)
(import \"system.sb\")
(allow file-read*)
(allow process*)
(allow file-write*
    (subpath \"/private/tmp\")
    (subpath \"/private/var/folders/1l/_cqfvyd16_g5jdgmngk59kl40000gp/T\")
)
(deny file-write*
    (subpath \"/Users/martin.redington/Documents/Source/SwiftlintSPM\")
)
(allow file-write*
    (subpath \"/Users/martin.redington/Library/Developer/Xcode/DerivedData/SwiftlintSPM-gpryfmjngiiqusaomhwhzywcgzli/SourcePackages/plugins/swiftlintspm.output/SwiftlintSPM/SwiftLintPlugin\")
)
" "/${BUILD_DIR}/${CONFIGURATION}/swiftlint" lint --quiet --force-exclude --cache-path /Users/martin.redington/Library/Developer/Xcode/DerivedData/SwiftlintSPM-gpryfmjngiiqusaomhwhzywcgzli/SourcePackages/plugins/swiftlintspm.output/SwiftlintSPM/SwiftLintPlugin --config /Users/martin.redington/Documents/Source/SwiftlintSPM/.swiftlint.yml /Users/martin.redington/Documents/Source/SwiftlintSPM/Sources/SwiftlintSPM/Example.swift

The file “swiftlint” doesn’t exist.

I can see

 % find  ~/Library/Developer/Xcode/DerivedData/SwiftlintSPM-gpryfmjngiiqusaomhwhzywcgzli -name swiftlint    
/Users/martin.redington/Library/Developer/Xcode/DerivedData/SwiftlintSPM-gpryfmjngiiqusaomhwhzywcgzli/SourcePackages/artifacts/extract/swiftlint
/Users/martin.redington/Library/Developer/Xcode/DerivedData/SwiftlintSPM-gpryfmjngiiqusaomhwhzywcgzli/SourcePackages/artifacts/swiftlint
/Users/martin.redington/Library/Developer/Xcode/DerivedData/SwiftlintSPM-gpryfmjngiiqusaomhwhzywcgzli/SourcePackages/artifacts/swiftlint/SwiftLintBinary.artifactbundle/swiftlint-0.52.4-macos/bin/swiftlint
/Users/martin.redington/Library/Developer/Xcode/DerivedData/SwiftlintSPM-gpryfmjngiiqusaomhwhzywcgzli/SourcePackages/checkouts/SwiftLint/Source/swiftlint
% find  ~/Library/Developer/Xcode/DerivedData/SwiftlintSPM-gpryfmjngiiqusaomhwhzywcgzli -name SwiftLintPlugin
/Users/martin.redington/Library/Developer/Xcode/DerivedData/SwiftlintSPM-gpryfmjngiiqusaomhwhzywcgzli/SourcePackages/plugins/SwiftLintPlugin
/Users/martin.redington/Library/Developer/Xcode/DerivedData/SwiftlintSPM-gpryfmjngiiqusaomhwhzywcgzli/SourcePackages/plugins/swiftlintspm.output/SwiftlintSPM/SwiftLintPlugin
/Users/martin.redington/Library/Developer/Xcode/DerivedData/SwiftlintSPM-gpryfmjngiiqusaomhwhzywcgzli/SourcePackages/plugins/SwiftLintPlugin.dSYM/Contents/Resources/DWARF/SwiftLintPlugin
/Users/martin.redington/Library/Developer/Xcode/DerivedData/SwiftlintSPM-gpryfmjngiiqusaomhwhzywcgzli/SourcePackages/checkouts/SwiftLint/Plugins/SwiftLintPlugin

Not sure how to get that working :-(

@mildm8nnered
Copy link
Collaborator

So did a little bit more digging on this. I still can't get it to build though :-(

So looking in SwiftLint's Package.swift, I can see the following:

    targets: [
        .plugin(
            name: "SwiftLintPlugin",
            capability: .buildTool(),
            dependencies: [
                .target(name: "SwiftLintBinary", condition: .when(platforms: [.macOS])),
                .target(name: "swiftlint", condition: .when(platforms: [.linux]))
            ]
        ),
        ...
        .binaryTarget(
            name: "SwiftLintBinary",
            url: "https://github.com/realm/SwiftLint/releases/download/0.52.4/SwiftLintBinary-macos.artifactbundle.zip",
            checksum: "8a8095e6235a07d00f34a9e500e7568b359f6f66a249f36d12cd846017a8c6f5"
        )
]

I think the plugin is just executing that binary, and I can see the binary under ~/Library/Developer/Xcode/DerivedData/SwiftlintSPM-gpryfmjngiiqusaomhwhzywcgzli, in SourcePackages/artifacts/swiftlint/SwiftLintBinary.artifactbundle/swiftlint-0.52.4-macos/bin/swiftlint

When I build your SwiftLintSPM project, I can see it building SwiftLint again, but I can't actually see any new artifacts under DerivedData.

If I'm right about swiftlint-0.52.4-macos/bin/swiftlint, then that would explain why you're getting the warning about strict, and that would mean that you'd need to wait for the next release to use it :-(

Still don't really understand why we're compiling SwiftLint again if we are just going to use the binary.

@mildm8nnered
Copy link
Collaborator

ok - finally worked it out - #5032 was blocking me - Xcode 14.3 and up is required to get plugins to work, and that was also why I was getting the strange recompilation behavior.

So even though the correct source is being downloaded, the plugin is going to use the 0.52.4 binary, which is why you're getting the error.

You could create a branch that points to a different binary and use that as a workaround I guess, until these changes make it into a release.

@juanuribeo13
Copy link
Author

Ohhh ok, thanks for looking into it! I'll keep an eye for the next release then 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Ideas for improvements of existing features and rules. good first issue Issue to be taken up by new contributors yet unfamiliar with the project.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants