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

Dont fail when there is no file #397

Merged
merged 2 commits into from
Dec 8, 2020
Merged

Conversation

MadsBogeskov
Copy link
Contributor

In my process into moving to Danger Swift from Danger JS I got this error:

Fatal error: Unexpectedly found nil while unwrapping an Optional value: file Danger/SwiftLintViolation.swift, line 23
Stack dump:
0.	Program arguments: /Applications/Xcode_12.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/swift -frontend -interpret /var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/_tmp_dangerfile.swift -enable-objc-interop -stack-check -sdk /Applications/Xcode_12.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk -I /Users/runner/work/ios-app/ios-app/.build/debug -target-sdk-version 10.15.6 -module-name _tmp_dangerfile -lDanger -- runner /usr/local/lib/node_modules/danger/distribution/commands/danger-ci.js --process .build/debug/danger-swift --passURLForDSL /var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/danger-dsl.json /var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/danger-response.json 
1.	Apple Swift version 5.3 (swiftlang-1200.0.29.2 clang-1200.0.30.1)
2.	While running user code "/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/_tmp_dangerfile.swift"
0  swift                    0x0000000112e57865 llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 37
1  swift                    0x0000000112e56865 llvm::sys::RunSignalHandlers() + 85
2  swift                    0x0000000112e57e1f SignalHandler(int) + 111
3  libsystem_platform.dylib 0x00007fff7210e5fd _sigtramp + 29
4  libsystem_notify.dylib   0x00007fff721046da notify_register_plain + 711
5  libswiftCore.dylib       0x00007fff7156dcf7 $ss17_assertionFailure__4file4line5flagss5NeverOs12StaticStringV_A2HSus6UInt32VtFySRys5UInt8VGXEfU_yAMXEfU_ + 87
6  libswiftCore.dylib       0x00007fff7156d9c3 $ss17_assertionFailure__4file4line5flagss5NeverOs12StaticStringV_A2HSus6UInt32VtFySRys5UInt8VGXEfU_ + 99
7  libswiftCore.dylib       0x00007fff7156d5d5 $ss17_assertionFailure__4file4line5flagss5NeverOs12StaticStringV_A2HSus6UInt32VtF + 533
8  libDanger.dylib          0x0000000118287ed7 $s6Danger18SwiftLintViolationV10toMarkdownSSyF + 743
9  libDanger.dylib          0x0000000118285a4b $s6Danger9SwiftLintV4lint6danger13shellExecutor13swiftlintPath6inline9directory10configFile6strict0D8AllFiles07currentI8Provider06outputmI013reportDeleter14markdownAction04failW00x6InlineW004warnyW004readM0SayAA0bC9ViolationVGAA0A3DSLV_0a5ShellG014ShellExecuting_pSSSbSSSgA0_S2bAA07CurrentiR0_pSSAA23SwiftlintReportDeleting_pySSXEySSXEySS_SSSitXEySS_SSSitXES2SXEtFZSSAVXEfU1_ + 27
10 libDanger.dylib          0x0000000118285ab6 $s6Danger18SwiftLintViolationVSSs5Error_pIggozo_ACSSsAD_pIegnrzo_TR + 86
11 libDanger.dylib          0x0000000118286024 $s6Danger18SwiftLintViolationVSSs5Error_pIggozo_ACSSsAD_pIegnrzo_TRTA + 20
12 libswiftCore.dylib       0x00007fff7154d505 $sSlsE3mapySayqd__Gqd__7ElementQzKXEKlF + 757
13 libDanger.dylib          0x00000001182834f2 $s6Danger9SwiftLintV4lint6danger13shellExecutor13swiftlintPath6inline9directory10configFile6strict0D8AllFiles07currentI8Provider06outputmI013reportDeleter14markdownAction04failW00x6InlineW004warnyW004readM0SayAA0bC9ViolationVGAA0A3DSLV_0a5ShellG014ShellExecuting_pSSSbSSSgA0_S2bAA07CurrentiR0_pSSAA23SwiftlintReportDeleting_pySSXEySSXEySS_SSSitXEySS_SSSitXES2SXEtFZ + 3218
14 libDanger.dylib          0x0000000118282086 $s6Danger9SwiftLintV4lint6inline9directory10configFile6strict0D8AllFiles13swiftlintPathSayAA0bC9ViolationVGSb_SSSgANS2bANtFZ + 1078
15 libDanger.dylib          0x00000001184c809d $s6Danger9SwiftLintV4lint6inline9directory10configFile6strict0D8AllFiles13swiftlintPathSayAA0bC9ViolationVGSb_SSSgANS2bANtFZ + 2384973
16 swift                    0x000000010ea165cf llvm::orc::runAsMain(int (*)(int, char**), llvm::ArrayRef<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, llvm::Optional<llvm::StringRef>) + 2495
17 swift                    0x000000010e9f39cb swift::RunImmediately(swift::CompilerInstance&, std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > > const&, swift::IRGenOptions const&, swift::SILOptions const&, std::__1::unique_ptr<swift::SILModule, std::__1::default_delete<swift::SILModule> >&&) + 6747
18 swift                    0x000000010e9ceafa performCompileStepsPostSILGen(swift::CompilerInstance&, swift::CompilerInvocation const&, std::__1::unique_ptr<swift::SILModule, std::__1::default_delete<swift::SILModule> >, llvm::PointerUnion<swift::ModuleDecl*, swift::SourceFile*>, swift::PrimarySpecificPaths const&, int&, swift::FrontendObserver*) + 2426
19 swift                    0x000000010e9beae0 swift::performFrontend(llvm::ArrayRef<char const*>, char const*, void*, swift::FrontendObserver*) + 21152
20 swift                    0x000000010e93fc07 main + 1255
21 libdyld.dylib            0x00007fff71f11cc9 start + 1
22 libdyld.dylib            0x0000000000000017 start + 18446603338604536655
�[31mERROR: Dangerfile eval failed at Dangerfile.swift
�[0;0m�[31mERROR: Could not get the results JSON file at /var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/danger-response.json
�[0;0m

The Dangerfile is pretty basic:

import Danger 
let danger = Danger()

SwiftLint.lint()

When I ran SwiftLint locally I got this output:

  {
    "character" : null,
    "file" : "\/Users\/REDACTED\/Developer\/path\/ios-app\/Danger\/App.swift",
    "line" : 6,
    "reason" : "Files should have a single trailing newline.",
    "rule_id" : "trailing_newline",
    "severity" : "Warning",
    "type" : "Trailing Newline"
  },
  {
    "character" : 0,
    "file" : "",
    "line" : 0,
    "reason" : "Number of warnings exceeded threshold of 1.",
    "rule_id" : "warning_threshold",
    "severity" : "Error",
    "type" : "Warning Threshold"
  }

So the problem seems to be that the last block failed since Danger was force unwrapping the file field:

public func toMarkdown() -> String {
        let formattedFile = file.split(separator: "/").last! + ":\(line)"

This PR just fixes this issue by removing the force unwrap, however perhaps there should be a special case handling for errors of type: warning_threshold also.

@@ -20,7 +20,8 @@ public struct SwiftLintViolation: Decodable {
}

public func toMarkdown() -> String {
let formattedFile = file.split(separator: "/").last! + ":\(line)"
let filename = file.split(separator: "/").last ?? "unknown"
let formattedFile = filename + ":\(line)"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let formattedFile = filename + ":\(line)"
let formattedFile = "\(filename):\(line)"

IMO, unifying to interpolation or concatenation is better than mixed.

Copy link
Member

@f-meloni f-meloni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me, thank you for the PR :)

I agree with @417-72KI that would be better to not mix interpolation and concatenation, but I see it as minor so approving anyway.

Comment on lines +23 to +24
let filename = file.split(separator: "/").last ?? "unknown"
let formattedFile = filename + ":\(line)"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking that we could use an empty string instead, given is not relevant for that error.

Suggested change
let filename = file.split(separator: "/").last ?? "unknown"
let formattedFile = filename + ":\(line)"
let formattedFile = file.split(separator: "/").last.map { "\($0):\(line)" } ?? ""

@f-meloni f-meloni merged commit a9b120e into danger:master Dec 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants