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

B018 wrongly detects inline variable or attribute docstrings #208

Closed
xqt opened this issue Nov 29, 2021 · 16 comments · Fixed by #216
Closed

B018 wrongly detects inline variable or attribute docstrings #208

xqt opened this issue Nov 29, 2021 · 16 comments · Fixed by #216

Comments

@xqt
Copy link

xqt commented Nov 29, 2021

Having an inline attribute doc string or a module variable docstring, it is wrongly marked as B018 (sample from sphinx doc):

module_level_variable2 = 98765
"""int: Module level variable documented inline.

The docstring may span multiple lines. The type may optionally be specified
on the first line, separated by a colon.
"""

@PhilippSelenium
Copy link

Can confirm.

@Mischback
Copy link

Can confirm, too.

You may see a failing Github Action run here: https://github.com/Mischback/django-calingen/actions/runs/1514981629

# FLAKE 8 SETTINGS regarding plugin configuration

# Activate rules by plugins
extend-select =
    # flake8-assertive
    A,
    # flake8-bugbear
    B, B902, B903,
    # pydocstyle (by flake8-docstrings)
    D,
    # flake8-django
    DJ, DJ10, DJ11,

The full flake8 configuration can be found here: https://github.com/Mischback/django-calingen/blob/interfaces/.flake8

As flake8-bugbear is run by flake8 pre-commit hook, this is the relevant part of my pre-commit configuration:

repos:
  # ... cut ...
  # flake8 (linting)
  - repo: https://github.com/PyCQA/flake8
    rev: 4.0.1
    hooks:
      - id: flake8
        additional_dependencies: [flake8-bugbear, flake8-docstrings, flake8-django, flake8-assertive]

The full pre-commit configuration can be found here: https://github.com/Mischback/django-calingen/blob/interfaces/.pre-commit-config.yaml

Mischback added a commit to Mischback/django-calingen that referenced this issue Nov 29, 2021
Release 21.11.28 of flake8-bugbear introduces a bug with B018 (see
PyCQA/flake8-bugbear#208).

As of now, simply disable that rule in flake8 configuration.
@henzef
Copy link
Contributor

henzef commented Nov 29, 2021

B018 is also triggered for multi-line strings that are used as a comment aside from docstrings. Is this behaviour intentional?

@oliver-sanders
Copy link

For info, attribute docstrings aren't a Python feature, however, are widely used and supported by the Sphinx documentation system via autodoc.

@cooperlees
Copy link
Collaborator

Damn. Sorry all, definitely missed this and it's not intended. A PR adding a unit test showing we no longer flag this will definitely be accepted. I'll try get to it but welcome to beat me to it!

cc: @kasium - Checks creator to see if they can workout a fix quicker than I :D

gforsyth pushed a commit to xonsh/xonsh that referenced this issue Nov 29, 2021
* chore: adopt NEP-0029 for py version deprecation policy

fixes #4560

* chore: drop py3.6 from CI

* docs: add news item

* fix: failing qa because latest version to flake8-bugbear

see PyCQA/flake8-bugbear#208

* chore: require >=py3.7
@kasium
Copy link
Contributor

kasium commented Nov 29, 2021

@cooperlees I'm very sorry for that. As a fast workaround, let's just skip strings and work with anything else. Then we can look for a better solution

@kasium
Copy link
Contributor

kasium commented Nov 29, 2021

Hi all. I created #209

@cooperlees
Copy link
Collaborator

21.11.29 released reverting checking of strings for now. We will work on adding some pattern matching support into the string detection and probably default to ignoring this edge use case.

I also, 21.11.28 yanked from PyPI so it can not be installed to create noise.

Mischback added a commit to Mischback/django-calingen that referenced this issue Nov 29, 2021
Wow, that was quick! PyCQA/flake8-bugbear#208
already fixed!
@Mischback
Copy link

Wow, that was fast!

@cooperlees @kasium Thx a lot!

@cooperlees
Copy link
Collaborator

cooperlees commented Nov 29, 2021

Unwanted CI noise is the worst so I didn’t want us to help create it. This needs more thought and this will give us time rather than rushing out something.

wmfgerrit pushed a commit to wikimedia/pywikibot that referenced this issue Nov 30, 2021
PyCQA/flake8-bugbear#208

Change-Id: Ifc3854032ea6f6144e22830dfaf4a4d83f564d4a
@kasium
Copy link
Contributor

kasium commented Dec 1, 2021

@cooperlees the issue is, that the used quotes (""" or " or ') are not accessible in the AST. Therefore we need to use tokens. Since it's used for docstrings even in the middle of code, I would suggest to keep strings out. If it's fine for you, I would enhance the tests, code and README to reflect it.

@cooperlees
Copy link
Collaborator

So you think there's not way to look at the string contents themselves by keywords or regex to try reduce the noise and reenable string? I was thinking of (worst case) a way for people to specify from regex's and have some defaults ...

@kasium
Copy link
Contributor

kasium commented Dec 5, 2021

I think that this is somehow error prone. What's possible is, to only allow this pattern in certain cases, e.g. on module level and in __init__ methods. So, e.g. in the middle of a custom function such a string makes no sense.

However, some users use multiline strings as comments...

@Mischback
Copy link

I think that this is somehow error prone. What's possible is, to only allow this pattern in certain cases, e.g. on module level and in init methods. So, e.g. in the middle of a custom function such a string makes no sense.

Personally, I use numpy doc style documentation in combination with Sphinx, sphinx-autodoc on a Django project. The attribute docstrings are used for class variables (or, in particular: Django model fields).

However, some users use multiline strings as comments...

I think this is bad behaviour and can and should be marked by the corresponding rule, perhaps even with the hint to use # ... lines for that.

@johnthagen
Copy link
Contributor

johnthagen commented Jan 10, 2022

I am hitting this issue for format strings used as attribute docstrings.

Note that attribute docstrings are listed as an official convention in PEP 257.

String literals occurring immediately after a simple assignment at the top level of a module, class, or __init__ method are called "attribute docstrings".

Here is a false positive:

from dataclasses import dataclass

x = 0


@dataclass
class Foo:
    bar: int
    f"""Attribute docstring {x}"""

Output:

main.py:9:5: B018 Found useless expression. Either assign it to a variable or remove it.

@cooperlees Do format string checks need to be temporarily rolled back too?

Environment

  • Python 3.9.8
pip list
Package        Version
-------------- --------
attrs          21.4.0
click          8.0.1
flake8         4.0.1
flake8-bugbear 21.11.29
mccabe         0.6.1
pep517         0.10.0
pip            21.3.1
pip-tools      6.4.0
pycodestyle    2.8.0
pyflakes       2.4.0
setuptools     57.0.0
toml           0.10.2
wheel          0.36.2

@kasium
Copy link
Contributor

kasium commented Jan 10, 2022

My fault. The ignore was only for strings and not for f-strings (aka JoinedStr). PR is open

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants