List Instantiation #17
Conversation
tfij
force-pushed
the
listInstantiation
branch
from
51ffb3b
to
18607e1
Compare
| @@ -65,7 +72,7 @@ Rule ZeroArgumentMethodCall() { | |||
|
|
|||
| Rule FieldAccess() { | |||
| return FirstOf( | |||
| Sequence("[ ", AdditiveExpression(), "] ", push(nodeFactory.mapAccess(pop(1), pop()))), | |||
| Sequence("[ ", Expression(), "] ", push(nodeFactory.mapAccess(pop(1), pop()))), | |||
There was a problem hiding this comment.
It'a bug? Can we hava a simple test for it ?
There was a problem hiding this comment.
yes, and I add some test
| return Sequence( | ||
| "[ ", | ||
| "] ", | ||
| push(nodeFactory.emptyListInstantiation())); |
There was a problem hiding this comment.
I suggest to put this push at the beginning of Args() (the same way I did in Declarations()) so you don't need EmptyList()
There was a problem hiding this comment.
The same way ZeroArgsCall() may be removed (not sure)
| private final Optional<ArgumentsListExpressionNode> listElements; | ||
|
|
||
| public ListInstantiationExpressionNode(ArgumentsListExpressionNode listElements) { | ||
| this.listElements = Optional.of(listElements); |
| return parsingResult.resultValue.getValue(embeddedEvalContext); | ||
| } | ||
|
|
||
| private String additionalErrorMsq(ParsingResult<OpelNode> parsingResult) { |
There was a problem hiding this comment.
is this format of error available in parse - eval flow ?
There was a problem hiding this comment.
when expression has error return parsingResult.resultValue.getValue(embeddedEvalContext) throw npe. This as additional error message of OpelExeption.
| boolean hasAdditionalMeg = parsingResult.parseErrors.stream() | ||
| .map(ParseError::getErrorMessage) | ||
| .anyMatch(it -> it != null); | ||
| if (hasAdditionalMeg) { |
There was a problem hiding this comment.
Not all errors have errorMessage but we always have position where parsing error occured. Maybe reporting position is also a good idea?
There was a problem hiding this comment.
maybe, this is better then npe from return parsingResult.resultValue.getValue(embeddedEvalContext);
| return parsingResult.resultValue.getValue(embeddedEvalContext); | ||
| } | ||
|
|
||
| private String additionalErrorMsq(ParsingResult<OpelNode> parsingResult) { | ||
| boolean hasAdditionalMeg = parsingResult.parseErrors.stream() |
tfij
force-pushed
the
listInstantiation
branch
from
6e9f0d9
to
2b20996
Compare
| ); | ||
| } | ||
|
|
||
| Rule Object() { | ||
| return FirstOf(FunctionCall(),StringLiteral(), NamedValue(), ListInstantiation(), Sequence("( ", Expression(), ") ")); |
There was a problem hiding this comment.
One change here: we can access field on string literal, but this doesn't have to be detected by parsing engine, it can be detected later
| this.listElements = listElements; | ||
| } | ||
|
|
||
| public ListInstantiationExpressionNode() { |
| return Sequence(Expression(), | ||
| ZeroOrMore(", ", Expression(), push(nodeFactory.argumentsList(pop(), getFunctionArguments())))); | ||
| return Sequence( | ||
| push(nodeFactory.emptyArgumentsList()), |
|
|
||
| class OpelEngineListIntegrationSpec extends Specification { | ||
| @Unroll | ||
| def 'should instantiation list in #input'() { |
There was a problem hiding this comment.
should instantiate / should create ?
No description provided.