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

[Bug] Ruff doesn't detect DJ012 rule after first fix #9150

Closed
vldmrrz opened this issue Dec 15, 2023 · 7 comments · Fixed by #9151
Closed

[Bug] Ruff doesn't detect DJ012 rule after first fix #9150

vldmrrz opened this issue Dec 15, 2023 · 7 comments · Fixed by #9151
Assignees
Labels
bug Something isn't working

Comments

@vldmrrz
Copy link

vldmrrz commented Dec 15, 2023

Bug description

I got an error DJ012 with some models when running command.

apps/app/models.py:113:5: DJ012 Order of model's inner classes, methods, and fields does not follow the Django Style Guide: field declaration should come before `Meta` class

Code example:

class MyModel(models.Model):
    class Meta:
        db_table = "my_model_table"

    model_field = models.TextField()

class MySecondModel(models.Model):
    class Meta:
        db_table = "my_second_model_table"

    model_field = models.TextField()

After fixing the first model and running the ruff check . I didn't get a warning about the second model

class MyModel(models.Model):
    model_field = models.TextField()

    class Meta:
        db_table = "my_model_table"

class MySecondModel(models.Model):
    class Meta:
        db_table = "my_second_model_table"

    model_field = models.TextField()

My configuration

  • Command I run: ruff check .
  • Ruff settings:
[tool.ruff]
# Enabled rules
select = [
    "E", # pycodestyle
    "F", # pyflakes
    "W", # pycodestyle
    "B", # bugbear
    "I", # isort
    "RUF", # ruff
    "UP", # pyupgrade
    "DJ", # django
    "N", # pep8-naming
    "PL", # pylint
    "TD", # todos
    "TCH", # flake8-type-checking
    "SIM", # flake8-simplify
    "Q", # flake8-quotes
    "T20", # flake8-print
    "COM", # flake8-comas
    "C90", # mccabe
]
# Ignored rules
ignore = [
    "RUF012", # mutable default values in class attributes
    "I001", # isort import ordering and blank lines in imports
    "DJ001", # Avoid using `null=True` on string-based fields such as TextField
]

# Allow fix for all enabled rules (when `--fix`) is provided.
fixable = ["B", "C", "D", "E", "F", "G", "I", "N", "Q", "S", "T", "W", "ANN", "ARG", "BLE", "COM", "DJ", "DTZ", "EM", "ERA", "EXE", "FBT", "ICN", "INP", "ISC", "NPY", "PD", "PGH", "PIE", "PL", "PT", "PTH", "PYI", "RET", "RSE", "RUF", "SIM", "SLF", "TCH", "TID", "TRY", "UP", "YTT"]
unfixable = []

line-length = 120

# Allow unused variables when underscore-prefixed.
dummy-variable-rgx = "^(_+|(_+[a-zA-Z0-9_]*[a-zA-Z0-9]+?))$"

# Assume Python 3.11
target-version = "py311"

# Exclude a variety of commonly ignored directories.
exclude = [
    ".direnv",
    ".git",
    ".mypy_cache",
    ".pytype",
    ".ruff_cache",
    ".tox",
    ".venv",
    "__pypackages__",
    "_build",
    "buck-out",
    "build",
    "dist",
    "venv",
]

[tool.ruff.mccabe]
max-complexity = 10

[tool.ruff.flake8-quotes]
docstring-quotes = "double"
  • ruff 0.1.8
  • python 3.11.7
  • Django 4.2.8
@dhruvmanila
Copy link
Member

I might be missing some context here but the playground shows the warning with the second code snippet: https://play.ruff.rs/407c7303-ed56-4cc0-8300-fd523728e97a

@vldmrrz
Copy link
Author

vldmrrz commented Dec 15, 2023

@dhruvmanila I found the problem.

Warning not fires when model inherited from another model (even non-abstract), like this https://play.ruff.rs/b7dbc350-21a2-4f97-9490-165d6226720c

But still looks like bug for me

@dhruvmanila
Copy link
Member

Thanks! Yes, the rule wouldn't trigger if the model isn't inherited directly from Django's Model class. I'll have to check if it's currently possible to resolve inheritance all the way to the base class \cc @charliermarsh ?

@vldmrrz
Copy link
Author

vldmrrz commented Dec 15, 2023

@dhruvmanila thank you so much for your attention. Hope it will be possible to fix this issue :)

@charliermarsh
Copy link
Member

We can do it within a single file, but not across files. So we can fix the single-file case, but the multi-file analysis case should just be merged into the larger tracking issue since it’s not at all specific to the Django rules.

@vldmrrz
Copy link
Author

vldmrrz commented Dec 15, 2023

@charliermarsh appreciate it.
It would be nice if we will have this functionality across multiple files in the future

@charliermarsh charliermarsh self-assigned this Dec 15, 2023
@charliermarsh charliermarsh added the bug Something isn't working label Dec 15, 2023
@charliermarsh
Copy link
Member

I can fix within the same file and make a note in the multi-file issue!

charliermarsh added a commit that referenced this issue Dec 15, 2023
## Summary

As elsewhere, this only applies to classes defined within the same file.

Closes #9150.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants