chore: upgrade script for next + rewriting RuleConfiguration to C# + expressions #17097
Conversation
… data processing rules
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ 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 |
github-actions
bot
added
skip-releasenotes
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| { | ||
| Span<char> buffer = stackalloc char[len]; | ||
| var indexedPath = formDataWrapper.AddIndexToPath(path, rowIndexes, buffer); | ||
| return indexedPath.IsEmpty ? false : formDataWrapper.Set(indexedPath, value); |
Check notice
Code scanning / CodeQL
Unnecessarily complex Boolean expression Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 18 hours ago
To fix the unnecessarily complex boolean expression on line 155, replace the ternary statement indexedPath.IsEmpty ? false : formDataWrapper.Set(indexedPath, value) with the simplified boolean logic !indexedPath.IsEmpty && formDataWrapper.Set(indexedPath, value). This preserves the logic: the result is false if indexedPath.IsEmpty is true, and otherwise the result is whatever is returned by formDataWrapper.Set(indexedPath, value). Only a single line in method Set within the extension class FormDataWrapperExtensions in file src/App/backend/src/Altinn.App.Core/Internal/Data/IFormDataWrapper.cs needs updating.
| @@ -152,7 +152,7 @@ | ||
| { | ||
| Span<char> buffer = stackalloc char[len]; | ||
| var indexedPath = formDataWrapper.AddIndexToPath(path, rowIndexes, buffer); | ||
| return indexedPath.IsEmpty ? false : formDataWrapper.Set(indexedPath, value); | ||
| return !indexedPath.IsEmpty && formDataWrapper.Set(indexedPath, value); | ||
| } | ||
|
|
||
| char[] pool = System.Buffers.ArrayPool<char>.Shared.Rent(len); |
| try | ||
| { | ||
| var indexedPath = formDataWrapper.AddIndexToPath(path, rowIndexes, pool); | ||
| return indexedPath.IsEmpty ? false : formDataWrapper.Set(indexedPath, value); |
Check notice
Code scanning / CodeQL
Unnecessarily complex Boolean expression Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 18 hours ago
To fix the unnecessarily complex Boolean expression in line 162, replace the ternary conditional indexedPath.IsEmpty ? false : formDataWrapper.Set(indexedPath, value) with the logical expression !indexedPath.IsEmpty && formDataWrapper.Set(indexedPath, value). This removes the ternary operator where a logical operator suffices, making the code simpler and more idiomatic in C#. Only line 162 of file src/App/backend/src/Altinn.App.Core/Internal/Data/IFormDataWrapper.cs needs to be changed. No new imports or methods are needed.
| @@ -159,7 +159,7 @@ | ||
| try | ||
| { | ||
| var indexedPath = formDataWrapper.AddIndexToPath(path, rowIndexes, pool); | ||
| return indexedPath.IsEmpty ? false : formDataWrapper.Set(indexedPath, value); | ||
| return !indexedPath.IsEmpty && formDataWrapper.Set(indexedPath, value); | ||
| } | ||
| finally | ||
| { |
| catch | ||
| { | ||
| return false; | ||
| } |
Check notice
Code scanning / CodeQL
Generic catch clause Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 18 hours ago
To fix the problem, the generic catch clause should be replaced by one or more specific catch clauses for exceptions that may reasonably occur when setting property values via reflection. For property assignment (prop.SetValue()), exceptions typically encountered include ArgumentException, MethodAccessException, TargetException, and InvalidCastException. The same applies for setting a value in a list (list[...] = ...). The best fix is therefore to replace both generic catch blocks (lines 187 and 219) with explicit catches for these specific exception types. No additional imports are required as these exceptions are present in the base class library.
All changes are confined to the blocks containing the catch statements in the method.
| @@ -184,10 +184,22 @@ | ||
| prop.SetValue(currentModel, convertedValue); | ||
| return true; | ||
| } | ||
| catch | ||
| catch (ArgumentException) | ||
| { | ||
| return false; | ||
| } | ||
| catch (InvalidCastException) | ||
| { | ||
| return false; | ||
| } | ||
| catch (MethodAccessException) | ||
| { | ||
| return false; | ||
| } | ||
| catch (System.Reflection.TargetException) | ||
| { | ||
| return false; | ||
| } | ||
| } | ||
| else | ||
| { | ||
| @@ -215,10 +224,22 @@ | ||
| list[groupIndex.Value] = convertedValue; | ||
| return true; | ||
| } | ||
| catch | ||
| catch (ArgumentException) | ||
| { | ||
| return false; | ||
| } | ||
| catch (InvalidCastException) | ||
| { | ||
| return false; | ||
| } | ||
| catch (MethodAccessException) | ||
| { | ||
| return false; | ||
| } | ||
| catch (System.Reflection.TargetException) | ||
| { | ||
| return false; | ||
| } | ||
| } | ||
| } | ||
|
|
| catch | ||
| { | ||
| return false; | ||
| } |
Check notice
Code scanning / CodeQL
Generic catch clause Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 18 hours ago
The best way to fix this issue is to replace the generic bare catch clauses on lines 187-190 and 218-221 with catch clauses for only the exception types that are reasonably expected during property assignment or list element assignment via reflection. For reflection property assignment (prop.SetValue()), the relevant exceptions per MSDN are: ArgumentException, TargetException, TargetInvocationException, MethodAccessException. For list assignment (list[groupIndex.Value] = convertedValue;), ArgumentOutOfRangeException and InvalidCastException may be relevant (plus general assignment errors).
We'll change each catch block to list these individual exception types, and for each, return false. No imports are required as these are all in the System namespace.
Edits are only needed to the try/catch blocks around prop.SetValue(currentModel, convertedValue); and list[groupIndex.Value] = convertedValue;, i.e. lines 182-190 and 213-221.
| @@ -184,10 +184,22 @@ | ||
| prop.SetValue(currentModel, convertedValue); | ||
| return true; | ||
| } | ||
| catch | ||
| catch (ArgumentException) | ||
| { | ||
| return false; | ||
| } | ||
| catch (TargetException) | ||
| { | ||
| return false; | ||
| } | ||
| catch (TargetInvocationException) | ||
| { | ||
| return false; | ||
| } | ||
| catch (MethodAccessException) | ||
| { | ||
| return false; | ||
| } | ||
| } | ||
| else | ||
| { | ||
| @@ -215,10 +224,18 @@ | ||
| list[groupIndex.Value] = convertedValue; | ||
| return true; | ||
| } | ||
| catch | ||
| catch (ArgumentOutOfRangeException) | ||
| { | ||
| return false; | ||
| } | ||
| catch (InvalidCastException) | ||
| { | ||
| return false; | ||
| } | ||
| catch (ArgumentException) | ||
| { | ||
| return false; | ||
| } | ||
| } | ||
| } | ||
|
|
| catch | ||
| { | ||
| return null; | ||
| } |
Check notice
Code scanning / CodeQL
Generic catch clause Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 18 hours ago
To fix the problem, replace the generic catch block with explicit catch clauses that handle only the exceptions that are expected during type conversion: FormatException, InvalidCastException, and OverflowException. This ensures that only conversion-related errors are quietly handled by returning null, while other, unexpected exceptions will propagate and can be diagnosed appropriately. No new methods or complex changes are needed—just replace the generic catch with specific ones within the existing method in ReflectionFormDataWrapper, in the file src/App/backend/src/Altinn.App.Core/Internal/Data/ReflectionFormDataWrapper.cs.
| @@ -336,10 +336,18 @@ | ||
| // Try using Convert.ChangeType as a fallback | ||
| return Convert.ChangeType(value, targetType, CultureInfo.InvariantCulture); | ||
| } | ||
| catch | ||
| catch (FormatException) | ||
| { | ||
| return null; | ||
| } | ||
| catch (InvalidCastException) | ||
| { | ||
| return null; | ||
| } | ||
| catch (OverflowException) | ||
| { | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| private static object? GetElementAt(System.Collections.IEnumerable enumerable, int index) |
1 participant
Description
🚧
Verification