atproto: some constructors return uncopyable values

[davecheney]

As originally observed here #1306, atproto/identity.NewCacheDirectory returns CacheDirectory. The issue is CacheDirectory contains several sync.Map fields which are marked nocopy. In practice all uses of this return value immediately take its address, because *CacheDirectory implements identity.Directory. go vet and other linters will spot any attempt to copy a CacheDirectory by value, ie cdir := identity.NewCacheDirectory(..); somestruct.dir = cdir, but we shouldn't lean on linters to catch this. I think we should correct the return type to be a reference.

@bnewbold suggested that there may be other occurrences which we should address in one commit because changing the return type is a breaking change, albeit a valuable one. An assay of atproto/ highights:

• identity.NewCacheDirectory

• apidir.NewAPIDirectory which requires the caller to refence the return value to fit the APIDirectory interface.

• identify.NewMockDirectory, as above, also contains a lock and map fields, which represents a correctness issue if the lock is copied.

• syntax.NewTIDClock returns TIDClock. TIDClock contains a sync.Mutex which protects the lastUnixMicro field. Copying a TIDClock copies the lock, which is a party foul, but it also copies lastUnixMicro. There are some questions about the correctness of the use of this type.

Additionally

• atproto/lexicon.NewBaseCatalog and atproto/lexicon.NewResolvingCatalog return by value. Inside indigo the result is used immediately, but returing by value means the result does not satisfy the lexicon.Catalog interface. This may be why ResolvingCatalog references its Base field as BaseCatalog, not the interface type.
• identify.ParseIdentity returns an Identity by value which is a moderately complex object. Benchmarking might be useful to determine if this is more expensive to copy vs gc the reference later. This may also apply to Identity.DIDDocument

[bnewbold]

Of these, identity.NewCacheDirectory is the most used in our code/ecosystem, with the lexicon catalogs being used a bit but not much. Those two will require a bit of coordination to roll out.

TIDClock is used a bit in indigo, and in hailey's cocoon PDS, but i'm fairly confident not much outside of that.

identify.ParseIdentity is pretty core / in-use, but mostly internal to the identity package. I'd maybe lean towards verifying the perf impact before making a change? But not a strong feeling, I guess we could just roll it up at the same time.

[davecheney]

@bnewbold #1318 addresses the first three items.

[bnewbold]

PRs for some of the other changes. As mentioned going to hold off on the identity.ParseIdentity change for now.

• syntax: have NewTIDClock() return ptr value #1325
• lexicon: have catalog constructors return ptr values #1326