Skip to content

Mention and recommend to use PHP-CS-Fixer when contributing code #8246

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed

Mention and recommend to use PHP-CS-Fixer when contributing code #8246

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5c06fe0
Mention and recommend to use PHP-CS-Fixer when contributing code
javiereguiluz Aug 1, 2017
a6fdc96
Fixed all reported issues
javiereguiluz Aug 3, 2017
7b31773
Fixed the link
javiereguiluz Aug 3, 2017
30e9e3e
Fixed the command again
javiereguiluz Aug 3, 2017
5def3f1
Minor fixes
javiereguiluz Jan 4, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 31 additions & 15 deletions contributing/code/standards.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,32 @@
Coding Standards
================

When contributing code to Symfony, you must follow its coding standards. To
make a long story short, here is the golden rule: **Imitate the existing
Symfony code**. Most open-source Bundles and libraries used by Symfony also
follow the same guidelines, and you should too.
Symfony code is contributed by thousands of developers around the world. To make
every piece of code look and feel familiar, Symfony defines some coding standards
that all contributions must follow.

Remember that the main advantage of standards is that every piece of code
looks and feels familiar, it's not about this or that being more readable.
These Symfony coding standards are based on the `PSR-0`_, `PSR-1`_, `PSR-2`_ and
`PSR-4`_ standards, so you may already know most of them.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you sure about it? both psr0 and psr4 ?


Symfony follows the standards defined in the `PSR-0`_, `PSR-1`_, `PSR-2`_ and `PSR-4`_
documents.
Making your Code Follow the Coding Standards
--------------------------------------------

Since a picture - or some code - is worth a thousand words, here's a short
example containing most features described below:
Instead of reviewing your code manually, Symfony makes it simple to ensure that
your contributed code matches the expected code syntax. First, install the
`PHP-CS-Fixer tool`_ and then, run this command to fix any problem:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The project name doesn't use dashes: it should be PHP CS Fixer.


.. code-block:: terminal

$ php php-cs-fixer.phar fix /path/to/project --rules=@Symfony
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Symfony project has a .php_cs.dist file that holds config for the tool (files to fix and rules to apply). Isn't it better to use this file instead of --rules=@Symfony?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it must use it's own config instead of manually provided ruleset, as @Symfony ruleset is the base, and then in config file there are adjustments, which could even be different for each branch like 2.8/3.1/3.3

Copy link
Member

@keradus keradus Aug 3, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

providing a path is also bad, as it will override path configured in .php_cs.dist file, leading to fixing even ignored files., command shall be just php php-cs-fixer.phar fix -v executed from project root dir

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are aware that this gonna provide bad UX ?
when you just clone the project and execute the command, it will find violations. why? because Symfony doesn't eats its own food and allowed to have violations originally never fixing all of them.

fabbot is configured to run PHP CS Fixer only on files modified in PR, if user will run it locally before sending a PR, whole project will be fixed.

While idea is nice and I'm 👍 for asking user to check the styles himself before sending a PR, I would do it only after Symfony repo would not raise CS violations itself.

Copy link
Member Author

@javiereguiluz javiereguiluz Aug 3, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we change it like this:

$ php php-cs-fixer.phar fix /path/to/modified/file.php

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then you would override finder from config and fix the file even if it is excluded via config.
$ php php-cs-fixer.phar fix --path-mode=intersection /path/to/modified/file.php would handle it properly, but this approach basically means you ask end-user to manually provide all modified files. still, bad UX
I would rather propose to keep command simple (to fix all, cache mechanism is on by default, so only first execution would be slower), but just fix project before providing this info in docs


If you forget to run this command and make a pull request with any syntax issue,
our automated tools will warn you about that and will provide the solution.

Symfony Coding Standards in Detail
----------------------------------

If you want to learn about the Symfony coding standards in detail, here's a
short example containing most features described below:

.. code-block:: html+php

Expand Down Expand Up @@ -122,7 +135,7 @@ example containing most features described below:
}

Structure
---------
~~~~~~~~~

* Add a single space after each comma delimiter;

Expand Down Expand Up @@ -181,7 +194,7 @@ Structure
* Do not use spaces around ``[`` offset accessor and before ``]`` offset accessor.

Naming Conventions
------------------
~~~~~~~~~~~~~~~~~~

* Use camelCase, not underscores, for variable, function and method
names, arguments;
Expand Down Expand Up @@ -223,7 +236,7 @@ Service Naming Conventions
* A group name uses the underscore notation.

Documentation
-------------
~~~~~~~~~~~~~

* Add PHPDoc blocks for all classes, methods, and functions (though you may
be asked to remove PHPDoc that do not add value);
Expand All @@ -234,14 +247,17 @@ Documentation

* Omit the ``@return`` tag if the method does not return anything;

* The ``@package`` and ``@subpackage`` annotations are not used.
* The ``@package`` and ``@subpackage`` annotations are not used;

* Inline the ``@inheritdoc`` tag.

License
-------
~~~~~~~

* Symfony is released under the MIT license, and the license block has to be
present at the top of every PHP file, before the namespace.

.. _`PHP-CS-Fixer tool`: https://github.com/FriendsOfPHP/PHP-CS-Fixer
.. _`PSR-0`: http://www.php-fig.org/psr/psr-0/
.. _`PSR-1`: http://www.php-fig.org/psr/psr-1/
.. _`PSR-2`: http://www.php-fig.org/psr/psr-2/
Expand Down