Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
],
"require": {
"php": "^7.2 || ^8.0",
"phpstan/phpstan": "^1.7.0"
"phpstan/phpstan": "^1.7.3"
},
"conflict": {
"doctrine/collections": "<1.0",
Expand Down
40 changes: 24 additions & 16 deletions src/Rules/Doctrine/ORM/PropertiesExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace PHPStan\Rules\Doctrine\ORM;

use Doctrine\ORM\Mapping\ClassMetadataInfo;
use PHPStan\Reflection\PropertyReflection;
use PHPStan\Rules\Properties\ReadWritePropertiesExtension;
use PHPStan\Type\Doctrine\ObjectMetadataResolver;
Expand Down Expand Up @@ -47,26 +48,11 @@ public function isAlwaysWritten(PropertyReflection $property, string $propertyNa
return true;
}

if ($metadata->isIdentifierNatural()) {
return false;
}

if ($metadata->versionField === $propertyName) {
return true;
}

try {
$identifiers = $metadata->getIdentifierFieldNames();
} catch (Throwable $e) {
$mappingException = 'Doctrine\ORM\Mapping\MappingException';
if (!$e instanceof $mappingException) {
throw $e;
}

return false;
}

return in_array($propertyName, $identifiers, true);
return $this->isGeneratedIdentifier($metadata, $propertyName);
}

public function isInitialized(PropertyReflection $property, string $propertyName): bool
Expand All @@ -82,7 +68,29 @@ public function isInitialized(PropertyReflection $property, string $propertyName
return false;
}

if ($this->isGeneratedIdentifier($metadata, $propertyName)) {
return true;
}

return $metadata->isReadOnly && !$declaringClass->hasConstructor();
}

private function isGeneratedIdentifier(ClassMetadataInfo $metadata, string $propertyName): bool
{
if ($metadata->isIdentifierNatural()) {
return false;
}

try {
return in_array($propertyName, $metadata->getIdentifierFieldNames(), true);
} catch (Throwable $e) {
$mappingException = 'Doctrine\ORM\Mapping\MappingException';
if (!$e instanceof $mappingException) {
throw $e;
}

return false;
}
}

}
50 changes: 50 additions & 0 deletions tests/Rules/DeadCode/UnusedPrivatePropertyRuleTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\DeadCode;

use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use const PHP_VERSION_ID;

/**
* @extends RuleTestCase<UnusedPrivatePropertyRule>
*/
class UnusedPrivatePropertyRuleTest extends RuleTestCase
{

protected function getRule(): Rule
{
return self::getContainer()->getByType(UnusedPrivatePropertyRule::class);
}

public static function getAdditionalConfigFiles(): array
{
return [__DIR__ . '/../../../extension.neon'];
}

public function testRule(): void
{
if (PHP_VERSION_ID < 70400) {
self::markTestSkipped('Test requires PHP 7.4.');
}

$this->analyse([__DIR__ . '/data/unused-private-property.php'], [
[
'Property UnusedPrivateProperty\EntityWithAGeneratedId::$unused is never written, only read.',
23,
'See: https://phpstan.org/developing-extensions/always-read-written-properties',
],
[
'Property UnusedPrivateProperty\EntityWithAGeneratedId::$unused2 is unused.',
25,
'See: https://phpstan.org/developing-extensions/always-read-written-properties',
],
[
'Property UnusedPrivateProperty\ReadOnlyEntityWithConstructor::$id is never written, only read.',
53,
'See: https://phpstan.org/developing-extensions/always-read-written-properties',
],
]);
}

}
59 changes: 59 additions & 0 deletions tests/Rules/DeadCode/data/unused-private-property.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
<?php // lint >= 7.4

namespace UnusedPrivateProperty;

use Doctrine\ORM\Mapping as ORM;

/**
* @ORM\Entity
*/
class EntityWithAGeneratedId
{

/**
* @ORM\Id
* @ORM\GeneratedValue
* @ORM\Column
*/
private int $id; // ok, ID is generated

/**
* @ORM\Column
*/
private int $unused;

private int $unused2;

}

/**
* @ORM\Entity(readOnly=true)
*/
class ReadOnlyEntity
{

/**
* @ORM\Id
* @ORM\Column
*/
private int $id; // ok, entity is read only

}

/**
* @ORM\Entity(readOnly=true)
*/
class ReadOnlyEntityWithConstructor
{

/**
* @ORM\Id
* @ORM\Column
*/
private int $id;

public function __construct()
{
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Properties;

use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use const PHP_VERSION_ID;

/**
* @extends RuleTestCase<MissingReadOnlyByPhpDocPropertyAssignRule>
*/
class MissingReadOnlyByPhpDocPropertyAssignRuleTest extends RuleTestCase
{

protected function getRule(): Rule
{
return self::getContainer()->getByType(MissingReadOnlyByPhpDocPropertyAssignRule::class);
}

public static function getAdditionalConfigFiles(): array
{
return [__DIR__ . '/../../../extension.neon'];
}

public function testRule(): void
{
if (PHP_VERSION_ID < 70400) {
self::markTestSkipped('Test requires PHP 7.4.');
}

$this->analyse([__DIR__ . '/data/missing-readonly-property-assign-phpdoc.php'], [
[
'Class MissingReadOnlyPropertyAssignPhpDoc\EntityWithAGeneratedId has an uninitialized @readonly property $unassigned. Assign it in the constructor.',
25,
],
[
'@readonly property MissingReadOnlyPropertyAssignPhpDoc\EntityWithAGeneratedId::$doubleAssigned is already assigned.',
36,
],
[
'Class MissingReadOnlyPropertyAssignPhpDoc\ReadOnlyEntityWithConstructor has an uninitialized @readonly property $id. Assign it in the constructor.',
67,
],
]);
}

}
47 changes: 47 additions & 0 deletions tests/Rules/Properties/MissingReadOnlyPropertyAssignRuleTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Properties;

use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use const PHP_VERSION_ID;

/**
* @extends RuleTestCase<MissingReadOnlyPropertyAssignRule>
*/
class MissingReadOnlyPropertyAssignRuleTest extends RuleTestCase
{

protected function getRule(): Rule
{
return self::getContainer()->getByType(MissingReadOnlyPropertyAssignRule::class);
}

public static function getAdditionalConfigFiles(): array
{
return [__DIR__ . '/../../../extension.neon'];
}

public function testRule(): void
{
if (PHP_VERSION_ID < 80100) {
self::markTestSkipped('Test requires PHP 8.1.');
}

$this->analyse([__DIR__ . '/data/missing-readonly-property-assign.php'], [
[
'Class MissingReadOnlyPropertyAssign\EntityWithAGeneratedId has an uninitialized readonly property $unassigned. Assign it in the constructor.',
17,
],
[
'Readonly property MissingReadOnlyPropertyAssign\EntityWithAGeneratedId::$doubleAssigned is already assigned.',
25,
],
[
'Class MissingReadOnlyPropertyAssign\ReadOnlyEntityWithConstructor has an uninitialized readonly property $id. Assign it in the constructor.',
46,
],
]);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
<?php // lint >= 7.4

namespace MissingReadOnlyPropertyAssignPhpDoc;

use Doctrine\ORM\Mapping as ORM;

/**
* @ORM\Entity
*/
class EntityWithAGeneratedId
{

/**
* @ORM\Id
* @ORM\GeneratedValue
* @ORM\Column
* @readonly
*/
private int $id; // ok, ID is generated

/**
* @ORM\Column
* @readonly
*/
private int $unassigned;

/**
* @ORM\Column
* @readonly
*/
private int $doubleAssigned;

public function __construct(int $doubleAssigned)
{
$this->doubleAssigned = $doubleAssigned;
Copy link
Member

Choose a reason for hiding this comment

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

I just got an idea for a rule improvement in phpstan/src. If an extension says isInitialized=true about a property, even the first assignment should say "readonly property is already assigned" :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

interesting, I guess I can take a look at this in phpstan-src

$this->doubleAssigned = 17;
}

}

/**
* @ORM\Entity(readOnly=true)
*/
class ReadOnlyEntity
{

/**
* @ORM\Id
* @ORM\Column
* @readonly
*/
private int $id; // ok, entity is read only

}

/**
* @ORM\Entity(readOnly=true)
*/
class ReadOnlyEntityWithConstructor
{

/**
* @ORM\Id
* @ORM\Column
* @readonly
*/
private int $id;

public function __construct()
{
}

}
Loading