phpbb / phpbb-translation-validator

A tool checking packages for compliance with the policies https://area51.phpbb.com/docs/dev/3.2.x/language/guidelines.html
GNU General Public License v2.0
8 stars 9 forks source link

False fatal error (File must not contain a PHP closing tag, but end with one new line) #29

Closed ghost closed 8 years ago

ghost commented 8 years ago

I applied StyleCI guidelines as coding standards. It replaced, for example, array() elements with []. Note that's a language pack compatible with phpBB 3.2, which is fine because phpBB 3.2 requires > PHP 5.6. My language pack for phpBB 3.1 is untouched to be compatible with PHP 5.3.3.

However, the Translation Validator wrongly returns that four files, _helpbbcode.php, _helpfaq.php, _search_ignorewords.php and _searchsynonyms.php, must not contain a PHP closing tag, but end with one new line, but these files don't contain any PHP closing tag, and end with a new line.

It causes a fatal failure and wi*ll lead to an automatic rejection in the Customisation Database, which is really bad. Note that all the other files are not affected with this "bug".

You can see the issue here: https://travis-ci.org/maelsoucaze/phpbb/jobs/99797166#L148-L158

nickvergessen commented 8 years ago

help_bbcode.php and help_faq.php don't really matter. They are going to be removed anyway.

For the 5.4 array syntax, that needs a patch in https://github.com/phpbb/phpbb-translation-validator/blob/2ae856d34a0e2575a69afd4dd360bd1a19f7b413/src/Phpbb/TranslationValidator/Validator/FileValidator.php#L594-L595

Elsensee commented 8 years ago

@nickvergessen 5.4 array syntax should still work with this line since it will be something like: $lang = array_merge($lang, [...]);

@maelsoucaze Looks like everything is fine again? Do you know what the issue was?

ghost commented 8 years ago

@Elsensee No, he's right, these four files do not use $lang = array_merge($lang, [...]); but just $help = [...]; $words = [...]; or $synonyms = [...];.

It's fine right now because I reverted these changes, as I still don't know if I'm allowed to apply them without knowing if my language pack will be denied or not. So I applied some rules to pass StyleCI by following phpBB Coding Style Guidelines in the same time (and use long array syntax).

Elsensee commented 8 years ago

Ah right. So I guess the phpBB coding guidelines can be interpreted like this is allowed. As you said, there is no PHP ending tag and a newline.

nickvergessen commented 8 years ago

Merged, but I dont plan to release a new version anytime soon, if you still need it, feel free to patch it yourself or use the github version

ghost commented 8 years ago

The phpBB.com's CDB is using dev-master? If not, a new version should be released indeed to not be affected by this issue when phpBB 3.2 will be released. Thanks @Elsensee @nickvergessen!

nickvergessen commented 8 years ago

Will release it before the final, but until then we want to collect issues

ghost commented 8 years ago

Okay, no problem in that case, thanks!