-
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
- rule added i.e. redundant_extension #5359 #5397
base: main
Are you sure you want to change the base?
Conversation
To be honest, I don't deem this rule to be as useful as I intentionally thought. The OSS report contains a lot of false positives. It's pretty common to conform a foreign type to a local protocol. And quite often the type already comes with the requirements defined in the protocol causing an empty body. So blindly triggering on every extension with an empty body isn't the way to go in my opinion. What might be acceptable is that the rule only triggers on empty extension if the extended type is defined in the same file. In that case, the conformance can just be moved to the type's definition. This could be a convention that projects follow, and the rule could support them with it. That said, I think this rule requires some more thought and, admittedly, the implementation for what I suggest above is a bit more involved. |
It seems that the rule is really trying to flag:
but is also trapping good and correct code like:
I think the rule is just being too loose for what the Issue and Examples suggest. The OSS failures are calling out this deficiency. |
Rule added for avoiding redundant extensions resolve #5359