Fix a crash when a pylint must display unicode raising a UnicodeEncodeError #9732
Conversation
Pierre-Sassoulas
added
Work in progress
Crash π₯
|
We should probably aim to keep the |
|
|
| @@ -0,0 +1 @@ | |||
| comparison-of-constants:3:7:3:37::"Comparison between constants: 'π == ' has a constant value":HIGH | |||
There was a problem hiding this comment.
| comparison-of-constants:3:7:3:37::"Comparison between constants: 'π == ' has a constant value":HIGH | |
| comparison-of-constants:3:7:3:37::"Comparison between constants: '"\U00010000" == "\ud800\udc00"' has a constant value":HIGH |
Imo we should aim for this, but I have an intuition that the ast module internal might be involved.
There was a problem hiding this comment.
Yes, but I don't think we can. self.out is utf-8 encoded so we always need to encode/escape to utf-8.
There was a problem hiding this comment.
Had to be fixed in comparison-of-constant I think 6584761
Pierre-Sassoulas
force-pushed
the
fix-unicode-crash
branch
from
6c12004 to
96592c9
Compare
This comment has been minimized.
This comment has been minimized.
DanielNoord
added
the
Needs take over ποΈ
Pierre-Sassoulas
force-pushed
the
fix-unicode-crash
branch
from
96592c9 to
49bf1a7
Compare
This comment has been minimized.
This comment has been minimized.
Pierre-Sassoulas
force-pushed
the
fix-unicode-crash
branch
from
49bf1a7 to
6584761
Compare
Pierre-Sassoulas
added
Needs review π
| comparison-of-constants:3:6:3:12::"Comparison between constants: '2 == 2' has a constant value":HIGH | ||
| comparison-of-constants:6:6:6:11::"Comparison between constants: '2 > 2' has a constant value":HIGH | ||
| comparison-of-constants:16:3:16:15::"Comparison between constants: 'True == True' has a constant value":HIGH | ||
| comparison-of-constants:3:6:3:12::"Comparison between constants: ""2 == 2"" has a constant value":HIGH |
There was a problem hiding this comment.
It appears as double "" but it's due to the "csv" encoding (the string start with ") and it's actually fine.
| """This does not crash in the functional tests, but it did when called directly.""" | ||
|
|
||
| assert "\U00010000" == "\ud800\udc00" # [comparison-of-constants] |
There was a problem hiding this comment.
Didn't manage to reproduce the crash automatically. Might be due to some unicode's magic already present in the functional test.
|
π€ According to the primer, this change has no effect on the checked open source code. π€π This comment was generated for commit 6584761 |
Pierre-Sassoulas
removed
the
Needs review π
Type of Changes
Description
Fixes for #8736,
this is probably not the right fix, but it's a fix. Hoping for a surrogates/unicode expert to chime in with the right approach π !after becoming a surrogate expert myself. This prevent the crash in the general case (for all checkers even badly coded plugin) and make the message output for comparison of constant closer to the content of the code by using the AST instead of recalculating. We can backport half of this. Review is better commit by commit as there is two parts.