fix: LexicalPreservingPrinter can't print PrimitiveType#4817
fix: LexicalPreservingPrinter can't print PrimitiveType#4817seokjun7410 wants to merge 1 commit intojavaparser:masterfrom
Conversation
|
Your PR contains two separate changes. One is an interesting improvement to the existing code, and the other tries to fix a bug. It would be more practical to separate the two types of changes, as I don't think the bug fix is very relevant. My observation is that when propagating the change in the list of members (here the fields) of the class declaration, we try to modify the parent node of the list of fields (more precisely in the Node.setParentNode(...) method). According to my debugger, this is when the data field of the node to be added, which is supposed to contain the text form of the field declaration, is modified. Before the assignment ‘parentNode = newParentNode;’, my debugger indicates that the data field (of the node to be added) is null, and after the assignment, the data field is initialised, which subsequently disrupts lexical preservation. I don't know if this is a bug in the debugger or if it's a side effect related to changing the parent node of the node to be added. Perhaps you could confirm this analysis in your work environment. |
|
Thank you for the insightful review. I'll debug the |
2 participants
fixes. #4781
Issue Analysis
When adding primitive type fields via LexicalPreservingPrinter, keywords like
int,booleanget omitted:Root Cause
Dynamically created
PrimitiveTypenodes lack proper NodeText initialization. The existingprettyPrintingTextNode()relied onnode.toString()which returns empty strings for uninitialized nodes.My Solution: CSM-Based Architecture Approach
Removed Special Case Handling: Eliminated the hardcoded PrimitiveType switch statement in
prettyPrintingTextNode()(38 lines removed).Enhanced
getOrCreateNodeText(): Added proper handling for dynamically created PrimitiveType nodes:Made Primitive enum implement Stringable: For consistency with JavaParser's design patterns.
nstead of special-casing PrimitiveType, I leveraged JavaParser's existing CSM (ConcreteSyntaxModel) infrastructure which already knows how to render primitive types correctly.
Question
Does this CSM-based approach align well with JavaParser's lexical preservation architecture? I chose this over the simpler hardcoded string approach because it maintains consistency with how other AST nodes are handled.