From 6127a02a8b32487f7f5951dbc507066079ebb8d3 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 12 Dec 2019 10:43:45 +0000 Subject: [PATCH 01/10] phpstan 0.12.2 --- tests/phpstan/configs/phpstan-bugs.neon | 6 ------ tests/travis.sh | 2 +- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/tests/phpstan/configs/phpstan-bugs.neon b/tests/phpstan/configs/phpstan-bugs.neon index 696fcc8ab..6e3312bd8 100644 --- a/tests/phpstan/configs/phpstan-bugs.neon +++ b/tests/phpstan/configs/phpstan-bugs.neon @@ -25,9 +25,3 @@ parameters: message: "#^Offset \\(int\\|string\\) does not exist on array\\(\\)\\.$#" count: 1 path: ../../../src/pocketmine/MemoryManager.php - - - - message: "#^Array \\(array\\) does not accept key int\\.$#" - count: 1 - path: ../../../src/pocketmine/plugin/PluginDescription.php - diff --git a/tests/travis.sh b/tests/travis.sh index ba90d4a19..ee5053262 100755 --- a/tests/travis.sh +++ b/tests/travis.sh @@ -21,7 +21,7 @@ if [ $? -ne 0 ]; then exit 1 fi -[ ! -f phpstan.phar ] && echo "Downloading PHPStan..." && curl -sSLO https://github.com/phpstan/phpstan/releases/download/0.12.0/phpstan.phar +[ ! -f phpstan.phar ] && echo "Downloading PHPStan..." && curl -sSLO https://github.com/phpstan/phpstan/releases/download/0.12.2/phpstan.phar "$PHP_BINARY" phpstan.phar analyze --no-progress --memory-limit=2G || exit 1 echo "PHPStan scan succeeded" From b3cfa5a3a098da106fa02c7a5cc11459ac7b84f6 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 12 Dec 2019 11:48:34 +0000 Subject: [PATCH 02/10] TimingsHandler: use weak comparison for stopTiming start check this could be an int or float, and 0 !== 0.0. --- src/pocketmine/timings/TimingsHandler.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pocketmine/timings/TimingsHandler.php b/src/pocketmine/timings/TimingsHandler.php index 4018ee730..87a6100c5 100644 --- a/src/pocketmine/timings/TimingsHandler.php +++ b/src/pocketmine/timings/TimingsHandler.php @@ -168,7 +168,7 @@ class TimingsHandler{ public function stopTiming(){ if(self::$enabled){ - if(--$this->timingDepth !== 0 or $this->start === 0){ + if(--$this->timingDepth !== 0 or $this->start == 0){ return; } From 73c5fe5cf9a3311e8152114654a432101cd0cd52 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 12 Dec 2019 11:58:00 +0000 Subject: [PATCH 03/10] BaseInventory: correctly annotate content type of slots field fixes warnings on phpstan level 4 --- src/pocketmine/inventory/BaseInventory.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pocketmine/inventory/BaseInventory.php b/src/pocketmine/inventory/BaseInventory.php index 4f64680a2..4702f90ee 100644 --- a/src/pocketmine/inventory/BaseInventory.php +++ b/src/pocketmine/inventory/BaseInventory.php @@ -46,7 +46,7 @@ abstract class BaseInventory implements Inventory{ protected $name; /** @var string */ protected $title; - /** @var \SplFixedArray|Item[] */ + /** @var \SplFixedArray|(Item|null)[] */ protected $slots; /** @var Player[] */ protected $viewers = []; From b8778cb791edfa95dd76ea08bc4ea0a25c78a5a9 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 12 Dec 2019 11:59:12 +0000 Subject: [PATCH 04/10] LevelProvider: fix doc comment of ::generate() this raised false positives on phpstan level 4 --- src/pocketmine/level/format/io/LevelProvider.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pocketmine/level/format/io/LevelProvider.php b/src/pocketmine/level/format/io/LevelProvider.php index 2854f0e38..5fa272c17 100644 --- a/src/pocketmine/level/format/io/LevelProvider.php +++ b/src/pocketmine/level/format/io/LevelProvider.php @@ -71,7 +71,7 @@ interface LevelProvider{ * @param string $name * @param int $seed * @param string $generator - * @param array[] $options + * @param array $options */ public static function generate(string $path, string $name, int $seed, string $generator, array $options = []); From a9fafbc5eb6f3b3d69d6fd21a2b46d656710a3c5 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 12 Dec 2019 11:59:41 +0000 Subject: [PATCH 05/10] BanEntry: remove nullable return from parseDate() this function never returns null --- src/pocketmine/permission/BanEntry.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pocketmine/permission/BanEntry.php b/src/pocketmine/permission/BanEntry.php index 7c8cbf3ab..addf5287a 100644 --- a/src/pocketmine/permission/BanEntry.php +++ b/src/pocketmine/permission/BanEntry.php @@ -136,10 +136,10 @@ class BanEntry{ /** * @param string $date * - * @return \DateTime|null + * @return \DateTime * @throws \RuntimeException */ - private static function parseDate(string $date) : ?\DateTime{ + private static function parseDate(string $date) : \DateTime{ $datetime = \DateTime::createFromFormat(self::$format, $date); if(!($datetime instanceof \DateTime)){ throw new \RuntimeException("Error parsing date for BanEntry: " . implode(", ", \DateTime::getLastErrors()["errors"])); From 26230c1f9bf6bb1cd9cc22ea1b98b2a8efd3a4d1 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 12 Dec 2019 12:17:52 +0000 Subject: [PATCH 06/10] Player: don't report never-played for a disconnected player this should catch fire like everything else does. --- src/pocketmine/Player.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pocketmine/Player.php b/src/pocketmine/Player.php index 16451d733..a76add108 100644 --- a/src/pocketmine/Player.php +++ b/src/pocketmine/Player.php @@ -464,11 +464,11 @@ class Player extends Human implements CommandSender, ChunkLoader, IPlayer{ } public function getFirstPlayed(){ - return $this->namedtag instanceof CompoundTag ? $this->namedtag->getLong("firstPlayed", 0, true) : null; + return $this->namedtag->getLong("firstPlayed", 0, true); } public function getLastPlayed(){ - return $this->namedtag instanceof CompoundTag ? $this->namedtag->getLong("lastPlayed", 0, true) : null; + return $this->namedtag->getLong("lastPlayed", 0, true); } public function hasPlayedBefore() : bool{ From 62069bc7af73595b11e118ae1e596e9ec4ffa733 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 12 Dec 2019 12:18:35 +0000 Subject: [PATCH 07/10] Level: fix return type content doc comment for getAdjacentChunks() --- src/pocketmine/level/Level.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pocketmine/level/Level.php b/src/pocketmine/level/Level.php index 59754eb92..57b16f5bb 100644 --- a/src/pocketmine/level/Level.php +++ b/src/pocketmine/level/Level.php @@ -2442,7 +2442,7 @@ class Level implements ChunkManager, Metadatable{ * @param int $x * @param int $z * - * @return Chunk[] + * @return (Chunk|null)[] */ public function getAdjacentChunks(int $x, int $z) : array{ $result = []; From 92be8c8ec0e978be291b39bedc806910829c56ff Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 12 Dec 2019 12:19:54 +0000 Subject: [PATCH 08/10] PopulationTask: don't assume anything about TLS return values while these SHOULD be what we say they are, it's possible for something else to overwrite them with junk and make the server catch fire. --- src/pocketmine/level/generator/PopulationTask.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/pocketmine/level/generator/PopulationTask.php b/src/pocketmine/level/generator/PopulationTask.php index 007a6f87f..d061c415b 100644 --- a/src/pocketmine/level/generator/PopulationTask.php +++ b/src/pocketmine/level/generator/PopulationTask.php @@ -56,11 +56,9 @@ class PopulationTask extends AsyncTask{ } public function onRun(){ - /** @var SimpleChunkManager $manager */ $manager = $this->getFromThreadStore("generation.level{$this->levelId}.manager"); - /** @var Generator $generator */ $generator = $this->getFromThreadStore("generation.level{$this->levelId}.generator"); - if($manager === null or $generator === null){ + if(!($manager instanceof SimpleChunkManager) or !($generator instanceof Generator)){ $this->state = false; return; } From 46930b98b774e2b2f9c00e883f79eba95d0ff57b Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 12 Dec 2019 12:59:01 +0000 Subject: [PATCH 09/10] Timings: add a dedicated field for checking initialization --- src/pocketmine/timings/Timings.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/pocketmine/timings/Timings.php b/src/pocketmine/timings/Timings.php index a8d1e646c..5192728f5 100644 --- a/src/pocketmine/timings/Timings.php +++ b/src/pocketmine/timings/Timings.php @@ -31,6 +31,8 @@ use pocketmine\tile\Tile; use function dechex; abstract class Timings{ + /** @var bool */ + private static $initialized = false; /** @var TimingsHandler */ public static $fullTickTimer; @@ -104,9 +106,10 @@ abstract class Timings{ public static $pluginTaskTimingMap = []; public static function init(){ - if(self::$serverTickTimer instanceof TimingsHandler){ + if(self::$initialized){ return; } + self::$initialized = true; self::$fullTickTimer = new TimingsHandler("Full Server Tick"); self::$serverTickTimer = new TimingsHandler("** Full Server Tick", self::$fullTickTimer); From 70c3008b7b2bad4aff51abc629add73bb0b17d07 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 12 Dec 2019 13:00:57 +0000 Subject: [PATCH 10/10] phpstan: green on level 4 --- phpstan.neon.dist | 4 +- tests/phpstan/configs/debug-const-checks.neon | 21 +++++++++ tests/phpstan/configs/phpstan-bugs.neon | 44 +++++++++++++++++++ .../phpstan/configs/runtime-type-checks.neon | 31 +++++++++++++ 4 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 tests/phpstan/configs/debug-const-checks.neon create mode 100644 tests/phpstan/configs/runtime-type-checks.neon diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 170897410..c8556a7fd 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -1,12 +1,14 @@ includes: + - tests/phpstan/configs/debug-const-checks.neon - tests/phpstan/configs/gc-hacks.neon - tests/phpstan/configs/optional-com-dotnet.neon - tests/phpstan/configs/optional-leveldb.neon - tests/phpstan/configs/phpstan-bugs.neon - tests/phpstan/configs/pthreads-bugs.neon + - tests/phpstan/configs/runtime-type-checks.neon parameters: - level: 3 + level: 4 autoload_files: - tests/phpstan/bootstrap.php - src/pocketmine/PocketMine.php diff --git a/tests/phpstan/configs/debug-const-checks.neon b/tests/phpstan/configs/debug-const-checks.neon new file mode 100644 index 000000000..bfe2fa5c6 --- /dev/null +++ b/tests/phpstan/configs/debug-const-checks.neon @@ -0,0 +1,21 @@ +parameters: + ignoreErrors: + - + message: "#^If condition is always true\\.$#" + count: 1 + path: ../../../src/pocketmine/Server.php + + - + message: "#^Ternary operator condition is always true\\.$#" + count: 1 + path: ../../../src/pocketmine/Server.php + + - + message: "#^If condition is always false\\.$#" + count: 1 + path: ../../../src/pocketmine/updater/AutoUpdater.php + + - + message: "#^Negated boolean expression is always false\\.$#" + count: 1 + path: ../../../src/pocketmine/updater/AutoUpdater.php diff --git a/tests/phpstan/configs/phpstan-bugs.neon b/tests/phpstan/configs/phpstan-bugs.neon index 6e3312bd8..2efbbb37b 100644 --- a/tests/phpstan/configs/phpstan-bugs.neon +++ b/tests/phpstan/configs/phpstan-bugs.neon @@ -25,3 +25,47 @@ parameters: message: "#^Offset \\(int\\|string\\) does not exist on array\\(\\)\\.$#" count: 1 path: ../../../src/pocketmine/MemoryManager.php + + - + message: "#^Comparison operation \"\\>\\=\" between 0 and 2 is always false\\.$#" + count: 1 + path: ../../../src/pocketmine/block/Liquid.php + + - + #adjacentSources comparison FP + message: "#^If condition is always false\\.$#" + count: 1 + path: ../../../src/pocketmine/block/Liquid.php + + - + #$class::NETWORK_ID false positive + message: "#^Strict comparison using \\!\\=\\= between \\-1 and \\-1 will always evaluate to false\\.$#" + count: 1 + path: ../../../src/pocketmine/entity/Entity.php + + - + message: "#^Call to function assert\\(\\) with false and 'unknown hit type' will always evaluate to false\\.$#" + count: 1 + path: ../../../src/pocketmine/entity/projectile/Projectile.php + + - + message: "#^Strict comparison using \\=\\=\\= between int\\ and 4 will always evaluate to false\\.$#" + count: 1 + path: ../../../src/pocketmine/level/Level.php + + - + message: "#^Instanceof between int and PharFileInfo will always evaluate to false\\.$#" + count: 1 + path: ../../../src/pocketmine/plugin/PharPluginLoader.php + + - + #ReflectionFunction::getClosureThis() should be nullable + message: "#^Else branch is unreachable because ternary operator condition is always true\\.$#" + count: 1 + path: ../../../src/pocketmine/utils/Utils.php + + - + #ReflectionFunction::getClosureScopeClass() should be nullable + message: "#^Unreachable statement \\- code above always terminates\\.$#" + count: 1 + path: ../../../src/pocketmine/utils/Utils.php diff --git a/tests/phpstan/configs/runtime-type-checks.neon b/tests/phpstan/configs/runtime-type-checks.neon new file mode 100644 index 000000000..7d48cacd9 --- /dev/null +++ b/tests/phpstan/configs/runtime-type-checks.neon @@ -0,0 +1,31 @@ +parameters: + ignoreErrors: + - + #::add() / ::remove() thread parameter + message: "#^If condition is always true\\.$#" + count: 2 + path: ../../../src/pocketmine/ThreadManager.php + + - + #::get() tags parameter + message: "#^If condition is always false\\.$#" + count: 1 + path: ../../../src/pocketmine/item/ItemFactory.php + + - + #::get() tags parameter + message: "#^Strict comparison using \\!\\=\\= between null and null will always evaluate to false\\.$#" + count: 1 + path: ../../../src/pocketmine/item/ItemFactory.php + + - + #::get() tags parameter + message: "#^Else branch is unreachable because ternary operator condition is always true\\.$#" + count: 1 + path: ../../../src/pocketmine/item/ItemFactory.php + + - + #->sendBlocks() blocks parameter + message: "#^Else branch is unreachable because ternary operator condition is always true\\.$#" + count: 2 + path: ../../../src/pocketmine/level/Level.php