A few classes under Upload/ seem to use FileRepo and LocalRepo somewhat interchangeably, which is wrong and makes phan v6 complain a lot while trying to upgrade (T406326). In particular, UploadFromChunks::__construct and UploadStash::__construct take FileRepo, but assign the parameter to properties documented as LocalRepo. It's not clear which of the two should be used:
- UploadStash calls $this->repo->getPrimaryDB() multiple times, and that method only exists in LocalRepo.
- But FileRepo has a getUploadStash method, and it passes $this as argument, so there's no guarantee that we're passing a LocalRepo
We don't have any errors in production, and this code has probably been like this for a long time, so I suppose there's some kind of implicit assumption that makes everything work correctly despite the type mismatch. However, as someone unfamiliar with this area of the code, this is really confusing and it would be nice to sort it out ahead of the phan v6 upgrade. To me it seems like we should change all those parameters to LocalRepo, and move getUploadStash to LocalRepo, plus any other changes needed to support that, but I'm not in a position to make this decision.