Python S5890: Type returned by __new__ constructor is not correctly managed

Clean Code Report False-positive / False-negative...
, ,
1

Language: Python
Rule #: S5890
Product: SonarLint - Pycharm 2023.1.4 (Professional Edition)

Step to reproduced:

testObjectA.py:

class ObjectA:
    def __init__(self, name: str):
        self.name = name

testObjectB.py:

class ObjectB:
    def __init__(self, name: str):
        self.name = name

testConvertToObjectB.py:

from testObjectB import ObjectB 
from testObjectA import ObjectA


class ConvertToObjectB:
    def __new__(cls, object: ObjectA) -> ObjectB:
        result: ObjectB = ObjectB(name=object.name)
        return result

test.py:

from typing import List
from testObjectB import ObjectB
from testObjectA import ObjectA
from testConvertToObjectB import ConvertToObjectB


class convert:
    def __new__(cls) -> List[ObjectB]:
        objectA: ObjectA = ObjectA(name='test')
        result: List[ObjectB] = list()
        newObject: ObjectB = ConvertToObjectB(object=objectA)
        result.append(newObject)
        return result

Sonarlint shows:
test.py:
(11,29) Assign to “newObject” a value of type “ObjectB” instead of “ConvertToObjectB” or update its type hint.

ConvertToObjectB.new return an ObjectB and therefore shouldn’t generate a warning.

2

Hey there!

What version of SonarLint are you using?

3

9.0.0.75308

6

Hello @Hal ,

Thanks for reporting this.

It is true that, currently, the Python analyzer will assume that a call to a constructor A() will return an object A and will ignore both the type annotation of __new__ as well as the actual return type. I created the following ticket to track this.

Note that, because the idea that a call to a constructor will return an instance of that class is a core assumption of our type inference engine (which currently relies on type annotation only in a limited way), this may take some time to be fixed. That said, we have some planned work to improve the engine in the next few months and this ticket will be a good reason for us to revisit that assumption.

Cheers,
Guillaume