Code Sniffer Rules

[jim-parry]

Looking for someone familiar with PHP_CodeSniffer to create a ruleset matching our style guide.
This is initially intended for use inside an IDE, and ultimately perhaps as part of our travis-ci build.

[louisl]

Hi,

I've previously built sniffers for CodeIgniter 2 and 3 in PHP_CodeSniffer 2 which included phpcbf functionality, I've been using these pretty much daily in SublimeText as linters and fixing up old code for a few years.

I never announced these publicly as I included a bunch of personal preferences and best practices which were being argued about at the time, these were mostly commenting, spacing alignment, string concatenation to name a few.

[deprecated link removed]

I'm currently in the process of converting these to PHP_CodeSniffer 3 and I have a basic working CI 4 version.

You can't make these with just a ruleset.xml as some CodeIgniter style guidelines are not available anywhere within the existing PHP_CodeSniffer sniffs so I have some sniffs created purely for CodeIgniter 2, 3 and 4.

I have some questions about interpreting the new style guide and I guess it could change a bit if needed?, how would you like me to proceed?, should I ask here or would you like to start a thread on the forum?, my username there is also 'louisl'.

Regards

Louis

[lonnieezell]

Hey Louis, that would be awesome! I started to look at it a week or so ago, realizing I would need to write sniffs for it and put that aside, since I don't have any experience with that :)

Post your questions here, that's fine. If we open it up on the forums we get too many opinions lol. Or you could simply shoot me an email and we could discuss it there.

[louisl]

OK, lets get the ball rolling.
https://bcit-ci.github.io/CodeIgniter4/contributing/styleguide.html

The example titled 'Good control structure examples:'

if ( $foo )
{
    $bar += $baz;
}
else
{
    $baz = 'bar';
}

conflicts with:
Other

• Opening parentheses SHOULD NOT be followed by a space character.
• Closing parentheses SHOULD NOT be preceeded by a space character. (1 'e' in preceded BTW)

So we want this right?

if ($foo)
{
    $bar += $baz;
}
else
{
    $baz = 'bar';
}

[lonnieezell]

Good catch. You're correct. There should not be spaces inside the parentheses.

[louisl]

I've run CodeIgniter4 through and it's showing up a ton of inconsistencies... Lots of space indenting, tab aligning, wrong indenting, mixed tab and space indenting, non visibility declared functions, spaces missing around operators, upper case keywords, missing newline at end of file, OR, AND !=, ==, if(,

} else
{

to name a few.

Thats good though right?, proof we're getting somewhere! Most of it's fixable with the phpcbf.

OK some questions...

It's not mentioned in the current style guide, but am I ok to require === and !== (good practice). There's a mixture in current CodeIgniter4.

Quote marks for strings. - I think we should to pick one or another, my preference is single quotes unless containing a single quote then I use double quotes. Current CodeIgniter4 is mostly using single quotes. (Already have the sniff with fixer made for this too).

String concatenation - guidelines are suggesting no space around ., however spaces around operators and such are the norm elsewhere in the guide. Current CodeIgniter4 has a mixture of both. (I vote spaces around but either is fine).

[lonnieezell]

And that's exactly why we need this! :)

1. I'm fine with requiring strict comparisons. The only issue I can think of is if you want a possible type conversion (like testing for truthiness) and we can fix that by being a little more specific, I suppose. Not sure if there's anywhere in the framework that would affect, though.

2. Quote marks. I've always preferred single in the past. According to stats from Blackfire, string concatenation is actually faster within double-quotes now, so double-quotes makes sense from that micro-optimization level. :) But since my habit is single quotes and I'm the primary, I guess we go with that. It won't automatically change those, I assume.

3. Concatenation. You're right it says no spaces. My habit before working on this was more complex. no space around the string portion, space between variables, but we can't make a rule for that I"m sure lol. Not sure on this. I'd be tempted to leave as is, but I have no specific reason for that. @jim-parry what are your thoughts on this?

[jim-parry]

I have always used spaces around concatenation operators, in both PHP and Java. It makes more sense to me.

[lonnieezell]

Then let's go with a space around it to maintain consistency.

[louisl]

Preview version ;)

https://github.com/louisl/CodeIgniter4-Standard

Have a few issues to sort out but it's linting well and fixing most errors, although there's currently a problem with PHP_Codesniffer 3.0.1 phpcbf not saving files.

[louisl]

Couple of updates, grab the latest develop branch.

• It's now checking snake case functions (out of scope ie helpers).
• Fixed an error with file comment missing being flagged when it wasn't missing.

TODO

• Php tags (allowing '<?=') is this just going to be for views?
• View syntax checks
• Vertical function spacing.
• Eliminate blank lines after '{' opener.
• Composer install.

The core of it's done. I've taken a mix of CodeIgniter 3, CodeIgniter 4 and PSR2 guides, consistencies in the CodeIgniter 4 code and some general good practices to shape it so far, so let me know what you think.

[louisl]

OK, it's been added to composer and install instructions have been done.

[lonnieezell]

It's looking pretty good so far! Thanks so much for doing this. The only that stood out to me is you seem to have both lowerCaseConstants and UpperCaseConstant rules listed in ruleset.xml?

[louisl]

No problem,

Generic.PHP.LowerCaseConstant Checks TRUE, FALSE, NULL are lower case.
Generic.NamingConventions.UpperCaseConstantName Checks user defined constants are upper case.

[lonnieezell]

Oh, that makes sense, then. Great.