Make comma_inheritance opt-in #4047
Conversation
The Swift language grammar doesn't allow a comma in place of an ampersand in all cases. Specifically, if a protocol's associated type is required to conform to a composition of protocols and one of those protocols in turn requires conformance to one of the same protocols, the compiler will generate an error: "redundant conformance constraint". For example:
protocol Transformable {
associatedtype Unit: Equatable & CaseIterable & UnitConvertible
...
}
protocol UnitConvertible where Self: Equatable {
...
}
If comma_inheritance is enabled by default, autocorrection can introduce a new compile error. Making the rule opt-in allows code bases where it isn't a problem to opt-in and continue to use autocorrect, while code bases where it is a problem can leave it disabled.
Generated by 🚫 Danger |
|
Thank you! |
|
Shouldn't the right fix here be to avoid reporting violations for associated types? |
There is no difference between associated types and protocol definitions. In the example protocol UnitConvertible where Self: Equatable {}
protocol Transformable: Equatable, UnitConvertible { // ⚠️
associatedtype Unit: Equatable, UnitConvertible // ⚠️
}we get the same message from the compiler at both definitions. But: The messages are only warnings, as I see now. So I wonder why the SwiftLint rule and its fix does anything wrong here. Isn't it helpful to see that there are redundant conformances? |
|
Now that you say that, I'm pretty sure I have "warnings as errors" turned on. I still don't think it's great for SwiftLint's default configuration to potentially introduce compiler warnings into a codebase when |
|
Well, I think introducing this kind of warning is fine. It is not caused by SwiftLint doing anything wrong but is just made visible to the compiler. What is your view on this, @jpsim? |
|
Yeah there's no hard policy, the obvious ideal would be for SwiftLint to never have any false positives, any false negatives and never correct violations to cause previously compiling code to not compile. However, we're limited by operating on the pre-typechecked AST in lint rules, so we should strike a balance between offering broadly useful rules that as much as possible do the right thing. If the false-positive rate is too high and can't easily be reduced, then marking a rule as opt-in seems like a reasonable compromise to me. |
This addresses issue #4027.
The Swift language grammar doesn't allow a comma in place of an ampersand in all cases. Specifically, if a protocol's associated type is required to conform to a composition of protocols and one of those protocols in turn requires conformance to one of the same protocols, the compiler will generate an error: "redundant conformance constraint". For example:
When
&is replaced with,, this producesRedundant conformance constraint: 'Self.Unit' : 'Equatable'.My first thought was to disable autocorrect, but this penalizes those that want to use the rule without solving the problem for problematic code bases. It feels better to make the rule opt-in, so that developers who want to enforce the rule can still take advantage of autocorrection, while code bases where it is an issue can either leave it disabled or opt-in and selectively disable where necessary.