v2: дефолтные значения в конструкторе сценария +тест #1482
Conversation
WalkthroughThe pull request introduces enhancements to the script engine's handling of constructor parameters, focusing on improving parameter management during instance creation. The changes modify the Changes
Sequence DiagramsequenceDiagram
participant ScriptEngine
participant UserScriptContextInstance
participant ScriptMethod
ScriptEngine->>UserScriptContextInstance: Create Instance
UserScriptContextInstance-->>ScriptEngine: Check Constructor Parameters
alt Parameters Insufficient
UserScriptContextInstance->>UserScriptContextInstance: Create Extended Parameters Array
UserScriptContextInstance->>ScriptMethod: Call with Filled Parameters
else Parameters Sufficient
UserScriptContextInstance->>ScriptMethod: Call with Existing Parameters
end
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/native-lib/test-native-use.os (1)
90-94: Add an assertion to validate the constructor’s result
Currently, this test only instantiates the object without checking its state or behavior. Consider adding assert statements to confirm that the object is correctly initialized.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/ScriptEngine/Machine/Contexts/UserScriptContextInstance.cs(2 hunks)tests/native-lib/test-native-use.os(2 hunks)tests/native-lib/Классы/ПараметрыКонструктораПоУмолчанию.os(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tests/native-lib/Классы/ПараметрыКонструктораПоУмолчанию.os
🔇 Additional comments (3)
tests/native-lib/test-native-use.os (1)
17-18: New test addition looks good!
The new test name is consistent and helps expand coverage for default constructor parameters.
src/ScriptEngine/Machine/Contexts/UserScriptContextInstance.cs (2)
68-68: Good switch to using .Length
Using ConstructorParams.Length avoids overhead and is more idiomatic than Count().
86-97: Constructor default parameter handling is well-structured
This logic appropriately copies constructor arguments and assigns default values to any missing parameters.
To validate usage across the codebase, consider searching for calls to OnInstanceCreation and confirming that default parameters are managed as expected:
✅ Verification successful
Let me verify the usage of default parameters by checking the method information and parameter handling.
Let me check one more aspect to verify the parameter handling implementation.
Default parameter handling implementation is correctly verified
The code's handling of default parameters is well-implemented and verified through the following findings:
-
The
UserScriptContextInstancecorrectly:- Checks for required parameters count vs provided parameters
- Validates that any skipped parameters have default values
- Properly copies constructor parameters and fills missing ones with their default values
- Uses proper error handling for parameter mismatches
-
The implementation is supported by a robust default value system throughout the codebase:
MethodSignaturetracks default values throughHasDefaultValueandDefaultValueIndexMachineInstancehandles default value application during execution- Parameter validation ensures all required parameters are provided
The original review comment's approval of the constructor default parameter handling is accurate and supported by the implementation details found in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for usage of OnInstanceCreation and verify calls with default parameter usage.
rg -A 5 'OnInstanceCreation'
Length of output: 6180
Script:
#!/bin/bash
# Search for GetMethodInfo and related parameter handling
ast-grep --pattern 'GetMethodInfo($_)'
# Also check for any other relevant parameter handling in UserScriptContextInstance
rg -A 10 'parameters\[' src/ScriptEngine/Machine/Contexts/UserScriptContextInstance.cs
Length of output: 918
Script:
#!/bin/bash
# Check the implementation of GetMethodInfo and DefaultValue handling
rg -A 5 'DefaultValue' src/ScriptEngine/Machine/
Length of output: 11623
Исправляет падение в native.
Summary by CodeRabbit
New Features
Tests