From aad2bce9e43c44d4e6f32ded0a5a40080caf0016 Mon Sep 17 00:00:00 2001 From: "Dylan T." Date: Sun, 9 Mar 2025 00:23:59 +0000 Subject: [PATCH 1/4] readme: call out Easy tasks issues --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index b9e2e1888..6f2b715ab 100644 --- a/README.md +++ b/README.md @@ -65,6 +65,8 @@ PocketMine-MP accepts community contributions! The following resources will be u * [Building and running PocketMine-MP from source](BUILDING.md) * [Contributing Guidelines](CONTRIBUTING.md) +New here? Check out [issues with the "Easy task" label](https://github.com/pmmp/PocketMine-MP/issues?q=is%3Aissue%20state%3Aopen%20label%3A%22Easy%20task%22) for things you could work to familiarise yourself with the codebase. + ## Donate PocketMine-MP is free, but it requires a lot of time and effort from unpaid volunteers to develop. Donations enable us to keep delivering support for new versions and adding features your players love. From 22915466105e9bffd8924365e184e44688f4664a Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sun, 9 Mar 2025 00:51:12 +0000 Subject: [PATCH 2/4] phpstan: added rule to ban new $class see #6635 for rationale on why we want to get rid of this for now, this rule will prevent this anti-feature from being used in new code --- phpstan.neon.dist | 1 + tests/phpstan/configs/actual-problems.neon | 30 ++++++++++ .../phpstan/rules/DisallowDynamicNewRule.php | 55 +++++++++++++++++++ 3 files changed, 86 insertions(+) create mode 100644 tests/phpstan/rules/DisallowDynamicNewRule.php diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 48bfd4a78..13f35c121 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -11,6 +11,7 @@ includes: rules: - pocketmine\phpstan\rules\DeprecatedLegacyEnumAccessRule + - pocketmine\phpstan\rules\DisallowDynamicNewRule - pocketmine\phpstan\rules\DisallowEnumComparisonRule - pocketmine\phpstan\rules\DisallowForeachByReferenceRule - pocketmine\phpstan\rules\ExplodeLimitRule diff --git a/tests/phpstan/configs/actual-problems.neon b/tests/phpstan/configs/actual-problems.neon index 73b7a65c1..e54bb166e 100644 --- a/tests/phpstan/configs/actual-problems.neon +++ b/tests/phpstan/configs/actual-problems.neon @@ -18,6 +18,12 @@ parameters: count: 1 path: ../../../src/Server.php + - + message: '#^Dynamic new is not allowed\.$#' + identifier: pocketmine.new.noDynamic + count: 1 + path: ../../../src/Server.php + - message: '#^Method pocketmine\\Server\:\:getCommandAliases\(\) should return array\\> but returns array\\>\.$#' identifier: return.type @@ -54,6 +60,12 @@ parameters: count: 1 path: ../../../src/VersionInfo.php + - + message: '#^Dynamic new is not allowed\.$#' + identifier: pocketmine.new.noDynamic + count: 1 + path: ../../../src/block/Block.php + - message: '#^Parameter \#1 \$x of method pocketmine\\world\\World\:\:getBlockAt\(\) expects int, float\|int given\.$#' identifier: argument.type @@ -510,6 +522,12 @@ parameters: count: 3 path: ../../../src/block/tile/Spawnable.php + - + message: '#^Dynamic new is not allowed\.$#' + identifier: pocketmine.new.noDynamic + count: 1 + path: ../../../src/block/tile/TileFactory.php + - message: '#^Parameter \#1 \$x of method pocketmine\\world\\World\:\:getPotentialLightAt\(\) expects int, float\|int given\.$#' identifier: argument.type @@ -936,6 +954,12 @@ parameters: count: 4 path: ../../../src/plugin/PluginManager.php + - + message: '#^Dynamic new is not allowed\.$#' + identifier: pocketmine.new.noDynamic + count: 1 + path: ../../../src/plugin/PluginManager.php + - message: '#^Method pocketmine\\resourcepacks\\ZippedResourcePack\:\:getPackSize\(\) should return int but returns int\<0, max\>\|false\.$#' identifier: return.type @@ -1248,6 +1272,12 @@ parameters: count: 1 path: ../../../src/world/format/io/region/RegionLoader.php + - + message: '#^Dynamic new is not allowed\.$#' + identifier: pocketmine.new.noDynamic + count: 1 + path: ../../../src/world/generator/GeneratorRegisterTask.php + - message: '#^Method pocketmine\\world\\generator\\biome\\BiomeSelector\:\:pickBiome\(\) should return pocketmine\\world\\biome\\Biome but returns pocketmine\\world\\biome\\Biome\|null\.$#' identifier: return.type diff --git a/tests/phpstan/rules/DisallowDynamicNewRule.php b/tests/phpstan/rules/DisallowDynamicNewRule.php new file mode 100644 index 000000000..c6c15a5aa --- /dev/null +++ b/tests/phpstan/rules/DisallowDynamicNewRule.php @@ -0,0 +1,55 @@ + + */ +final class DisallowDynamicNewRule implements Rule{ + + public function getNodeType() : string{ + return New_::class; + } + + public function processNode(Node $node, Scope $scope) : array{ + /** @var New_ $node */ + if($node->class instanceof Expr){ + return [ + RuleErrorBuilder::message("Dynamic new is not allowed.") + ->tip("For factories, use closures instead. Closures can implement custom logic, are statically analyzable, and don't restrict the constructor signature.") + ->identifier("pocketmine.new.noDynamic") + ->build() + ]; + } + + return []; + } +} From 95284bc9decef6869e7d42c1d5b213a0d683562e Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sun, 9 Mar 2025 00:54:39 +0000 Subject: [PATCH 3/4] change error identifier --- tests/phpstan/configs/actual-problems.neon | 10 +++++----- tests/phpstan/rules/DisallowDynamicNewRule.php | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/phpstan/configs/actual-problems.neon b/tests/phpstan/configs/actual-problems.neon index e54bb166e..d3adde422 100644 --- a/tests/phpstan/configs/actual-problems.neon +++ b/tests/phpstan/configs/actual-problems.neon @@ -20,7 +20,7 @@ parameters: - message: '#^Dynamic new is not allowed\.$#' - identifier: pocketmine.new.noDynamic + identifier: pocketmine.new.dynamic count: 1 path: ../../../src/Server.php @@ -62,7 +62,7 @@ parameters: - message: '#^Dynamic new is not allowed\.$#' - identifier: pocketmine.new.noDynamic + identifier: pocketmine.new.dynamic count: 1 path: ../../../src/block/Block.php @@ -524,7 +524,7 @@ parameters: - message: '#^Dynamic new is not allowed\.$#' - identifier: pocketmine.new.noDynamic + identifier: pocketmine.new.dynamic count: 1 path: ../../../src/block/tile/TileFactory.php @@ -956,7 +956,7 @@ parameters: - message: '#^Dynamic new is not allowed\.$#' - identifier: pocketmine.new.noDynamic + identifier: pocketmine.new.dynamic count: 1 path: ../../../src/plugin/PluginManager.php @@ -1274,7 +1274,7 @@ parameters: - message: '#^Dynamic new is not allowed\.$#' - identifier: pocketmine.new.noDynamic + identifier: pocketmine.new.dynamic count: 1 path: ../../../src/world/generator/GeneratorRegisterTask.php diff --git a/tests/phpstan/rules/DisallowDynamicNewRule.php b/tests/phpstan/rules/DisallowDynamicNewRule.php index c6c15a5aa..c25e6a18b 100644 --- a/tests/phpstan/rules/DisallowDynamicNewRule.php +++ b/tests/phpstan/rules/DisallowDynamicNewRule.php @@ -45,7 +45,7 @@ final class DisallowDynamicNewRule implements Rule{ return [ RuleErrorBuilder::message("Dynamic new is not allowed.") ->tip("For factories, use closures instead. Closures can implement custom logic, are statically analyzable, and don't restrict the constructor signature.") - ->identifier("pocketmine.new.noDynamic") + ->identifier("pocketmine.new.dynamic") ->build() ]; } From 7af5eb3da2bb50cdb11c06622e003283fdffd0ca Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sun, 9 Mar 2025 01:10:03 +0000 Subject: [PATCH 4/4] crafting: validate array inputs this makes sure wrong parameters don't show up as core errors, as seen in crash report 12373907 closes #6642 --- src/crafting/ShapedRecipe.php | 2 ++ src/crafting/ShapelessRecipe.php | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/crafting/ShapedRecipe.php b/src/crafting/ShapedRecipe.php index 4c40eb0f5..2af3f5134 100644 --- a/src/crafting/ShapedRecipe.php +++ b/src/crafting/ShapedRecipe.php @@ -97,6 +97,7 @@ class ShapedRecipe implements CraftingRecipe{ $this->shape = $shape; + Utils::validateArrayValueType($ingredients, function(RecipeIngredient $_) : void{}); foreach(Utils::stringifyKeys($ingredients) as $char => $i){ if(!str_contains(implode($this->shape), $char)){ throw new \InvalidArgumentException("Symbol '$char' does not appear in the recipe shape"); @@ -105,6 +106,7 @@ class ShapedRecipe implements CraftingRecipe{ $this->ingredientList[$char] = clone $i; } + Utils::validateArrayValueType($results, function(Item $_) : void{}); $this->results = Utils::cloneObjectArray($results); } diff --git a/src/crafting/ShapelessRecipe.php b/src/crafting/ShapelessRecipe.php index 7a4a22fda..b139439ef 100644 --- a/src/crafting/ShapelessRecipe.php +++ b/src/crafting/ShapelessRecipe.php @@ -53,7 +53,9 @@ class ShapelessRecipe implements CraftingRecipe{ if(count($ingredients) > 9){ throw new \InvalidArgumentException("Shapeless recipes cannot have more than 9 ingredients"); } + Utils::validateArrayValueType($ingredients, function(RecipeIngredient $_) : void{}); $this->ingredients = $ingredients; + Utils::validateArrayValueType($results, function(Item $_) : void{}); $this->results = Utils::cloneObjectArray($results); }