Should we change how attributes are defined on certain nodes classes?

[DanielNoord]

I ran into this while working on the typing errors that we need to solve before turning on mypy. A similar issue was encountered in https://github.com/PyCQA/astroid/pull/1244/files.

Some of our nodes classes have attributes that get defined in a postinit. Take for example func on nodes.Call:
https://github.com/PyCQA/astroid/blob/907818246c89ef938b23448e0cecc52b7c8693e0/astroid/nodes/node_classes.py#L1635-L1642

https://github.com/PyCQA/astroid/blob/907818246c89ef938b23448e0cecc52b7c8693e0/astroid/nodes/node_classes.py#L1660-L1664

We know that this func attribute can't technically be None. If it were None the Call wouldn't have a function to call. In the postinit you can clearly see the difference between attributes that can and can not be None. For example, args and keywords can be None. (They should probably be [] in that case to allow iteration, but that's another issue).

I wonder what would happen if we were to change self.func: Optional[NodeNG] = None to self.func: NodeNG. This would simplify a lot of our typing as we wouldn't need to add checks for the existence of Call.func everywhere. Perhaps there are solutions to this which we should consider, but I think there must be a way to show that nodes.Call must have a func attribute, but that we don't know it at initialisation time.
One issue I already thought of is what would happen if something accessed func before the postinit is called. I don't think this is an issue and any exception raised is correct: the instance shouldn't be accessed before the postinit is complete.

I have created DanielNoord#3 to show that this does indeed pass our test suite. The added ValueError is there to immediately break a test.

My proposal would be to identify all of these non-optional attributes and change their typing.

I'm pinging @cdce8p as you often have valuable insights for typing related issues.

[cdce8p]

I've already spend some time working on this exact issue 😅
The next step is to deprecate additional default values for arguments in postinit. I.e. don't allow a default value of None for func. If you're ok with it taking a bit longer, I would appreciate if you let me do it. (Or just open fewer PRs, maybe then I have some time left to work on my own ones 😉)

[DanielNoord]

I'll dim down the typing issues for now 😄

Let me know if you need any help!

[Pierre-Sassoulas]

Don't panic, there's enough issues for everyone 😄

[DanielNoord]

Don't panic, there's enough issues for everyone 😄

I'm going to take another try at the dataclass crash blocking 2.12. I have spent some time in the brain today, so perhaps I can find the problem now.

[cdce8p]

@DanielNoord You don't need to fix the dataclass brain to fix the issue. It's much easier. See (2) here: pylint-dev/pylint#5321 (comment). Just haven't had the time to prepare a PR with tests. The brain fix can be done later.

[DanielNoord]

Yeah I know, but I thought I would try and do them both. We can use the tests I already wrote in that case 😄

I'll try again and if I don't succeed I'll create a PR with the fix in (2).

[cdce8p]

(2) should be addressed even if you find the issue 😄