Feat/expr in var origin #45
Conversation
WalkthroughThis PR enhances Numscript by extending its grammar to support function calls within value expressions and broadening variable origin definitions. It refactors internal analysis and hover logic to use generalized expression checking, introduces a new mid-script function call feature flag in the CLI, and improves balance management with new types and methods. Parser components and AST definitions have been updated to handle the new application context and expression types. Additional tests validate balance operations, mid-script function calls, and error handling for invalid nested meta calls. Changes
Sequence Diagram(s)sequenceDiagram
participant Eval as evaluateExpr
participant Flag as FeatureFlag Checker
participant Handler as handleFnCall
Eval ->> Flag: checkFeatureFlag("ExperimentalMidScriptFunctionCall")
alt Feature Enabled
Eval ->> Handler: Invoke handleFnCall(fnCall)
Handler -->> Eval: Return evaluated value
else Feature Disabled
Eval -->> Eval: Return error
end
sequenceDiagram
participant Query as runBalancesQuery
participant Cache as CachedBalances
participant Store as StaticStore
Query ->> Cache: filterQuery(currentQuery)
Cache -->> Query: filteredQuery
Query ->> Store: GetBalances(filteredQuery)
Store -->> Query: queriedBalances
Query ->> Cache: mergeBalance(queriedBalances)
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code Definitions (1)internal/interpreter/interpreter_test.go (3)
🔇 Additional comments (8)
✨ Finishing Touches
🪧 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 (
|
ascandone
force-pushed
the
feat/expr-in-var-origin
branch
4 times, most recently
from
9f5a254 to
9101a5a
Compare
ascandone
force-pushed
the
feat/expr-in-var-origin
branch
from
04c66df to
50d8dd2
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #45 +/- ##
==========================================
+ Coverage 62.13% 62.25% +0.11%
==========================================
Files 32 33 +1
Lines 7113 7235 +122
==========================================
+ Hits 4420 4504 +84
- Misses 2473 2505 +32
- Partials 220 226 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
internal/cmd/run.go (1)
219-219: Fix typo in feature flag descriptionThere's a small typo in the feature flag description.
- cmd.Flags().BoolVar(&midScriptFunctionCallFeatureFlag, interpreter.ExperimentalMidScriptFunctionCall, false, "allows to use function call as expression, and to use any expression when definining variables") + cmd.Flags().BoolVar(&midScriptFunctionCallFeatureFlag, interpreter.ExperimentalMidScriptFunctionCall, false, "allows to use function call as expression, and to use any expression when defining variables")🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 219-219: internal/cmd/run.go#L219
Added line #L219 was not covered by testsnumscript_test.go (1)
420-458: Well-structured test for mid-script function calls.This test effectively verifies the new functionality that allows
balance()to be called mid-script when the feature flag is enabled. It correctly tests that the balance is properly captured in the transaction metadata.One suggestion - consider adding a test case that verifies the error condition when
meta()is called within another expression, as this was mentioned as a limitation in the PR objectives.internal/parser/antlrParser/numscript_parser.go (1)
825-870: Add unit-test coverage for your new parse context.The newly introduced
ApplicationContextand its methods are functionally correct for capturing function calls as expressions. However, static analysis indicates that lines [839-840], [853-854], [859-862], and [865-868] lack test coverage. It’s advisable to add or expand tests to ensure these code paths are exercised (e.g., parsing expressions containing function calls) to improve overall confidence in the parser.Would you like me to propose a sample test that parses a function call expression, ensuring coverage of these methods?
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 839-840: internal/parser/antlrParser/numscript_parser.go#L839-L840
Added lines #L839 - L840 were not covered by tests
[warning] 853-854: internal/parser/antlrParser/numscript_parser.go#L853-L854
Added lines #L853 - L854 were not covered by tests
[warning] 859-862: internal/parser/antlrParser/numscript_parser.go#L859-L862
Added lines #L859 - L862 were not covered by tests
[warning] 865-868: internal/parser/antlrParser/numscript_parser.go#L865-L868
Added lines #L865 - L868 were not covered by testsinternal/interpreter/interpreter.go (1)
241-248: Potentially unusedvarOriginPositionfield.The code sets
st.varOriginPosition = trueand later resets it to false, but there’s no observable usage. Consider removing the field or documenting how it should be used to avoid confusion.Also applies to: 283-283
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
go.modis excluded by!**/*.modinternal/parser/__snapshots__/parser_fault_tolerance_test.snapis excluded by!**/*.snap,!**/*.snapinternal/parser/__snapshots__/parser_test.snapis excluded by!**/*.snap,!**/*.snap
📒 Files selected for processing (19)
Numscript.g4(1 hunks)internal/analysis/check.go(3 hunks)internal/analysis/hover.go(2 hunks)internal/cmd/run.go(3 hunks)internal/interpreter/balances.go(1 hunks)internal/interpreter/batch_balances_query.go(1 hunks)internal/interpreter/evaluate_expr.go(1 hunks)internal/interpreter/interpreter.go(11 hunks)internal/interpreter/interpreter_error.go(1 hunks)internal/interpreter/interpreter_test.go(3 hunks)internal/interpreter/value.go(1 hunks)internal/parser/antlrParser/Numscript.interp(1 hunks)internal/parser/antlrParser/numscript_base_listener.go(1 hunks)internal/parser/antlrParser/numscript_listener.go(2 hunks)internal/parser/antlrParser/numscript_parser.go(54 hunks)internal/parser/ast.go(2 hunks)internal/parser/parser.go(2 hunks)internal/parser/parser_test.go(1 hunks)numscript_test.go(2 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
internal/parser/antlrParser/numscript_base_listener.go
[warning] 54-54: internal/parser/antlrParser/numscript_base_listener.go#L54
Added line #L54 was not covered by tests
[warning] 57-57: internal/parser/antlrParser/numscript_base_listener.go#L57
Added line #L57 was not covered by tests
internal/cmd/run.go
[warning] 133-135: internal/cmd/run.go#L133-L135
Added lines #L133 - L135 were not covered by tests
[warning] 215-217: internal/cmd/run.go#L215-L217
Added lines #L215 - L217 were not covered by tests
[warning] 219-219: internal/cmd/run.go#L219
Added line #L219 was not covered by tests
internal/interpreter/value.go
[warning] 218-222: internal/interpreter/value.go#L218-L222
Added lines #L218 - L222 were not covered by tests
internal/interpreter/balances.go
[warning] 50-51: internal/interpreter/balances.go#L50-L51
Added lines #L50 - L51 were not covered by tests
internal/parser/ast.go
[warning] 21-21: internal/parser/ast.go#L21
Added line #L21 was not covered by tests
internal/parser/antlrParser/numscript_parser.go
[warning] 839-840: internal/parser/antlrParser/numscript_parser.go#L839-L840
Added lines #L839 - L840 were not covered by tests
[warning] 853-854: internal/parser/antlrParser/numscript_parser.go#L853-L854
Added lines #L853 - L854 were not covered by tests
[warning] 859-862: internal/parser/antlrParser/numscript_parser.go#L859-L862
Added lines #L859 - L862 were not covered by tests
[warning] 865-868: internal/parser/antlrParser/numscript_parser.go#L865-L868
Added lines #L865 - L868 were not covered by tests
[warning] 1411-1411: internal/parser/antlrParser/numscript_parser.go#L1411
Added line #L1411 was not covered by tests
[warning] 1441-1441: internal/parser/antlrParser/numscript_parser.go#L1441
Added line #L1441 was not covered by tests
[warning] 5497-5497: internal/parser/antlrParser/numscript_parser.go#L5497
Added line #L5497 was not covered by tests
[warning] 5500-5500: internal/parser/antlrParser/numscript_parser.go#L5500
Added line #L5500 was not covered by tests
internal/interpreter/interpreter_error.go
[warning] 228-229: internal/interpreter/interpreter_error.go#L228-L229
Added lines #L228 - L229 were not covered by tests
🔇 Additional comments (43)
internal/parser/antlrParser/Numscript.interp (1)
112-112: Auto-generated file change.This is an expected change in the ANTLR-generated parser definition, updating the state transition value from 252 to 253 to accommodate the grammar changes for function calls in value expressions.
internal/parser/antlrParser/numscript_listener.go (2)
25-27: Added method for handling application production entry.This addition to the listener interface is necessary to support the new grammar rule for function calls in expression contexts.
154-156: Added method for handling application production exit.This complements the EnterApplication method, ensuring proper listener integration with the modified grammar.
internal/parser/parser_test.go (2)
441-449: Good test for expression in variable origin.This test validates that the new grammar correctly supports using expressions (like numeric literals) in variable origins, which is one of the main objectives of this PR.
451-461: Valuable test for mid-script function calls.This test verifies the ability to use function calls as expressions within other function calls, which is a key enhancement mentioned in the PR objectives. The test demonstrates the nested function call pattern with
balance(@user, EUR/2)as an argument.internal/parser/antlrParser/numscript_base_listener.go (1)
53-57: Implementation aligns with PR objective to support function calls in expressions.The addition of
EnterApplicationandExitApplicationmethods to theBaseNumscriptListenerstruct enables parsing support for application expressions, which is essential for the new feature allowing function calls to be treated as expressions in variable origins.Note that these methods currently have empty implementations as they're meant to be overridden by actual listeners. The static analysis indicates these lines aren't covered by tests. Consider adding tests that exercise this new parsing functionality specifically.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 54-54: internal/parser/antlrParser/numscript_base_listener.go#L54
Added line #L54 was not covered by tests
[warning] 57-57: internal/parser/antlrParser/numscript_base_listener.go#L57
Added line #L57 was not covered by testsinternal/interpreter/evaluate_expr.go (1)
87-96: Well-implemented feature flag check for experimental function call usage.The implementation correctly:
- Handles function calls within expressions
- Enforces the
ExperimentalMidScriptFunctionCallfeature flag when not in a variable origin position- Provides a clean interface to the existing
handleFnCallmethodThis code fulfills the PR objective to allow function calls to be treated as expressions.
internal/interpreter/interpreter_error.go (1)
224-230:✅ Verification successful
Error handling for nested meta function restriction.
The addition of
InvalidNestedMetaerror type aligns with the PR objective to maintain the limitation that themeta()function can only be called at the top level of a variable origin. The error message clearly communicates this restriction to users.The static analysis indicates these lines aren't covered by tests. Consider adding a test case that specifically verifies this error is thrown when attempting to use the meta function in a nested expression.
🏁 Script executed:
#!/bin/bash # Check if there's a test for InvalidNestedMeta error rg -g "*.go" "TestInvalidNestedMeta" --context=5Length of output: 688
Action Required: Confirmed Test Coverage for Nested Meta Function Error Handling
The new
InvalidNestedMetaerror type ininternal/interpreter/interpreter_error.gosuccessfully enforces that themeta()function cannot be nested, and the error message is clear. Additionally, the test caseTestInvalidNestedMetaCallininternal/interpreter/interpreter_test.goverifies that the restriction is enforced properly. No further tests are needed.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 228-229: internal/interpreter/interpreter_error.go#L228-L229
Added lines #L228 - L229 were not covered by testsinternal/analysis/hover.go (2)
78-78: Enhanced hover support for expressions in variable origins.Changed from function call-specific handling to general expression handling for variable origins, which aligns with the PR objective to allow any valid expression in variable origins.
186-187: Added hover support for function calls in expressions.This change ensures that hover functionality works correctly when function calls are used within expressions, completing the editor integration for the new feature.
internal/parser/ast.go (1)
310-310: Broaden the Origin field typeChanging the Origin field type from
*FnCallto*ValueExpris a key change that allows variable origins to be defined with any expression, not just function calls. This aligns with the PR objective of enhancing variable origins.Numscript.g4 (2)
24-25: Enhance grammar to support function calls in value expressionsThis change adds support for function calls to be used as value expressions, which aligns with the implementation in the AST. The semicolon has been removed from the previous line for consistent formatting.
31-31: Enhance variable origin to support any value expressionModifying the
varOriginrule to accept any value expression instead of just function calls is a key part of the enhancement. This change allows for more flexible variable declarations such asnumber $b = 1 + 2.internal/cmd/run.go (2)
32-32: Add feature flag for mid-script function callsAdding a feature flag is appropriate for this experimental feature, aligning with the PR's intent to control this new capability.
133-135: Include the feature flag in runtime configurationThe conditional check correctly adds the feature flag to the
featureFlagsmap when enabled, which will allow the interpreter to recognize and use the mid-script function call feature.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 133-135: internal/cmd/run.go#L133-L135
Added lines #L133 - L135 were not covered by testsinternal/analysis/check.go (3)
165-165: Update check method to use general expression checkingThis change correctly updates the check method to use
checkExpressioninstead of the now-removedcheckVarOriginmethod. This is consistent with treating variable origins as general expressions.
305-319: Add checkFnCall method to determine function return typeThis method properly determines the return type of function calls, which is necessary for type checking variable origins that are function calls. The implementation correctly handles the resolution of built-in functions and arity checking.
384-386: Add function call handling to checkTypeOfThis case handles function calls in the
checkTypeOfmethod, calling the newcheckFnCallmethod to determine the return type. This is consistent with the overall changes to support function calls as expressions.internal/parser/parser.go (3)
116-120: LGTM! Good refactoring to support expressions in variable origins.This refactoring allows variable origins to be defined as any valid expression, not just function calls, which is in line with the PR objectives. The change from using
parseFnCalltoparseValueExpris a good approach to enable more flexible syntax.
126-126: Origin field updated correctly.The field assignment is properly updated to use the new
originvariable of type*ValueExprinstead of the previous function call specific variable.
350-352: Good addition to support function calls as expressions.This new case for handling
ApplicationContextis essential for enabling function calls to be treated as valid expressions within value expressions, as described in the PR objectives.numscript_test.go (2)
170-170: Good use of the constant reference.Using the constant
interpreter.ExperimentalOneofFeatureFlaginstead of a string literal improves code maintainability.
460-530: Good test for interleaved balance batching.This test thoroughly validates the complex scenario of interleaved balance batching with metadata references. It correctly verifies the postings and balance queries that result from the transaction.
The test also properly uses the
ExperimentalMidScriptFunctionCallfeature flag as specified in the PR objectives.internal/interpreter/batch_balances_query.go (2)
59-59: Simplified type handling.Good simplification of the
slices.Containscall by removing the explicit type parameters, making the code cleaner.
65-80: Balance query logic refactored to use new helper methods.The refactoring of the
runBalancesQuerymethod to use the newfilterQueryandmergeBalancemethods fromCachedBalancesimproves code modularity and reusability.internal/interpreter/balances.go (1)
5-29: Well-structured balance access methods.The
fetchAccountBalances,fetchBalance, andhasmethods provide a clean API for accessing account balances with proper handling of missing data.internal/parser/antlrParser/numscript_parser.go (1)
59-106: No issues found with auto-generated code.These lines appear to be part of the ANTLR-generated serialized ATN data structure. Typically, we do not manually modify or add tests for this auto-generated content.
internal/interpreter/interpreter_test.go (7)
156-189: Thorough test coverage for partial retrieval fromStaticStore.These changes effectively verify that queried balances can be selectively fetched, aligning well with the new
GetBalanceslogic. No issues found.
2255-2285: Straightforward verification of a singlebalance(...)usage.This test properly checks that a mid-script balance retrieval succeeds and matches the expected posting outcome. Nicely done.
2286-2315: Ensures multiplebalancereferences remain consistent.While the test name suggests querying the same balance more than once, it still validates that reusing computed balance data works seamlessly. No concerns here.
4045-4072: Validates mid-script balance sending.This test confirms that
send balance(@acc, USD/2)results in the correct posting, demonstrating the new mid-script function call feature. Good coverage.
4074-4115: Checks updated balance after intermediate decrease.By subtracting from the account before calling
balance, the test ensures the interpreter respects the new lower balance. Implementation looks solid.
4117-4133: Tests arithmetic expression in a variable origin.This verifies that expressions like
number $x = 1 + 2parse and evaluate correctly at variable declaration time under the new feature flag. Looks good.
4135-4155: Verifies that nestedmeta()calls fail as intended.This test accurately checks the interpreter’s rejection of nested
meta()usage. The negative path is well-covered.internal/interpreter/interpreter.go (9)
38-59: RefinedGetBalancesmethod logic.The new approach returns only requested balances, avoiding extraneous accounts/currencies. This appears correct and aligns with the associated tests.
134-148: Properly enforces feature flag for mid-script function calls.By checking
ExperimentalMidScriptFunctionCallfor non-meta()function origins, the interpreter cleanly separates top-levelmeta()usage from experimental calls. Nicely structured.
150-166: Clear handling of meta-based variable origins.This logic rejects invalid nested
meta()usage and correctly parses valid metadata calls. Implementation seems solid.
218-218: New feature flag constant.Introducing
ExperimentalMidScriptFunctionCallclearly separates mid-script call functionality from default behavior. No concerns.
367-367: Switched toCachedBalances.fetchBalance.This aligns with the unified approach of fetching balances consistently and helps simplify code paths. Implementation is fine.
Also applies to: 370-370
387-387: Consistent balance fetching.Using
fetchBalance(*account, *asset)improves clarity and matches the refactoring effort. No issues noticed.
473-473: Further alignment withfetchBalance.All references now route through
CachedBalances, reducing potential duplication. Looks good.
570-570: Maintaining uniform fetch strategy.Again, this change ensures consistent usage of
fetchBalance. Implementation is straightforward and coherent.
896-896: Standardized balance retrieval.Replacing direct map access with the helper function keeps the code uniform and maintainable. Approved.
| func (b Balances) mergeBalance(update Balances) { | ||
| // merge queried balance | ||
| for acc, accBalances := range update { | ||
| cachedAcc := defaultMapGet(b, acc, func() AccountBalance { | ||
| return AccountBalance{} | ||
| }) | ||
|
|
||
| for curr, amt := range accBalances { | ||
| cachedAcc[curr] = amt | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Effective balance merging implementation.
The mergeBalance method effectively combines balances from multiple sources, which is crucial for maintaining accurate state during script execution.
Note: Static analysis suggests lines 50-51 are not covered by tests. Consider adding a test case that exercises the scenario where a new account is added during a merge operation.
🏁 Script executed:
#!/bin/bash
# Check test coverage for the mergeBalance method, specifically for lines 50-51
# Find test files that might test the mergeBalance function
echo "Looking for tests that might cover the mergeBalance function..."
grep -r "mergeBalance" --include="*_test.go" .Length of output: 183
Action: Add test coverage for the mergeBalance update scenario.
The current implementation in internal/interpreter/balances.go (lines 46-57) correctly merges balance updates; however, our grep check shows no test case covers the branch where a new account is introduced (specifically affecting lines 50-51). Please add a unit test that exercises this scenario to ensure that when an account does not exist in the cached balance, it is correctly initialized and updated.
- File:
internal/interpreter/balances.go(lines 46-57) - Concern: The merging logic for adding new accounts (lines 50-51) lacks test coverage.
- Recommendation: Add a test case in the appropriate
*_test.gofile that simulates merging an update with a new account.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 50-51: internal/interpreter/balances.go#L50-L51
Added lines #L50 - L51 were not covered by tests
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
internal/interpreter/balances_test.go (1)
10-31: Good test coverage for filterQuery.The test effectively verifies that filterQuery correctly identifies and returns only the currencies that aren't already cached in the balance. It covers all key scenarios: existing and non-existing currencies for existing accounts, as well as completely new accounts.
Consider adding tests for edge cases like empty balances or an empty query to ensure robust behavior in all scenarios.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
internal/interpreter/balances.go(1 hunks)internal/interpreter/balances_test.go(1 hunks)internal/utils/utils.go(1 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
internal/interpreter/balances.go
[warning] 56-57: internal/interpreter/balances.go#L56-L57
Added lines #L56 - L57 were not covered by tests
🔇 Additional comments (4)
internal/utils/utils.go (1)
36-44: Well-implemented generic filter function.This is a clean implementation of a functional-style filter operation using Go's generics. The function correctly creates a new slice without modifying the original, has O(n) time complexity, and handles the generic type parameter properly.
internal/interpreter/balances.go (3)
9-49: Well-structured implementation of balance management.The balance management functions are well-designed with appropriate handling of default values to prevent nil reference errors. The code consistently follows the pattern of defaulting to empty structures or zero values when data doesn't exist.
36-49: Improved filterQuery implementation addresses previous review feedback.The new implementation correctly uses the Filter function to only include currencies that aren't already cached in the balance, addressing the issue mentioned in previous reviews. The approach of filtering at the currency level rather than the account level is more precise and eliminates the potential issue where all currencies for an account would be included even if only one was missing.
51-63: Add test coverage for mergeBalance new account scenario.The mergeBalance implementation correctly handles the case where an account doesn't exist in the cached balance, but this scenario isn't covered by tests according to static analysis.
#!/bin/bash # Check test coverage for the mergeBalance method # Find any tests that cover the mergeBalance method echo "Looking for existing tests for mergeBalance..." grep -r "mergeBalance" --include="*_test.go" . # Suggest a test implementation echo -e "\nSuggested test implementation:" cat << 'EOF' func TestMergeBalance(t *testing.T) { // Test merging when account doesn't exist in the cached balance cached := Balances{ "alice": AccountBalance{ "EUR": big.NewInt(100), }, } update := Balances{ "bob": AccountBalance{ "USD": big.NewInt(200), }, } cached.mergeBalance(update) // Verify the new account was added require.Equal(t, big.NewInt(200), cached["bob"]["USD"]) // Verify existing account wasn't affected require.Equal(t, big.NewInt(100), cached["alice"]["EUR"]) } EOF🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 56-57: internal/interpreter/balances.go#L56-L57
Added lines #L56 - L57 were not covered by tests
| @@ -220,3 +220,11 @@ type InvalidAccountName struct { | |||
| func (e InvalidAccountName) Error() string { | |||
| return fmt.Sprintf("Invalid account name: @%s", e.Name) | |||
| } | |||
|
|
|||
| type InvalidNestedMeta struct { | |||
There was a problem hiding this comment.
You should implements Is(target error) bool method for your errors.
It allow callers (the ledger for example) to check for concrete errors.
| @@ -2218,6 +2252,67 @@ func TestVariableBalance(t *testing.T) { | |||
| // test(t, tc) | |||
| // } | |||
|
|
|||
| func TestBalanceSimple(t *testing.T) { | |||
| script := ` | |||
There was a problem hiding this comment.
add t.Parallel() call on your tests except if it's not parallelizable, but I guess it is the case here
There was a problem hiding this comment.
oh yeah that's true.. yeah I think so far 100% of the tests are parallelizable, but I keep forgetting about adding it.. is there a way to declare that the whole module is parallelizable ?
| @@ -32,3 +32,13 @@ func MaxBigInt(a *big.Int, b *big.Int) *big.Int { | |||
|
|
|||
| return &max | |||
| } | |||
|
|
|||
| func Filter[T any](slice []T, predicate func(x T) bool) []T { | |||
There was a problem hiding this comment.
You have some helpers in go libs to play with generics.
https://github.com/formancehq/go-libs/tree/main/collectionutils
There was a problem hiding this comment.
yeah I'm aware of this lib but for such simple utilities that are basically one-liners it's just better to duplicate them
(I've also tried to avoid this so far because I though it's sort of non-idiomatic in go, but it's just so much clearer this way..)
NOTE FOR REVIEWERS: commits are rubbish. The PR is not meant to be read commit by commit, and it'll be squashed before merge (as always, discard the antlr autogenerated part)
Specs
This PR changes the numscript grammar so that the variable origin is not just a function call but any expression. Conversely, a function call is now an expression.
This means that:
is now possible, and at the same time:
is now allowed as well
important caveat: it will only be possible to call the meta() function in a toplevel position of a var origin (same way as it was until this PR). This is because otherwise it would be tricky (and even trickier for the user) to determine the return type (which is needed to parse correctly the stringified meta)
feature flags: the feature is under the
experimental-mid-script-function-callfeature flag