Hackificator tries to hackify PHARs

[simonwelsh]

Running the Hackificator in a folder that contains a phar causes it to try to convert the phar to Hack. Given that a phar tends to be a binary file, this fails rather spectularily.

[jwatzman]

Whoops, good catch! Not something I'm sure we'll have time to get to, but would be happy to accept a PR for this.

[ghost]

I'll work on that.

[steelbrain]

@lucastadeu, any updates?

[Stelian]

@lucastadeu are you still looking into this? I have some available time and could submit a PR for this.

[paulbiss]

Doubtful. @Stelian if you want to take over that would be awesome!

[Stelian]

@paulbiss Will have a look at it today and come up with a PR

[Stelian]

@paulbiss I need a little help here. The problem originates in https://github.com/facebook/pfff/blob/master/lang_php/parsing/lib_parsing_php.ml#L40 that also evaluates phars to true.

There are two ways of fixing this: one would be to add another function like is_php_not_phar that would be called either in is_php_file or in find_source_files_of_dir_or_files; the other one would be to just change the is_php_script itself to exclude phars.

Since I do not know of all the usage of the lib_parsing_php around, not sure which approach would be the sensitive one.

[paulbiss]

@Stelian I would avoid modifying the behavior of is_php_script, but @jwatzman knows this code much better than I do, so he might be able to shed some light on the cleanest approach here. Thanks for digging into this!

[jwatzman]

Given that pfff can't parse phar files anyways, I think modifying the behavior of is_php_file makes the most sense. I'm pretty sure I have permission to merge code into the pfff repo -- can you send a PR over there?

[Stelian]

@jwatzman I created a PR at facebookarchive/pfff#105 but I am not sure how to add tests to that particular usecase. I did test it on my box and it worked, but some automation would help.

[simonwelsh]

This appears to be fixed