Our previous blog post covered tips on code style fixing while working on legacy projects. The blog post was very generic and the ideas are applicable on projects in any language. I mentioned some tips that help your peer developers review your pull requests. What I didn’t mention are code style standards and code style details.
In this blog post I would like to share some thoughts on code style details for cleaner diffs.
Cleaner diffs make it easier for your peer developers to review code changes. When your peers don’t get distracted by awkward diff lines you can expect a more thoughtful review, less overlooked bugs, and more honest feedback. Here are a couple of code style tips that allow for cleaner diffs.
When you’re appending new lines to a list, you might need to add a comma to the previous last item in order
to comply with the syntax. Some languages allow to append a comma to the last line without throwing syntax errors.
This first example has a clean diff when you’re appending a new item to the array initialisation:
This second example is less clean:
Is it not possible to add a comma to the last item of a block/array/… because of syntax limitations?
You can sometimes avoid appending to the back of the list by ordering the list items. That way you’ll mostly insert items in the middle and not at the end.
Are you a Composer user? It’s actually a bad practise to touch your
Therefore, use a special flag to have Composer sort your items for you:
composer require monolog/monolog --sort-packages.
Don’t align things like consecutive assignments (aligning equal signs) or list definitions (aligning arrows or semicolons). When the longest line changes in length, or when a new longer line is added, you will need to adjust the other lines by adding or removing spaces, resulting in larger diffs. If you didn’t align them, it’s more clear on what has been changed or added.
This is just a suggestion, though. People like to align assignments for various reasons. Because it’s pretty, or because it’s visually easier to scan a file. If you like the benefits of aligning over the downside of the heavier diffs, go ahead: align.
It’s sometimes possible to leave off certain braces. For example: calling a constructor in PHP when passing zero arguments is possible without braces
$client = new GuzzleHttpClient; but when you need to change that and need to pass one or more arguments to the constructor, you’ll still need to add the braces. You might notice I’m nitpicking here, this doesn’t actually result in super ugly diffs. But what about when you delete the last argument of a constructor? Do you delete the braces from every existing constructor call? If not, you’ll end up with inconsistent notation.
Often language syntax allows you to use control structures (if, for, while, …) without braces as well: they just use the single next line as the code block to execute conditionally, if no braces are used. Apart from the risk of bugs this also results in harder diffs when you add lines, or inconsistencies when you remove lines but don’t remove the braces.
Have your editor and code style tool be clear on whitespace: where to have how many invisible characters (spaces, tabs, new lines). EditorConfig is an editor independent config file that can help you with that. In PHP we like to remove trailing whitespace at the end of a line, and empty lines must be completely empty. We also like to have a single empty line at the end of every
When a diff contains whitespace changes one can often ignore those lines. Use
git diff -bw (short for
git diff --ignore-space-change --ignore-all-space) or add
?w=0 in the address bar when viewing a diff on GitHub.
The php-cs-fixer tool has some options to guard some of the tips mentioned in this post previously.
multiline_array_trailing_commafixer can be used to append commas everywhere, but is never enabled by default.
new_with_bracesfixer is not enabled on the PSR-2 level, but it is on the Symfony level.
phpdoc_paramsis included in the Symfony level, it aligns docblock items so I like to disable it instead.
align_equalsoptions are available, but I rather leave them off.
Of course, these aren’t golden rules. I just have my reasons to enable or disable them. Define the rules you like for your projects and make sure to run the code style fixer regularly. For example before every commit, when your code base already adheres to all those rules, or after every commit on the files that have been changes, to slowly move to a cleaner code base.
Happy CS fixing!