Skip to content

Commit

Permalink
Address level 4 PHPStan issues
Browse files Browse the repository at this point in the history
Doctrine extention was added to the Stan setup. Preventing unused
field warnings.

Three warnings have been ignored. They are regarding a hidden use of
three RegistraoinAuthority fields. Static analysis does not evaluate
that they are written when persisted to db storage

The rest of the fixes are mainly removal of unused code fragments. Or
removal of redundant sections of logical expressions.
  • Loading branch information
MKodde committed Mar 21, 2024
1 parent 402a02a commit acb6a8d
Show file tree
Hide file tree
Showing 35 changed files with 170 additions and 292 deletions.
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
"overtrue/phplint": "*",
"phpmd/phpmd": "^2.15",
"phpstan/phpstan": "^1.11.x-dev",
"phpstan/phpstan-doctrine": "^1.3",
"phpstan/phpstan-mockery": "^1.1",
"phpstan/phpstan-symfony": "^1.3",
"phpunit/phpunit": "^9.5",
Expand Down
74 changes: 73 additions & 1 deletion composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

29 changes: 7 additions & 22 deletions src/Surfnet/Stepup/DateTime/DateTime.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
use DateTime as CoreDateTime;
use Stringable;
use Surfnet\Stepup\Exception\InvalidArgumentException;
use TypeError;

/**
* @SuppressWarnings(PHPMD.TooManyMethods)
Expand All @@ -37,14 +38,15 @@ class DateTime implements Stringable

/**
* Allows for mocking of time.
*
* @see DateTimeHelper::setCurrentTime here you can see how now can be overridden using reflection
* @var self|null
*/
private static ?DateTime $now;
private static ?DateTime $now; // @phpstan-ignore-line PHPStan can not see that the DateTimeHelper is able to set the now value using reflection

private readonly CoreDateTime $dateTime;

/**
* @see DateTimeHelper::setCurrentTime here you can see how now can be overridden using reflection
* @return self
*/
public static function now(): DateTime
Expand Down Expand Up @@ -118,34 +120,17 @@ public function comesBeforeOrIsEqual(DateTime $dateTime): bool

public function comesAfter(DateTime $dateTime): bool
{
$end = $this->dateTime->getTimestamp();
$start = $dateTime->dateTime->getTimestamp();
return $end > $start;
return $this->dateTime > $dateTime->dateTime;
}

public function comesAfterOrIsEqual(DateTime $dateTime): bool
{
return $this->dateTime >= $dateTime->dateTime;
}

/**
* @param $format
* @return string
*/
public function format($format): string
public function format(string $format): string
{
$formatted = $this->dateTime->format($format);

if ($formatted === false) {
throw new InvalidArgumentException(
sprintf(
'Given format "%s" is not a valid format for DateTime',
$format,
),
);
}

return $formatted;
return $this->dateTime->format($format);
}

/**
Expand Down
24 changes: 4 additions & 20 deletions src/Surfnet/Stepup/Identity/AuditLog/Metadata.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,36 +39,20 @@ final class Metadata
*/
public Institution $identityInstitution;

/**
* @var Institution
*/
public Institution $raInstitution;
public ?Institution $raInstitution;

/**
* @var SecondFactorId|null
*/
public ?SecondFactorId $secondFactorId;

/**
* @var SecondFactorType|null
*/
public ?SecondFactorType $secondFactorType;

/**
* @var SecondFactorIdentifier|null
*/
public ?SecondFactorIdentifier $secondFactorIdentifier;

/** @var VettingType */
public VettingType $vettingType;
public ?VettingType $vettingType;

/**
* @var RecoveryTokenId
*/
public RecoveryTokenId $recoveryTokenId;
public ?RecoveryTokenId $recoveryTokenId;

/**
* @var RecoveryTokenType
*/
public RecoveryTokenType $recoveryTokenType;
public ?RecoveryTokenType $recoveryTokenType;
}
6 changes: 3 additions & 3 deletions src/Surfnet/Stepup/Identity/Entity/RegistrationAuthority.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ final class RegistrationAuthority extends SimpleEventSourcedEntity
{
private ?RegistrationAuthorityRole $role = null;

private ?Location $location = null;
private ?Location $location = null; // @phpstan-ignore-line PHPStan can not see that this field is written when serialized to the database

private ?ContactInformation $contactInformation = null;
private ?ContactInformation $contactInformation = null; // @phpstan-ignore-line PHPStan can not see that this field is written when serialized to the database

private ?Institution $institution = null;
private ?Institution $institution = null; // @phpstan-ignore-line PHPStan can not see that this field is written when serialized to the database

public static function accreditWith(
RegistrationAuthorityRole $role,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,7 @@ class SecondFactorVettedEvent extends IdentityEvent implements Forgettable, Righ
*/
public Locale $preferredLocale;

/** @var VettingType */
public VettingType $vettingType;
public ?VettingType $vettingType;

/**
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,7 @@ class SecondFactorVettedWithoutTokenProofOfPossession extends IdentityEvent impl
*/
public Locale $preferredLocale;

/** @var VettingType */
public VettingType $vettingType;
public ?VettingType $vettingType;

/**
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
Expand Down
14 changes: 8 additions & 6 deletions src/Surfnet/Stepup/Identity/Identity.php
Original file line number Diff line number Diff line change
Expand Up @@ -821,19 +821,21 @@ public function complyWithSecondFactorRevocation(SecondFactorId $secondFactorId,
public function revokeRecoveryToken(RecoveryTokenId $recoveryTokenId): void
{
$this->assertNotForgotten();
$recoveryToken = $this->recoveryTokens->get($recoveryTokenId);
if (!$recoveryToken) {
throw new DomainException('Cannot revoke recovery token: no token with given id exists.');
try {
$recoveryToken = $this->recoveryTokens->get($recoveryTokenId);
} catch (DomainException $e) {
throw new DomainException('Cannot revoke recovery token: no token with given id exists.', 0, $e);
}
$recoveryToken->revoke();
}

public function complyWithRecoveryTokenRevocation(RecoveryTokenId $recoveryTokenId, IdentityId $authorityId): void
{
$this->assertNotForgotten();
$recoveryToken = $this->recoveryTokens->get($recoveryTokenId);
if (!$recoveryToken) {
throw new DomainException('Cannot revoke recovery token: no token with given id exists.');
try {
$recoveryToken = $this->recoveryTokens->get($recoveryTokenId);
} catch (DomainException $e) {
throw new DomainException('Cannot revoke recovery token: no token with given id exists.', 0, $e);
}
$recoveryToken->complyWithRevocation($authorityId);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public function assertRegistrationOfSelfAssertedTokensIsAllowed(IdentityId $iden
}
// The Identity is not allowed to do a SAT when he had a RT, but lost it. And also currently has no SF
$hasActiveRecoveryToken = $this->recoveryTokenService->identityHasActiveRecoveryToken($identity);
if ($options->possessedSelfAssertedToken && !$hasActiveRecoveryToken && !$hasVettedSecondFactorToken) {
if ($options->possessedSelfAssertedToken && !$hasActiveRecoveryToken) {
return $this->deny('Identity lost both Recovery and Second Factor token, SAT is not allowed');
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ final class InstitutionAuthorizationProjector extends Projector
{
public function __construct(
private readonly InstitutionAuthorizationRepository $institutionAuthorizationRepository,
private readonly InstitutionConfigurationOptionsRepository $institutionConfigurationOptionsRepository,
) {
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,7 @@ public function applyInstitutionConfigurationRemovedEvent(InstitutionConfigurati
$this->repository->removeRaLocationsFor($event->institution);
}

/**
* @return RaLocation
*/
private function fetchRaLocationById(RaLocationId $raLocationId)
private function fetchRaLocationById(RaLocationId $raLocationId): RaLocation
{
$raLocation = $this->repository->findByRaLocationId($raLocationId);

Expand All @@ -101,12 +98,6 @@ private function fetchRaLocationById(RaLocationId $raLocationId)
);
}

if (!$raLocation instanceof RaLocation) {
throw new RuntimeException(
'Tried to update an RA Locations contact information, but location is of the wrong type',
);
}

return $raLocation;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,9 @@ public function __construct(ManagerRegistry $registry)
}

/**
* @return InstitutionConfigurationOptions
* @throws NonUniqueResultException
*/
public function findConfigurationOptionsFor(Institution $institution)
public function findConfigurationOptionsFor(Institution $institution): ?InstitutionConfigurationOptions
{
return $this->createQueryBuilder('ico')
->where('ico.institution = :institution')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,7 @@ public function search(RaLocationQuery $query)
->getResult();
}

/**
* @return RaLocation[]
*/
public function findByRaLocationId(RaLocationId $raLocationId)
public function findByRaLocationId(RaLocationId $raLocationId): ?RaLocation
{
return $this->createQueryBuilder('rl')
->where('rl.id = :id')
Expand All @@ -84,7 +81,7 @@ public function remove(RaLocation $raLocation): void
/**
* @return RaLocation[]
*/
public function findByInstitution(Institution $institution)
public function findByInstitution(Institution $institution): array
{
return $this->createQueryBuilder('rl')
->where('rl.institution = :institution')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,11 @@
use Surfnet\Stepup\Configuration\Value\Institution;
use Surfnet\StepupMiddleware\ApiBundle\Configuration\Entity\AllowedSecondFactor;
use Surfnet\StepupMiddleware\ApiBundle\Configuration\Repository\AllowedSecondFactorRepository;
use Surfnet\StepupMiddleware\ApiBundle\Configuration\Repository\ConfiguredInstitutionRepository;

class AllowedSecondFactorListService
{
public function __construct(
private readonly AllowedSecondFactorRepository $allowedSecondFactorRepository,
private readonly ConfiguredInstitutionRepository $configuredInstitutionRepository,
) {
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,12 @@ public function __construct(private readonly RaLocationRepository $repository)
/**
* @return null|RaLocation[]
*/
public function search(RaLocationQuery $query)
public function search(RaLocationQuery $query): ?array
{
return $this->repository->search($query);
}

/**
* @return RaLocation[]
*/
public function findByRaLocationId(RaLocationId $raLocationId)
public function findByRaLocationId(RaLocationId $raLocationId): ?RaLocation
{
return $this->repository->findByRaLocationId($raLocationId);
}
Expand All @@ -50,7 +47,7 @@ public function findByRaLocationId(RaLocationId $raLocationId)
/**
* @return RaLocation[]
*/
public function listRaLocationsFor(Institution $institution)
public function listRaLocationsFor(Institution $institution): ?array
{
return $this->repository->findByInstitution($institution);
}
Expand Down
Loading

0 comments on commit acb6a8d

Please sign in to comment.