Skip to content

NPE in CompositeASTVisitor#1896

Merged
BoykoAlex merged 3 commits into
mainfrom
NPE-CompositeADTVisitor
May 25, 2026
Merged

NPE in CompositeASTVisitor#1896
BoykoAlex merged 3 commits into
mainfrom
NPE-CompositeADTVisitor

Conversation

@BoykoAlex
Copy link
Copy Markdown
Contributor

No description provided.

@martinlippert
Copy link
Copy Markdown
Member

I still think that the add method should have a @nonnull contract and null checks should happen before calling this method, but this is just a minor detail, so please go ahead with whatever you prefer.

@BoykoAlex
Copy link
Copy Markdown
Contributor Author

I've explored returning Optional<ASTVisitor> for JDT reconciler and code action handlers... I have also noted that semantic token computations use the same CompositeASTVisitor API and would also need to switch to Optional<ASTVisitor> At this point in terms of changes adding a null check in the CompositeASTVisitor seemed like a good solution... Having an Optional return type rather than ASTVisitor seemed better than having null checks and jspecify annotations as it prevents this problem almost entirely. Maybe I'll revisit the Optional solution in conjunctions with @NonNull

@BoykoAlex BoykoAlex force-pushed the NPE-CompositeADTVisitor branch from 43a4523 to f7061be Compare May 25, 2026 18:26
BoykoAlex added 3 commits May 25, 2026 11:26
Signed-off-by: BoykoAlex <alex.boyko@broadcom.com>
Signed-off-by: BoykoAlex <alex.boyko@broadcom.com>
Signed-off-by: BoykoAlex <alex.boyko@broadcom.com>
@BoykoAlex BoykoAlex force-pushed the NPE-CompositeADTVisitor branch from f7061be to 80c6437 Compare May 25, 2026 18:26
@BoykoAlex BoykoAlex merged commit 063f822 into main May 25, 2026
4 of 7 checks passed
@BoykoAlex BoykoAlex deleted the NPE-CompositeADTVisitor branch May 25, 2026 20:35
@martinlippert martinlippert added this to the 5.2.0.RELEASE milestone May 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants