michael/luau
michael
/
luau
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

RFC: Deprecate getfenv/setfenv (#51)

This commit is contained in:
Arseny Kapoulkine 2021-06-24 23:02:57 -07:00 committed by GitHub
parent 9a4487f3e9
commit 7e71295c27
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 36 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
rfcs/deprecate-getfenv-setfenv.md Normal file
View File

@ -0,0 +1,36 @@
# Deprecate getfenv/setfenv
## Summary
Mark getfenv/setfenv as deprecated
## Motivation
getfenv and setfenv are problematic for a host of reasons:
- They allow uncontrolled mutation of global environment, which results in deoptimization; various important performance features
like builtin calls or imports are disabled when these functions are used.
- Because of the uncontrolled mutation code that uses getfenv/setfenv can't be typechecked correctly; in particular, injecting new
globals is going to produce "unknown globals" warnings, and modifying existing globals can trivially violate soundness wrt type
checking
- While these functions can be used for good (once you ignore the issues above), such as custom module systems, statistically speaking
they are mostly used to obfuscate code to hide malicious intent.
## Design
We will mark getfenv and setfenv as deprecated. The only consequence of this change is that the linter will start emitting warnings when they are used.
Removing support for getfenv/setfenv, while tempting, is not planned in the foreseeable future because it will cause significant backwards compatibility issues.
## Drawbacks
There are valid uses for getfenv/setfenv, that include extra logging (in Roblox code this manifests as `getfenv(1).script`), monkey patching for mocks in unit tests, and custom
module systems that inject globals into the calling environment. We do have a replacement for logging use cases, `debug.info`, and we do have an officially recommended replacement
for custom module systems, which is to use `require` that doesn't result in issues that fenv modification carries and can be understood by the type checker, we do not have an
alternative for mocks. As such, testing frameworks that implement mocking via setfenv/getfenv will need to use `--!nolint DeprecatedGlobal` to avoid this warning.
## Alternatives
Besides the obvious alternative "do nothing", we could also consider implementing Lua 5.2 support for _ENV. However, since we do not have a way to load script files other than
via `require` that doesn't support _ENV, and `loadstring` is supported but discouraged, we do not currently plan to implement `_ENV` although it's possible that this will happen
in the future.