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

Swiftlint analyzer fails when the app's Display Name contains a blank #3021

Closed
2 tasks done
gereons opened this issue Jan 6, 2020 · 20 comments
Closed
2 tasks done

Swiftlint analyzer fails when the app's Display Name contains a blank #3021

gereons opened this issue Jan 6, 2020 · 20 comments
Labels
bug Unexpected and reproducible misbehavior.

Comments

@gereons
Copy link

gereons commented Jan 6, 2020

New Issue Checklist

Describe the bug

I'm running swiftlint analyze with rule explicit_self enabled in my project: https://github.com/gereons/SwiftLintBug2 (use the enclosed lint.sh to reproduce).

Expected result: swiftlint reports the violations

Actual result: swiftlint does not analyze any files

$ ./lint.sh
note: Using new build system

** CLEAN SUCCEEDED **

Loading configuration from '.swiftlint.yml'
Analyzing Swift files at paths
Done analyzing! Found 0 violations, 0 serious in 0 files.

Instead of the expected output of

Analyzing Swift files at paths
Analyzing 'ExplicitSelf.swift' (1/4)
Analyzing 'AppDelegate.swift' (2/4)
Analyzing 'ContentView.swift' (3/4)
Analyzing 'SceneDelegate.swift' (4/4)
... offending files
Done analyzing! Found 3 violations, 0 serious in 4 files.

Opening the .xcodeproj and removing the blank from the app's "Display Name" makes swiftlint work again.

Swiftlint version: 0.38.1, installed via homebrew
Xcode version: 11.3

@jpsim jpsim added the bug Unexpected and reproducible misbehavior. label Jan 7, 2020
@jpsim
Copy link
Collaborator

jpsim commented Jan 7, 2020

Thanks for taking the time to create a project to reproduce the issue.

@DimDL
Copy link

DimDL commented Jan 23, 2020

Hi There. I can confirm I hit the same issue, but in my case changing the BUNDLE_DISPLAY_NAME wasn't enough, I also had to change the target name (which IMHO makes more sense) to make SwiftLint analyze work. Config: SwiftLint 0.38.2.

@stale
Copy link

stale bot commented Nov 8, 2020

This issue has been automatically marked as stale because it has not had any recent activity. Please comment to prevent this issue from being closed. Thank you for your contributions!

@stale stale bot added the wontfix Issues that became stale and were auto-closed by a bot. label Nov 8, 2020
@gereons
Copy link
Author

gereons commented Nov 9, 2020

This issue is still present in v0.41.0 and should not be closed, IMO

@stale stale bot removed the wontfix Issues that became stale and were auto-closed by a bot. label Nov 9, 2020
@stephengroombbc
Copy link

stephengroombbc commented Dec 16, 2020

The issue is that CompilerArgumentsExtractor.allCompilerInvocations does a .components(separatedBy: " ") and doesn't account for app\ name.

Doing something like

let invocation = String(line[swiftcIndex...])
                    .split(usingRegex: "(?<!\\\\) ")
                    .map({ $0.replacingOccurrences(of: "\\", with: "") })
                    .expandingResponseFiles
                    .joined(separator: " ")

(splitting by spaces not preceded by a "\" and then replacing "\" with "") solves the problem.

EDIT: split(usingRegex) would have to be implemented of course. I have a version of it found on StackOverflow that I'm really not happy with which works but is bad, hence not submitting this as a PR.

@stephengroombbc
Copy link

stephengroombbc commented Dec 17, 2020

and then replacing "\" with ""

This introduces another bug. It appears that some the \s in compiler arguments is required as removing them all breaks the sourcekitten index query for some files

@stale
Copy link

stale bot commented Feb 15, 2021

This issue has been automatically marked as stale because it has not had any recent activity. Please comment to prevent this issue from being closed. Thank you for your contributions!

@stale stale bot added the wontfix Issues that became stale and were auto-closed by a bot. label Feb 15, 2021
@gereons
Copy link
Author

gereons commented Feb 16, 2021

This issue is still present in v0.42.0 and should not be closed, IMO

@stale stale bot removed the wontfix Issues that became stale and were auto-closed by a bot. label Feb 16, 2021
@jpsim
Copy link
Collaborator

jpsim commented Feb 25, 2021

This can be worked around by using the --compile-commands option and passing a JSON file in the compilation database format instead of relying on extracting compilation arguments from the Xcode build log. Support for that was added in this PR, which might give more context: #2962

@gereons
Copy link
Author

gereons commented Feb 26, 2021

How would I generate such compilation database file using xcodebuild? I've tried using

xcodebuild -project SwiftLintTest.xcodeproj -scheme SwiftLintTest | xcpretty -r json-compilation-database -o compile_commands.json
swiftlint analyze --compile-commands compile_commands.json

in the example project I've linked, but this still outputs Done analyzing! Found 0 violations, 0 serious in 0 files.. I'm using xcpretty v0.3.0.

compile_commands.json only contains an empty JSON array, [].

@jpsim
Copy link
Collaborator

jpsim commented Feb 26, 2021

If the the compilation database is empty, maybe that's a bug or limitation in xcpretty? Maybe it fails for the same reason SwiftLint is failing here (the space in the scheme name)?

Maybe this tool works? https://github.com/jerrymarino/XcodeCompilationDatabase

@gereons
Copy link
Author

gereons commented Feb 27, 2021

Thanks for the pointer. The tool you've linked does indeed produce a compile_commands.json with content that looks reasonable, but feeding this into SwiftLint yields:

Could not read compilation database at path: 'compile_commands.json' Missing or invalid 
(must be an array of strings) 'arguments' key in compile_commands.json at index 0

This seems to be a bug in the parser for the compilation database, if I understand the documentation correctly - arguments is optional when command is present, which it is in this case.

@jpsim
Copy link
Collaborator

jpsim commented Mar 2, 2021

Is there any chance you could share step-by-step instructions to reproduce this? It’d be easier for me to help you if have a sample project to start debugging with.

@gereons
Copy link
Author

gereons commented Mar 3, 2021

Sure. I've updated the https://github.com/gereons/SwiftLintBug2 repo, simply clone it and run the enclosed lint.sh script

@jpsim
Copy link
Collaborator

jpsim commented Mar 3, 2021

Thanks for doing that, so here's the compilation database format SwiftLint is expecting:

[
  {
    "arguments": [
      "swiftc",
      "..."
    ],
    "file": "Modules/Module1/Sources/Source1.swift"
  },
  {
    "arguments": [
      "swiftc",
      "..."
    ],
    "file": "Modules/Module1/Sources/Source2.swift"
  }
]

And here's what's produced by XcodeCompilationDatabase in your sample project.

[
  {
    "command": "  /Applications/Xcode-12.4.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/swift -frontend -c -primary-file ...",
    "file": "/full/path/to/project/SwiftLintBug2/SwiftLintTest/AppDelegate.swift",
    "directory": ""
  },
  {
    "command": "  /Applications/Xcode-12.4.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/swift -frontend -c ...",
    "file": "/full/path/to/project/SwiftLintBug2/SwiftLintTest/ExplicitSelf.swift",
    "directory": ""
  }
]

There are lots of differences:

Expected Format XcodeCompilationDatabase
Entries have arguments Entries have command
Entries have swiftc invocations Entries have swift -frontend invocations
Files paths are relative Files paths are absolute

According to clang's compilation database format, either the command or the arguments value should be specified, so it seems that both are equally valid, however SwiftLint only checks for arguments.

  • command: The compile command executed. After JSON unescaping, this must be a valid command to rerun the exact compilation step for the translation unit in the environment the build system uses. Parameters use shell quoting and shell escaping of quotes, with " and \ being the only special characters. Shell expansion is not supported.
  • arguments: The compile command executed as list of strings. Either arguments or command is required.

I'll see if parsing the command values and extracting the arguments the same way we parse them from Xcode logs works for your project.

@jpsim
Copy link
Collaborator

jpsim commented Mar 3, 2021

It seems that SourceKit isn't satisfied if you give it swift frontend arguments, it needs swiftc arguments.

It logs these errors:

error: unknown argument: '-primary-file'
error: unknown argument: '-emit-module-doc-path'
error: unknown argument: '-emit-dependencies-path'
error: unknown argument: '-emit-reference-dependencies-path'
error: unknown argument: '-enable-objc-interop'
error: unknown argument: '-serialize-debugging-options'
error: unknown argument: '-enable-anonymous-context-mangled-names'
error: unknown argument: '-target-sdk-version'
error: unknown argument: '-index-system-modules'
error: option '-serialize-diagnostics-path' is not supported by 'swiftc'; did you mean to use 'swift'?

And if I filter these out:

let unsupportedArguments = [
    "-primary-file",
    "-emit-module-doc-path",
    "-emit-dependencies-path",
    "-emit-reference-dependencies-path",
    "-enable-objc-interop",
    "-serialize-debugging-options",
    "-enable-anonymous-context-mangled-names",
    "-target-sdk-version",
    "-index-system-modules"
]

arguments.removeAll(where: unsupportedArguments.contains)

let unsupportedOptions = ["-serialize-diagnostics-path"]
for option in unsupportedOptions {
    if let optionIndex = arguments.firstIndex(of: option) {
        arguments.remove(at: optionIndex) // Option parameter
        arguments.remove(at: optionIndex) // Option value
    }
}

the analysis appears to run without errors but produces no results:

Analyzing Swift files in current working directory
Analyzing 'AppDelegate.swift' (1/4)
Analyzing 'ExplicitSelf.swift' (2/4)
Analyzing 'ContentView.swift' (3/4)
Analyzing 'SceneDelegate.swift' (4/4)
Done analyzing! Found 0 violations, 0 serious in 4 files.
Program ended with exit code: 0

@jpsim
Copy link
Collaborator

jpsim commented Mar 3, 2021

I found a fix for the initial issue you reported, I should have a PR up later today.

@jpsim
Copy link
Collaborator

jpsim commented Mar 3, 2021

@gereons @DimDL @stephengroombbc can you please confirm that #3549 fixes this issue in your projects?

@stephengroombbc
Copy link

Works for me 🎉

@jpsim
Copy link
Collaborator

jpsim commented Mar 3, 2021

Thanks for checking! I've merged the fix to master.

@jpsim jpsim closed this as completed Mar 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unexpected and reproducible misbehavior.
Projects
None yet
Development

No branches or pull requests

4 participants