interpreter: Introduce StringHolder #9207
Conversation
|
This pull request introduces 1 alert when merging ff38350 into d67850b - view on LGTM.com new alerts:
|
|
This pull request introduces 2 alerts when merging 77fcf43 into d67850b - view on LGTM.com new alerts:
|
Codecov Report
@@ Coverage Diff @@
## master #9207 +/- ##
==========================================
+ Coverage 66.88% 66.91% +0.03%
==========================================
Files 386 388 +2
Lines 85171 85199 +28
Branches 17559 17497 -62
==========================================
+ Hits 56966 57015 +49
- Misses 23431 23452 +21
+ Partials 4774 4732 -42
Continue to review full report at Codecov.
|
mensinda
force-pushed
the
strHolder
branch
2 times, most recently
from
3735e0a
to
802c4a2
Compare
|
This pull request fixes 2 alerts when merging 802c4a2 into 1dbb6d6 - view on LGTM.com fixed alerts:
|
I don't see a FeatureNew for this. |
That's because there is no way to check for this after the refactoring. |
|
Nevermind, I forgot that the |
|
This pull request fixes 2 alerts when merging 47cc82d into f291b63 - view on LGTM.com fixed alerts:
|
|
ping |
|
⚔️ conflicts. |
Another commit in my quest to rid InterpreterBase from all higher level object processing logic. Additionally, there is a a logic change here, since `str.join` now uses varargs and can now accept more than one argument (and supports list flattening).
|
This pull request fixes 2 alerts when merging 829ee89 into 64c267c - view on LGTM.com fixed alerts:
|
| # Implement some basic FeatureNew check on the AST level | ||
| assert isinstance(self.current_node, MethodNode) | ||
| n_args = self.current_node.args.arguments | ||
| if len(n_args) != 1 or not isinstance(n_args[0], ArrayNode) or not all(isinstance(x, StringNode) for x in n_args[0].args.arguments): |
There was a problem hiding this comment.
I've just accidentally created a case where this is triggering:
Apparently this is an IndexNode instead of a StringNode.
There was a problem hiding this comment.
Since this is in we need an issue we can attach to the 0.60 milestone so we don't accidentally release with this.
There was a problem hiding this comment.
@mensinda Thoughts on fixing this?
There was a problem hiding this comment.
Oh... This looks like it's going to be a huge pain. The only real way of fixing I can think of is to call evaluate_statement again on the nodes to check the type. But then this will break: ', '.join([run_command('./must_only_be_called_once.sh').stdout()]).
I really don't know how to properly implement this check then... Would it be ok to completely remove it or has someone a better idea?
There was a problem hiding this comment.
Since we are right on top of rc1 and no one has proposed a better idea, let's remove it. #9382
Another commit in my quest to rid InterpreterBase from all higher
level object processing logic.
Additionally, there is a a logic change here, since
str.joinnowuses varargs and can now accept more than one argument (and supports
list flattening).