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

Include modname in AST warnings #2380

Merged
merged 2 commits into from
Feb 9, 2024
Merged

Conversation

cdce8p
Copy link
Member

@cdce8p cdce8p commented Feb 9, 2024

Type of Changes

Type
✨ New feature

Description

Python 3.12 change the DeprecationWarning for invalid escape sequence to a SyntaxWarning. These are now printed when the module is imported / the ast tree build. An example

# currently
<unknown>:158: SyntaxWarning: invalid escape sequence '\.'

# with this change
plexapi.base:158: SyntaxWarning: invalid escape sequence '\.'

Usually it's an actual filename for warnings and not just the modname. However, it should be good enough for now.

Long term it might make sense to adjust the warnings filter for astroid to make all SyntaxWarnings -> SyntaxErrors which would get re-raised. Thus it would be possible to add a pylint: disable (but only because the whole import isn't analyzed at all / skipped after the error).

For know this will at least help debug where the warning is coming from. Users can still choose to ignore them

python -Wignore:invalid\ escape\ sequence:SyntaxWarning -m pylint ...

@cdce8p cdce8p added this to the 3.1.0 milestone Feb 9, 2024
Copy link

codecov bot commented Feb 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (5accd50) 92.75% compared to head (04f266d) 92.75%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2380   +/-   ##
=======================================
  Coverage   92.75%   92.75%           
=======================================
  Files          94       94           
  Lines       11065    11067    +2     
=======================================
+ Hits        10263    10265    +2     
  Misses        802      802           
Flag Coverage Δ
linux 92.56% <100.00%> (+<0.01%) ⬆️
pypy 90.89% <100.00%> (+<0.01%) ⬆️
windows 92.34% <100.00%> (+<0.01%) ⬆️

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

Files Coverage Δ
astroid/_ast.py 100.00% <100.00%> (ø)
astroid/builder.py 94.11% <100.00%> (ø)

Comment on lines +421 to +424
with pytest.raises(AstroidSyntaxError, match="invalid escape sequence") as ctx:
self.builder.string_build("'\\d+\\.\\d+'")
assert isinstance(ctx.value.error, SyntaxError)
assert ctx.value.error.filename == "<unknown>"
Copy link
Member Author

Choose a reason for hiding this comment

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

Not exactly sure why the SyntaxError is raised for all Python version. Additionally this should only emit a SyntaxWarning at the moment, not a SyntaxError (which is reraised as AstroidSyntaxError).

Might be some pytest magic though..

astroid/builder.py Show resolved Hide resolved
astroid/_ast.py Show resolved Hide resolved
Copy link
Collaborator

@DanielNoord DanielNoord left a comment

Choose a reason for hiding this comment

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

Thanks!

@cdce8p cdce8p merged commit 35dba66 into pylint-dev:main Feb 9, 2024
20 checks passed
@cdce8p cdce8p deleted the ast-syntax-warning branch February 9, 2024 11:25
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.

2 participants