> we all copied it... I learned the idiom require_once( 'path.php' ); from
* My wiki's LocalSettings.php , generated by includes/installer/LocalSettingsGenerator.php * The LocalSettings.php of labs instances, in puppet labs-localsettings * The installation instructions for every extension on mediawiki.org All should change. php.net has a comment agreeing with this change: "it will prevent your peers from giving you a hard time and a trivial conversation about what require really is" :-) On Wed, May 8, 2013 at 5:26 PM, Krinkle <[email protected]> wrote: > 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 > [email protected] > https://lists.wikimedia.org/mailman/listinfo/wikitech-l -- =S Page software engineer on E3 _______________________________________________ Wikitech-l mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/wikitech-l
