Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Prototype a means to detect non-strict types in Pydantic models at lint time #13336

Closed
DMRobertson opened this issue Jul 20, 2022 · 1 comment · Fixed by #13502
Closed

Prototype a means to detect non-strict types in Pydantic models at lint time #13336

DMRobertson opened this issue Jul 20, 2022 · 1 comment · Fixed by #13502
Assignees
Labels
T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. Z-Dev-Wishlist Makes developers' lives better, but doesn't have direct user impact

Comments

@DMRobertson
Copy link
Contributor

I don't like not-strict-by-default either. FWIW there is the intention to have that feature At Some Point (pydantic/pydantic#1098).

For enforcement, we could write a linter which parses the AST and complains if a type inheriting from BaseModel has an annotation which involves one of the non-strict types, or involves a constrained type without strict=True. I don't like the idea of writing something home-grown for that, but

  • we don't want a repeat of stringy power levels
  • maybe it wouldn't be too bad to write? Reminds be a bit of the mypy plugin...

Originally posted by @DMRobertson in #13188 (comment)

@DMRobertson DMRobertson added T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. Z-Dev-Wishlist Makes developers' lives better, but doesn't have direct user impact labels Jul 20, 2022
@DMRobertson
Copy link
Contributor Author

Rather than parsing syntax or writing plugins, I think we could do something simpler. I'm imagining a new linting script which does the following.

  1. Patch pydantic's constrained types to complain if strict is False. I'd suggest wrapping ConstrainedStr.__get_validators__ with something that asserts cls.strict is Truthy.
  2. Patch something to inspect the list of fields at model definition time. If some field has a non-strict type, complain. Note that this involves recursively checking type parameters, e.g. dictionary values, list elements. Might be able to define an __init_subclass__ on BaseModel, or else patch something on ModelMetaclass.
  3. Import everything from Synapse, recursively so that we define all the models.

DMRobertson pushed a commit that referenced this issue Aug 10, 2022
Really need a lint for this, see e.g. #13336
@DMRobertson DMRobertson self-assigned this Aug 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. Z-Dev-Wishlist Makes developers' lives better, but doesn't have direct user impact
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant