-
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 include_bare_init
option to explicit_init
rule
#5203
Add include_bare_init
option to explicit_init
rule
#5203
Conversation
include_bare_init
option to explicit_init
rule
Generated by 🚫 Danger |
bb3bd36
to
5dc6cb5
Compare
Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/ExplicitInitConfiguration.swift
Outdated
Show resolved
Hide resolved
Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/ExplicitInitConfiguration.swift
Outdated
Show resolved
Hide resolved
private(set) var severityConfiguration = SeverityConfiguration<Parent>(.warning) | ||
@ConfigurationElement(key: "include_explicit_init") | ||
private(set) var includeExplicitInit = true | ||
@ConfigurationElement(key: "include_bare_init") |
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.
Swift calls expression like .init(...)
"implicit member expression". Not sure we should use the term in an ExplicitInitRule, though. So "bare" might be okay. What do you think?
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.
So I'm not super-happy with "bare". "inferred" sounds a little bit better, and is at least in keeping with existing terms of art, but "inferred_init" doesn't feel quite right, as it's not the "init" that is inferred ... totally open to suggestions if anyone has something better.
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'm struggling as well. include_implict_init_members
maybe?
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.
Maybe bare
is ok - "implicit member expression" feels a little obscure
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 name would not matter so much if there was a description added to each option. The infrastructure is there already. I may prepare a PR in the upcoming days to open it up and add a few examples. More can then be attached over time.
Long story short, let's keep "bare". 😉
5dc6cb5
to
919d399
Compare
ed14259
to
6f2d8ad
Compare
Adds
include_bare_init
option to theexplicit_init
rule.This will produce violations where
.init
is called using type inference to determine the type.For example:
This option is off by default (as many code bases use the
.init
construct), and these violations are not correctable, as SwiftLint cannot infer the intended type.Also adds an
include_explicit_init
option, on by default, which allows the original behavior ofexplicit_init
to be suppressed, so that users can tune the rule's behavior to suit their particular use case.Replaces #5170, and resolves #2960