Skip to content

gh-71765: Fix inspect.getsource() on empty file #20809

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

Merged
merged 21 commits into from
Mar 18, 2024

Conversation

kernc
Copy link
Contributor

@kernc kernc commented Jun 11, 2020

For modules from empty files, inspect.getsource() instead of raising inaccurate OSError now returns an empty string, and inspect.getsourcelines() returns a list of one empty string, fixing the expected invariant.

As indicated by exec(''), empty strings are valid Python source code.

https://bugs.python.org/issue27578

For modules from empty files, `inspect.getsource()` now
returns an empty string, and `inspect.getsourcelines()` returns
a list of one empty string, fixing the expected invariant.

As indicated by `exec('')`, empty strings are valid Python
source code.
Copy link
Contributor

@remilapeyre remilapeyre left a comment

Choose a reason for hiding this comment

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

Thanks for looking into this. I think it would make more sense to change https://github.com/python/cpython/blob/3.9/Lib/linecache.py#L140-L141 to:

    if not lines:
        lines = ['\n']
    elif not lines[-1].endswith('\n'):
        lines[-1] += '\n'

and then inspect would not need to be changed.

@kernc
Copy link
Contributor Author

kernc commented Jun 15, 2020

Thanks @remilapeyre! Indeed, using the whole existing pipeline makes plenty more sense. 😅

With trailing '\n' forced even on the first and only line of a blank module, this makes its inspect.getsource() return a truthy value, which required changing a seemingly unrelated test.

Is the discrepancy between linecache.getlines() and file.readlines() documented anywhere in prose, or is it just at one time someone's misplaced convenience for splicing tracebacks? I understand that's likely set in stone now either way?

@remilapeyre
Copy link
Contributor

Thanks @remilapeyre! Indeed, using the whole existing pipeline makes plenty more sense. 😅

With trailing '\n' forced even on the first and only line of a blank module, this makes its inspect.getsource() return a truthy value, which required changing a seemingly unrelated test.

The change in the pdb test is related to this issue, it had nowhere to put the current line marker, but I think having the current line marker on an empty line when the file is empty is not an issue.

Is the discrepancy between linecache.getlines() and file.readlines() documented anywhere in prose, or is it just at one time someone's misplaced convenience for splicing tracebacks? I understand that's likely set in stone now either way?

linecache.getlines is specialised so I think it's OK to have it differs from readlines(). The documentation of linecache.getline says:

This function will never raise an exception — it will return '' on errors (the terminating newline character will be included for lines that are found).

It is useful to have a sentinel value that is a string as you don't have to worry to handle None when you call it and I think it makes sense to have linecache.getlines in similar fashion.

For the change to be accepted, you will have to add a test for the change in linecache and one for inspect as it's the initial issue on bpo.

@kernc
Copy link
Contributor Author

kernc commented Jun 16, 2020

Thanks. Test for inspect re-added. linecache on empty file is already changed/covered:

class EmptyFile(GetLineTestsGoodData, unittest.TestCase):
file_list = []
test_list = ['\n']

Copy link
Contributor

@remilapeyre remilapeyre left a comment

Choose a reason for hiding this comment

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

Thanks for making the change, I just have a last remark for the tests.

@kernc
Copy link
Contributor Author

kernc commented Nov 14, 2020

I didn't expect the Spanish Inquisition

@bedevere-bot
Copy link

Nobody expects the Spanish Inquisition!

: please review the changes made to this pull request.

Copy link
Contributor

@furkanonder furkanonder left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@furkanonder furkanonder left a comment

Choose a reason for hiding this comment

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

LGTM

@erlend-aasland erlend-aasland changed the title bpo-27578: Fix inspect.getsource() on empty file gh-71765: Fix inspect.getsource() on empty file Jan 8, 2024
@encukou encukou merged commit 52ef443 into python:main Mar 18, 2024
vstinner pushed a commit to vstinner/cpython that referenced this pull request Mar 20, 2024
* bpo-27578: Fix inspect.getsource() on empty file

For modules from empty files, `inspect.getsource()` now
returns an empty string, and `inspect.getsourcelines()` returns
a list of one empty string, fixing the expected invariant.

As indicated by `exec('')`, empty strings are valid Python
source code.

Co-authored-by: Oleg Iarygin <[email protected]>
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 25, 2024
* bpo-27578: Fix inspect.getsource() on empty file

For modules from empty files, `inspect.getsource()` now
returns an empty string, and `inspect.getsourcelines()` returns
a list of one empty string, fixing the expected invariant.

As indicated by `exec('')`, empty strings are valid Python
source code.

Co-authored-by: Oleg Iarygin <[email protected]>
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
* bpo-27578: Fix inspect.getsource() on empty file

For modules from empty files, `inspect.getsource()` now
returns an empty string, and `inspect.getsourcelines()` returns
a list of one empty string, fixing the expected invariant.

As indicated by `exec('')`, empty strings are valid Python
source code.

Co-authored-by: Oleg Iarygin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants