fix: Cast null to 0 in arithmetic expressions#4209
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughArithmetic expression functions now coerce ChangesArithmetic Functions Null-Handling Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
TomasEng
added
squad/data
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/features/expressions/expression-functions.ts (1)
1035-1040: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winUpdate return type to reflect that
nullis never returned.The function signature declares a return type of
number | null, but with the new implementation on line 1039 that coercesnulloperands to0usinga || 0, b || 0, the function now always returnsnumberand never returnsnull. This type inconsistency may mislead callers about the actual behavior.♻️ Update the return type
function applyNullableBinaryOperation( operation: (a: number, b: number) => number, [a, b]: [number | null, number | null], -): number | null { +): number { return operation(a || 0, b || 0); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/expressions/expression-functions.ts` around lines 1035 - 1040, The function applyNullableBinaryOperation currently declares a return type of number | null but its implementation coerces null operands to 0 (using a || 0, b || 0) so it never returns null; update the function signature return type to number (i.e., remove | null) to match actual behavior and keep callers' types accurate, referencing applyNullableBinaryOperation to locate and change the signature accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/features/expressions/expression-functions.ts`:
- Around line 1035-1040: The function applyNullableBinaryOperation currently
declares a return type of number | null but its implementation coerces null
operands to 0 (using a || 0, b || 0) so it never returns null; update the
function signature return type to number (i.e., remove | null) to match actual
behavior and keep callers' types accurate, referencing
applyNullableBinaryOperation to locate and change the signature accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f466d98c-d203-4105-a9a9-287506958c90
📒 Files selected for processing (5)
src/features/expressions/expression-functions.tssrc/features/expressions/shared-tests/functions/divide/divide.jsonsrc/features/expressions/shared-tests/functions/minus/minus.jsonsrc/features/expressions/shared-tests/functions/multiply/multiply.jsonsrc/features/expressions/shared-tests/functions/plus/plus.json
Good catch! I have fixed this now. |
|
Tip For best results, initiate chat on the files or code changes.
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
|
1 participant
Description
This pull request changes the bahaviour of the arithmetic functions in our expression language when
nullis given as an argument. Instead of always returningnullwhen one of the arguments isnull, the functions will castnullto the number0.Here is the corresponding fix in backend: Altinn/app-lib-dotnet#1751
Related Issue(s)
Verification/QA
kind/*andbackport*label to this PR for proper release notes groupingSummary by CodeRabbit