Thank you for catching this. I think we should add confirmation of FIRDatabaseConnectionContextProvider class to FIRDatabaseConnectionContextProvider protocol in the header instead of casting. It seems that originally the method could return different classes but it is not the case any more, so an explicit confirmation looks reasonable to me.

cc @schmidt-sebastian

@maksymmalyhin It looks like the unit tests depend on the different classes:

Yeah, that's correct. Sorry if I was not clear. Let me try to explain my thoughts.

The warning with the original method implementation is actually correct an helpful, because the method is supposed to return an instance that conforms to the FIRDatabaseConnectionContextProvider protocol, but in fact it returns an instance of FIRDatabaseConnectionContextProvider class (the same name as the protocol to add more confusion 😄 ) which doesn't declare it's conforming to the protocol. My suggested fix for this is adding the protocol confirmation to the class, e.g. by modifying

@interface FIRDatabaseConnectionContextProvider : NSObject <FIRDatabaseConnectionContextProvider>

In this case casting won't be required and if protocol changes the compiler will suggest us changing the class (which is not the case currently). I needed to do this initially, but missed it, sorry about that.

WDYT?

Thanks for explaining. That makes sense and is much cleaner to avoid the casts.

I was confused by the same name for both the protocol and the interface. Is that a typical style?

I usually prefer to add Protocol suffix if there is a name conflict with a class, but here I followed the existing name convention and only applied renaming "AuthTokenProvider" -> "ConnectionContextProvider". I actually don't mind renaming the protocol.