issue_484: Null pointer passed to DPLParserCatalystVisitor - #485
issue_484: Null pointer passed to DPLParserCatalystVisitor#485Abigael-JT wants to merge 10 commits into
Conversation
Abigael-JT
commented
Jan 24, 2025
Abigael-JT
commented
- throw an exception when DPLParserCatalystContext is missing instead of passing a Null pointer to DPLParserCatalystVisitor
- return NullNode instead of null
Abigael-JT
changed the title
elliVM
left a comment
elliVM
left a comment
There was a problem hiding this comment.
Change looks good. Some changes requested to improve code quality
|
Removed DPLParserCatalystContext null check, added ifInfoEnabled guard to logger messages with method call and removed commented code |
|
Found an issue in testing when |
51-code
left a comment
51-code
left a comment
There was a problem hiding this comment.
Seems good. I don't think there is a solution that can avoid instanceof calls for this.
eemhu
left a comment
eemhu
left a comment
There was a problem hiding this comment.
Would it be a good alternative to add isStub() or similar method to the Node interface, and NullNode would return true, otherwise false? This way it could be used instead of using instanceof checks?
|
added isStub() method to Node interface and rebased |
|
Rebased |
|
Rebased |
…emoved commented code
… is NullNode before casting to ColumnNode
… have Nullnode return true
| final Node logicalNode = visit(ctx.logicalStatement(0)); | ||
| if (directoryNode.isStub()) { | ||
| LOGGER.info("Directory statement node was a NullNode"); | ||
| rv = directoryNode; |
There was a problem hiding this comment.
why is stub set as return value?
There was a problem hiding this comment.
if directoryNode is stub and logical node isn't wouldn't it make more sense to return the logical node and not the stub
6 participants
