phpstan level 7

CHANGELOG.md

@@ -3,12 +3,22 @@
 See README.md for info about version numbering. Follows https://keepachangelog.com/en/1.0.0/
 
 ## 2.2.0 - 2018-08-10
-- Temporarily ignoring semver because no one else is using htis library at the moment. Will add a note to the changelog when we follow semver again
+- Temporarily ignoring semver because no one else is using this library at the moment. Will add a note to the changelog when we follow semver again
 ### Changed
 - Coding style fixes
+- phpstan passes at level 7 (max)
 - Stricter types on some classes. Gradually migrating to strict_types=1
 - Removed the Public Key Store: it was horrible to use on the Connection class making it hard to use dependency injection. Storage is a separate concern. The API assumes you also store the threema id separately as well, so this is no extra burden on the caller (though it will hurt a bit for the command line)
 - Migrated command line to Symfony console: to ease dependency injection; to make the commands more self documenting / easier to use; to get private keys and api secrets off the command line (may be insecure)
+- Removed CryptTool/Encryptor singleton for dependency injection. Renamed CryptTool to Encryptor.
+- Use sodium_hex2bin() and _bin2hex()
+- Split cUrl stuff out of Connection class into HttpDriver and created an interface for dependency injection
+- Close the curl connection after finished (for long running processes)
+- Created ConnectionFactory to hold the runtime environment together
+- Made helper methods on Connection class for end to end encrypted messages: now the user does not have to know about E2EHelper, they can just call methods on the Connection class
+- Made all key and nonce parameters hex on the Connection class so users to not have to know / remember which is hex and which is binary. Everything outside is hex. The Encryptor class needs some binary things, but the parameters are now clearly labelled (we hope!)
+- Moved some internal classes around to put related things together
+- Simplified a lot of complex conditions that were checking for null even when null could never be returned
 
 ## 2.1.1 - 2018-08-07
 ### Added

README.md

@@ -141,11 +141,18 @@ or
 vendor/bin/phpunit
 ```
 
+Also check with phpstan, which must be green (zero errors) at maximum level.
+```
+composer phpstan
+```
+
 ## To Do
 
-* Refactor the Connection to split out the cUrl calls to a separate driver class. Then add a Guzzle Driver. This will also make the Connection testable because it is trivial to create a Mock Driver. 
+* Replace Receiver class with 3 methods on Connection
 * ReceiveMessageResult assumes you want to store file attachments on the local filesystem. This may not be true eg if using Amazon infrastructure. Refactor to allow for FileAcceptors(?) which can be overloaded to use Flysystem, local file system, or a null object pattern that ignores the file
 * There are some useful Exception classes defined but they are not used in some places.
+* Url class is probably not needed
+* AssocArray class is probably not needed 
 
 ## Other platforms (Java and Python)
 

composer.json

@@ -56,6 +56,6 @@
  "scripts": {
  "test": "./vendor/bin/phpunit",
  "phpunit": "./vendor/bin/phpunit",
- "phpstan": "./vendor/bin/phpstan analyse -l 7 src"
+ "phpstan": "./vendor/bin/phpstan analyse -l max src"
  }
 }

src/Threema/Console/Symfony/AbstractLocalCommand.php

@@ -22,12 +22,12 @@ abstract class AbstractLocalCommand extends Command
 {
  private const KEY_FILE = 'default.key';
 
- /** @var array default option and argument values stored in self::KEY_FILE */
- private $defaults = [];
-
  /** @var \Threema\MsgApi\ConnectionFactory */
  protected $connectionFactory;
 
+ /** @var array default option and argument values stored in self::KEY_FILE */
+ private $defaults = [];
+
  public function __construct(ConnectionFactory $connectionFactory)
  {
  parent::__construct();
@@ -122,7 +122,11 @@ protected function getEncryptor(): AbstractEncryptor
  private function readStdInput(): string
  {
  // read console standard input stream. Strip empty / blank lines
- return join("\n", array_filter(array_map('trim', file('php:https://stdin'))));
+ $file = file('php:https://stdin');
+ if (empty($file)) {
+ return '';
+ }
+ return join("\n", array_filter(array_map('trim', $file)));
  }
 
  private function getKey(string $key, string $optionName, string $keyPrefix): string

src/Threema/Console/Symfony/CreditsCommand.php

@@ -25,7 +25,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
  {
  $result = $this->getConnection($input, $output)->credits();
  $this->assertSuccess($result);
- $output->writeln($result->getCredits());
+ $output->writeln($result->getCredits() . ' remaining');
  return 0;
  }
 }

src/Threema/Core/AssocArray.php

@@ -8,7 +8,6 @@
 
 class AssocArray
 {
-
  /**
  * @var array
  */
@@ -24,41 +23,34 @@ public function __construct(array $data)
  }
 
  /**
- * @param string  $string
- * @param array|null $requiredKeys
+ * @param string $string
+ * @param array  $requiredKeys
  * @return AssocArray
  * @throws Exception
  */
- public static final function byJsonString($string, array $requiredKeys = null)
+ public static final function byJsonString($string, array $requiredKeys)
  {
  $v = json_decode($string, true);
  if (null === $v || false === $v) {
  throw new Exception('invalid json string');
  }
 
- //validate first
- if (null !== $requiredKeys) {
- //validate array first
- foreach ($requiredKeys as $requiredKey) {
- if (false === [$v]) {
- throw new Exception('required key ' . $requiredKey . ' failed');
- }
+ //validate array first
+ foreach ($requiredKeys as $requiredKey) {
+ if (!isset($v[$requiredKey])) {
+ throw new Exception('required key ' . $requiredKey . ' is not present');
  }
  }
  return new AssocArray($v);
  }
 
  /**
- * @param string $key
- * @param null  $defaultValue
+ * @param string  $key
+ * @param mixed|null $defaultValue
  * @return mixed|null return the key value or the default value
  */
  public function getValue($key, $defaultValue = null)
  {
- if (false === array_key_exists($key, $this->data)) {
- return $defaultValue;
- }
-
- return $this->data[$key];
+ return $this->data[$key] ?? $defaultValue;
  }
 }

src/Threema/Core/Url.php

@@ -27,31 +27,31 @@ class Url
  * @param string $path
  * @param string $host
  */
- public function __construct($path, $host = null)
+ public function __construct(string $path, string $host)
  {
  $this->path = $path;
  $this->host = $host;
  }
 
- public static function parametersToArray($urlParameter)
- {
- $result = [];
-
- while (strlen($urlParameter) > 0) {
- // name
- $keyPosition = strpos($urlParameter, '=');
- $keyValue = substr($urlParameter, 0, $keyPosition);
- // value
- $valuePosition = strpos($urlParameter, '&') ? strpos($urlParameter, '&') : strlen($urlParameter);
- $valueValue = substr($urlParameter, $keyPosition + 1, $valuePosition - $keyPosition - 1);
-
- // decoding the response
- $result[$keyValue] = urldecode($valueValue);
- $urlParameter = substr($urlParameter, $valuePosition + 1, strlen($urlParameter));
- }
-
- return $result;
- }
+// public static function parametersToArray($urlParameter)
+// {
+// $result = [];
+//
+// while (strlen($urlParameter) > 0) {
+// // name
+// $keyPosition = strpos($urlParameter, '=');
+// $keyValue = substr($urlParameter, 0, $keyPosition);
+// // value
+// $valuePosition = strpos($urlParameter, '&') ? strpos($urlParameter, '&') : strlen($urlParameter);
+// $valueValue = substr($urlParameter, $keyPosition + 1, $valuePosition - $keyPosition - 1);
+//
+// // decoding the response
+// $result[$keyValue] = urldecode($valueValue);
+// $urlParameter = substr($urlParameter, $valuePosition + 1, strlen($urlParameter));
+// }
+//
+// return $result;
+// }
 
  /**
  * @param string $key

src/Threema/MsgApi/Commands/Capability.php

@@ -7,6 +7,7 @@
 namespace Threema\MsgApi\Commands;
 
 use Threema\MsgApi\Commands\Results\CapabilityResult;
+use Threema\MsgApi\Commands\Results\Result;
 
 class Capability implements CommandInterface
 {
@@ -38,11 +39,11 @@ public function getPath(): string
 
  /**
  * @param int $httpCode
- * @param object $res
+ * @param string $response
  * @return CapabilityResult
  */
- public function parseResult($httpCode, $res): \Threema\MsgApi\Commands\Results\Result
+ public function parseResult(int $httpCode, string $response): Result
  {
- return new CapabilityResult($httpCode, $res);
+ return new CapabilityResult($httpCode, $response);
  }
 }

src/Threema/MsgApi/Commands/CommandInterface.php

@@ -24,8 +24,8 @@ public function getParams(): array;
 
  /**
  * @param int $httpCode
- * @param object $res
+ * @param string $response
  * @return Result
  */
- public function parseResult($httpCode, $res): \Threema\MsgApi\Commands\Results\Result;
+ public function parseResult(int $httpCode, string $response): Result;
 }

src/Threema/MsgApi/Commands/Credits.php

@@ -25,11 +25,11 @@ public function getPath(): string
 
  /**
  * @param int $httpCode
- * @param object $res
+ * @param string $response
  * @return CreditsResult
  */
- public function parseResult($httpCode, $res): \Threema\MsgApi\Commands\Results\Result
+ public function parseResult(int $httpCode, string $response): \Threema\MsgApi\Commands\Results\Result
  {
- return new CreditsResult($httpCode, $res);
+ return new CreditsResult($httpCode, $response);
  }
 }

src/Threema/MsgApi/Commands/DownloadFile.php

@@ -41,11 +41,11 @@ public function getPath(): string
 
  /**
  * @param int $httpCode
- * @param object $res
+ * @param string $response
  * @return DownloadFileResult
  */
- public function parseResult($httpCode, $res): \Threema\MsgApi\Commands\Results\Result
+ public function parseResult(int $httpCode, string $response): \Threema\MsgApi\Commands\Results\Result
  {
- return new DownloadFileResult($httpCode, $res);
+ return new DownloadFileResult($httpCode, $response);
  }
 }

src/Threema/MsgApi/Commands/FetchPublicKey.php

@@ -41,11 +41,11 @@ public function getPath(): string
 
  /**
  * @param int $httpCode
- * @param object $res
+ * @param string $response
  * @return FetchPublicKeyResult
  */
- public function parseResult($httpCode, $res): \Threema\MsgApi\Commands\Results\Result
+ public function parseResult(int $httpCode, string $response): \Threema\MsgApi\Commands\Results\Result
  {
- return new FetchPublicKeyResult($httpCode, $res);
+ return new FetchPublicKeyResult($httpCode, $response);
  }
 }

src/Threema/MsgApi/Commands/LookupBulk.php

@@ -62,9 +62,8 @@ public function getParams(): array
 
  public function getJson(): string
  {
-
  return json_encode(['emailHashes' => array_keys($this->hashedEmail),
- 'phoneHashes' => array_keys($this->hashedPhone)]);
+ 'phoneHashes' => array_keys($this->hashedPhone)]) ?: '';
  }
 
  public function calculateHashes(AbstractEncryptor $encryptor)
@@ -87,11 +86,11 @@ public function getPath(): string
 
  /**
  * @param int $httpCode
- * @param object $res
+ * @param string $response
  * @return LookupBulkResult
  */
- public function parseResult($httpCode, $res): Result
+ public function parseResult(int $httpCode, string $response): Result
  {
- return new LookupBulkResult($httpCode, $res, $this);
+ return new LookupBulkResult($httpCode, $response, $this);
  }
 }

src/Threema/MsgApi/Commands/LookupEmail.php

@@ -64,11 +64,11 @@ public function getPath(): string
 
  /**
  * @param int $httpCode
- * @param object $res
+ * @param string $response
  * @return LookupIdResult
  */
- public function parseResult($httpCode, $res): Result
+ public function parseResult(int $httpCode, string $response): Result
  {
- return new LookupIdResult($httpCode, $res);
+ return new LookupIdResult($httpCode, $response);
  }
 }

src/Threema/MsgApi/Commands/LookupPhone.php

@@ -61,11 +61,11 @@ public function getPath(): string
 
  /**
  * @param int $httpCode
- * @param object $res
+ * @param string $response
  * @return LookupIdResult
  */
- public function parseResult($httpCode, $res): Result
+ public function parseResult(int $httpCode, string $response): Result
  {
- return new LookupIdResult($httpCode, $res);
+ return new LookupIdResult($httpCode, $response);
  }
 }

src/Threema/MsgApi/Commands/Results/CapabilityResult.php

@@ -80,7 +80,7 @@ public function can($key)
  /**
  * @param string $response
  */
- protected function processResponse($response)
+ protected function processResponse(string $response)
  {
  $this->capabilities = array_unique(array_filter(explode(',', $response ?? '')));
  }
@@ -89,7 +89,7 @@ protected function processResponse($response)
  * @param int $httpCode
  * @return string
  */
- protected function getErrorMessageByErrorCode($httpCode)
+ protected function getErrorMessageByErrorCode(int $httpCode): string
  {
  switch ($httpCode) {
  case 401:

src/Threema/MsgApi/Commands/Results/CreditsResult.php

@@ -24,7 +24,7 @@ public function getCredits()
  /**
  * @param string $response
  */
- protected function processResponse($response)
+ protected function processResponse(string $response)
  {
  $this->credits = intval($response, 10);
  }
@@ -33,7 +33,7 @@ protected function processResponse($response)
  * @param int $httpCode
  * @return string
  */
- protected function getErrorMessageByErrorCode($httpCode)
+ protected function getErrorMessageByErrorCode(int $httpCode): string
  {
  switch ($httpCode) {
  case 401:

src/Threema/MsgApi/Commands/Results/DownloadFileResult.php

@@ -24,18 +24,18 @@ public function getData()
  }
 
  /**
- * @param string $data
+ * @param string $response
  */
- protected function processResponse($data)
+ protected function processResponse(string $response)
  {
- $this->data = $data;
+ $this->data = $response;
  }
 
  /**
  * @param int $httpCode
  * @return string
  */
- protected function getErrorMessageByErrorCode($httpCode)
+ protected function getErrorMessageByErrorCode(int $httpCode): string
  {
  switch ($httpCode) {
  case 401: