classification
Title: test_global_err_then_warn in test_syntax is no longer valid
Type: Stage: resolved
Components: Tests Versions: Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: gvanrossum, levkivskyi, serhiy.storchaka
Priority: normal Keywords: 3.6regression, patch

Created on 2016-12-11 12:33 by serhiy.storchaka, last changed 2017-10-26 21:29 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
syntax-error-order.patch levkivskyi, 2016-12-11 14:15 review
syntax-error-order-v2.patch levkivskyi, 2016-12-17 23:32 review
Pull Requests
URL Status Linked Edit
PR 4097 merged levkivskyi, 2017-10-23 21:47
Messages (16)
msg282916 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-12-11 12:33
Issue27999 invalidated test_global_err_then_warn in Lib/test/test_syntax.py:567. That test was added in issue763201 for testing that SyntaxWarning emitted in symtable_global() doesn't clobber a SyntaxError. In issue27999 a SyntaxWarning was converted to SyntaxError, and the test no longer serves its purpose.

Issue28512 makes tests more strong (it tests the position of SyntaxError), but it can't be applied in 3.6, because _check_error() catches different SyntaxError.
msg282920 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2016-12-11 14:15
Serhiy, here is a patch that might be helpful. It detects global-and-parameter errors sooner (when possible). This will cause the following:

>>> if 1:
...     def error(a):
...         global a
...     def error2():
...         b = 1
...         global b
... 
  File "<stdin>", line 3
SyntaxError: name 'a' is parameter and global

However, in more complex (nested) cases, the global-after-assign will still be detected sooner:

>>> def error(a):
...     def inner():
...         global a
...     def inner2():
...         b = 1
...         global b
... 
  File "<stdin>", line 6
SyntaxError: name 'b' is assigned to before global declaration

Maybe there is a way to delay the detection of this error until second pass in symtable.c, but don't see now how to do this.
msg283437 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-12-16 20:58
I don't know what should we do with this. Is there a bug that needs to be fixed? Or maybe the test can be just removed?
msg283522 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2016-12-17 22:01
I don't think this is a bug, but detecting first a SyntaxError that appears textually first might be seen as an improvement. I would say such behaviour seems more intuitive. A possible downside could be a (probably very minor) slow-down of compilation.

I don't have any strong opinion here.
msg283523 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2016-12-17 23:32
Updated patch as proposed by Serhiy.
msg304817 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-23 16:01
Do you mind to create a pull request Ivan?
msg304847 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2017-10-23 21:52
OK, I made a PR with the fix and a test that checks the line number for syntax error (so that the original purpose test_global_err_then_warn is preserved).
msg304850 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2017-10-23 23:26
Am I needed here?
msg304861 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2017-10-24 06:06
> Am I needed here?

As I understand, Serhiy is going to review the PR, so I think no.
msg304864 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-24 06:45
Sorry for disturbing you Guido. I had added you as the committer of issue27999. Do you have thoughts about this issue?
msg304915 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2017-10-24 15:10
I think this is very minor but if you two can agree that the code is right I think it's a nice little improvement, and I like that that particular test's usefulness is restored.


PS. Long-term we should really build error recovery into our antiquated parser and report as many errors as we can, without cascading. But that's probably a Python 4 project.
msg305045 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-26 11:31
Is it correct that the parameter can be annotated in the function body?

def f(x):
    x: int

Or that the local variable can be annotated after assignment?

def f():
    x = 1
    x: int
msg305086 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2017-10-26 19:39
Those seem things that the type checker should complain about, but I don't think Python should.
msg305089 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2017-10-26 20:40
> Is it correct that the parameter can be annotated in the function body?

I agree with Guido, this is rather a task for type checkers.
msg305091 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-26 21:28
New changeset 8c83c23fa32405aa9212f028d234f4129d105a23 by Serhiy Storchaka (Ivan Levkivskyi) in branch 'master':
bpo-28936: Detect lexically first syntax error first (#4097)
https://github.com/python/cpython/commit/8c83c23fa32405aa9212f028d234f4129d105a23
msg305092 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-26 21:29
Thank you for your patch Ivan.
History
Date User Action Args
2017-10-26 21:29:58serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg305092

stage: patch review -> resolved
2017-10-26 21:28:41serhiy.storchakasetmessages: + msg305091
2017-10-26 20:40:51levkivskyisetmessages: + msg305089
2017-10-26 19:39:11gvanrossumsetmessages: + msg305086
2017-10-26 11:31:51serhiy.storchakasetmessages: + msg305045
2017-10-24 15:10:08gvanrossumsetmessages: + msg304915
2017-10-24 06:45:59serhiy.storchakasetmessages: + msg304864
2017-10-24 06:06:00levkivskyisetmessages: + msg304861
2017-10-23 23:26:08gvanrossumsetnosy: - Jeremy.Hylton
messages: + msg304850
2017-10-23 21:52:33levkivskyisetmessages: + msg304847
2017-10-23 21:47:50levkivskyisetstage: patch review
pull_requests: + pull_request4068
2017-10-23 16:01:16serhiy.storchakasetmessages: + msg304817
2016-12-17 23:32:40levkivskyisetfiles: + syntax-error-order-v2.patch

messages: + msg283523
2016-12-17 22:01:01levkivskyisetmessages: + msg283522
2016-12-16 20:58:02serhiy.storchakasetmessages: + msg283437
2016-12-11 14:15:15levkivskyisetfiles: + syntax-error-order.patch
keywords: + patch
messages: + msg282920
2016-12-11 12:33:10serhiy.storchakacreate