@sandersn you were right that the merged symbols essentially go both ways (target and source don't share a mergeId, but target gets there by cloneSymbol(resolvedTarget) and being mutated; then source gets added here.) Introducing a unidirectonal option to prevent adding a mergeId to source accomplishes your suggestion.

Hm. I've been burned pretty badly in the past by trying to extend the architecture of symbol merging. Merging is where state touches the checker, which is otherwise stateless:
Symbols from the binder are created when loading a file and only re-created when that file is editted. But merging mutates those symbols. It's important to only do that once, and tricky to make sure that happens as checkers are re-created over and over again and run merge* each time. I'd rather not change the architecture if it's not necessary.

Can you instead investigate copying constituent symbols from mainModule.exports and moduleAugmentation.exports to a fresh symbol created from cloneSymbol(moduleAugmentation) (or maybe it's called createSymbol)? This method is more likely to break symbol sharing, so you'll need to add tests for find-all-refs and rename-symbol to make they work correctly.

The patternAmbientModuleAugmentation map is still probably needed.

Not sure I follow—moduleAgumentation (the target) isn't transient, so mergeSymbol clones it rather than mutates it, and mainModule (the source) doesn't get mutated because it's the source. I think the only thing getting mutated here is the mergedSymbols array, which is the problem—I need to avoid calling recordMergedSymbol with these two symbols, because you should never be able to get from mainModule to moduleAugmentation via getMergedSymbol.

Hm. I've been burned pretty badly in the past by trying to extend the architecture of symbol merging. Merging is where state touches the checker, which is otherwise stateless:
Symbols from the binder are created when loading a file and only re-created when that file is editted. But merging mutates those symbols. It's important to only do that once, and tricky to make sure that happens as checkers are re-created over and over again and run merge* each time. I'd rather not change the architecture if it's not necessary.

Can you instead investigate copying constituent symbols from mainModule.exports and moduleAugmentation.exports to a fresh symbol created from cloneSymbol(moduleAugmentation) (or maybe it's called createSymbol)? This method is more likely to break symbol sharing, so you'll need to add tests for find-all-refs and rename-symbol to make they work correctly.

The patternAmbientModuleAugmentation map is still probably needed.

here's a terrifying test case

declare module "*.foo" {
  export interface OhNo { star: string }
}
declare module "a.foo" {
  export interface OhNo { a: string }
}
import { OhNo } from "b.foo"
declare let ohno: OhNo;
ohno.a // oh no

If we can give an error here, we should. Or we could try to recursively do what this PR does.

😱 OH NO

Doing the "unidirectional" thing recursively fixed this immediately. It feels like I'm on the right track, so ping me if you get a chance to elaborate on your concerns about symbol mutation, and hopefully I can address them in a way that continues to work well recursively like this.