Skip to content
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.

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

Typechecker allows require_once $nullable_string #2926

Closed
fredemmott opened this issue Jun 12, 2014 · 4 comments
Closed

Typechecker allows require_once $nullable_string #2926

fredemmott opened this issue Jun 12, 2014 · 4 comments

Comments

@fredemmott
Copy link
Contributor

[fredemmott@devbig076 ~/hhtest] hh_client
No errors!
[fredemmott@devbig076 ~/hhtest] cat test.php
<?hh

function foo(?string $bar) {
  require_once $bar;
}
@jwatzman
Copy link
Contributor

I assume from the context that require_once cannot take null?

@jwatzman
Copy link
Contributor

Oof this is going to be obnoxious, our AST totally drops requires at the parse level.

@fredemmott
Copy link
Contributor Author

Nope, require_once must take a string.

Couldn't the parser just rewrite it to require_once($foo) ? then we could just add an hhi

@lexidor
Copy link
Collaborator

lexidor commented Jul 17, 2020

I am going over old issues against this repository to reduce the amount of open issues.

The issue of require_once's "first argument" not being typechecked is still true in hhvm today.
It joins the other statement keywords in being untyped echo f.e.

However, because of changes in the ecosystem, this is becoming less and less relevant. When this issue was made hhvm-autoload did not exist yet, so require_once's in application code were far more common. These days, most Hack applications only require an autoloader vendor/autoload.hack and invoke Facebook\AutoloadMap\initialize(). Since top-level code (with the exception of declarations) was banned (even in partial mode), requiring a file for its side effects is no longer a thing. The only thing you can achieve using a require call, it bringing named entities into the request. However, because of autoloading, this is rarely if ever needed.

@lexidor lexidor closed this as completed Jul 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants