-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Add prefer_scaled_to_fit opt-in rule
#6609
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,70 @@ | ||||||||
| import SwiftLintCore | ||||||||
| import SwiftSyntax | ||||||||
|
|
||||||||
| @SwiftSyntaxRule(optIn: true) | ||||||||
| struct PreferScaledToFitRule: Rule { | ||||||||
| var configuration = SeverityConfiguration<Self>(.warning) | ||||||||
|
|
||||||||
| static let description = RuleDescription( | ||||||||
| identifier: "prefer_scaled_to_fit", | ||||||||
| name: "Prefer Scaled To Fit", | ||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Title style:
Suggested change
|
||||||||
| description: "Prefer `scaledToFit()` or `scaledToFill()` over " + | ||||||||
| "`aspectRatio(contentMode:)` with a constant content mode", | ||||||||
| kind: .idiomatic, | ||||||||
| nonTriggeringExamples: [ | ||||||||
| Example("view.aspectRatio(ratio, contentMode: .fit)"), | ||||||||
| Example("view.aspectRatio(ratio, contentMode: .fill)"), | ||||||||
| Example("view.aspectRatio(contentMode: contentMode)"), | ||||||||
| Example("view.aspectRatio(contentMode: shouldFit ? .fit : .fill)"), | ||||||||
| Example("view.scaledToFit()"), | ||||||||
| Example("view.scaledToFill()"), | ||||||||
| ], | ||||||||
| triggeringExamples: [ | ||||||||
| Example("view.↓aspectRatio(contentMode: .fit)"), | ||||||||
| Example("view.↓aspectRatio(contentMode: .fill)"), | ||||||||
| Example("↓aspectRatio(contentMode: .fit)"), | ||||||||
| Example("↓aspectRatio(contentMode: .fill)"), | ||||||||
| ] | ||||||||
| ) | ||||||||
| } | ||||||||
|
|
||||||||
| private extension PreferScaledToFitRule { | ||||||||
| final class Visitor: ViolationsSyntaxVisitor<ConfigurationType> { | ||||||||
| override func visitPost(_ node: FunctionCallExprSyntax) { | ||||||||
| let functionName: String | ||||||||
| let violationPosition: AbsolutePosition | ||||||||
|
|
||||||||
| if let memberAccess = node.calledExpression.as(MemberAccessExprSyntax.self) { | ||||||||
| functionName = memberAccess.declName.baseName.text | ||||||||
| violationPosition = memberAccess.declName.baseName.positionAfterSkippingLeadingTrivia | ||||||||
| } else if let declRef = node.calledExpression.as(DeclReferenceExprSyntax.self) { | ||||||||
| functionName = declRef.baseName.text | ||||||||
| violationPosition = declRef.baseName.positionAfterSkippingLeadingTrivia | ||||||||
| } else { | ||||||||
| return | ||||||||
| } | ||||||||
|
|
||||||||
| guard functionName == "aspectRatio" else { | ||||||||
| return | ||||||||
| } | ||||||||
|
|
||||||||
| guard | ||||||||
| node.arguments.count == 1, | ||||||||
| let argument = node.arguments.first, | ||||||||
|
Comment on lines
+52
to
+53
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| argument.label?.text == "contentMode" | ||||||||
| else { | ||||||||
| return | ||||||||
| } | ||||||||
|
|
||||||||
| guard | ||||||||
| let memberValue = argument.expression.as(MemberAccessExprSyntax.self), | ||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we also check that |
||||||||
| let valueName = memberValue.declName.baseName.text as String?, | ||||||||
| valueName == "fit" || valueName == "fill" | ||||||||
|
Comment on lines
+61
to
+62
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| else { | ||||||||
| return | ||||||||
| } | ||||||||
|
|
||||||||
| violations.append(violationPosition) | ||||||||
| } | ||||||||
| } | ||||||||
| } | ||||||||
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.
There are a few
Legacy...rules already. I wonder whether we should name this one in a similar way. What do you think?All of them are enabled by default (not opt-in) and support auto-correction using
LegacyFunctionVisitor.