Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not allow super() without parameters in generator expressions #2310

Closed
sobolevn opened this issue Dec 26, 2021 · 19 comments · Fixed by #2530
Closed

Do not allow super() without parameters in generator expressions #2310

sobolevn opened this issue Dec 26, 2021 · 19 comments · Fixed by #2530
Assignees
Labels
good first issue Entrypoint to the project help wanted Extra attention is needed level:starter Good for newcomers rule request Adding a new rule

Comments

@sobolevn
Copy link
Member

sobolevn commented Dec 26, 2021

This bug: https://bugs.python.org/issue46175

We need to require super(cls, self) in this case.

@sobolevn sobolevn added help wanted Extra attention is needed level:starter Good for newcomers rule request Adding a new rule good first issue Entrypoint to the project labels Dec 26, 2021
@sobolevn sobolevn changed the title Do not allow super() without parameters in generator exmpressions Do not allow super() without parameters in generator expressions Dec 26, 2021
@ghost
Copy link

ghost commented Jan 3, 2022

Hi ! I would like to work on this issue :)

@sobolevn
Copy link
Member Author

sobolevn commented Jan 3, 2022

@nasdev-cmd thank you!

@sobolevn sobolevn assigned ghost Jan 3, 2022
@kitsiosvas
Copy link

Hello, is this issue still open?
If so, I would appreciate it greatly if I could provided with the information of where is the code linked to this behavior located.

@sobolevn
Copy link
Member Author

@kitsiosvas I think that we can add this check here: https://github.com/wemake-services/wemake-python-styleguide/blob/master/wemake_python_styleguide/visitors/ast/classes.py

Are you familiar with python's AST?

@kitsiosvas
Copy link

@sobolevn I am somewhat familiar with AST indeed. Are there any other similar checks that I could consult?
I've been studying the code you included in your previous message to understand what exactly I will need to implement.
Thank you.

@sobolevn
Copy link
Member Author

You can take a look at this:

def _check_wrong_function_called(self, node: ast.Call) -> None:
function_name = functions.given_function_called(
node, FUNCTIONS_BLACKLIST,
)
if function_name:
self.add_violation(
WrongFunctionCallViolation(node, text=function_name),
)

It just checks for wrong functions. But, in this case you will also need the context. See https://github.com/wemake-services/wemake-python-styleguide/blob/master/wemake_python_styleguide/logic/walk.py

@kitsiosvas
Copy link

If I understand it correctly, I need to somehow check if super() exists as a node with parent a generator expression and it's kids
are not in the form you have pointed out initially (super(cls, self)).
I don't know if I am missing something. Any input is more than welcome.

@sobolevn
Copy link
Member Author

Yes, sounds right!

If super call:

  • is inside a gen expr
  • and does not have two args

We raise a violation.

@kitsiosvas
Copy link

kitsiosvas commented Jan 27, 2022

@sobolevn Thank you for your quick reply.
I am having trouble figuring out how do I check if a node is inside a generator expression.
Is there a tactic that I am missing?
Sorry for making this threat a long one.

Edit: I am thinking of something like using get_closest_parent(node, parents) method. As first argument there will be the super() call node, and as second argument I will have to include generator expressions (not sure on how that is implemented).
If it returns null then super() is not in a gen expr.

@sobolevn
Copy link
Member Author

I would go this way:

  1. Find ast.Call nodes, check name to be super
  2. Check args count (it is fast and cheap)
  3. Then get_closest_parent(call, ast.GeneratorExpr)

@kitsiosvas
Copy link

In order to get the node's name I would have to use given_function_called(node, to_check: Container[str]) method.
What would I use as Container in this case. I couldn't find anything clear on this. Or maybe there is another way of getting the name of the node?

@sobolevn
Copy link
Member Author

sobolevn commented Jan 27, 2022

given_function_called(node, {'super'})

@kitsiosvas
Copy link

I have implemented the feature. Should I issue a Pull Request for review?

@sobolevn
Copy link
Member Author

Please! 👍

@kitsiosvas
Copy link

kitsiosvas commented Jan 27, 2022

According to the "Raise a violation" part, I tried doing something similar to #2310 (comment) , but it raises errors apparently. I couldn't figure out what is the proper way of achieving this, so far. Will keep searching, but any help is amazing.
Thank you.

@akshaybhadange
Copy link

does this issue still exists , I want to contribute in this project , actually its my first opensource contribution , so any help regarding this really appreciated

@lensvol
Copy link
Collaborator

lensvol commented Oct 12, 2022

Can I take this issue over?

@sobolevn
Copy link
Member Author

sobolevn commented Oct 12, 2022

Please! Can you also check other comprehensions? And nested functions?

@lensvol
Copy link
Collaborator

lensvol commented Oct 12, 2022

I'll do what I can.

@lensvol lensvol self-assigned this Oct 14, 2022
lensvol added a commit to lensvol/wemake-python-styleguide that referenced this issue Oct 14, 2022
sobolevn pushed a commit that referenced this issue Oct 15, 2022
* Detect `super()` call being used in the wrong context.

Resolves #2310.

* Replace custom node tree walking code with the existing helper.

* Fix incorrect class name in the line pertaining to this PR.

* Fix version in the unrelated version.

* Add ability to detect calls to `super()` in the nested methods.

* Add missing test case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Entrypoint to the project help wanted Extra attention is needed level:starter Good for newcomers rule request Adding a new rule
Projects
None yet
4 participants