Squiz/PropertyDeclaration: add fixer for 'var' keyword

[klausi]

This pull request adds a fixer for the var keyword on class properties.

Description

We have been running this fixer for many years in Drupal's Coder https://github.com/pfrenssen/coder/blob/8.3.x/coder_sniffer/Drupal/Sniffs/Classes/PropertyDeclarationSniff.php#L59 .

The var keyword is equivalent to public, so we can replace it with that.

Suggested changelog entry

PSR2.Classes.PropertyDeclaration.VarUsed: Added a fixer that converts the var keyword into public.

Related issues/external references

https://github.com/pfrenssen/coder/blob/8.3.x/coder_sniffer/Drupal/Sniffs/Classes/PropertyDeclarationSniff.php

Types of changes

• New feature (non-breaking change which adds functionality)

PR checklist

• I have checked there is no other PR open for the same change.
• I have read the Contribution Guidelines.
• I grant the project the right to include and distribute the code under the BSD-3-Clause license (and I have the right to grant these rights).
• I have added tests to cover my changes.
• I have verified that the code complies with the projects coding standards.

[jrfnl]

@klausi Thanks for this PR.

Technically, the PR is perfectly fine as the var keyword is equivalent to a property being public.

However, the var keyword stems from a time that property visibility could not yet be declared on a property and quite often, the actual visibility of a property should not have been public, but protected or private. This is sometimes even annotated in the docblock via an annotation (@private).

In other words, what the visibility of a property should be, MUST be a developer decision.
And when this means the property visibility changes from implicitly public to explicitly private or protected, this may also need some careful planning as this could be a breaking chance and is likely to have to be timed for a major release.

Auto-fixing var properties to public hides this decision away and increases technical debt in a project by having property visibility set without a developer having reviewed the visibility is correct for the use-case.

For this reason, this error was/is not auto-fixable. This is also consistent with the PSR12/ConstantVisibility and the Squiz/MethodScope sniffs, neither of which auto-fix missing visibility to public as in all these cases, the developer should determine the intended visibility.

With this in mind, I propose closing the PR.

[klausi]

Thanks for the detailed explanation! I disagree a bit, but since the var keyword is not in wide use anyway I'm fine to close this.

[jrfnl]

@klausi Thanks for understanding and I'm with you on the "disagree a bit", but I'm trying to honour the original vision and premise of the PHP_CodeSniffer project and I'm fairly sure Greg would have rejected the PR for the above reasons.
I seem to recall having a discussion about this before in the old repo, but can't seem to find it at the moment.