Supports standard library nodes

[tristanlatr]

Fix #45.

It's still works with gast of course. But starting at Pyhton 3.6 beniget provides a beniget.standard module which has the same public interface as the main module except it works with standard library nodes.

All test classes have been adjusted to allow changing the reference ast module, standard library test are implemented as a subclass fo the original test case.

[serge-sans-paille]

Hey Tristan. The only reason for not using gast would be to speedup processing, and that would need to be backed with numbers. We currently don't need to think about ast representation thanks to gast, and I consider this a good point. So my question would be: what motivates this PR? (And by the way, thanks for your interest in beniget, are you using it for some specific project / purpose?)

[tristanlatr]

The main motivation to use standard library ast nodes is to be compatible with all the code that already exist that is using the standard library. It's not about performance really to me, but I agree it should improve it a little.

More specifically, I'm looking at beginet to be the core of a new library which will aim at analyzing the code structure of entire packages and the relationship between them for documentation purposes. My plan it to use beniget to solve these issues twisted/pydoctor#295 twisted/pydoctor#348, by creating a code model which will be used by pydoctor to do the name resolution. One of the challenges is to find the canonical name for each symbols (because one symbol can be imported and accessible in several modules). Another challenge is the handling of __all__ variable, etc...

Hope this answer your question

[serge-sans-paille]

gast has a ast_to_gast and a gast_to_ast functions. Your change here is being too intrusive for the benefit, I'm going to decline... Sorry for being conservative, but keeping many projects manageable in my small brain is of primary importance to me :-)

And thanks for sharing your plan. I'll try to support this by making beniget more robust, as in fixing #44 !

[tristanlatr]

I get it... But it's too bad because making beniget compatible with the standard library is probably much less work that making pydoctor compatible with gast.

But I'll see what I can do about this...

[tristanlatr]

I’m reopening this pull request (and I’ll update it as well) because I think it’s highly relevant in terms of performance gain. Generating the def use chains takes about a third of the time taken to produce the gast…

[tristanlatr]

We could use a subclass for the standard library nodes, It might be even more clean and do not require to change all the isinstance calls.

[tristanlatr]

I'm marking this PR as draft until I implement a much less intrusive diff

[tristanlatr]

Salut @serge-sans-paille,

J'espère que tu vas bien.

Je me permet de te relancer quant à ces changements. L'idée c'est ne plus utiliser isisntance pour determiner le type des noeuds, mais de comparer type(x).__name__ à la place. Je pense même que ça améliore les performances un chouya puisque qu'on ne vas pas chercher la MRO des classes.

Le reste du code spécifique à la librairie standard est dans un nouveau module: beniget.standard, qui offre la même interface publique que le module beniget. L'idée est d'être de moins invasif possible tout en offrant le support pour les noeuds standards, qui ne sont pas si différents au final.

La suggestion initiale d'utiliser un mapping ast -> gast ne semble pas viable, en plus d'être tres inefficace, j'ai pas réussi a faire passer les tests de beniget avec la version modifiée.

C'est pour ça que j'espère vraiment que tu vas considérer ces changements,

Merci,

[serge-sans-paille]

Hey @tristanlatr . I'm not a big fan of the name based-check, and the additional checks for two different kind of nodes.
Do you think it would work to pass the ast module in the constructor and have it just be a member of the analysis, so that we would write:

if isinstance(self.node, self.ast.Name):

instead of

if isinstance(self.node, ast.Name):

?

[tristanlatr]

Yes that could work! But Def and _CollectFutureImports will need to be changed as well.
Let me try to refactor this PR.

[tristanlatr]

Après quelques analyses, Le but ici est de rendre la classe DefUseChains assez flexible et auto-dépendante afin de pouvoir implementer tous les changement pour la librairie standard en créant une classe dérivée.

Donc ça implique créer les attributs de classes suivant:

• DefUseChains.ast: Le module ast de référence
• DefUseChains.Def: La classe Def de référence
• DefUseChains.collect_future_imports et _CollectFutureImports
• DefUseChains.collect_locals et CollectLocals
• DefUseChains._validate_comprehension et autres
• DefUseChains.lookup_annotation_name_defs et autres

Pour UseDefChains y'a rien a faire.
Pour Ancestors y'a rien a faire.

Ensuite:

• Changer toutes les occurrences de ast dans DefUseChains par self.ast.
• Changer toutes les occurrences de Def(...) par self.Def(...)
• Ajuster standard.DefUseChains utiliser le module ast standard et utiliser la version adaptée de Def

Je ne suis pas certain que passer le module ast de reference par le constructeur est le plus simple: pourquoi pas une classe dérivée et assigner le module ast en tant que variable de classe ? (Comme je fais dans les tests d’ailleurs)

Merci,