Skip to content
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.

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

Fix Psalm-issues batch #3 #1162

Open
wants to merge 12 commits into
base: master
from

Conversation

@tvdijen
Copy link
Member

commented Jul 9, 2019

Fixes all Psalm-issues in all if SimpleSAML/lib/.

@tvdijen tvdijen force-pushed the psalm-lib branch 4 times, most recently from e2f0213 to a44bafc Jul 10, 2019

@tvdijen tvdijen force-pushed the psalm-lib branch 4 times, most recently from 72abd75 to cc7f4bd Jul 22, 2019

@@ -505,8 +505,10 @@ public function handleLogoutRequest(array &$state, $assocId)
$this->authSource->logout($returnTo);
$handler = $this->getLogoutHandler();
$handler->startLogout($state, $assocId);
if ($assocId !== null) {

This comment has been minimized.

Copy link
@tvdijen

tvdijen Jul 24, 2019

Author Member

Twelve lines above we also test for $assocId being null, so I'm guessing this could be a valid situation, but I'm not too sure about this

@@ -526,8 +528,11 @@ public function handleLogoutResponse($assocId, $relayState, Error\Exception $err
assert(is_string($assocId));
assert(is_string($relayState) || $relayState === null);
$index = strpos($assocId, ':');

This comment has been minimized.

Copy link
@tvdijen

tvdijen Jul 24, 2019

Author Member

Perhaps raising an exception if $assocId does not meet a certain format would be better.. But what would that format be?

/**
* @var bool
*/
private static $initializing = false;

This comment has been minimized.

Copy link
@tvdijen

tvdijen Jul 24, 2019

Author Member

By introducing a new property, we can keep the typedecl of $loggingHandler as simple as possible..

} elseif (class_exists(_Memcached::class)) {
$memcache = new _Memcached();
self::$extension = strtolower(_Memcached::class);
} else {

This comment has been minimized.

Copy link
@tvdijen

tvdijen Jul 24, 2019

Author Member

This prevents us from using string classes. It's unlikely there will be new Memcache(d) classes to add.. Alternative would be to suppress the string-class-issue in Psalm-config.

@tvdijen tvdijen force-pushed the psalm-lib branch from cc7f4bd to 4071133 Aug 10, 2019

@tvdijen tvdijen force-pushed the psalm-lib branch from 7535331 to 1eb61cd Aug 30, 2019

@tvdijen tvdijen added this to the 1.18 milestone Aug 30, 2019

@tvdijen tvdijen force-pushed the psalm-lib branch from c69f5ac to 19884de Aug 31, 2019

@tvdijen tvdijen force-pushed the psalm-lib branch from 19884de to 823f8ef Aug 31, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.