-
Notifications
You must be signed in to change notification settings - Fork 121
Fix NullPointerException in UselessTernaryOperatorDiagnostic for malformed ternary operators #3728
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Comment |
Add null check for ctx.children before accessing its size to prevent NPE when parsing malformed ternary operators Co-authored-by: nixel2007 <[email protected]>
...a/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java
Outdated
Show resolved
Hide resolved
...m/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnosticTest.java
Outdated
Show resolved
Hide resolved
- Replace ctx.children null check with ctx.getChildCount() method - Rewrite test to properly get diagnostics list and assert with assertj Co-authored-by: nixel2007 <[email protected]>
...m/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnosticTest.java
Show resolved
Hide resolved
- Import assertThatCode from assertj - Replace isEmpty() assertion with doesNotThrowAnyException() - Specifically test that getDiagnostics call doesn't throw NPE Co-authored-by: nixel2007 <[email protected]>
|
/buildJar |
|
✅ Собраны JAR-файлы для этого PR по команде Артефакт: 5010181738 Файлы внутри:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a NullPointerException that occurs in the UselessTernaryOperatorDiagnostic when parsing malformed ternary operators where the third expression is missing or malformed (e.g., Возврат ?(таблица.Количество() = 1, Неопределено, );).
Key changes:
- Replaced unsafe direct access to
ctx.children.size()with the safectx.getChildCount()method - Added a test case to verify the fix handles malformed ternary operators without throwing exceptions
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java |
Changed line 120 to use getChildCount() instead of direct children.size() access to safely handle null children |
src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnosticTest.java |
Added test method to verify malformed ternary operators don't cause NullPointerException |
src/test/resources/diagnostics/UselessTernaryOperatorDiagnosticMalformed.bsl |
Added test resource file with malformed ternary operator example |
The code changes are well-implemented and follow the project's conventions. The fix correctly addresses the root cause by using ANTLR's built-in getChildCount() method which safely handles null children. The test follows the established pattern in the codebase for testing exception-free behavior using assertThatCode().doesNotThrowAnyException().
Fix NullPointerException in UselessTernaryOperatorDiagnostic
The issue occurs when parsing malformed ternary operators like
Возврат ?(таблица.Количество() = 1, Неопределено, );where the third expression is missing or malformed.Plan:
Changes Made:
UselessTernaryOperatorDiagnostic.java: Usectx.getChildCount() == 1instead of manual null check in thegetBooleanTokenmethod (line 120)UselessTernaryOperatorDiagnosticMalformed.bslwith malformed ternary operator exampletestMalformedTernaryOperatorDoesNotThrowNPE()to useassertThatCode(() -> getDiagnostics(documentContext)).doesNotThrowAnyException()pattern to specifically test that getting diagnostics doesn't throwRoot Cause:
The parser can create an
ExpressionContextwith nullchildrenwhen parsing malformed code. The original code tried to callctx.children.size()without checking ifchildrenwas null first, causing a NullPointerException.Fix:
Use the safe
getChildCount()method which handles null children internally:ctx.getChildCount() == 1Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.