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

Multiple binary | operation in a single statement failed with "E1131: unsupported operand type(s) for | (unsupported-binary-operation)" #7381

Open
Wayonb opened this issue Aug 29, 2022 · 19 comments
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code Needs PR This issue is accepted, sufficiently specified and now needs an implementation Regression

Comments

@Wayonb
Copy link

Wayonb commented Aug 29, 2022

Bug description

If I am trying to OR more than two flags, pylint 2.15.0 fails with "E1131: unsupported operand type(s) for | (unsupported-binary-operation)". This worked with pylint 2.14.1

	class MosaicFlags(Flag):
		NONE = 0
		SUPPLY_MUTABLE = 1
		TRANSFERABLE = 2
		RESTRICTABLE = 4
		REVOKABLE = 8

value = MosaicFlags.SUPPLY_MUTABLE | MosaicFlags.RESTRICTABLE | MosaicFlags.REVOKABLE

Configuration

No response

Command used

python3 -m pylint --rcfile .pylintrc --load-plugins pylint_quotes

Pylint output

************* Module tests.test_RuleBasedTransactionFactory
tests/test_RuleBasedTransactionFactory.py:148:3: E1131: unsupported operand type(s) for | (unsupported-binary-operation)
tests/test_RuleBasedTransactionFactory.py:164:10: E1131: unsupported operand type(s) for | (unsupported-binary-operation)

Expected behavior

Expected no errors

Pylint version

pylint 2.15.0
Python 3.8.10

OS / Environment

Ubuntu 20.04

Additional dependencies

isort==5.10.1
pycodestyle==2.9.1
pylint==2.15.0
pylint-quotes==0.2.3

@Wayonb Wayonb added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Aug 29, 2022
@RemiCardona
Copy link
Contributor

Same issue when using Django's "Q objects" https://docs.djangoproject.com/en/3.1/topics/db/queries/#complex-lookups-with-q

2 Q objects |-ed or &-ed together is fine, 3 or more breaks pylint. We're getting hundreds of errors as Q objects are a fairly common sight in Django projects.

@mbyrnepr2
Copy link
Member

mbyrnepr2 commented Aug 30, 2022

Thanks for the report. I can't reproduce this one myself:

(venv310) Marks-MacBook-Air-2:programming markbyrne$ pip freeze |grep pylint
pylint==2.15.0
pylint-quotes==0.2.3
(venv310) Marks-MacBook-Air-2:programming markbyrne$ pylint --version
pylint 2.15.0
astroid 2.12.5
Python 3.10.4 (v3.10.4:9d38120e33, Mar 23 2022, 17:29:05) [Clang 13.0.0 (clang-1300.0.29.30)]
(venv310) Marks-MacBook-Air-2:programming markbyrne$ cat example.py 
'''test'''

from enum import Flag

class MosaicFlags(Flag):
    """test"""
    NONE = 0
    SUPPLY_MUTABLE = 1
    TRANSFERABLE = 2
    RESTRICTABLE = 4
    REVOKABLE = 8

value = MosaicFlags.SUPPLY_MUTABLE | MosaicFlags.RESTRICTABLE | MosaicFlags.REVOKABLE
print(value)
(venv310) Marks-MacBook-Air-2:programming markbyrne$ pylint example.py --load-plugins pylint_quotes
************* Module programming.p
p.py:1:0: C4003: Invalid docstring quote ''', should be """ (invalid-docstring-quote)

------------------------------------------------------------------
Your code has been rated at 8.89/10 (previous run: 8.89/10, +0.00)

@mbyrnepr2 mbyrnepr2 added Needs reproduction 🔍 Need a way to reproduce it locally on a maintainer's machine and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Aug 30, 2022
mbyrnepr2 added a commit to mbyrnepr2/pylint that referenced this issue Aug 30, 2022
@Wayonb
Copy link
Author

Wayonb commented Aug 30, 2022

Hi @mbyrnepr2, Thanks for taking a look. Sorry, I didnt actually try the sample code.
I was able to repo with the sample below.

pylint example.py --load-plugins pylint_quotes

from enum import Flag


class Module:
    """module"""

    class TestFlags(Flag):
        """test flags"""
        TEST_NONE = 0
        TEST_SUPPLY_MUTABLE = 1
        TEST_TRANSFERABLE = 2
        TEST_RESTRICTABLE = 4
        TEST_REVOKABLE = 8


class TestClass():
    """test class"""

    @staticmethod
    def _assert_flags_parser(value, expected):
        """assert"""
        pass

    def test_flags_parser_can_handle_multiple_string_flags(self):
        """test flags"""
        self._assert_flags_parser(
            'supply_mutable restrictable revokable',
            Module.TestFlags.TEST_SUPPLY_MUTABLE | Module.TestFlags.TEST_RESTRICTABLE | Module.TestFlags.TEST_REVOKABLE)

    @staticmethod
    def test_flags_parser_passes_non_parsed_values_as_is():
        """test parser"""
        value = Module.TestFlags.TEST_SUPPLY_MUTABLE | Module.TestFlags.TEST_RESTRICTABLE | Module.TestFlags.TEST_REVOKABLE
        print(value)

@SamyCookie
Copy link

SamyCookie commented Aug 30, 2022

if you want a shorter example without enum dependency you can reproduce this false positive bug with this:

class BinaryOperator:
    """ only make useless binary operations
    """
    def __or__(self, whatever):
        return 42
operators = [ BinaryOperator(), BinaryOperator() ]
def _fn(idx: int=0):
    print(operators[0] | operators[1])  # NO unsupported-binary-operation issue
    print(operators[idx] | operators[1])  # OUPS unsupported-binary-operation issue
_fn(0)

@mbyrnepr2 mbyrnepr2 added False Positive 🦟 A message is emitted but nothing is wrong with the code and removed Needs reproduction 🔍 Need a way to reproduce it locally on a maintainer's machine labels Aug 30, 2022
@mbyrnepr2
Copy link
Member

Thanks for the examples.

@Wayonb you're right - something between 2.14.1 & 2.15.0 has led to this false positive. Note it happens only for Python versions below 3.10 because of this logic: https://github.com/PyCQA/pylint/blob/main/pylint/checkers/typecheck.py#L1879-L1880

@mbyrnepr2 mbyrnepr2 added Needs PR This issue is accepted, sufficiently specified and now needs an implementation Regression labels Aug 30, 2022
@RemiCardona
Copy link
Contributor

Is there any chance for part or all of the new type checker code to be reverted until the issue is fixed?

Thanks

@Pierre-Sassoulas
Copy link
Member

@RemiCardona Without looking too much into it, the cost of fixing the issue is probably going to be smaller than the cost to partially revert (but the main problem is that we don't have contributions for either right now).

@DanielNoord
Copy link
Collaborator

@Pierre-Sassoulas I have been waiting on working on Enums more until pylint-dev/astroid#1598 gets merged. It's quite a substantial design change, but would facilitate more work on Enums very easily.

@mbyrnepr2
Copy link
Member

It could be worth taking a look at this change. If I revert that one locally, I can't reproduce using the following example, provided in the comment above

example from the comment
from enum import Flag


class Module:
    """module"""

    class TestFlags(Flag):
        """test flags"""
        TEST_NONE = 0
        TEST_SUPPLY_MUTABLE = 1
        TEST_TRANSFERABLE = 2
        TEST_RESTRICTABLE = 4
        TEST_REVOKABLE = 8


class TestClass():
    """test class"""

    @staticmethod
    def _assert_flags_parser(value, expected):
        """assert"""
        pass

    def test_flags_parser_can_handle_multiple_string_flags(self):
        """test flags"""
        self._assert_flags_parser(
            'supply_mutable restrictable revokable',
            Module.TestFlags.TEST_SUPPLY_MUTABLE | Module.TestFlags.TEST_RESTRICTABLE | Module.TestFlags.TEST_REVOKABLE)

    @staticmethod
    def test_flags_parser_passes_non_parsed_values_as_is():
        """test parser"""
        value = Module.TestFlags.TEST_SUPPLY_MUTABLE | Module.TestFlags.TEST_RESTRICTABLE | Module.TestFlags.TEST_REVOKABLE
        print(value)

Pierre-Sassoulas pushed a commit that referenced this issue Sep 22, 2022
@belm0
Copy link
Contributor

belm0 commented Sep 26, 2022

Here is my repro. Having the expression in the tuple seems to be necessary.

class MyFlag(enum.Flag):
    FOO = 1
    BAR = 2
    BAZ = 4

XX = (MyFlag.FOO | MyFlag.BAR | MyFlag.BAZ, 'hello')

@antonshack
Copy link

antonshack commented Dec 5, 2022

Here's another repro. Completing @belm0's one.

It seems placing the expression in a tuple, a list or passing it as a parameter of a function raises checker E1131.
The union of at least three operands is also still required.

This was tested on the latest version released this morning (pylint 2.15.8/Python 3.9.10)

Repro

from enum import Flag


class MyFlag(Flag):
    FOO = 1
    BAR = 2
    BAZ = 3


def spam(eggs):
    ...


# No errors
spam(MyFlag.FOO | MyFlag.BAR)  # two operands
eggs = (MyFlag.FOO | MyFlag.BAR, "hello")  # two operands
eggs = MyFlag.FOO | MyFlag.BAR | MyFlag.BAZ  # expression *not* in a tuple, list nor function

# E1131: unsupported operand type(s) for | (unsupported-binary-operation)
spam(MyFlag.FOO | MyFlag.BAR | MyFlag.BAZ)  # 3 operands as a parameter of a function
eggs = (MyFlag.FOO | MyFlag.BAR | MyFlag.BAZ, "hello")  # 3 operands in a tuple
eggs = [MyFlag.FOO | MyFlag.BAR | MyFlag.BAZ, "hello"]  # 3 operands in a list

Env

> pylint --version
pylint 2.15.8
astroid 2.12.13
Python 3.9.10 (main, Jun  9 2022, 15:45:30) 
[GCC 9.3.1 20200408 (Red Hat 9.3.1-2)]

Expected results

unsupported-binary-operation error should not be raised.

@Pierre-Sassoulas
Copy link
Member

Here's a django example from #7953 thanks to @masiak2:

from django.db import models
from django.db.models import Q


class Author(models.Model):
    name = models.CharField(max_length=200)
    email = models.EmailField()


# this is fine
qs = Author.objects.filter(Q(name="x") | Q(name="y"))

# this throws an unsupported-binary-operation error
qs2 = Author.objects.filter(Q(name="x") | Q(name="y") | Q(name="z"))

@Hubrrr88
Copy link

Another short example with Enum's Flag:

from enum import Flag, auto


class Selector(Flag):
    A = auto()
    B = auto()
    C = auto()


class Storage:
    selector = Selector.A | Selector.B


if __name__ == '__main__':
    storage = Storage()
    selector = storage.selector | Selector.C
    print(selector)

@Pierre-Sassoulas
Copy link
Member

Another example:

"https://github.com/PyCQA/pylint/issues/8297"
from dataclasses import dataclass

@dataclass
class C:
    "class doc"
    a: str
    b: int = 10
    c: float | int | None = None

@sevdog

This comment has been minimized.

@belm0

This comment has been minimized.

bbannier added a commit to zeek/zeekscript that referenced this issue Mar 27, 2023
@fechnert

This comment has been minimized.

@paulking86

This comment has been minimized.

@CareF
Copy link
Contributor

CareF commented Dec 22, 2023

Another example:

"""module comment"""""
from enum import auto, Flag

class MyFlag(Flag):
    """class comment"""
    A = auto()
    B = auto()
    C = auto()
    ALL = A | B | C

under py3.8 & 3.9.

However it's not showing issue with py3.11

bklang added a commit to Gig-o-Matic/GO3 that referenced this issue Mar 10, 2024
This is needed to make pylint pass:

```
 ************* Module band.models
band/models.py:162:16: E1131: unsupported operand type(s) for | (unsupported-binary-operation)
************* Module gig.models
gig/models.py:47:45: E1131: unsupported operand type(s) for | (unsupported-binary-operation)
```

Reference: pylint-dev/pylint#7381 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code Needs PR This issue is accepted, sufficiently specified and now needs an implementation Regression
Projects
None yet
Development

No branches or pull requests