doctrine / coding-standard Goto Github PK
View Code? Open in Web Editor NEWThe coding standard of the Doctrine project.
Home Page: https://www.doctrine-project.org/projects/coding-standard.html
License: MIT License
The coding standard of the Doctrine project.
Home Page: https://www.doctrine-project.org/projects/coding-standard.html
License: MIT License
Since the auto fixer for this rule sets the visibility to public, this may cause constants to get more visibility then really needed.
If they aren't auto fixed, the problem becomes more 'in your face', requiring you to consider what the exact visibility is that you need.
Version: 7.0.2
There is a problem using intersection types for type declaration in annotations.
These are valid since PHPStorm 2018.3.
https://blog.jetbrains.com/phpstorm/2018/09/phpstorm-2018-3-eap-183-2635-12/#intersection-types
Given the following code
/**
* @param ContainerInterface&ServiceManager $container
*/
public function __invoke(ContainerInterface $container) : Fubar {}
Expected:
no problem
Actual:
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
XX | ERROR | [x] Expected 4 spaces after parameter type; 0 found
Using phpcbf
to fix the issue resulted in this line:
* @param ContainerInterface &ServiceManager $container
๐๐พ there has been a lot of new sniffs added and options tweaked. Could we maybe tag a new version? Currently, I have nothing on my mind I'd like to contribute on top what's already here in the near future.
Or is there something I can do to speed it up?
Is this correct?
https://github.com/doctrine/coding-standard/pull/205/checks?check_run_id=788441821
As you can see on this job there are differences but the job is success.
Is this addressed here? https://github.com/doctrine/coding-standard/issues
Since PSR-2 has been deprecated are there any plans change the ruleset base to PSR-12 or integrate any of the new/changed features from the ruleset?
Why my phpcs print :
"phpcs: ERROR: Referenced sniff "SlevomatCodingStandard.Arrays.SingleLineArrayWhitespace" does not exist Run "phpcs --help" for usage information"
my errors when configured phpcs or error in the package?
Hi... I recently opened this #199
Instead of going wild and opening one by one a PR I thought about initiating a discussion here.
I believe there are more functions that can be forbidden:
'__autoload' => null, // no reason to use this function since composer is out there
'define' => null,
'each' => null, // @see http://php.net/manual/en/migration72.deprecated.php
'create_function' => null, // why using this?
// debug functions
'debug_backtrace' => null,
'debug_print_backtrace' => null,
'die' => null,
'exit' => null,
'phpinfo' => null,
'print_r' => null,
'var_dump' => null,
// Use a logger lib (preferably a PSR-3 one)
'error_log' => null,
// Framework debug functions
'dd' => null,
'dump' => null,
// use an HttpClient library instead.
'curl_exec' => null,
'curl_init' => null,
'curl_multi_exec' => null,
// Use a process library..
'exec' => null,
// The following functions should not be allowed inside a project and they should be set by your php.ini unless if you are building a very specific library.
'ini_alter' => null,
'ini_restore' => null,
'ini_set' => null,
'date_default_timezone_set' => null,
'apache_request_headers' => null, // Exists only on Apache webservers
'apache_response_headers' => null, // Exists only on Apache webservers
'getallheaders' => null, // Is an alias of apache_request_headers()
'putenv' => null, // use $_SERVER or $_ENV variable directly // https://github.com/laravel/framework/issues/7354
'getenv' => null,
ofc users can override this on their projects but it would be good to promote these as a good practice.
Let me know WDYT.. maybe discussing all this in one issue might be an overhead and I should open smaller ones?
I know that mostly this is checked by phpstan/psalm, but I think it will be good to double check this on phpcs level
I think these rules are the same.
If it is correct
<rule ref="PSR12">
<!-- checked by SlevomatCodingStandard.Classes.ClassConstantVisibility -->
<exclude name="PSR12.Properties.ConstantVisibility"/>
</rule>
I would like to start discussion on adding these rules to doctrine CS:
<rule ref="WordPress.Arrays.ArrayDeclarationSpacing">
<exclude name="WordPress.Arrays.ArrayDeclarationSpacing.NoSpaceAfterArrayOpener"/>
<exclude name="WordPress.Arrays.ArrayDeclarationSpacing.NoSpaceBeforeArrayCloser"/>
<exclude-pattern>tests/*</exclude-pattern>
</rule>
I got this idea when checking https://github.com/doctrine/orm/pull/10126/files#diff-71c09c517db3322ed613e4cdf6cd41ea2323bf44c1677d3c249dd83da4fbeb25R29-R30
The main argument for this change is that together with SlevomatCodingStandard.Arrays.TrailingArrayComma.MissingTrailingComma
which is already applied this makes for much cleaner diffs when making changes.
I also prefer:
#[DiscriminatorMap([
'cc' => Component\ConcreteComponent::class,
'cd' => Decorator\ConcreteDecorator::class,
])]
abstract class Component
over
#[DiscriminatorMap(['cc' => Component\ConcreteComponent::class,
'cd' => Decorator\ConcreteDecorator::class])]
abstract class Component
WDYT?
such as:
I do not believe that the coding standard should enforce the usage of union types over the nullable ones. From the original discussion in doctrine/orm#9886 (comment):
Personally, I believe arbitrary union types (e.g.
getValue(sting $key): int|string|false
) often reflect poor API design while nullable types are fine (e.g.findUser(int $id): ?User
).I think that enforcing a feature introduced primarily to support the existing poor design of the PHP's own API and the type system as a one-size-fits-all solution is not the right move.
[...]
IMO, union types are often poor because they make the consumer deal with a set of types instead of one by adding conditionals. Without pattern matching, it's error-prone since there's no guarantee that every consumer will handle each type from the union.
Specifically, in the PHP's standard library, union types go hand in hand with the APIs that are known as extremely error-prone, e.g.
if (strpos($str, substr)) ...
) orwhile ($value = $stmt->fetch(PDO::FETCH_COLUMN)) ...
. Union types are also needed to deal with array keys because there's no built-in support for maps in PHP and numeric array keys are converted to strings.All the
int|string
types used in the code that fetches data from the database are because there's no support for unsigned integers and decimals in PHP. Otherwise, it would be possible to implement an API that returns a specific type.Looking at the DBAL code, the only place I see where a union type is used deliberately is the expression builder where the methods accept strings or expressions but even there they could be split into two: one for stings, the other for expressions.
This is a release blocker for 7.0: currently we require a dev-master commit of slevomat/coding-standard. This needs to be changed to a stable release before releasing 7.0.
Consider the code example:
<?php
declare(strict_types=1);
namespace Test;
class Test
{
/**#@+
* Error codes.
*/
public const ERR_1 = 1;
public const ERR_2 = 2;
/**#@-*/
public function doStuff() : void
{
}
}
If a phpDoc template (example) is followed by a method without a docblock, then the "Squiz.Commenting.FunctionComment" sniff mistakenly attributes the closing template block to the method and marks it as invalid:
$ phpcs Test.php
------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------
14 | ERROR | There must be no blank lines after the function comment
| | (Squiz.Commenting.FunctionComment.SpacingAfter)
------------------------------------------------------------------------------------------------
Although this specific rule is implemented using a component from the "Squiz" standard,
coding-standard/lib/Doctrine/ruleset.xml
Line 436 in 931c039
As per squizlabs/PHP_CodeSniffer#2838 (comment), the "Squiz" standard enforces docblocks and doesn't support docblock templates.
I'm filing the issue here to see if this specific rule can be reimplemented using some Slevomat building blocks which are better composable by design.
While working on applying this standard on doctrine/orm
, we found with @SenseException that the following will not raise any error:
// declarations
class Foo
{
/** @var int */
private $bar;
/** @var int */
private $baz;
}
class FooBar
{
}
We would expect errors about the lack of space between property declarations
and class declarations. Does anyone know of a sniff that we could integrate to
doctrine/coding-standard
for this? @kukulich maybe?
Note that in PHP 7.4, I personally would be ok with a lack of space between property declarations when they fit on one line.
I have the following method
public function foo(Collection $collection)
{
$collection->add($this->bar);
$collection->add($this->bar);
if ($collection instanceof BOOM) {
$collection->add($this->some)
}
}
which reports an error because I didn't use an early exit.
But changing
if ($collection instanceof BOOM) {
$collection->add($this->some)
}
to
if (! $collection instanceof BOOM) {
return;
}
$collection->add($this->some)
is taking more lines and not really reducing complexity.
Also, I may change later to
public function foo(Collection $collection)
{
$collection->add($this->bar);
$collection->add($this->bar);
if ($collection instanceof Boom) {
$collection->add($this->some)
}
if ($collection instanceof Please) {
$collection->add($this->thanks)
}
}
and so on.
The slevomat Early Exit standard has some configuration available
https://github.com/slevomat/coding-standard#slevomatcodingstandardcontrolstructuresearlyexit-
I would say it's better to use some.
Functionally this is not a problem, however, if the rule should ever be removed the other one may be left in the ruleset (and cause an headache when trying to figure out why nothing was affected).
First instance:
coding-standard/lib/Doctrine/ruleset.xml
Lines 255 to 256 in db39894
Second instance:
coding-standard/lib/Doctrine/ruleset.xml
Lines 493 to 494 in db39894
Currently installing doctrine/coding-standard is not possible if we are using on root project squizlabs/php_codesniffer: 4.0.x-dev
williamdes@williamdes:/mnt/Dev/@phpmyadmin/coding-standard$ composer2 update
The "dealerdirect/phpcodesniffer-composer-installer" plugin was skipped because it requires a Plugin API version ("^1.0") that does not match your Composer installation ("2.0.0"). You may need to run composer update with the "--no-plugins" option.
Loading composer repositories with package information
Updating dependencies
Your requirements could not be resolved to an installable set of packages.
Problem 1
- dealerdirect/phpcodesniffer-composer-installer v0.6.2 requires composer-plugin-api ^1.0 -> found composer-plugin-api[2.0.0] but it does not match the constraint.
- doctrine/coding-standard 8.0.0 requires dealerdirect/phpcodesniffer-composer-installer ^0.6.2 -> satisfiable by dealerdirect/phpcodesniffer-composer-installer[v0.6.2].
- Root composer.json requires doctrine/coding-standard ^8.0 -> satisfiable by doctrine/coding-standard[8.0.0].
You are using a snapshot build of Composer 2, which some of your plugins seem to be incompatible with. Make sure you update your plugins or report an issue to them to ask them to support Composer 2. To work around this you can run Composer with --ignore-platform-req=composer-plugin-api, but this may result in broken plugins and bigger problems down the line.
williamdes@williamdes:/mnt/Dev/@phpmyadmin/coding-standard$ composer2 --version
The "dealerdirect/phpcodesniffer-composer-installer" plugin was skipped because it requires a Plugin API version ("^1.0") that does not match your Composer installation ("2.0.0"). You may need to run composer update with the "--no-plugins" option.
Composer version 2.0.0-alpha2 2020-06-24 21:36:18
williamdes@williamdes:/mnt/Dev/@phpmyadmin/coding-standard$
Fix: allow 0.7.0 (https://github.com/Dealerdirect/phpcodesniffer-composer-installer/releases/tag/v0.7.0)
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.