Reject duplicate interface conformances

[SupunS]

Issue to be solved

Suggestion from @bluesign (please feel free to update, add more info)

If Vault conforms to Receiver
i.e:

resource interface Vault: Receiver {}

And an implementation/or another interface conforms to both Receiver and Vault
i.e:

resource VaultImpl: Vault, Receiver

Then report an error, since Vault already conforms to Receiver, and thus, VaultImpl explicitly conforming to Receiver again makes it a duplicate conformance.
i.e:

resource VaultImpl: Vault, Receiver    // should be an error because `VaultImpl` conforms to `Receiver` twice
                                       // - one implicitly, and one explicitly

Suggested Solution

See above

[bluesign]

thanks @SupunS , the reason for this suggestion is, if we don't allow resource VaultImpl: Vault, Receiver case, then we can allow in interface conformances to override default implementations.

something like this:

resource interface Receiver {
       fun deposit(from: @FT){
           // original default implementation
       }
}

resource interface Vault: Receiver {
       fun deposit(from: @FT){
           // new default implementation
       }
}

resource VaultImpl: Vault {
    
}

[turbolent]

Re-stating all interfaces in the type declaration's conformance list that the concrete type conforms to, is currently allowed and not required.

• This issue / @bluesign is proposing to forbid re-stating interfaces the type conforms to.
• However, previously there have been suggestions from @joshuahannan and @dete to require the concrete type to re-state all interfaces, especially interfaces the type conforms to implicitly through interface inheritance.

For the latter, the following pros/cons come to mind:

• Pros: Increases readability. It is immediately visible what interfaces the type conforms to, there is no need to determine the full set manually or through tooling.
• Cons:
  • Verbosity
  • Friction for downstream code: If an upstream interface conformance list is updated, all downstream concrete types which conform to the type must be updated, too

The sentiment in the team is that the current behaviour is a good middle ground.

If there are strong opinions for changing the behaviour to either forbidding or requiring, we can discuss this further in an upcoming LDM and propose the change through a FLIP.

[joshuahannan]

I'm still leaning towards requiring it. I don't have an issue with a little more verbosity, and the issue about upstream updates updating and breaking downstream code doesn't seem like a real issue to me, because if upstream code adds new interface conformances, they're probably going to break downstream code anyway because the downstream code will still have to conform to the new interface.

I don't feel super strongly about it though, so if everyone else wants to just keep the current behavior, I can accept that

[bjartek]

For me what is important here is that I can override a default implementation defined in something i extend. So lets say i want to compose on ViewResolver to change what the default views returned are. Currently i cannot do this, and @bluesign mentioned to me this issue would fix that.

If we do not allow that, anybody that creates interfaces has to do extra work since they cannot have any default implementations in the interface they require.

[joshuahannan]

@bjartek correct me if I am wrong, but I think that is a separate issue. I think this issue is just about the conformance list, not the default implementations

[bjartek]

Ok, then I might be confusing things.

If you publish an interface you can always add a new method and thus breaking everything relying on you regardless.

[bluesign]

@joshuahannan this is a requirement for that one actually

Consider:

resource interface Receiver {
       fun deposit(from: @FT){
           // original default implementation
       }
}

resource interface Vault: Receiver {
       fun deposit(from: @FT){
           // new default implementation
       }
}

if I declare my resource as:

resource VaultImpl: Vault, Receiver {
 }

there is a conflict with Vault and Receiver default implementations, it is not easy to decide which one to use, so we reject.

But if we declare it as:

resource VaultImpl: Vault {
 }

Then we can use the Vault implementation safely.

[bjartek]

So in your example @bluesign it is not possible to say that since Vault is defined before Receiver its implementation will superseed Receivers?

[bluesign]

So in your example @bluesign it is not possible to say that since Vault is defined before Receiver its implementation will superseed Receivers?

It should be possible but I think order of interfaces makes it little confusing. But totally viable option. I think optimum solution is to look for distance even resource VaultImpl: Vault, Receiver and resource VaultImpl: Receiver, Vault both should get from Vault. ( I think Java does like this http://tpcg.io/_AQVEJS )

[joshuahannan]

I see the issue now. yeah, if possible, it seems like we could say that since Vault inherits from Receiver, then we can just use the default implementation from Vault. but I could imagine that there are corner cases that might be more complicated than that

[bluesign]

Btw this is not urgent at all, we can always say that we will implement something like java later and select interface defaults by distance. My suggestion was just disallowing now, and maybe later release when we have the complicated implementation in place, I don't have too hard opinion on this.

[j1010001]

potentially braking change for 1.0

[turbolent]

I guess this change won't go into Cadence 1.0, so we can maybe remove it from #2642, and maybe even close it? @onflow/cadence wdyt?

[bluesign]

I am in favour of closing, it is not that important considering the current usage of interfaces actually.

[j1010001]

This would require a breaking change and we don't have consensus, closing.