Repository navigation
π₯ remove: Drop an unused parser helper and two needless Sprintf calls - #165
Conversation
- Parser.prefixPrec: unused since unary operators went through parseExpr(precUnary) directly (staticcheck U1000). - Parser.Errors: Parse already returns the same errors, and nothing asks for them a second time. - Environment.Set: the interpreter has no reassignment to use it for. - meowrt.AsList: codegen unboxes only int, byte, float, string and bool, so no generated program calls it. - Two fmt.Sprintf calls with nothing to format (S1025, S1039). Every golden and every other test prints what it printed before; the examples the e2e job runs still run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Scc7p1RecmFoikhcXwnTMu
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
βΉοΈ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with π while any review is running, comments if it has suggestions, and reacts with π once all reviews finish with no findings. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. π βΉοΈ Recent review infoβοΈ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: π Files selected for processing (3)
π€ Files with no reviewable changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. π WalkthroughWalkthroughThe code generator and interpreter simplify expressions without changing their described behavior. The parser removes the unexported ChangesCode cleanup
Priority: β¬οΈ Low Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Refactor Merge Risk: βͺ Minimal Β· up to This cleanup preserves the described behavior and presents no identified merge-blocking risk. Architecture SummaryArchitecture risk: π΅ Low Β· up to The change affects 1 system. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
π₯ Pre-merge checks | β 5β Passed checks (5 passed)
β¨ Finishing Touchesπ Generate docstrings
π§ͺ 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. A rabbit hops past lines of code, Comment |
There was a problem hiding this comment.
π‘ Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94a2778567
βΉοΈ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with π.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Parser.Errors, Environment.Set and meowrt.AsList have no caller in this repository, but their packages are importable, so removing them would stop a module that uses them compiling on upgrade. Removing exported API is a decision for a documented release, not part of a cleanup; this one keeps to what nothing outside can reach. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Scc7p1RecmFoikhcXwnTMu
## Overview Removes three exported functions that nothing in this repository calls, and nothing in the Go the compiler generates calls either. PR #165 kept them because removing exported API breaks any module that imports these packages. The maintainer has since decided to remove them now, since the module is still at v0. | Removed | What to use instead | | --- | --- | | `parser.Parser.Errors` | `Parse` already returns the same errors as its second result | | `interpreter.Environment.Set` | Nothing: the interpreter has no reassignment, so nothing sets a binding after it is defined | | `meowrt.AsList` | `meowrt.TryAsList`, which returns the list, or the Furball instead of panicking | ### How it reaches the release notes - `CHANGELOG.md` is generated by semantic-release-gitmoji, and its template prints only β¨, π, π, π and π₯ commits. A π₯ commit would appear under "Breaking Changes", but it would also cut v1.0.0. So this commit is π₯, which cuts no release of its own. - GoReleaser's changelog groups a commit whose message says `remove` under **Removed**. So the GitHub release notes of the next release list this commit, and the commit message carries the replacements above. ## Verification - [x] `go test ./...` passes - [x] `go vet ./...` passes - [x] Golden files updated if needed: none moved - [x] `go build ./...` and the WASM playground build - [x] Nothing in the repository references the three names any more (`TryAsList` stays) - [x] staticcheck reports nothing new on the changed packages - [x] The examples the e2e job runs (`hello`, `fibonacci`, `fizzbuzz`, `list_ops`) run ## Impact - [x] Compiler (lexer / parser / checker / codegen): parser - [x] Runtime (meowrt / file / http): `meowrt.AsList` - [ ] CLI - [ ] Documentation - [x] Playground: `interpreter.Environment.Set` A dependent module that calls one of these stops compiling when it upgrades. That breaking change is the point of this PR. Generated programs are unaffected. ## AI Session - Session URL: https://claude.ai/code/session_01Scc7p1RecmFoikhcXwnTMu ## Checklist - [x] I have read the [CONTRIBUTING guide](../CONTRIBUTING.md) - [x] My changes follow the project's coding style - [x] I have added/updated tests for my changes. Nothing called these, so the existing suite is the check. - [x] Commit messages use [gitmoji](https://gitmoji.dev/) prefix π€ Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Scc7p1RecmFoikhcXwnTMu --- _Generated by [Claude Code](https://claude.ai/code/session_01Scc7p1RecmFoikhcXwnTMu)_ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **API Changes** * Removed the environment method for updating an existing binding, the parser accessor for accumulated errors, and the list conversion helper. These methods and helper are no longer available to callers. * Parser errors remain available as the second result returned by `Parse`. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude <noreply@anthropic.com>
Overview
Removes the code nothing can reach, and two
fmt.Sprintfcalls with nothing to format. Nothing a program does changes.Parser.prefixPrec(unexported)parseExpr(precUnary)directly, so nothing calls it (staticcheck U1000)fmt.Sprintf("%s", x)in codegen'sgenPartialCallxis already a string (S1025)fmt.Sprintf("Hiss! pipe target is not callable, nya~")in the interpreterKept on purpose
meowrt.AsList,Parser.ErrorsandEnvironment.Set. The first push removed them. Their packages are importable by other modules, though, so removing them would stop a dependent module compiling on upgrade; Codex review pointed this out. They are restored unchanged. Dropping them belongs in a documented breaking release, if at all.deadcodereports as unreachable (TryAsByte,AsByte,Nya,And,Or,BuiltinFunc,RunMain,ExitOnFurball): generated programs call them, directly or throughRunMain/AsByte.deadcodecan't see calls that exist only in Go the compiler generates.TrickType.String/Equals: they are what makeTrickTypeatypes.Type.The candidates came from staticcheck (the repo's
all,-ST1005) and fromdeadcode -testrun for both the native and the WASM (GOOS=js) build.Found while checking this, not changed here
The spec has a Reassignment section (
AssignStmt = identifier "=" Expr, "Rebinds an existing variable to a new value"). NoAssignStmtexists, though:nyan x = 1followed byx = 5is refused withVariable x already declared in this scope. Whether to implement reassignment or remove that spec section is a language decision, so it is not part of this PR.Verification
go test ./...passes. Every golden prints exactly what it did before, so the codegen change emits the same Go.go vet ./...passesgo build ./...and the WASM playground buildhello,fibonacci,fizzbuzz,list_ops) runImpact
Sprintfin the interpreterAI Session
Checklist
π€ Generated with Claude Code
https://claude.ai/code/session_01Scc7p1RecmFoikhcXwnTMu
Summary by CodeRabbit