-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Add new uiimage_requires_bundle
Rule
#5177
base: main
Are you sure you want to change the base?
Conversation
@@ -206,6 +206,7 @@ public let builtInRules: [Rule.Type] = [ | |||
UnavailableConditionRule.self, | |||
UnavailableFunctionRule.self, | |||
UnhandledThrowingTaskRule.self, | |||
UIImageIncludesBundleRule.self, |
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.
I added this and the lines in GeneratedTests.swift to check the rule, but I'm sure there's a more appropriate way to get Sourcery involved here.
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.
make sourcery
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.
Looks good. Thank you! Please add a CHANGELOG entry to make people aware of the new rule.
static let description = RuleDescription( | ||
identifier: "uiimage_requires_bundle", | ||
name: "UIImage Requires Bundle", | ||
description: "`UIImage(named:) must specify a bundle via the `in:` parameter", |
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.
description: "`UIImage(named:) must specify a bundle via the `in:` parameter", | |
description: "`UIImage(named:)` must specify a bundle via the `in:` parameter", |
@@ -206,6 +206,7 @@ public let builtInRules: [Rule.Type] = [ | |||
UnavailableConditionRule.self, | |||
UnavailableFunctionRule.self, | |||
UnhandledThrowingTaskRule.self, | |||
UIImageIncludesBundleRule.self, |
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.
make sourcery
} | ||
} | ||
|
||
private extension TupleExprElementListSyntax { |
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.
This is a copy of the same extension in NSLocalizedStringRequireBundleRule
. Please move it to SwiftSyntax+SwiftLint.swift
to make it reusable.
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.
This pushed that file beyond 400 lines! I tried working around this by moving any extensions on structures defined inside the generated SyntaxCollections
(these three: AttributeListSyntax
, ModifierListSyntax
, and TupleExprElementListSyntax
), but I get this error: testSwiftLintLints(): failed - File name should match a type or extension declared in the file (if any)
This is on a file I called SwiftSyntaxCollections+SwiftLint.swift. How would you recommend getting around this?
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 file_length
rule has meanwhile been disabled in SwiftSyntax+SwiftLint.swift
. You don't have to bother with that anymore. I'd rather like to have these extension in one place.
Adds a new rule to help ensure that use of
UIImage(named:)
includes a bundle, helpful for modularized codebases.This was based on the very similar
NSLocalizedStringRequireBundleRule