On 2013-05-08 9:26 PM, "Krinkle" krinklemail@gmail.com wrote:
Hi,
Since there appears to have been a little bit of trivia
Hi,
Since there appears to have been a little bit of trivia around fixing these phpcs warnings, I'll open a thread instead.
Both in javascript and PHP there are various keywords that can be used as if they are functions. In my opinion this is a misuse of the language and only causes confusion.
I'm referring to code like this (both javascript and php):
delete( mw.legacy ); new( mw.Title ); typeof( mw ); echo( $foo . $bar ); print( $foo . $bar ); return( $foo . $bar );… and, wait for it..
require_once( $foo . $bar );I think most experienced javascript developers know by now that using "delete" or "new" like it is a function is just silly and looks like you don't know what you're doing.
To give a bit of background, here's why these work at all (they aren't implemented both keywords and functions, just keywords). Though I'm sure the implementation details differ between PHP and javascript, the end result is the same: Keywords are given expressions which are then evaluated and the result is used as value. Since expressions can be wrapped in parenthesis for readability (or logic grouping), and since whitespace is insignificant to the interpreter, it is possible to do `return("test")`, which really is just `return ("test")` and eventually `return "test"`.
I'm obviously biased, but I think the same goes for "require_once" (and "include", "require" etc.). Right now this is causing quite a few warnings in our php-checkstyle report.
I didn't disable that rule because it appears (using our code base as status quo) that we do want this. There's 0 warnings I could find in our code base that violate this, except for when the keyword is include|require(_once)?
The check style sniffer does not (and imho should not) have a separate rule per keyword. Either you use constructs like this, or you don't.
But let's not have some weird exception just because someone didn't understand it[1] and we all copied it and want to keep it for no rational reason.
Because that would mean we have to either hack the sniffer to exclude this somehow, or we need to disable the rule, thus not catching the ones we do use.
See pending change in gerrit that does a quick pass of (most of) these in mediawiki/core:
https://gerrit.wikimedia.org/r/62753
-- Krinkle
[1] Or whatever the reason is the author originally wrote it like this. Perhaps PHP was different back then, or perhaps there was a different coding style.
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l