realm / SwiftLint

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

Rule Request: Verify that #imageLiteral references existing asset #2512

Open LinusU opened 5 years ago

LinusU commented 5 years ago

New rule request

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

Referencing a non-existing asset with #imageLiteral() will result in a crash at runtime. As seen e.g. here and as experienced by me today, it's very surprising that #imageLiteral() is not checked by the Swift compiler in the first place. I think the next best thing after that would be to have an error lint about it, which is why I created this suggestion ☺️

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

The following code would trigger a violation:

#imageLiteral(resourceNamed: "DoesNotExists")

The following code would not trigger a violation:

#imageLiteral(resourceNamed: "DoesExists")

3) Should the rule be configurable, if so what parameters should be configurable?

No.

4) 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 think the rule should be enabled by default (unless the implementation turns out to be slow) since it's a problem that will cause a crash at runtime, and I don't see that there will be any false positives.


I'd be willing to help contribute code for this ☺️

skagedal commented 5 years ago

This is indeed a surprising and annoying feature of image literals! To correctly check for this, you would have to check that the asset catalog containing the image is available in the target(s) that use the image literal. But maybe some simplified heuristics would be enough for many users.

I would still recommend using SwiftGen instead for a more stable solution to this problem.

LinusU commented 5 years ago

I would still recommend using SwiftGen instead for a more stable solution to this problem.

Personally, I prefer literals since they are statically analyzable, and doesn't require a separate tool for code generation. Each to their own though!

But maybe some simplified heuristics would be enough for many users.

Tried some very simple heuristics in my project and it works really well, but we only have a single asset catalog though...

jpsim commented 5 years ago

I'm in favor of this, but it would need some design discussion first. Right now, SwiftLint completely ignores anything that isn't in a Swift file, which includes collecting which assets are defined, and in which modules.