Skip to content
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

Fix crash when FK references unknown 'app_label' #1342

Merged
merged 1 commit into from
Jan 25, 2023

Conversation

flaeppe
Copy link
Member

@flaeppe flaeppe commented Jan 24, 2023

I have made things!

Plugin previously crashed running the provided test case, now it yields an intended error message instead.

Related issues

Ref: #1312

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@intgr intgr left a comment

Choose a reason for hiding this comment

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

Cool, this makes sense.

content: |
from django.db import models
class InstalledModel(models.Model):
non_installed = models.ForeignKey( # E: Cannot find model 'not_installed.NonInstalledModel' referenced in field 'non_installed'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it also possible to ignore this error using a # type: ignore comment?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I tested it and indeed it works.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

I thought the same. Yet the test passes successfully with the following diff:

diff --git a/tests/typecheck/fields/test_related.yml b/tests/typecheck/fields/test_related.yml
index 7e9758ae..cc863373 100644
--- a/tests/typecheck/fields/test_related.yml
+++ b/tests/typecheck/fields/test_related.yml
@@ -940,6 +940,6 @@
             content: |
                 from django.db import models
                 class InstalledModel(models.Model):
-                    non_installed = models.ForeignKey(  # E: Cannot find model 'not_installed.NonInstalledModel' referenced in field 'non_installed'
+                    non_installed = models.ForeignKey(  # type: ignore
                         "not_installed.NonInstalledModel", on_delete=models.CASCADE
                     )

I'm not complaining 😆

Copy link
Member Author

Choose a reason for hiding this comment

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

How interesting, have to compare how this error is emitted versus the one that couldn't be ignored

@intgr intgr merged commit e6d65d2 into typeddjango:master Jan 25, 2023
@flaeppe flaeppe deleted the fix/relation-unknown-app-label branch January 25, 2023 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants