realm / SwiftLint

A tool to enforce Swift style and conventions.
https://realm.github.io/SwiftLint
MIT License
18.68k stars 2.22k forks source link

Rule Request: Properly unwrap "implicitly unwrapped optionals" #3064

Open sindresorhus opened 4 years ago

sindresorhus commented 4 years ago

New Issue Checklist

New rule request

  1. Why should this rule be added? Share links to existing discussion about what the community thinks about this.

Many of Apple's API's export "implicitly unwrapped optionals" instead of plain optionals. Usually because those APIs doesn't yet have null annotations. This is a footgun as Swift will not force you to properly unwrap it, which often cause crashes in production.

One example is EKCalendarItem#title, which exports var title: String! { get set }. It's very easy to just use it directly as event.title instead of doing event.title ?? "" or guard/if.

It would be great to have a rule that forces you to explicitly unwrap "implicitly unwrapped optionals" from imported frameworks. I don't think it should trigger on "implicitly unwrapped optionals" in the same file as that's usually intentional.

Some relevant discussion: https://forums.swift.org/t/disable-objc-bridging-as-implicitly-unwrapped-optionals/21966

  1. Provide several examples of what would and wouldn't trigger violations.

Would trigger:

// event.title is EKCalendarItem#title

foo(event.title)

Wouldn't trigger:

foo(event.title ?? "")
guard let title = event.title else {
    return
}

foo(title)
  1. Should the rule be configurable, if so what parameters should be configurable?

No

  1. Should the rule be opt-in or enabled by default? Why? See README.md for guidelines on when to mark a rule as opt-in.

I would make it default as it helps catch a lot of potential crashes in production.


This should really be an opt-in warning in Swift, but it doesn't look like that will happen anytime soon:

iliaskarim commented 4 years ago

A good configuration option would be to allow it to only apply to imported sources. That way developers can still use implicitly unwrapped options as they are intended to be used, that is, without any callsite unwrap.

stale[bot] commented 4 years ago

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!

sindresorhus commented 4 years ago

Please keep it open.

stale[bot] commented 3 years ago

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!

sindresorhus commented 3 years ago

Please keep it open.