Skip to content

Pick correct file if two files with the same name but with different extensions exist #2722

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

Chasarr
Copy link

@Chasarr Chasarr commented Apr 12, 2025

Type of Changes

Type
βœ“ πŸ› Bug fix
✨ New feature
πŸ”¨ Refactoring
πŸ“œ Docs

Description

To replicate the error that this PR fixes, you need to create two files:

test.py    # Contains Pylint errors
test.py2   # Does not contain Pylint errors

and then I run pylint test.py2 it reports Pylint errors from test.py. (the wrong file)

I believe this was due to a typo/logical error. Before this PR, I don't think the prefer_stubs argument even did anything.

Closes pylint-dev/pylint#3631

@Chasarr
Copy link
Author

Chasarr commented Apr 12, 2025

Hmm, the CI isn't too happy. Investigating...

@Chasarr Chasarr changed the title Use correct file extension Pick correct file file if two files with the same name but with different extensions exist Apr 12, 2025
@Chasarr Chasarr changed the title Pick correct file file if two files with the same name but with different extensions exist Pick correct file if two files with the same name but with different extensions exist Apr 12, 2025
@Chasarr
Copy link
Author

Chasarr commented Apr 12, 2025

All tests pass on my machine now, so it should work

Copy link

codecov bot commented Apr 13, 2025

Codecov Report

All modified and coverable lines are covered by tests βœ…

Project coverage is 93.21%. Comparing base (ce81c29) to head (e278288).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2722   +/-   ##
=======================================
  Coverage   93.21%   93.21%           
=======================================
  Files          93       93           
  Lines       11071    11072    +1     
=======================================
+ Hits        10320    10321    +1     
  Misses        751      751           
Flag Coverage Ξ”
linux 93.08% <100.00%> (+<0.01%) ⬆️
pypy 93.21% <100.00%> (+<0.01%) ⬆️
windows 93.19% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Ξ”
astroid/modutils.py 89.51% <100.00%> (+0.03%) ⬆️
πŸš€ New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Chasarr
Copy link
Author

Chasarr commented Apr 15, 2025

I don't think automatic issue closing works between repos, so someone with the correct permissions needs to close it manually.

Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the fix @Chasarr, do you mind adding some automated tests please ?

@Chasarr
Copy link
Author

Chasarr commented Apr 18, 2025

Done :)

Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the automated tests, it looks great. Do you mind adding a changelog entry too ? You probably need to specify that the issue you're closing come from pylint, like this:

Closes pylint-dev/pylint#8589

And if you want to have an aliase for the contributors list you can do it in https://github.com/pylint-dev/astroid/blob/main/script/.contributors_aliases.json#L4

@Chasarr
Copy link
Author

Chasarr commented Apr 19, 2025

Done! Thanks for the guidance!

Comment on lines +341 to +342
os.path.join(package, "__init__.weird_ext"),
os.path.join(package, "standalone_file.weird_ext"),
Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas Apr 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
os.path.join(package, "__init__.weird_ext"),
os.path.join(package, "standalone_file.weird_ext"),
os.path.join(package, "__init__.weird_ext"),
os.path.join(package, "standalone_file.weird_ext"),
os.path.join(package, "standalone_file.py"),

I might have reviewed this still half asleep the first time* but shouldn't it be this instead (issue talks about 'two file with the same name but different extension') ? I suppose being able to also handle 'two file with the same extension but different name is also valuable.

* (just found and read the pylint issue to close it automatically when we merge this and it doesn't match)

Copy link
Author

@Chasarr Chasarr Apr 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There already exists an __init__.py, so I figured I could just reuse that file for my tests too. And I also threw in a standalone file without any weird naming and without any other file sharing it's base name just because Β―\_(ツ)_/Β―

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we perhaps test for __init__.py too? I just realized there doesn't seem to be a test for that. Most likely other test would catch that error though.

Comment on lines +44 to +47
* Fix bug where ``pylint code.custom_extension`` would analyze ``code.py`` or ``code.pyi`` instead if they existed.

Closes pylint-dev/pylint#3631

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Fix bug where ``pylint code.custom_extension`` would analyze ``code.py`` or ``code.pyi`` instead if they existed.
Closes pylint-dev/pylint#3631
* Fix a bug where astroid could not parse ``code.pyi`` if ``code.py`` existed.
Closes pylint-dev/pylint#3631

Copy link
Author

@Chasarr Chasarr Apr 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, not quite, this should not affect the behaviour of .pyi, .py or .pyw extensions. They worked as intended before my PR.

These three extensions seem to be picked according to some kind of hierarchy that can be changed with prefer_stubs = True/False.

With prefer_stubs = False:
.py, .pyw, .pyi (.pyw is only picked on Windows machines)
With prefer_stubs = True:
.pyi, .pyw, .py

So if I ran pylint mycode.pyi it would actually check mycode.py, if it exists. I’m not sure why it works in this way though, but it seems to be the intended behaviour.

The bug that I fixed appeared when using a file with another file extension than these three, such as .weird_ext, where it would completely disregard my non-standard file in favor of the correctly named file. In other words,pylint mycode.weird_ext would in reality analyze mycode.py instead if it existed.

I don’t think I want to go too deep into what bug was fixed in the changelog, so perhaps the first explanation was sufficient? I can’t think of a better way to explain it in one sentence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Results from wrong file when files have the same name but different extensions
2 participants