Inconsistent error for merging an identical method and callback

[LukeAbby]

For some reason stable TS accepts this:

interface Foo {
    foo(x: any): void;
    foo: (x: any) => void;
}

With no error.

However stable TS does NOT accept this:

interface Foo {
    foo: (x: any) => void;
    foo(x: any): void;
}

With the error Duplicate identifier 'foo'. twice.

Meanwhile TSGo does not accept either of these. I would not raise this as an issue if it weren't for two facts:

• I observed this actually happening with real code; @types/chai and vitest declare containSubset(val: any, exp: any, msg?: string): void; and containSubset: (val: any, exp: any, msg?: string) => void; respectively
• TSGo is aiming to be a 1:1 port so even weird behavior like this should be supported.

I would actually support revisiting these semantics such that either both are an error (like it is here) or neither are an error. I find the fact that order matters here surprising. If that is the decision I would like a clear answer so I can file an issue against packages with this issue.

[Andarist]

The main difference lies in the declared PropertyExcludes values - it's different between Strada and Corsa. This is what we get if we port the Corsa's adjustment to Strada: Andarist/TypeScript@d43d118

I can open the PR with the above later if it's going to be decided it's worth porting (I think it is). The commit introduces an issue related to expandos - I'd have to investigate that before opening the PR.

[jakebailey]

Given that this seems order dependent and that's a main source of differences in Corsa, I'd suggest sending that to Strada where we can try the extended tests and see if this makes sense to do early.

[jakebailey]

But if it's solely the flags, I don't quite know why we wouldn't just align Corsa?

[Andarist]

But if it's solely the flags, I don't quite know why we wouldn't just align Corsa?

I think the Corsa's behavior here makes more sense - that's why I have experimented with porting this to Strada instead of doing the opposite

[ahejlsberg]

The changes in Corsa are intentional. I made several corrections to the duplicate identifier detection logic as I was porting the code. As a result, Corsa finds duplication errors that Strada erroneously didn't, and Corsa consistently reports errors on all duplicate declarations as opposed to all but the "first" declaration. The repro reported in this issue is an example of the corrected behavior. I'm not sure back-porting to Strada is worth the effort, I think it is simply a bug fix that Corsa introduces.

[LukeAbby]

Thanks for the official confirmation @ahejlsberg! I'll file an issue with Vitest/@types/chai about this since it sounds like this it's an issue in their code now.

[jakebailey]

Ah, I somehow missed the sentence that said tsgo rejects both.

It seems like Andarist already has the commit and it's very simple, so I think it'd be worth looking into.

[ahejlsberg]

It seems like Andarist already has the commit and it's very simple, so I think it'd be worth looking into.

It's not that simple. Some 10 years ago we merged a PR that, due to a misunderstanding of the binder's duplication checking, set PropertyExcludes = None when really it should have had the value it now has. However, over the years Strada then subsequently replicated most of the duplication checking in the checker, but with different error messages, with unintended order sensitivity, and without erroring an all declaration sites. I left most of that logic out of the port and rewrote the rest to do the right thing. The changes really go hand-in-hand, so I'd caution against taking just the flags change. I'd much rather just leave it alone (I mean, it's been 10 years) and focus on adding value in the porting effort.

[DanielRosenwasser]

Possibly - I think if we had a PR to line things up in Strada, we wouldn't outright reject it though.