[issue28128] Improve the warning message for invalid escape sequences

Changes by Donald Stufft : -- pull_requests: +922 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.

[issue28128] Improve the warning message for invalid escape sequences

Eric V. Smith added the comment: Chi Hsuan Yen: I'll investigate, and open another issue as needed. -- resolution: -> fixed stage: commit review -> resolved status: open -> closed ___ Python tracker _

[issue28128] Improve the warning message for invalid escape sequences

Roundup Robot added the comment: New changeset ee82266ad35b by Eric V. Smith in branch '3.6': Issue 28128: Print out better error/warning messages for invalid string escapes. Backport to 3.6. https://hg.python.org/cpython/rev/ee82266ad35b New changeset 7aa001a48120 by Eric V. Smith in branch 'd

[issue28128] Improve the warning message for invalid escape sequences

Chi Hsuan Yen added the comment: The error message is much better now, thanks you all! Seems the ^ pointer is not always correct. For example, in the function scope it's correct: $ cat test.py def foo(): s = 'C:\Program Files\Microsoft' $ python3.7 -W error test.py File "test.py", line

[issue28128] Improve the warning message for invalid escape sequences

Tim Graham added the comment: The patch is working well to identify warnings when running Django's test suite. Thanks! -- ___ Python tracker ___

[issue28128] Improve the warning message for invalid escape sequences

Eric V. Smith added the comment: I'll work on this as soon as I can, coordinating with Ned. -- ___ Python tracker ___ ___ Python-bugs-

[issue28128] Improve the warning message for invalid escape sequences

Emanuel Barry added the comment: Even better than what I was aiming for :) -- dependencies: -Convert warnings to SyntaxWarning in parser priority: deferred blocker -> release blocker ___ Python tracker ___

[issue28128] Improve the warning message for invalid escape sequences

Eric V. Smith added the comment: I agree it would be nice to get this in to 3.6. I'm not sure I'd go so far as to say it's a must and can't wait for 3.6.1. It's a non-trivial change, and it's up to Ned to say if it can go in to 3.6. If you don't run with -Wall or -Werror, then you won't notice

[issue28128] Improve the warning message for invalid escape sequences

Ned Deily added the comment: I agree that the current behavior for 3.6 is very user-unfriendly so I think the risks of making such an extensive change at this point in the release cycle are outweighed by the severity of the problem. So let's get it into 3.6 now; there's still time for it to m

[issue28128] Improve the warning message for invalid escape sequences

Emanuel Barry added the comment: As Nick pointed out in an earlier message on this thread and as Serhiy observed on GitHub issues, backporting this patch to 3.6 is a must. Large projects' use of Python 3.6 has shown that it's hard to track down the actual cause of the error; it only makes sens

[issue28128] Improve the warning message for invalid escape sequences

Serhiy Storchaka added the comment: Searching on GitHub it seems to me that the most frequent issue with supporting Python 3.6 is eliminating or silencing warnings about invalid escape sequences. Any help with this is very important. -- ___ Python t

[issue28128] Improve the warning message for invalid escape sequences

Roundup Robot added the comment: New changeset 259745f9a1e4 by Eric V. Smith in branch 'default': Issue 28128: Print out better error/warning messages for invalid string escapes. https://hg.python.org/cpython/rev/259745f9a1e4 -- nosy: +python-dev ___

[issue28128] Improve the warning message for invalid escape sequences

Eric V. Smith added the comment: I've pushed this to the default branch. I'll watch the buildbots. Then Ned can decide if this goes in to 3.6. -- ___ Python tracker ___

[issue28128] Improve the warning message for invalid escape sequences

Eric V. Smith added the comment: I'm not in front of a computer at the moment, but the output looks good. Also, my very quick glance at -7.diff's warn_invalid_escape_sequence looks reasonable, although I can't say for sure whether raising the error in PyErr_WarnExplicitObject followed by raisi

[issue28128] Improve the warning message for invalid escape sequences

Serhiy Storchaka added the comment: Following patch just raises SyntaxError if DeprecationWarning was raised as error. Still needed tests for this. > Also, you'll note that with or without your patch, you get the same behavior. Not the same. New warnings contain correct information about a fil

[issue28128] Improve the warning message for invalid escape sequences

Eric V. Smith added the comment: Also, you'll note that with or without your patch, you get the same behavior. The code in hg already raises DeprecationWarning, just in a different place. So unless we can improve the DeprecationWarning output, we're better off doing nothing. -- _

[issue28128] Improve the warning message for invalid escape sequences

Serhiy Storchaka added the comment: Added additional tests. -- Added file: http://bugs.python.org/file45292/28128-6.diff ___ Python tracker ___ __

[issue28128] Improve the warning message for invalid escape sequences

Eric V. Smith added the comment: Serihy: I had tried this approach earlier, but it doesn't work. With your -5.diff patch, the output is (using Nick's test case): $ rm -rf __pycache__/ ; ./python -Werror escape_warning.py Traceback (most recent call last): File "escape_warning.py", line 1, i

[issue28128] Improve the warning message for invalid escape sequences

Serhiy Storchaka added the comment: Fixed bytes literals decoding (test_codecs was failed). -- Added file: http://bugs.python.org/file45290/28128-5.diff ___ Python tracker ___ __

[issue28128] Improve the warning message for invalid escape sequences

Serhiy Storchaka added the comment: Updated patch emits DeprecationWarning instead of SyntaxError. I still not tested it. -- Added file: http://bugs.python.org/file45288/28128-4.diff ___ Python tracker ___

[issue28128] Improve the warning message for invalid escape sequences

Changes by Serhiy Storchaka : Added file: http://bugs.python.org/file45287/28128-3.diff ___ Python tracker ___ ___ Python-bugs-list mailing li

[issue28128] Improve the warning message for invalid escape sequences

Eric V. Smith added the comment: I'll take a look at it, Emanuel. But I can't promise how much progress I'll be able to make today. I also think that at that point it becomes so complex that it fails Ned's test for inclusion in 3.6. -- ___ Python tr

[issue28128] Improve the warning message for invalid escape sequences

Emanuel Barry added the comment: Thank you Eric. Have you looked at making a new DeprecatedSyntaxWarning subclass of both DeprecationWarning and SyntaxWarning? Hopefully that's of some help. I don't see a review link, but from a quick glance this looks good. Thanks :) --

[issue28128] Improve the warning message for invalid escape sequences

Eric V. Smith added the comment: Oops, use 28128-3.diff. -- Added file: http://bugs.python.org/file45285/28128-3.diff ___ Python tracker ___ _

[issue28128] Improve the warning message for invalid escape sequences

Changes by Eric V. Smith : -- stage: needs patch -> patch review ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue28128] Improve the warning message for invalid escape sequences

Changes by Eric V. Smith : Added file: http://bugs.python.org/file45284/28128-2.diff ___ Python tracker ___ ___ Python-bugs-list mailing list

[issue28128] Improve the warning message for invalid escape sequences

Eric V. Smith added the comment: Here's an updated patch, that fixes some problems with the earlier patch, and adds equivalent support for bytes. HOWEVER, I can't get the warnings machinery to raise a DeprecationWarning that would have all of the equivalent information that an actual SyntaxErr

[issue28128] Improve the warning message for invalid escape sequences

Changes by Eric V. Smith : -- assignee: -> eric.smith ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https://

[issue28128] Improve the warning message for invalid escape sequences

Eric V. Smith added the comment: I'll work on this later today. -- ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscrib

[issue28128] Improve the warning message for invalid escape sequences

Changes by Ezio Melotti : -- nosy: +ezio.melotti ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.p

[issue28128] Improve the warning message for invalid escape sequences

Nick Coghlan added the comment: Petr, is there any chance you or someone from your team could take a look at this and the other issue Emanuel referenced? These warnings are likely to pose a usability problem when upgrading Python in Fedora in their current state, so it would be good to see the

[issue28128] Improve the warning message for invalid escape sequences

Emanuel Barry added the comment: Following Ned's email to Python-Dev, I'm upping the priority of this issue to "deferred blocker". I haven't had a lot of time to work on this, but I had time to think about it, and I think that resolving #28028 would be the best way to solved this. I pinged th

[issue28128] Improve the warning message for invalid escape sequences

Eric V. Smith added the comment: Sorry for not responding earlier. It's unlikely I'll have time for this before beta 2, although I can probably get to it after that and before beta 3. Don't let me stop someone else from improving on the patch. -- __

[issue28128] Improve the warning message for invalid escape sequences

Nick Coghlan added the comment: Eric's basic approach sounds fine to me, as it gets the traceback in the right place (i.e. blaming the code being compiled, not the code doing the import). For beta 2, how about we just go with a plain SyntaxWarning? Since users running pre-compiled modules won'

[issue28128] Improve the warning message for invalid escape sequences

Changes by Brett Cannon : -- nosy: -brett.cannon ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.

[issue28128] Improve the warning message for invalid escape sequences

Emanuel Barry added the comment: Ping. I'd like for this to get merged in beta 2; should I (or Eric if he wants to) get to work on this? Is the DeprecatedSyntaxWarning subclass route still desired? -- ___ Python tracker

[issue28128] Improve the warning message for invalid escape sequences

Nick Coghlan added the comment: Eric's basic approach sounds fine to me. The "pre-compiled .pyc files won't trigger SyntaxWarning" problem isn't new, as it exists for the old 3.5 warnings as well (-B prevents writing bytecode, which may be handy while working on this. Unfortunately, there's no

[issue28128] Improve the warning message for invalid escape sequences

Eric V. Smith added the comment: Tim: Cool! That's way more useful than I thought it would be. Serhiy: It's a proof of concept. Lots of design remains to be done. I'm not sure we've agreed on the concept yet, so I don't think it's worthwhile designing the API. --

[issue28128] Improve the warning message for invalid escape sequences

Serhiy Storchaka added the comment: The basic concept LGTM. first_invalid_escape_char is redundant, it is just s[first_invalid_escape_idx]. Or maybe better to return a pointer instead of an index. bytes literals need similar solution. -- ___ Python

[issue28128] Improve the warning message for invalid escape sequences

Tim Graham added the comment: Eric, your patch was good enough to allow me to easily identify and fix all the warnings in Django: https://github.com/django/django/pull/7254. Thanks! -- ___ Python tracker _

[issue28128] Improve the warning message for invalid escape sequences

Emanuel Barry added the comment: Personally I'd be fine with only one warning, reporting the first invalid escape. Presumably the string would be checked as a whole, or would get an r prefix. Patch seems like a good start; bytes would also need updating in that regard. Don't worry too much abo

[issue28128] Improve the warning message for invalid escape sequences

Eric V. Smith added the comment: Also, I assume this is a problem with all such syntax warnings: you only see this warning/error when the file is originally compiled. Once the .pyc file exists, you'll never see a warning or error. Maybe that's okay, but it means there's a certain class of inst

[issue28128] Improve the warning message for invalid escape sequences

Eric V. Smith added the comment: I forgot: this is what Nick's example now looks like: $ ./python -Wall escape_warning.py Traceback (most recent call last): File "escape_warning.py", line 1, in import bad_escape File "/home/eric/local/python/cpython/bad_escape.py", line 1 print('\d'

[issue28128] Improve the warning message for invalid escape sequences

Eric V. Smith added the comment: Here is an extremely rough patch that shows the basic concept. I named the private function _PyUnicode_DecodeUnicodeEscape. The problems with this patch are: 1. it always raises an error, not a warning 2. the private function isn't declared in a .h file 3. the n

[issue28128] Improve the warning message for invalid escape sequences

Eric V. Smith added the comment: Couldn't we create a private version of PyUnicode_DecodeUnicodeEscape, to be called by ast.c, which passes back invalid escape info? Then have the actual warning raised by ast.c, which knows enough about the context to generate a better error/warning. I think w

[issue28128] Improve the warning message for invalid escape sequences

Nick Coghlan added the comment: I've added Eric Smith to the nosy list as well, as he's been working on that part of the compilation pipeline for the f-string implementation. Eric, the context here is that the new deprecation warning for unknown escapes is currently being misattributed to the

[issue28128] Improve the warning message for invalid escape sequences

Emanuel Barry added the comment: Hmm, I see; I'll need to dig a bit deeper get to and understand that part of the compile process better. I'll look up where SyntaxErrors are generated (since they have access to at least the line number at that point), and try to hook it up from there. ---

[issue28128] Improve the warning message for invalid escape sequences

Nick Coghlan added the comment: Regarding when I think the error should be generated, we definitely want the warning to be happening at compile time, but the "compile step" is actually a series of substeps. The point where the string parser is processing string escapes is *not* the best place

[issue28128] Improve the warning message for invalid escape sequences

Nick Coghlan added the comment: I realised I wasn't entirely clear about the "warning misattribution" problem that's implied by Chi Hsuan's problem report, so here's the behaviour when using "-W all" rather than "-W error": $ echo "print('\d')" > bad_escape.py $ echo "import bad_escape

[issue28128] Improve the warning message for invalid escape sequences

Yury Selivanov added the comment: BTW, this will also help to make warnings more friendly in #26182. -- ___ Python tracker ___ ___ Pyt

[issue28128] Improve the warning message for invalid escape sequences

Emanuel Barry added the comment: Thank you Nick for the useful feedback! I think that a subclass of DeprecationWarning and SyntaxWarning would be a good idea; I'll play around with that. As far as when the warning should occur, I agree that erroring out at the compile step isn't optimal, howe

[issue28128] Improve the warning message for invalid escape sequences

Nick Coghlan added the comment: Adding a "DeprecatedSyntaxWarning" that's a subclass of both DeprecationWarning and SyntaxWarning (and hence silenced by default while still having the extra SyntaxWarning attributes) strikes me as a reasonable step to take here, even though we're into the beta

[issue28128] Improve the warning message for invalid escape sequences

Emanuel Barry added the comment: There definitely needs to be a better message for that. The problem is that the parser doesn't have access to the whole string (of course; it's being constructed!), so I think there are several possible venues here: - Change DeprecationWarning to display the li

[issue28128] Improve the warning message for invalid escape sequences

Tim Graham added the comment: I hope the message can be improved for Python 3.6 as the warnings I see when running Django's test suite are rather useless to help find and fix the issues: cpython/Lib/importlib/_bootstrap.py:205: DeprecationWarning: invalid escape sequence '\:' Grepping for the

[issue28128] Improve the warning message for invalid escape sequences

R. David Murray added the comment: Are SyntaxWarnings silent by default? If not it can't even go into 3.7. -- ___ Python tracker ___

[issue28128] Improve the warning message for invalid escape sequences

Emanuel Barry added the comment: Besides converting the DeprecationWarning to a Syntax{Error,Warning}, I don't see an easy way to include the offending line (or even file). The place in the code where the strings are created has no idea *where* they are being defined. AIUI, either we special-c

[issue28128] Improve the warning message for invalid escape sequences

Martin Panter added the comment: See also Issue 28028. Serhiy suggested translating warnings to SyntaxWarning in general. Looks like that may help narrowing down the location of escaping problems. -- ___ Python tracker

[issue28128] Improve the warning message for invalid escape sequences

Emanuel Barry added the comment: Hello, and thanks! I'll work on a patch this week, or at most next week. I will make it so that it's completely uncontroversial to apply it to 3.6 as well (won't change the actual feature, only prettify the error message), so no need to worry about that :) ---

[issue28128] Improve the warning message for invalid escape sequences

R. David Murray added the comment: The error can't be a SyntaxError, it must be a DeprecationWarning. If you can improve the deprecation warning text, I'd be in favor of calling that a fix to the original feature and put it in 3.6. The deprecation warning can even say that this will be a Syn

[issue28128] Improve the warning message for invalid escape sequences

Ned Deily added the comment: It sounds like a fix but let's see the final patch first. If a core developer wants to apply it to the default branch for 3.7, we can decide whether it should go into 3.6, too. -- ___ Python tracker

[issue28128] Improve the warning message for invalid escape sequences

New submission from Chi Hsuan Yen: In issue27364, invalid escape sequences in string literals are deprecated. Currently the deprecation message is not so useful when fixing lots of files in one or more large projects. For example, I have two files foo.py and bar.py: # foo.py import bar # bar.

[issue28128] Improve the warning message for invalid escape sequences

Chi Hsuan Yen added the comment: And I'd like to ask Ned: for me it's an improvement of an existing feature, so I guess it can enter the 3.6 branch? -- nosy: +ned.deily ___ Python tracker _