Fix dataclass inference for marshmallow_dataclass

[noirbee]

Steps

• For new features or bug fixes, add a ChangeLog entry describing what your PR does.
• Write a good description on what the PR does.

Description

This allows astroid/pylint to understand dataclasses as provided by marshmallow-dataclass, which are basically regular stdlib dataclasses with a few additional private attributes to generate marshmallow schemas from said dataclasses declaration.

Note that despite the fact that this affects inference for both dataclass and field, pylint still correctly detects that marshmallow-dataclass does not provide a field function.

Type of Changes

	Type
✓	🐛 Bug fix

(I'm putting this down as a bugfix because this was "broken" by the fact that recent astroid/pylint versions now correctly infers types from dataclasses, but I guess this could be considered a new feature)

[Pierre-Sassoulas]

Thank you for the MR. I discovered this package and it fix a problem I had with marshmallow's design 😄

Regarding the change itself, I'm thinking there might be a problem with our design if we need to add the name of all dataclasses modules to this constant. I don't have a better solution as it's astroid and not pylint we can't add an option to let user add their own data-classes.

[DanielNoord]

I took a quick look at how marshmallow defines their dataclasses but it will be difficult to do this with inference. Since functions don't inherit other functions like classes we'll need to look at their return values.
In the case of marshmallow this is difficult since we select their overloads instead of the actual function definitions and therefore don't actually see the return values of the functions that define the new dataclasses. I discuss this issue here: pylint-dev/pylint#5264 (comment)

Even if we did find the correct function definition it would be difficult to fix this. We currently infer this as the return value from marshmallow_dataclass.dataclass:
[<Lambda.<lambda> l.144 at 0x105951b50>, <Const.NoneType l.167 at 0x1059538e0>, Uninferable]
Code here

Making sure that all of these return values are dataclasses would require us to go several levels deeper and might be unfeasible. I think for now this might be the best thing we can do..

[cdce8p]

Thank you for the MR. I discovered this package and it fix a problem I had with marshmallow's design 😄

Regarding the change itself, I'm thinking there might be a problem with our design if we need to add the name of all dataclasses modules to this constant. I don't have a better solution as it's astroid and not pylint we can't add an option to let user add their own data-classes.

I think the way you're probably meant to deal with it would be trough a custom pylint / astroid plugin.
It's easy enough for us to add it now if it works good enough but in general such things are probably better done separately. Especially as we aren't able to support all packages. Someone has to maintain it in the end.

[Pierre-Sassoulas]

I think the way you're probably meant to deal with it would be trough a custom pylint / astroid plugin

Sounds good, but do we have astroid plugins ? If we go this route, each astroid brains could be an astroid plugins ? Seems to me a good idea as brains need to handle multiple versions of a package (?) and are rather naive in their current implementation. Having external astroid brains would permit to make them a lot more complicated than what they currently are. Then a pylint option could permits to add brain packages to your project. Thoughts @hippo91 ?

[cdce8p]

Sounds good, but do we have astroid plugins ?

Aren't those the Transform plugins? It looks like those should work with inference_tip. Although the documentations could definitely be updated.

If we go this route, each astroid brains could be an astroid plugins ?

Most brain modules are actually for builtin functions and classes. Those should definitely stay within astroid. For the external once (like numpy) there could be an argument to move them to new projects. The issue then just becomes who will maintain them? For simple brains it's certainly much easier to add them to astroid directly.

In the end, I would think of it as batteries included. If it isn't too difficult or a very popular library, add it to astroid. For more complex stuff that might also require more maintenance than we can provide, a separate package is probably better.

[hippo91]

@Pierre-Sassoulas i definitely agree with @cdce8p remarks!
By the way, i'm sorry to not have given news for a long period, but i have been very busy with work.
I hope i can be more present at the beginning of 2022.

[Pierre-Sassoulas]

i definitely agree with @cdce8p remarks!

Glad to see you're back and thanks for taking the time to check the problem ;)

For more complex stuff that might also require more maintenance than we can provide, a separate package is probably better.

I wonder the criteria we could use. Maybe popularity of the package and the complexity of maintaining multiple versions together. Libraries like numpy and pandas are complex but also used a lot so keeping them in astroid makes some sense. The same question arise when we're asking ourselves about what dependency should be installed during tests (should we install numpy ? If we had an astroid-numpy-brain package the tests would not need to install numpy).

[cdce8p]

For more complex stuff that might also require more maintenance than we can provide, a separate package is probably better.

I wonder the criteria we could use. Maybe popularity of the package and the complexity of maintaining multiple versions together. Libraries like numpy and pandas are complex but also used a lot so keeping them in astroid makes some sense. The same question arise when we're asking ourselves about what dependency should be installed during tests (should we install numpy ? If we had an astroid-numpy-brain package the tests would not need to install numpy).

The whole discussion should probably be moved to two separate issues:

• Improve pylint documentation around brain packages / plugins
• Which library brains should be included with astroid?

Some last comments, the package name should probably be something like pylint-numpy as it's a pylint plugin. Numpy is one of the things I also considered. As long as someone is willing to maintain it here, I think it's ok. Long term though, this would be a size for which a separate package could make sense.

--
As for this PR if you agree, I think it would be fine.

[Pierre-Sassoulas]

This is an external library but it's rather popular (350 star right now, but it's linked to marshmallow which is a lot more popular), and it's simple to maintain so following the rules we talked about, it can be included so astroid has "battery included".

Follow-up discussion in #1312