Skip to content

feat: improve builder #165

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ascandone wants to merge 2 commits into
base: main
Choose a base branch
from
+82 −1
Open

feat: improve builder #165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
95c0043
chore: removed todo comment
ascandone Jun 23, 2026
08b4640
feat: add flags
ascandone Jun 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 51 additions & 1 deletion builder/builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@ func renderVars(
return st.sb.String()
}

// TODO double check this one (do we need to handle vars?)
func BuildProgram(statements ...Statement) (map[string]string, VarsEnv, string) {
env := newEnv()
for _, stmt := range statements {
Expand All @@ -150,3 +149,54 @@ func BuildProgram(statements ...Statement) (map[string]string, VarsEnv, string)

return st.knownBindings, env.varsEnv, vars + env.builder.String()
}

// Check feature flag has only lower chars and "-" chars.
// Less declarative than a regex, but this way we don't need a more complex api user-wise
// just for the sake of perfs (this should be good enough)
func checkIsFlagValid(s string) bool {
if len(s) == 0 {
return false
}

for i := 0; i < len(s); i++ {
ch := s[i]
if !(ch >= 'a' && ch <= 'z') && ch != '-' {
return false
}
}
return true
}

// Same as `BuildProgram`, but accepts features flag.
//
// IMPORTANT NOTE: this function will panic if a feature flag doesn't match `^[a-z-]+$`. Flags are meant to be passed directly, and a sintactically
// incorrect flag is treated as an argument error panic right away, not returned as an "error".
// Also note we don't keep the list of valid flags here
func BuildProgramWithFeatureFlags(
featureFlags []string,
statements ...Statement,
) (map[string]string, VarsEnv, string) {
knownBindings, varsEnv, script := BuildProgram(statements...)

var flagsArgs strings.Builder
for index, flag := range featureFlags {
if !checkIsFlagValid(flag) {
// Yes, we are panicking instead of returning an error here.
// That's desidered: flags are meant to be passed manually. Not computed, created conditionally, etc.
// If a feature flag is wrong we want to crash the thing immediately instead of having the user handle that, log that, or whatever.
panic(fmt.Sprintf("Invalid argument: the `%s` feature flag is invalid. Only flags matching `^[a-z-]+$` are accepted.", flag))
}

Comment on lines +175 to +189

@coderabbitai coderabbitai Bot Jun 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Validate flags before building the script to preserve fail-fast behavior.

Line 179 builds statements before validating featureFlags. Since Statement is executable code, callers can trigger side effects/panics even when flags are invalid, which contradicts the “panic right away” contract documented in Lines 172-173.

Suggested fix 
 func BuildProgramWithFeatureFlags(
 	featureFlags []string,
 	statements ...Statement,
 ) (map[string]string, VarsEnv, string) {
-	knownBindings, varsEnv, script := BuildProgram(statements...)
-
 	var flagsArgs strings.Builder
 	for index, flag := range featureFlags {
 		if !checkIsFlagValid(flag) {
 			panic(fmt.Sprintf("Invalid argument: the `%s` feature flag is invalid. Only flags matching `^[a-z-]+$` are accepted.", flag))
 		}
@@
 		flagsArgs.WriteString(flag)
 		flagsArgs.WriteByte('"')
 	}
 
+	knownBindings, varsEnv, script := BuildProgram(statements...)
 	updatedScript := fmt.Sprintf("#![feature(%s)]\n%s", flagsArgs.String(), script)
 	return knownBindings, varsEnv, updatedScript
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func BuildProgramWithFeatureFlags(
featureFlags []string,
statements ...Statement,
) (map[string]string, VarsEnv, string) {
knownBindings, varsEnv, script := BuildProgram(statements...)
var flagsArgs strings.Builder
for index, flag := range featureFlags {
if !checkIsFlagValid(flag) {
// Yes, we are panicking instead of returning an error here.
// That's desidered: flags are meant to be passed manually. Not computed, created conditionally, etc.
// If a feature flag is wrong we want to crash the thing immediately instead of having the user handle that, log that, or whatever.
panic(fmt.Sprintf("Invalid argument: the `%s` feature flag is invalid. Only flags matching `^[a-z-]+$` are accepted.", flag))
}
func BuildProgramWithFeatureFlags(
featureFlags []string,
statements ...Statement,
) (map[string]string, VarsEnv, string) {
var flagsArgs strings.Builder
for index, flag := range featureFlags {
if !checkIsFlagValid(flag) {
// Yes, we are panicking instead of returning an error here.
// That's desidered: flags are meant to be passed manually. Not computed, created conditionally, etc.
// If a feature flag is wrong we want to crash the thing immediately instead of having the user handle that, log that, or whatever.
panic(fmt.Sprintf("Invalid argument: the `%s` feature flag is invalid. Only flags matching `^[a-z-]+$` are accepted.", flag))
}
flagsArgs.WriteString(flag)
flagsArgs.WriteByte('"')
}
knownBindings, varsEnv, script := BuildProgram(statements...)
updatedScript := fmt.Sprintf("#![feature(%s)]\n%s", flagsArgs.String(), script)
return knownBindings, varsEnv, updatedScript
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@builder/builder.go` around lines 175 - 189, The BuildProgramWithFeatureFlags
function validates featureFlags after calling BuildProgram on the statements,
which allows side effects from BuildProgram to execute even when invalid flags
would cause a panic. Move the flag validation loop that calls checkIsFlagValid
before the call to BuildProgram to ensure invalid flags are caught immediately
and prevent any statement execution or side effects from occurring before the
panic.

if index != 0 {
flagsArgs.WriteString(", ")
}

flagsArgs.WriteByte('"')
flagsArgs.WriteString(flag)
flagsArgs.WriteByte('"')

}

updatedScript := fmt.Sprintf("#![feature(%s)]\n%s", flagsArgs.String(), script)
return knownBindings, varsEnv, updatedScript
}
31 changes: 31 additions & 0 deletions builder/builder_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,37 @@ send [$asset_0 42] (
)`))
}

func TestSimpleSendWithFlag(t *testing.T) {
stmt := builder.StmtSend(
builder.ExprMonetary(
builder.ExprAsset("USD/2"),
builder.ExprNumberBigInt(big.NewInt(42)),
),
builder.SrcAccount(
builder.ExprAccount("src"),
),
builder.DestAccount(
builder.ExprAccount("dest"),
),
)

_, _, script := builder.BuildProgramWithFeatureFlags([]string{
"my-flag",
"another-flag",
}, stmt)
snaps.MatchInlineSnapshot(t, script, snaps.Inline(`#![feature("my-flag", "another-flag")]
vars {
account $account_0
account $account_1
asset $asset_0
}

send [$asset_0 42] (
source = $account_0
destination = $account_1
)`))
}

func TestSendAll(t *testing.T) {
stmt := builder.StmtSendAll(
builder.ExprAsset("COIN"),
Expand Down
Loading