Skip to content

[ty] Only calculate information for unresolved-reference subdiagnostic if we know we'll emit the diagnostic#18465

Merged
AlexWaygood merged 1 commit intomainfrom
alex/cleanup-unresolved-ref
Jun 4, 2025
Merged

[ty] Only calculate information for unresolved-reference subdiagnostic if we know we'll emit the diagnostic#18465
AlexWaygood merged 1 commit intomainfrom
alex/cleanup-unresolved-ref

Conversation

@AlexWaygood
Copy link
Member

Summary

This optimizes some of the logic added in #18444. In general, we only calculate information for subdiagnostics if we know we'll actually emit the diagnostic. The check to see whether we'll emit the diagnostic is work we'll definitely have to do whereas the the work to gather information for a subdiagnostic isn't work we necessarily have to do if the diagnostic isn't going to be emitted at all.

This PR makes us lazier about gathering the information we need for the subdiagnostic, and moves all the subdiagnostic logic into one function rather than having some unresolved-reference subdiagnostic logic in infer.rs and some in diagnostic.rs.

Test Plan

cargo test -p ty_python_semantic

@AlexWaygood AlexWaygood requested a review from carljm as a code owner June 4, 2025 17:08
@AlexWaygood AlexWaygood added the internal An internal refactor or improvement label Jun 4, 2025
@AlexWaygood AlexWaygood added performance Potential performance improvement ty Multi-file analysis & type inference labels Jun 4, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jun 4, 2025

mypy_primer results

No ecosystem changes detected ✅

@AlexWaygood AlexWaygood force-pushed the alex/cleanup-unresolved-ref branch from f95171c to de23fec Compare June 4, 2025 17:13
@AlexWaygood AlexWaygood requested a review from MichaReiser as a code owner June 4, 2025 17:13
Copy link
Contributor

@carljm carljm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see where this is delaying work compared to the prior version from #18444? It seems like in both the old and new version, all the work is in deciding whether an attribute exists in the first place, and in both old and new, that work is done if we are definitely going to emit a diagnostic, and not otherwise. Edit: ah, never mind, I forgot about the early return if context.report_lint(...) returns None.

But this looks like a nice refactor to collect more of the work in one place, either way!

@AlexWaygood AlexWaygood merged commit ce8b744 into main Jun 4, 2025
35 checks passed
@AlexWaygood AlexWaygood deleted the alex/cleanup-unresolved-ref branch June 4, 2025 19:41
carljm added a commit to mtshiba/ruff that referenced this pull request Jun 4, 2025
* main:
  [ty] Only calculate information for unresolved-reference subdiagnostic if we know we'll emit the diagnostic (astral-sh#18465)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal An internal refactor or improvement performance Potential performance improvement ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants