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

sprintf() with variables do not pass type checker #3725

Closed
milesj opened this issue Sep 11, 2014 · 6 comments
Closed

sprintf() with variables do not pass type checker #3725

milesj opened this issue Sep 11, 2014 · 6 comments

Comments

@milesj
Copy link
Contributor

milesj commented Sep 11, 2014

I receive the standard FormatString errors. I'm aware that this works:

sprintf('%s ago', '5 min');

But this does not.

$format = '%s ago'; sprintf($format, '5min');

Are there any plans to support this? I've tried forcing the type cast with (string) but no luck.

This is the type error for reference.

/vagrant/src/Titon/Utility/Format.hh:234:24,46: Invalid argument (Typing[4110])
  /tmp/hh_server_vagrant/hhi_1f54f164/printf.hhi:87:18,29: This is an object of type FormatString
  /vagrant/src/Titon/Utility/Format.hh:234:25,30: It is incompatible with a string
@paulbiss paulbiss added the hack label Sep 11, 2014
@milesj
Copy link
Contributor Author

milesj commented Sep 12, 2014

The same could be said for the inst_meth() and related functions.

/vagrant/src/Titon/Common/Attachable.hh:248:52,58: The argument to inst_meth() must be an expression and a constant literal string representing a valid method name. (Naming[2025])

@paulbiss
Copy link
Contributor

I'm going to let the hack folks elaborate more on this when they have a chance. I imagine the reason we require literals (and have no plans to support anything else) for these parameters is so that the functions can be safely typed.

Hack can't type-check the variadic arguments of sprintf without having the format string available, nor can it verify that an instance method exists and accepts a set of parameters or returns a particular value without having the name of the method available.

While the compiler doesn't require this type of static knowledge (the code is valid as far as hhvm is concerned), the type checker can't statically analyze types if it doesn't have the information readily available.

@jwatzman
Copy link
Contributor

Yep, @paulbiss is exactly right. This is so we can statically check printf and sprintf, a common source of errors. Similar logic applies to inst_meth and friends, which are actually totally useless if you can't statically check them.

If you really want unchecked versions -- which I think is very ill-advised -- you can create your own partial mode wrappers or something similar.

@milesj
Copy link
Contributor Author

milesj commented Sep 12, 2014

But correct me if I am wrong, you have the format string available, although it's through a variable. Couldn't you determine the type based on the variable? Or secondly, why wouldn't typing it to a string not provide what the checker needs?

How would you accomplish i18n then? For scenarios where message strings would be passed to sprintf() or something similar. There's so many situations where providing these values dynamically are the only approach.

@jwatzman
Copy link
Contributor

But correct me if I am wrong, you have the format string available, although it's through a variable. Couldn't you determine the type based on the variable?

We could, but this goes down the road of dependent types, which we don't want to cross if we can avoid it. If it's really a constant string, then just write it inline!

Or secondly, why wouldn't typing it to a string not provide what the checker needs?

We need to know what the actual value of the string is to parse it as a format string and know how many other parameters to sprintf there should be and what their types should be.

How would you accomplish i18n then?

FB does i18n through another mechanism. sprintf isn't expressive enough anyways due to issues with word order, etc etc. You need a real i18n framework :)

There's so many situations where providing these values dynamically are the only approach.

I disagree, but there's still nothing stopping you from writing an untyped wrapper, using UNSAFE, or similarly deliberately hiding what you're doing from the typechecker.

@milesj
Copy link
Contributor Author

milesj commented Sep 13, 2014

D: Guess I'll figure something out. Thanks for the info!

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