Skip to content

Conversation

UnknownPlatypus
Copy link
Contributor

I have made things!

I've tried to avoid as much as possible false positive on loosely typed cases.

cc @asottile-sentry , would be awesome if can try to cherry-pick a1f80ea to see if it fixes your issue.

Related issues

Fixes #2818

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, except one question :)

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.

one nit :)

),
mod_name=None,
)
else:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
else:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've made the change, should we add RET505 from ruff to enforce this convention ?

I can make a followup PR for this, it does affect ~10 files. I also find these valueable:

  "RET504",  # Unnecessary assignment to {name} before return statement
  "RET505",  # Unnecessary {branch} after return statement
  "RET506",  # Unnecessary {branch} after raise statement
  "RET507",  # Unnecessary {branch} after continue statement
  "RET508",  # Unnecessary {branch} after break statement

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.

Thanks a lot!

@UnknownPlatypus
Copy link
Contributor Author

btw, asotille recently left sentry so we wont probably hear from him on this issue.

@sobolevn sobolevn merged commit 5844f3a into typeddjango:master Oct 5, 2025
39 checks passed
@sobolevn
Copy link
Member

sobolevn commented Oct 5, 2025

master branch is now broken :(
Please, take a look

@UnknownPlatypus
Copy link
Contributor Author

Seem to be a conflict with the branch removing automatically added contentypes, will send a fix

@UnknownPlatypus UnknownPlatypus deleted the allow-annotate-existing-field-after-values branch October 5, 2025 11:16
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.

5.2.3 regression with .annotate that forbids overriding existing fields
2 participants