Skip to content

feat: support DisableBuiltin("$env") to hide the $env variable#949

Open
boinger wants to merge 2 commits intoexpr-lang:masterfrom
boinger:feat/disable-env-builtin
Open

feat: support DisableBuiltin("$env") to hide the $env variable#949
boinger wants to merge 2 commits intoexpr-lang:masterfrom
boinger:feat/disable-env-builtin

Conversation

@boinger
Copy link
Copy Markdown

@boinger boinger commented Mar 27, 2026

Summary

$env is a hardcoded special variable with custom handling in the checker and compiler. It's not in the Builtins map, so DisableBuiltin("$env") previously had no effect.

This PR checks config.Disabled["$env"] before each $env special-case handler. When disabled, $env falls through to normal identifier resolution, which errors in strict mode as "unknown name $env". This is consistent with how DisableBuiltin works for regular builtins.

Changes

  • checker/checker.go: Guard $env handling in identifierNode, memberNode, callNode, and callBuiltin (the get($env, ...) path) with !v.config.Disabled["$env"]
  • compiler/compiler.go: Guard OpLoadEnv emission in IdentifierNode with !c.config.Disabled["$env"]
  • test/issues/710/issue_test.go: 7 subtests covering all disabled paths (standalone, member access, call, get()), non-strict mode behavior, and regression tests for default behavior

Use case

Users embedding expr as a DSL may have domain variables that conflict with $env, or may want to restrict access to the full environment object for sandboxing.

Fixes #710

boinger added 2 commits March 26, 2026 15:58
$env is a hardcoded special variable, not a regular builtin, so
DisableBuiltin("$env") previously had no effect. Check config.Disabled
before each $env special-case handler in the checker and compiler.

When disabled, $env falls through to normal identifier resolution
(errors in strict mode as "unknown name $env"), consistent with how
DisableBuiltin works for regular builtins.

Fixes expr-lang#710
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR makes the hardcoded special variable $env respect DisableBuiltin("$env"), allowing users to hide $env (e.g., to avoid naming conflicts or to restrict environment access) and have it fall back to normal identifier resolution.

Changes:

  • Guard $env special-case handling in the checker so it only applies when $env is not disabled.
  • Guard $env bytecode emission (OpLoadEnv) in the compiler so it only applies when $env is not disabled.
  • Add regression tests covering $env disabled/enabled behaviors across identifier, member access, call, and get($env, ...) paths.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
checker/checker.go Adds !config.Disabled["$env"] guards around $env special handling so disabling causes normal name resolution/errors.
compiler/compiler.go Prevents OpLoadEnv emission when $env is disabled, aligning runtime behavior with checker resolution.
test/issues/710/issue_test.go Adds coverage for strict and non-strict behavior when $env is disabled, plus default enabled regressions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Request/Question] Allow control over if/how the $env var is exposed

2 participants