Interface implementations of more general parameters confuse code inspector

Issue #1465 new
Aidan Harding created an issue

If I have an interface like this:

public interface SObjectFunction {
    SObject call(SObject anSObject);
}

Then a legitimate (i.e. SF will compile and run it) implementation is:

public class SObjectFunctionImplementation implements SObjectFunction {
    public SObject call(Object objectOrSObject) {
        return null;
    }
}

I’ve made my method more generic in the implementation so that it takes not just an SObject, but any Object. The code inspector reports this as an incomplete implementation of the SObjectFunction interface.

Comments (6)

  1. Scott Wells repo owner

    Just came back around to this. I'm actually very surprised that this is allowed (I verified that it isn't in Java) and am debating what to do about it. This means that the following holds true:

    SObjectFunction fn = new SObjectFunctionImplementation();
    Account a = null;
    // These compile
    fn.call(a);
    ((SObjectFunctionImpl) fn).call('Hello');
    // This does not
    fn.call('Hello');
    

    and that's just weird. What's the use case for this vs. having distinct call signatures for SObject vs. Object?

    I'm tempted to leave it as-is even though Salesforce will compile it as it just doesn't seem that's really ever what someone wants, but perhaps I'm about to learn something new...

  2. Aidan Harding reporter

    Hey Scott,

    Thanks for picking up on this. I have no idea what I had in mind in 2019!

    A lot of the time when I’m doing this sort of thing, I’m trying to work around the lack of generics.

    The behaviour in Salesforce is specific to interfaces. You cannot have contravariant parameters in methods that you override from a base class (which is like Java and C++). That makes sense because, in some circumstances, it’s not clear which method the language should call.

    But, you can have contravariant parameters when implementing an interface.

    I can’t remember exactly what I was trying to do, but here’s an example from C#: https://docs.microsoft.com/en-us/dotnet/standard/generics/covariance-and-contravariance#generic-interfaces-with-contravariant-type-parameters

    This makes sense because an interface is not about having multiple implementations of a function, it’s just about guaranteeing that there is a compatible one. If the compatible one happens to be more generic (i.e. contravariant parameters), that should be fine.

    While this is all good fun, I don’t imagine many of your users will care, so I won’t be offended if you don’t fix this! I scanned the codebase where I came across this, and I must have chosen another approach because I don’t have this issue in there.

  3. Scott Wells repo owner

    Yeah, I've also done some unnatural things in Apex to try to allow for code reuse where the language has limitations. And I'm certainly aware of narrowing the parameter/return types when implementing/overriding a method via covariance, but this is first time I've seen widening (or at least seen it work) via contravariance.

    I won't close this one just yet, but I probably won't address it just yet either. Thanks for the context!

  4. Eric Kintzer

    Here’s another use case that is legit Apex but inspector says Class Foo must implement the following methods or be declared abstract: doStuff(Map<Id, Account>)

    Interface

    public interface IFoo {
        void doStuff(Map<Id,Account> recs);  // specific SObjectType here: Account
    }
    

    Class implementing interface

    public inherited sharing class Foo  implements IFoo {
    
        public void doStuff(Map<Id,SObject> recs) {  // generic SObject type here
            System.debug(LoggingLevel.INFO,recs);
        }
    }
    

    Invoked as:

    IFoo f = new Foo();
    f.doStuff(new Map<Id,Account> ([SELECT Id FROM Account])); // executes just fine
    

    Visual proof

    That said, I think it is best practice to have the existing inspector exploit the interface definition, because if spec’d as intended, the interface should be adhered to by implementing classes. I’m not arguing for any fix here.

  5. Scott Wells repo owner

    Yeah, that's effectively the same thing where the concrete implementation widens the type. Again, I'm not going to close this one just yet, but I'm also not in a hurry to address it. Well...to be clear, there's an aspect of my pride that tells me, "if it compiles, IC2 should be fine with it", but that's a very slippery slope given some of the corner cases that actually compile in Apex.

    I think if I do implement a fix for this, I'll likely also add a code inspection that flags it as a warning.

  6. Log in to comment