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

Features for refactoring automatically offending code (pylint autofix) #7438

Open
idiomaticrefactoring opened this issue Sep 9, 2022 · 1 comment
Labels
Discussion 🤔 High effort 🏋 Difficult solution or problem to solve Needs design proposal 🔒 This is a huge feature, some discussion should happen before a PR is proposed Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning

Comments

@idiomaticrefactoring
Copy link

idiomaticrefactoring commented Sep 9, 2022

Current problem

Recently, a paper could automatically refactor non-idiomatic Python code with nine Python idioms: List/set/dict-comprehension, chain-comparison, truth-value-test, loop-else, star-in-function-call, assign-multiple-targets, for-multiple-targets. And they pulled some requests to GitHub projects and many project members like the feature. And some developers proposed issues for Pylint project such as #5800, a developer said “Would it be an idea for Pylint to output the suggested refactor to the user” in Pylint.

Therefore, I would like to try to integrate the functionality into Pylint, what do you think of the idea?

For example, for the list-comprehension, the paper could refactor the code from GitHub project,

input_files = []
for root, dirs, files in os.walk(path):
             for file in files:
                 if file.endswith(".pbxproj"):
                     input_files.append(path + file)

into

input_files = [path + file for (root, dirs, files) in os.walk(path) 
                                        for file in files if file.endswith('.pbxproj')]

.
For another example, for the for-multiple-targets, the paper could refactor the code from GitHub project

for items in params_grads:
                 assert items[0].name is not model.embed1.weight.name
                 assert items[0].name is not model.linear_1.weight.name

into

for (items_0, *items_len) in params_grads:
                 assert items_0.name is not model.embed1.weight.name
                 assert items_0.name is not model.linear_1.weight.name

Desired solution

If I could do this, currently I want to refer to the source code of Refactoring checker. Maybe I want to create a new class for each python idiom and register them in https://github.com/PyCQA/pylint/blob/main/pylint/checkers/refactoring/__init__.py. And then, I maybe refer to https://github.com/PyCQA/pylint/blob/055c1920140ebf36501d30307d5ab86965ae185f/pylint/checkers/refactoring/refactoring_checker.py to implement the details of code refactoring.

For example,

  1. adding msg for each refactoring
  2. checking code whether is refactorable
  3. giving suggestions with added msg like Refactoring checker, such as printing "RXXXX: we could refactor ... into ...".

How about the method or some suggestions?

Additional context

https://arxiv.org/abs/2207.05613

@idiomaticrefactoring idiomaticrefactoring added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Sep 9, 2022
@Pierre-Sassoulas
Copy link
Member

Thank you for creating the issue. This is something we often talk about as there's some message that give the replacement outright in text and auto fixing would make a lot of sense. But we did not have an issue for it yet.

Take for example the for_any_all checker, the expected code is clearly given in the message.

It was proposed that it would be better as a standalone command and project with a different release cycle than pylint, for simplicity. Even if most pylint contributors would probably be interested in participating.

There's some design to consider:

  • Do we reuse the code of the checkers from pylint (maybe adding a hook to an autofix when using add_message) ? This would be more suited to something integrated in pylint
  • Do we code it from scratch instead with a completely new design ? It would be more work at the beginning but less later on (?). We could refactor checkers without breaking the "pylint-autofix" (btw we need a name).

The organization we would create the repository in is also something we need to discuss. (Right now we do not have admin right in PyCQA and can't create a new repository among other issues).

@Pierre-Sassoulas Pierre-Sassoulas added Discussion 🤔 Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning Needs design proposal 🔒 This is a huge feature, some discussion should happen before a PR is proposed High effort 🏋 Difficult solution or problem to solve and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Sep 9, 2022
@jacobtylerwalls jacobtylerwalls changed the title features for refactoring non-idiomaitc code with Python idioms features for refactoring non-idiomatic code with Python idioms Sep 9, 2022
@Pierre-Sassoulas Pierre-Sassoulas changed the title features for refactoring non-idiomatic code with Python idioms Features for refactoring automatically offending code (pylint autofix) Sep 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion 🤔 High effort 🏋 Difficult solution or problem to solve Needs design proposal 🔒 This is a huge feature, some discussion should happen before a PR is proposed Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning
Projects
None yet
Development

No branches or pull requests

2 participants