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

Error if an invalid field name is used with Field #6797

Merged
merged 2 commits into from
Jul 21, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 11 additions & 4 deletions pydantic/_internal/_model_construction.py
Original file line number Diff line number Diff line change
Expand Up @@ -308,16 +308,23 @@ def inspect_namespace( # noqa C901
elif isinstance(value, ModelPrivateAttr):
if var_name.startswith('__'):
raise NameError(
f'Private attributes "{var_name}" must not have dunder names; '
'use a single underscore prefix instead.'
'Private attributes must not use dunder names;'
f' instead of {var_name!r} use a single underscore prefix instead.'
dmontagu marked this conversation as resolved.
Show resolved Hide resolved
)
elif is_valid_field_name(var_name):
raise NameError(
f'Private attributes "{var_name}" must not be a valid field name; '
f'use sunder names, e.g. "_{var_name}"'
'Private attributes must not use valid field names;'
f' instead of {var_name!r} use sunder names, e.g. {"_" + var_name!r}'
)
private_attributes[var_name] = value
del namespace[var_name]
elif isinstance(value, FieldInfo) and not is_valid_field_name(var_name):
suggested_name = var_name.lstrip('_') or 'my_field' # don't suggest '' for all-underscore name
raise NameError(
f'Fields must not use names with leading underscores;'
Copy link
Member

@hramezani hramezani Jul 21, 2023

Choose a reason for hiding this comment

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

Suggested change
f'Fields must not use names with leading underscores;'
f'Field() must not use names with leading underscores;'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We say Private attributes above, which I was trying to mirror here with the description of what it was, rather than the function used to create it. I actually think it's less confusing like this because people might not realize that calling Field() signals to pydantic that that thing should be a field, which is what I want to emphasize.

Copy link
Member

Choose a reason for hiding this comment

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

Agree, I've edited my suggestion

f' e.g., instead of {var_name!r} use {suggested_name!r}'
)

elif var_name.startswith('__'):
continue
elif is_valid_privateattr_name(var_name):
Expand Down
23 changes: 23 additions & 0 deletions tests/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -2746,3 +2746,26 @@ class Model(BaseModel):
)

assert 'class Model' in module.help_result_string


def test_cannot_use_leading_underscore_field_names():
with pytest.raises(
NameError, match="Fields must not use names with leading underscores; e.g., instead of '_x' use 'x'"
):

class Model1(BaseModel):
_x: int = Field(alias='x')

with pytest.raises(
NameError, match="Fields must not use names with leading underscores; e.g., instead of '__x__' use 'x__'"
):

class Model2(BaseModel):
__x__: int = Field()

with pytest.raises(
NameError, match="Fields must not use names with leading underscores; e.g., instead of '___' use 'my_field'"
):

class Model3(BaseModel):
___: int = Field(default=1)
5 changes: 3 additions & 2 deletions tests/test_private_attributes.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ class Model(BaseModel):
def test_private_attribute_invalid_name():
with pytest.raises(
NameError,
match='Private attributes "foo" must not be a valid field name; use sunder names, e.g. "_foo"',
match="Private attributes must not use valid field names; instead of 'foo' use sunder names, e.g. '_foo'",
):

class Model(BaseModel):
Expand Down Expand Up @@ -266,7 +266,8 @@ class Model(ParentAModel, ParentBModel):
def test_private_attributes_not_dunder() -> None:
with pytest.raises(
NameError,
match='Private attributes "__foo__" must not have dunder names; use a single underscore prefix instead.',
match="Private attributes must not use dunder names;"
" instead of '__foo__' use a single underscore prefix instead.",
):

class MyModel(BaseModel):
Expand Down