Skip to content

fix: Avoid crash in the presence of decorators. #184

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 15, 2025
Merged

Conversation

varungandhi-src
Copy link
Contributor

@varungandhi-src varungandhi-src commented Aug 15, 2025

Changes the method lookup logic to use the same
logic as Pyright internals instead of our own ad-hoc
implementation of name resolution, which does not
match Python's method resolution order (MRO), which
is based purely on names, and not on types.

This gets rid of a code path where we were doing a !
operation, which triggered a crash when indexing
a prospect's codebase.

Fixes GRAPH-1278.

Copy link
Contributor Author

varungandhi-src commented Aug 15, 2025

@varungandhi-src varungandhi-src requested a review from jupblb August 15, 2025 08:03
@varungandhi-src varungandhi-src marked this pull request as ready for review August 15, 2025 08:04
@varungandhi-src varungandhi-src force-pushed the vg/fix-bug branch 2 times, most recently from e330ce9 to 34a3753 Compare August 15, 2025 08:07
# ^^^ reference python-stdlib 3.11 builtins/int#
def matched_despite_different_type(self, x: int, y: int):
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ definition snapshot-util 0.1 inherits_class/B#matched_despite_different_type().
# relationship implementation scip-python python snapshot-util 0.1 inherits_class/A#matched_despite_different_type().
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This relationship is new. Python's overriding is based purely on name, not on types, so this is more accurate if we're looking at the runtime behavior.

@@ -54,7 +54,6 @@ def three(self):
def shared(self) -> bool:
# ^^^^^^ definition snapshot-util 0.1 multiinherits_test/Multi#shared().
# relationship implementation scip-python python snapshot-util 0.1 multiinherits_test/Left#shared().
# relationship implementation scip-python python snapshot-util 0.1 multiinherits_test/Right#shared().
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This relationship is done, because based on MRO, only one of the parent class methods will be considered for overriding.

}

let decl = parentMethodType.details.declaration!;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This ! triggered the error reported in GRAPH-1278.

}
}

// NOTE(tech-debt): typeToSymbol's signature doesn't make sense. It returns the
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -0,0 +1,21 @@
from typing import TypeVar, Generic, Callable, Iterator, ParamSpec
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test cases seems a bit large, but I haven't been able to minimize it further.

Base automatically changed from vg/run-all-tests to scip August 15, 2025 12:04
@varungandhi-src varungandhi-src merged commit f519201 into scip Aug 15, 2025
4 checks passed
@varungandhi-src varungandhi-src deleted the vg/fix-bug branch August 15, 2025 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant