Slots added to many classes, but not their bases

[ariebovenberg]

👋 Hi there!

I found you use add_slots to add slots to a lot of classes (e.g. libcst._nodes.expression.Annotation). However, it seems __slots__ is not present in many of their base classes (e.g. libcst._nodes.base.CSTNode), meaning the benefits are reduced.

Unless I'm missing something, we should be able to fix this for a free performance boost!

If you agree, I'd be happy to submit a PR

edit: a full list of slotless bases:

       - libcst._nodes.base.BaseLeaf
       - libcst._nodes.base.BaseValueToken
       - libcst._nodes.base.CSTNode
       - libcst._nodes.expression.BaseAssignTargetExpression
       - libcst._nodes.expression.BaseComp
       - libcst._nodes.expression.BaseDelTargetExpression
       - libcst._nodes.expression.BaseDict
       - libcst._nodes.expression.BaseDictElement
       - libcst._nodes.expression.BaseElement
       - libcst._nodes.expression.BaseExpression
       - libcst._nodes.expression.BaseFormattedStringContent
       - libcst._nodes.expression.BaseList
       - libcst._nodes.expression.BaseNumber
       - libcst._nodes.expression.BaseSet
       - libcst._nodes.expression.BaseSimpleComp
       - libcst._nodes.expression.BaseSlice
       - libcst._nodes.expression.BaseString
       - libcst._nodes.expression._BaseElementImpl
       - libcst._nodes.expression._BaseExpressionWithArgs
       - libcst._nodes.expression._BaseParenthesizedNode
       - libcst._nodes.expression._BasePrefixedString
       - libcst._nodes.expression._BaseSetOrDict
       - libcst._nodes.op.BaseAugOp
       - libcst._nodes.op.BaseBinaryOp
       - libcst._nodes.op.BaseBooleanOp
       - libcst._nodes.op.BaseCompOp
       - libcst._nodes.op.BaseUnaryOp
       - libcst._nodes.op._BaseOneTokenOp
       - libcst._nodes.op._BaseTwoTokenOp
       - libcst._nodes.statement.BaseCompoundStatement
       - libcst._nodes.statement.BaseSmallStatement
       - libcst._nodes.statement.BaseStatement
       - libcst._nodes.statement.BaseSuite
       - libcst._nodes.statement._BaseSimpleStatement
       - libcst._nodes.whitespace.BaseParenthesizableWhitespace
       - libcst._parser.types.config.BaseWhitespaceParserConfig

[zsol]

Thanks for checking! Yes, I'd be happy to review a PR :) I wonder if we could build a linter to run automatically as part of CI for this. Maybe something using https://github.com/instagram/fixit ?

[ariebovenberg]

Hi! Well, I found these errors because that's exactly what I'm building myself, haha. Found some mistakes in the stdlib even 👀

I'm assuming fixit is a static analysis tool? I've found it's quite challenging to trace inheritance with all the import shenanigans many libraries use. So in the end it's a runtime checker.

[zsol]

I'm assuming fixit is a static analysis tool? I've found it's quite challenging to trace inheritance with all the import shenanigans many libraries use. So in the end it's a runtime checker.

Hmm, fair enough. If you make a pre-commit hook, I'm happy to include that in our dev/CI process after we've fixed all the offending classes listed above.