[red-knot] Avoid undeclared path when raising conflicting declarations

[dhruvmanila]

Summary

This PR updates the logic when raising conflicting declarations diagnostic to avoid the undeclared path if present.

The conflicting declaration diagnostics is added when there are two or more declarations in the control flow path of a definition whose type isn't equivalent to each other. This can be seen in the following example:

if flag:
	x: int
x = 1  # conflicting-declarations: Unknown, int

After this PR, we'd avoid considering "Unknown" as part of the conflicting declarations. This means we'd still flag it for the following case:

if flag:
	x: int
else:
	x: str
x = 1  # conflicting-declarations: int, str

A solution that's local to the exception control flow was also explored which required updating the logic for merging the flow snapshot to avoid considering declarations using a flag. This is preserved here: https://github.com/astral-sh/ruff/compare/dhruv/control-flow-no-declarations?expand=1.

The main motivation to avoid that is we don't really understand what the user experience is w.r.t. the Unknown type and the conflicting-declaration diagnostics. This makes us unsure on what the right semantics are as to whether that diagnostics should be raised or not and when to raise them. For now, we've decided to move forward with this PR and could decide to adopt another solution or remove the conflicting-declaration diagnostics in the future.

Closes: #13966

Test Plan

Update the existing mdtest case. Add an additional case specific to exception control flow to verify that the diagnostic is not being raised now.

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

[carljm]

Tests look great! I think we should move the fix inside declarations_ty, and add a test for the "three paths, conflicting declarations in two of them" case.

[carljm]

I think this has an issue: there could be three or more conflicting types, e.g. int, str, and Unknown; this should still emit a diagnostic for the conflict between int and str.

It's also a little weird that we just search for Unknown, because there could be an actual annotation x: Foo in one path, where Foo resolves to Unknown (e.g. because it is imported from a module we can't find), and we would still treat this Unknown the same as the undeclared-path Unknown.

I think it would be easier to make a more correct fix if we do it inside declarations_ty instead, so that in the cases where we don't want to emit the diagnostic, we don't even return an Err result at all.

[dhruvmanila]

Ah yes, thanks for pointing that out. I've pushed a commit that moves the logic to declarations_ty which doesn't consider the undeclared_ty when looking for conflicting types. It does add the type to the final declared type.

[carljm]

Love to see these go away!

[carljm]

Looks great, thank you!