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

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

Open
LinusU opened this issue Dec 17, 2018 · 3 comments
Open

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

LinusU opened this issue Dec 17, 2018 · 3 comments
Labels
rule-request Requests for a new rules.

Comments

@LinusU
Copy link

LinusU commented Dec 17, 2018

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
Copy link

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
Copy link
Author

LinusU commented Dec 20, 2018

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
Copy link
Collaborator

jpsim commented Dec 24, 2018

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.

@marcelofabri marcelofabri added the rule-request Requests for a new rules. label Dec 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule-request Requests for a new rules.
Projects
None yet
Development

No branches or pull requests

4 participants