feat: Use Nix & Shipfox & Just for DevXP #77
Conversation
…; remove obsolete workflows
WalkthroughThis update introduces a Nix flake and Justfile to standardize the development environment and automation tasks, replacing previous shell scripts for code generation and coverage. It also updates error handling throughout the codebase by explicitly ignoring unused return values and propagating errors properly. Minor parser and generated code updates are included, along with a small logic adjustment in range handling. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant Just
participant Go Tools
participant ANTLR
participant Nix Flake
Developer->>Just: Run task (e.g., pre-commit, generate, lint)
Just->>Go Tools: Run go mod tidy, golangci-lint, go test, etc.
Just->>ANTLR: Generate parser code (via generate task)
Just->>Go Tools: Rename lexer file after generation
Developer->>Nix Flake: Enter dev shell
Nix Flake->>Go Tools: Provide Go 1.23 and other tools
Nix Flake->>ANTLR: Provide ANTLR tool
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 5
🧹 Nitpick comments (2)
flake.nix (1)
44-53: Consider pinning specific versions of ancillary tools
antlr,goreleaser-pro, andgolangci-lintmay receive breaking updates. Pinning them (e.g. viapkgs.antlrs_4_13or a commit-hash overlay) guards against CI drift.Justfile (1)
20-23: Missing coverage file cleanup
go test -coverprofile coverage.txtoverwrites but never removes the file. Addrm -f coverage.txtbefore the test run to avoid stale data contaminating local coverage tools.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
.github/actions/default/action.ymlis excluded by!**/*.yml.github/actions/env/action.ymlis excluded by!**/*.yml.github/workflows/checks.ymlis excluded by!**/*.yml.github/workflows/main.ymlis excluded by!**/*.yml.github/workflows/release.ymlis excluded by!**/*.yml.github/workflows/releases.ymlis excluded by!**/*.ymlflake.lockis excluded by!**/*.lock,!**/*.lock
📒 Files selected for processing (23)
.envrc(1 hunks)Justfile(1 hunks)flake.nix(1 hunks)generate-cover.sh(0 hunks)generate-parser.sh(0 hunks)internal/analysis/hover.go(7 hunks)internal/cmd/check.go(1 hunks)internal/cmd/lsp.go(1 hunks)internal/cmd/run.go(5 hunks)internal/interpreter/batch_balances_query.go(1 hunks)internal/interpreter/interpreter.go(1 hunks)internal/jsonrpc2/jsonrpc2.go(5 hunks)internal/jsonrpc2/jsonrpc2_test.go(1 hunks)internal/lsp/code_actions.go(1 hunks)internal/lsp/handlers.go(2 hunks)internal/lsp/handlers_test.go(1 hunks)internal/lsp/object_stream_test.go(1 hunks)internal/numscript/numscript.go(1 hunks)internal/parser/antlrParser/lexer.go(1 hunks)internal/parser/antlrParser/numscript_base_listener.go(1 hunks)internal/parser/antlrParser/numscript_listener.go(1 hunks)internal/parser/antlrParser/numscript_parser.go(1 hunks)internal/parser/parser.go(1 hunks)
💤 Files with no reviewable changes (2)
- generate-cover.sh
- generate-parser.sh
🧰 Additional context used
🧠 Learnings (10)
internal/parser/antlrParser/numscript_listener.go (4)
Learnt from: ascandone
PR: formancehq/numscript#31
File: internal/parser/parser_test.go:115-126
Timestamp: 2025-02-06T17:48:48.962Z
Learning: The numscript parser's fault tolerance is built into the infrastructure and handles errors gracefully for all clause types. Tests in parser_fault_tolerance_test.go cover both inorder and oneof constructs as they share the same parsing infrastructure.
Learnt from: ascandone
PR: formancehq/numscript#55
File: internal/parser/antlrParser/numscript_parser.go:2953-2967
Timestamp: 2025-04-22T16:45:07.842Z
Learning: In numscript, type constraints for color expressions are not enforced at the grammar level but are instead handled during static analysis or at runtime. The grammar is intentionally kept permissive (accepting any valueExpr after the RESTRICT token) to maintain separation of concerns.
Learnt from: ascandone
PR: formancehq/numscript#39
File: internal/parser/parser.go:0-0
Timestamp: 2025-02-19T11:57:21.654Z
Learning: Percentage literals in NumScript are inherently bounded as they represent percentages, making overflow checks unnecessary. The uint16 type for floating digits is more than sufficient as it can handle up to 65535 decimal places.
Learnt from: ascandone
PR: formancehq/numscript#31
File: internal/parser/parser_test.go:115-126
Timestamp: 2025-02-06T17:48:48.961Z
Learning: Edge cases for oneof destination parsing in numscript are covered by the existing inorder parsing fault tolerance tests, as both constructs share similar parsing logic.
internal/parser/antlrParser/numscript_base_listener.go (4)
Learnt from: ascandone
PR: formancehq/numscript#31
File: internal/parser/parser_test.go:115-126
Timestamp: 2025-02-06T17:48:48.962Z
Learning: The numscript parser's fault tolerance is built into the infrastructure and handles errors gracefully for all clause types. Tests in parser_fault_tolerance_test.go cover both inorder and oneof constructs as they share the same parsing infrastructure.
Learnt from: ascandone
PR: formancehq/numscript#55
File: internal/parser/antlrParser/numscript_parser.go:2953-2967
Timestamp: 2025-04-22T16:45:07.842Z
Learning: In numscript, type constraints for color expressions are not enforced at the grammar level but are instead handled during static analysis or at runtime. The grammar is intentionally kept permissive (accepting any valueExpr after the RESTRICT token) to maintain separation of concerns.
Learnt from: ascandone
PR: formancehq/numscript#39
File: internal/parser/parser.go:0-0
Timestamp: 2025-02-19T11:57:21.654Z
Learning: Percentage literals in NumScript are inherently bounded as they represent percentages, making overflow checks unnecessary. The uint16 type for floating digits is more than sufficient as it can handle up to 65535 decimal places.
Learnt from: ascandone
PR: formancehq/numscript#31
File: internal/parser/parser_test.go:115-126
Timestamp: 2025-02-06T17:48:48.961Z
Learning: Edge cases for oneof destination parsing in numscript are covered by the existing inorder parsing fault tolerance tests, as both constructs share similar parsing logic.
internal/parser/antlrParser/lexer.go (1)
Learnt from: ascandone
PR: formancehq/numscript#39
File: internal/parser/antlr/numscript_base_listener.go:78-82
Timestamp: 2025-02-19T11:57:58.444Z
Learning: Skip suggesting tests for ANTLR-generated code files (containing "antlr" in path and having header "Code generated ... DO NOT EDIT") as they are auto-generated and will be regenerated when the grammar changes.
internal/parser/parser.go (1)
Learnt from: ascandone
PR: formancehq/numscript#39
File: internal/parser/parser.go:0-0
Timestamp: 2025-02-19T11:57:21.654Z
Learning: Percentage literals in NumScript are inherently bounded as they represent percentages, making overflow checks unnecessary. The uint16 type for floating digits is more than sufficient as it can handle up to 65535 decimal places.
internal/interpreter/interpreter.go (3)
Learnt from: ascandone
PR: formancehq/numscript#62
File: internal/interpreter/evaluate_expr.go:143-144
Timestamp: 2025-05-30T13:51:20.130Z
Learning: In the numscript interpreter color system: `nil` color means "any color" (no color filter applied), while an empty string `""` color is treated as a specific color value, distinct from nil. The `evaluateColor` function in `internal/interpreter/evaluate_expr.go` should return a pointer to an empty string when no color expression is provided, not nil, to maintain this semantic distinction.
Learnt from: ascandone
PR: formancehq/numscript#27
File: internal/interpreter/interpreter.go:667-668
Timestamp: 2024-12-05T11:42:58.472Z
Learning: In Go test files within the `internal/interpreter` package (e.g., `reconciler_test.go` and `interpreter_test.go`), it's acceptable to use hardcoded `"<kept>"` strings in test data and comments, and they do not need to be replaced with the `KEPT_ADDR` constant.
Learnt from: ascandone
PR: formancehq/numscript#31
File: internal/interpreter/interpreter.go:479-488
Timestamp: 2025-01-20T23:01:48.857Z
Learning: In the `sendAll` method of the Numscript interpreter, the `oneof` operator only uses the first source because none of the branches can fail in a "send all" context. This differs from `trySendingUpTo` where backtracking is needed as sources can fail when trying to send a specific amount.
internal/interpreter/batch_balances_query.go (2)
Learnt from: ascandone
PR: formancehq/numscript#62
File: internal/interpreter/evaluate_expr.go:143-144
Timestamp: 2025-05-30T13:51:20.130Z
Learning: In the numscript interpreter color system: `nil` color means "any color" (no color filter applied), while an empty string `""` color is treated as a specific color value, distinct from nil. The `evaluateColor` function in `internal/interpreter/evaluate_expr.go` should return a pointer to an empty string when no color expression is provided, not nil, to maintain this semantic distinction.
Learnt from: ascandone
PR: formancehq/numscript#27
File: internal/interpreter/interpreter.go:667-668
Timestamp: 2024-12-05T11:42:58.472Z
Learning: In Go test files within the `internal/interpreter` package (e.g., `reconciler_test.go` and `interpreter_test.go`), it's acceptable to use hardcoded `"<kept>"` strings in test data and comments, and they do not need to be replaced with the `KEPT_ADDR` constant.
internal/numscript/numscript.go (2)
Learnt from: ascandone
PR: formancehq/numscript#31
File: internal/parser/parser_test.go:115-126
Timestamp: 2025-02-06T17:48:48.962Z
Learning: The numscript parser's fault tolerance is built into the infrastructure and handles errors gracefully for all clause types. Tests in parser_fault_tolerance_test.go cover both inorder and oneof constructs as they share the same parsing infrastructure.
Learnt from: ascandone
PR: formancehq/numscript#62
File: internal/interpreter/evaluate_expr.go:143-144
Timestamp: 2025-05-30T13:51:20.130Z
Learning: In the numscript interpreter color system: `nil` color means "any color" (no color filter applied), while an empty string `""` color is treated as a specific color value, distinct from nil. The `evaluateColor` function in `internal/interpreter/evaluate_expr.go` should return a pointer to an empty string when no color expression is provided, not nil, to maintain this semantic distinction.
internal/lsp/handlers.go (1)
Learnt from: ascandone
PR: formancehq/numscript#52
File: internal/jsonrpc2/jsonrpc2.go:128-164
Timestamp: 2025-03-25T09:47:10.768Z
Learning: In the jsonrpc2 package, the Conn.Close() method is designed to be called only once during shutdown, not concurrently from multiple goroutines.
internal/parser/antlrParser/numscript_parser.go (3)
undefined
<retrieved_learning>
Learnt from: ascandone
PR: #31
File: internal/parser/parser_test.go:115-126
Timestamp: 2025-02-06T17:48:48.962Z
Learning: The numscript parser's fault tolerance is built into the infrastructure and handles errors gracefully for all clause types. Tests in parser_fault_tolerance_test.go cover both inorder and oneof constructs as they share the same parsing infrastructure.
</retrieved_learning>
<retrieved_learning>
Learnt from: ascandone
PR: #55
File: internal/parser/antlrParser/numscript_parser.go:2953-2967
Timestamp: 2025-04-22T16:45:07.842Z
Learning: In numscript, type constraints for color expressions are not enforced at the grammar level but are instead handled during static analysis or at runtime. The grammar is intentionally kept permissive (accepting any valueExpr after the RESTRICT token) to maintain separation of concerns.
</retrieved_learning>
<retrieved_learning>
Learnt from: ascandone
PR: #39
File: internal/parser/parser.go:0-0
Timestamp: 2025-02-19T11:57:21.654Z
Learning: Percentage literals in NumScript are inherently bounded as they represent percentages, making overflow checks unnecessary. The uint16 type for floating digits is more than sufficient as it can handle up to 65535 decimal places.
</retrieved_learning>
internal/jsonrpc2/jsonrpc2.go (1)
Learnt from: ascandone
PR: formancehq/numscript#52
File: internal/jsonrpc2/jsonrpc2.go:128-164
Timestamp: 2025-03-25T09:47:10.768Z
Learning: In the jsonrpc2 package, the Conn.Close() method is designed to be called only once during shutdown, not concurrently from multiple goroutines.
🧬 Code Graph Analysis (7)
internal/lsp/handlers_test.go (1)
internal/lsp/lsp_types/bindings.go (1)
DidOpenTextDocumentParams(1510-1515)
internal/cmd/lsp.go (1)
internal/lsp/handlers.go (1)
RunServer(244-247)
internal/interpreter/interpreter.go (1)
internal/utils/utils.go (1)
NonExhaustiveMatchPanic(9-11)
internal/cmd/run.go (5)
internal/utils/utils.go (1)
Unmarshal(41-48)internal/interpreter/interpreter.go (1)
Balances(24-24)numscript.go (2)
Balances(58-58)ParseErrorsToString(44-44)internal/parser/parser.go (1)
ParseErrorsToString(79-85)internal/parser/ast.go (1)
Source(122-125)
internal/interpreter/batch_balances_query.go (1)
internal/utils/utils.go (1)
NonExhaustiveMatchPanic(9-11)
internal/lsp/handlers.go (2)
internal/analysis/check.go (1)
Diagnostic(119-123)internal/lsp/lsp_types/bindings.go (2)
Diagnostic(1337-1385)PublishDiagnosticsParams(3042-3057)
internal/jsonrpc2/jsonrpc2.go (1)
internal/jsonrpc2/messages.go (6)
Response(52-59)Response(61-61)Request(65-73)Request(75-75)ID(25-27)ErrMethodNotFound(13-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Tests
- GitHub Check: Dirty
🔇 Additional comments (26)
internal/parser/antlrParser/numscript_base_listener.go (1)
1-1: Confirm the toolchain is actually on ANTLR 4.13.2The header now advertises 4.13.2. Double-check that:
- The Nix flake/Just recipe installs and invokes exactly this version.
- All other generated ANTLR files were regenerated in the same run, avoiding mixed-version artefacts.
If both conditions already hold, nothing else to do.
internal/parser/antlrParser/numscript_parser.go (1)
1-1: Validate runtime/library compatibility after bumping the generator version
The header indicates the grammar was regenerated with ANTLR 4.13.2. Make sure:
go.modpulls a runtime (github.com/antlr4-go/antlr/v4) version that is ≥ 4.13.2.- Any CI caches are invalidated so they don’t mix 4.13.1 artefacts with 4.13.2-generated sources.
No other action needed if the above are already satisfied.
internal/parser/antlrParser/lexer.go (1)
1-1: ANTLR version update looks good.The lexer code has been regenerated with ANTLR 4.13.2, which aligns with the build tooling improvements mentioned in the PR objectives.
internal/lsp/code_actions.go (1)
32-32: Change fromprogram.Vars.Range.Endtoprogram.Vars.Endis correctThe
VarDeclarationstype embedsRange(internal/parser/ast.go:315–318), soprogram.Vars.Endis a promoted alias forprogram.Vars.Range.End. This access is equivalent and preserves existing functionality. No further updates are needed.internal/lsp/object_stream_test.go (1)
48-48: Good practice: Explicitly ignoring unused return values.The change from
in.Write([]byte(fmt.Sprintf(...)))to_, _ = fmt.Fprintf(in, ...)explicitly acknowledges that the return values are being ignored, which improves code clarity and suppresses unused variable warnings.internal/numscript/numscript.go (2)
29-29: Appropriate explicit ignoring of stderr write result.The change explicitly ignores the return values from
os.Stderr.Write, which is appropriate for error reporting scenarios where the write result doesn't affect control flow.
35-35: Verify that ignoring sentry.Init error is intentional.The change explicitly ignores the error from
sentry.Init. Ensure this is intentional for non-critical telemetry setup, as initialization errors might indicate configuration issues that could be worth logging.flake.nix (1)
38-39: Verify thatgo_1_23exists in your pinned nixpkgs
The previous attempt to auto-list Go versions failed (nix-instantiate: command not found), so please manually confirm that your flake’s pinned nixpkgs actually exportsgo_1_23. For example, run locally:# Evaluate whether go_1_23 is defined nix eval '(import <nixpkgs> {}).go_1_23'If this errors or returns “attribute not found,” you will need to either bump your pinned nixpkgs revision to one that includes Go 1.23 or fall back to
go_1_22in flake.nix..envrc (1)
1-1: LGTM – minimal and effectiveThe directive cleanly delegates environment setup to the flake.
internal/parser/antlrParser/numscript_listener.go (1)
1-1: No functional change – safe to ignoreOnly the generated comment was updated to 4.13.2; interfaces are untouched.
internal/lsp/handlers_test.go (1)
114-120: LGTM: Explicit error ignoring in test context.The explicit ignoring of
SendNotificationerrors is appropriate in a test environment where the focus is on testing the core functionality rather than notification error handling.internal/lsp/handlers.go (3)
32-32: LGTM: Improved slice initialization.Using type inference for slice initialization is cleaner and more idiomatic Go code.
37-40: LGTM: Explicit error ignoring for diagnostics publishing.Explicitly ignoring errors from
SendNotificationis consistent with the broader codebase pattern and appropriate for diagnostics publishing where failure to send notifications shouldn't interrupt the main LSP flow.
283-283: LGTM: Explicit error ignoring for exit notification.Explicitly ignoring errors from the "exit" notification is appropriate during shutdown since there's no meaningful error recovery possible at this point.
internal/interpreter/interpreter.go (1)
540-541: LGTM: Consistent error handling pattern.Although
NonExhaustiveMatchPanicalways panics and never returns normally, explicitly assigning to_maintains consistency with the broader codebase pattern of making return value handling explicit.internal/cmd/lsp.go (1)
15-17: LGTM: Proper error handling for LSP server.This is a significant improvement that adds proper error handling to the LSP command. Previously, errors from
lsp.RunServer()were silently ignored, now they are properly captured and reported usingcmd.PrintErr().internal/parser/parser.go (1)
257-257: LGTM: More idiomatic string replacement.Using
strings.ReplaceAllis more idiomatic and clearer thanstrings.Replacewith-1for replacing all occurrences. The functionality remains identical.internal/cmd/check.go (1)
16-16: Good practice: Explicitly ignoring return values.This change correctly follows Go best practices by explicitly ignoring the return values from
os.Stderr.Writeusing the blank identifier, making the intention clear that these values are intentionally discarded.internal/cmd/run.go (4)
72-72: LGTM: Consistent error handling pattern.The explicit ignoring of return values from
os.Stderr.Writeusing_, _ = ...is consistent and follows Go best practices for intentionally discarded errors.Also applies to: 81-81, 90-90, 99-99
84-84: LGTM: Explicit error ignoring for JSON unmarshaling.The explicit ignoring of errors from
json.Unmarshalcalls is appropriate here since these operations are used for optional configuration loading where failures are handled gracefully by continuing with defaults.Also applies to: 93-93, 102-102
119-119: LGTM: Consistent error output handling.The explicit ignoring of return values from error output operations is appropriate since these are error reporting calls where the write failure itself doesn't need to be handled.
Also applies to: 135-135, 137-137, 138-138
163-163: LGTM: Explicit handling of stdout write.The explicit ignoring of return values from
os.Stdout.Writeis appropriate for output operations where write failures are not critical to handle.internal/analysis/hover.go (1)
73-73: LGTM: Improved encapsulation with direct Contains method.The change from
Range.Contains(position)toContains(position)improves encapsulation by using the node's own method instead of accessing the Range field directly. This aligns well with the parser regeneration mentioned in the summary and provides better abstraction.Also applies to: 104-104, 122-122, 241-241, 291-291, 314-314, 341-341, 345-345
internal/jsonrpc2/jsonrpc2.go (3)
56-56: LGTM: Consistent error handling in request handlers.The explicit ignoring of errors from
WriteMessagecalls is appropriate for response sending operations where the error handling is delegated to the connection-level error handling.Also applies to: 71-71
220-220: LGTM: Appropriate error ignoring for Close operation.Based on the retrieved learning that the
Close()method is designed for single-use during shutdown, ignoring the error froms.stream.Close()is appropriate since Close operations are typically best-effort.
239-243: LGTM: Asynchronous error response handling.Wrapping the error response in a goroutine is appropriate since it's just sending an error notification for an unknown method, and shouldn't block the main message handling loop.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #77 +/- ##
==========================================
- Coverage 73.08% 72.47% -0.62%
==========================================
Files 36 36
Lines 4054 4080 +26
==========================================
- Hits 2963 2957 -6
- Misses 968 995 +27
- Partials 123 128 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
* feat: add Nix flake for Go 1.23 development environment * feat: add .envrc and Justfile for environment setup; remove obsolete scripts * feat: add antlr to Nix flake dependencies * chore: update ANTLR version in generated files to 4.13.2 * feat: add GitHub Actions for environment setup and release management; remove obsolete workflows * chore: remove SPEAKEASY_API_KEY from workflows to streamline environment variables * refactor: simplify range checks in hover functions and update variable end position * refactor: improve error handling and logging across multiple files * feat: Update flake.nix Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
No description provided.