-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Add legacy_uigraphics_functions
rule
#6269
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
base: main
Are you sure you want to change the base?
Conversation
Generated by 🚫 Danger |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the prompt implementation of your issue!
Please consider my comments ... Have you checked whether the OSS findings reported in the bot comment make sense?
* Add `legacy_uigraphics_functions` rule to encourage the use of modern | ||
UIGraphicsImageRenderer instead of legacy UIGraphics{Begin|End}ImageContext. | ||
The modern replacement is safer, cleaner, Retina-aware and more performant. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* Add `legacy_uigraphics_functions` rule to encourage the use of modern | |
UIGraphicsImageRenderer instead of legacy UIGraphics{Begin|End}ImageContext. | |
The modern replacement is safer, cleaner, Retina-aware and more performant. | |
* Add `legacy_uigraphics_functions` rule to encourage the use of modern | |
`UIGraphicsImageRenderer `instead of legacy `UIGraphics{Begin|End}ImageContext`. | |
The modern replacement is safer, cleaner, Retina-aware and more performant. |
@@ -0,0 +1,82 @@ | |||
import SwiftSyntax | |||
|
|||
@SwiftSyntaxRule |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rule should be opt-in as you have described in the issue:
@SwiftSyntaxRule | |
@SwiftSyntaxRule(optIn: true) |
kind: .idiomatic, | ||
nonTriggeringExamples: [ | ||
Example(""" | ||
let renderer = UIGraphicsImageRenderer(size: bounds.size) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests can be a bit easier, as the rule effectively only checks function names. So the let screenshot = ...
part isn't actually necessary.
I wasn't actually aware of Edit: I remove the helper in #6271 so that visitor and rewrite can directly be used. |
Add new
legacy_uigraphics_functions
rule to encourage the use of modern UIGraphicsImageRenderer instead of legacy UIGraphics{Begin|End}ImageContext.The modern replacement is safer, cleaner, Retina-aware and more performant.
The legacy API is deprecated
Resolves #6268