It turns out that in both PHP and JS the following control structure isn't picked up as odd:
if ( foo ) {
bar()
} {
baz()
}… and baz() is always executed. (Found by me accidentally forgetting to write else into some code.)
Roan suggested this'd be worth sniffing and warning about. Thoughts?
| Subject | Repo | Branch | Lines +/- | |
|---|---|---|---|---|
| New sniff: `MissingElseBetweenBracketsSniff` | mediawiki/tools/codesniffer | master | +126 -0 |
https://3v4l.org/6SthD I guess in PHP you can just put brackets around a statement that don't do anything...? Are there any valid uses of this syntax feature?
Looks like an atavism from C where these worked as scope limiters, but they're pointless in PHP.
I don't know how easy this is to detect, but from an end result perspective this seems uncontroversial indeed. Obviously a mistake or obscure code we shouldn't commit as-is.
Could be as easy as checking for a } token directly followed by { (skipping optional whitespace). Is there any valid PHP syntax that is allowed to contain such a combination of tokens? I can't think of any, and can't find it in any of my local codebases. It appears in strings, e.g. "Handler {$method} {$spec['path']}", or in Mustache-like syntax. But this is not even tokenized.
Change 652397 had a related patch set uploaded (by Majavah; owner: Majavah):
[mediawiki/tools/codesniffer@master] New sniff: MissingControlStructureBetweenBracketsSniff
Change 652397 merged by jenkins-bot:
[mediawiki/tools/codesniffer@master] New sniff: MissingElseBetweenBracketsSniff