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

Incorrect horizontal position of fixme warnings (W0511) #4218

Closed
bersbersbers opened this issue Mar 9, 2021 · 10 comments · Fixed by #4246
Closed

Incorrect horizontal position of fixme warnings (W0511) #4218

bersbersbers opened this issue Mar 9, 2021 · 10 comments · Fixed by #4246
Assignees
Labels
Enhancement ✨ Improvement to a component Good first issue Friendly and approachable by new contributors Help wanted 🙏 Outside help would be appreciated, good for new contributors

Comments

@bersbersbers
Copy link

The horizontal positions of the following four W0511 warning are all 2:

bug.py:

"""Horizontal position of fixme."""

# TODO 1
print(1)  # TODO 2


def fun():
    """Test in function."""
    # TODO 3
    if True & True:
        pass
        # TODO 4

pylint bug.py:

************* Module bug
bug.py:3:2: W0511: TODO 1 (fixme)
bug.py:4:2: W0511: TODO 2 (fixme)
bug.py:9:2: W0511: TODO 3 (fixme)
bug.py:12:2: W0511: TODO 4 (fixme)

This is how that is rendered by VS Code:
image

Shouldn't the position be the start of the comment (or the start of TODO)?

@Pierre-Sassoulas Pierre-Sassoulas added the Enhancement ✨ Improvement to a component label Mar 10, 2021
@Pierre-Sassoulas
Copy link
Member

Thanks for the report. We added a column check to the functional test recently so it's probably just making sure that the proper number is used in fixme's functional tests and changing the naive value in the checker.

@Pierre-Sassoulas Pierre-Sassoulas added Good first issue Friendly and approachable by new contributors Help wanted 🙏 Outside help would be appreciated, good for new contributors labels Mar 10, 2021
@bersbersbers
Copy link
Author

bersbersbers commented Mar 10, 2021

I may be misunderstanding your comment, but just to be on the safe this: this is a bug in a real-life application, not just a failing test.

@Pierre-Sassoulas
Copy link
Member

Yes I was just talking about the way we can simply modify the functional test text and the naive value in the checker because of the amazing #4013 instead of having to create a new test 😛

@ksaketou
Copy link
Contributor

Hello! I am a new contributor and I would like to work on this issue. I notice that for some reason the column index of the output depends on the position of #. For example, if there is no space between # and TODO, the column number will change to 1, regardless of the column index of #. Do you think I should check the functional test for that or the checker itself?

@Pierre-Sassoulas
Copy link
Member

Hello @ksaketou thank you for working on this, much appreciated ! I assigned you to the issue.

The fix need to be done on the checker itself but the column will change inside the test to reflect that :) You will be able to automatically update the expected output in the functional tests if the output is not the same anymore (python tests/test_functional.py --update-functional-output -k "test_functional[fixme]") or you can change the value in the test manually first in order to check if your code is correct with python3 -m pytest -k test_functional[fixme]. The order is up to your personal preference :)

@ksaketou
Copy link
Contributor

Great! I am working on it. I'll let you know. :)

@ksaketou
Copy link
Contributor

Hello, I have a question. I made some changes on the checker and I successfully tested the code with python -m pytest -k test_functional[fixme]. However, when I use python tests/test_functional.py --update-functional-output -k "test_functional[fixme]" the column indexes at fixme.txt are updated with the wrong values. Why is this happening?

@Pierre-Sassoulas
Copy link
Member

When you launch python tests/test_functional.py --update-functional-output -k "test_functional[fixme]" it updates the test with the current pylint output. You can use it to check the real output of your code. The problem here is that we're only testing columns for python 3.8 or above (the column information got different and better in python 3.8). So even if the column is wrong python -m pytest -k test_functional[fixme] will work everytime as long as the lines of the messages are correct. I think you might be using python 3.7 or below ? You will probably need to switch to python 3.8 for the test on column to be useful for you. You can develop under python 3.7 if you check the output yourself though. Then github actions will check all versions of python online.

@ksaketou
Copy link
Contributor

Hello, sorry for the late reply. I see what you mean, thank you. Also, I use Python 3.8.1. I linked a pull request to this issue, please let me know if I need to make any changes.

@bersbersbers
Copy link
Author

Thanks everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement ✨ Improvement to a component Good first issue Friendly and approachable by new contributors Help wanted 🙏 Outside help would be appreciated, good for new contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants