From 642260e2fe8e2b2fd22f4a42423a7e1205b1cb39 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Fri, 3 Feb 2023 14:08:00 -0600 Subject: [PATCH 1/5] Prefer caching the relative path to a class in the ClassLoader (#140) Adds Filesystem->isAbsolutePath() and changes the ClassLoader->autoloadPackage() logic to prefer storing a relative path to a class file over an absolute one. --- src/Filesystem/Filesystem.php | 17 ++++++++++++- src/Support/ClassLoader.php | 13 +++++++--- tests/Filesystem/FilesystemTest.php | 39 +++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 4 deletions(-) create mode 100644 tests/Filesystem/FilesystemTest.php diff --git a/src/Filesystem/Filesystem.php b/src/Filesystem/Filesystem.php index ba8c3df9c..3eb3542ad 100644 --- a/src/Filesystem/Filesystem.php +++ b/src/Filesystem/Filesystem.php @@ -123,6 +123,21 @@ public function localToPublic(string $path): ?string return $result; } + /** + * Returns whether the file path is an absolute path. + * @see Symfony\Component\Filesystem\Filesystem::isAbsolutePath() + */ + public function isAbsolutePath(string $file): bool + { + return '' !== $file && (strspn($file, '/\\', 0, 1) + || (\strlen($file) > 3 && ctype_alpha($file[0]) + && ':' === $file[1] + && strspn($file, '/\\', 2, 1) + ) + || null !== parse_url($file, \PHP_URL_SCHEME) + ); + } + /** * Determines if the given path is a local path. * @@ -231,7 +246,7 @@ public function isPathSymbol(string $path): bool * @param string $path * @param string $contents * @param bool|int $lock - * @return bool|int + * @return int|false */ public function put($path, $contents, $lock = false) { diff --git a/src/Support/ClassLoader.php b/src/Support/ClassLoader.php index e43a44e56..345422791 100644 --- a/src/Support/ClassLoader.php +++ b/src/Support/ClassLoader.php @@ -158,7 +158,7 @@ class_alias($alias, $class); */ protected function resolvePath(string $path): string { - if (!Str::startsWith($path, ['/', '\\', $this->basePath . DIRECTORY_SEPARATOR])) { + if (!$this->files->isAbsolutePath($path)) { $path = $this->basePath . DIRECTORY_SEPARATOR . $path; } return $path; @@ -228,10 +228,17 @@ public function build(): void /** * Add a namespace prefix to the autoloader + * + * @param string $namespacePrefix The namespace prefix for this package + * @param string $path The path to this package, either relative to the base path or absolute */ - public function autoloadPackage(string $namespacePrefix, string $relativePath): void + public function autoloadPackage(string $namespacePrefix, string $path): void { - $this->autoloadedPackages[ltrim(Str::lower($namespacePrefix), '\\')] = $relativePath; + // Normalize the path to an absolute path and then attempt to use the relative path + // if the path is contained within the basePath + $path = Str::after($this->resolvePath($path), $this->basePath . DIRECTORY_SEPARATOR); + + $this->autoloadedPackages[ltrim(Str::lower($namespacePrefix), '\\')] = $path; // Ensure packages are sorted by length of the prefix to prevent a greedier prefix // from being matched first when attempting to autoload a class diff --git a/tests/Filesystem/FilesystemTest.php b/tests/Filesystem/FilesystemTest.php new file mode 100644 index 000000000..3ffa36362 --- /dev/null +++ b/tests/Filesystem/FilesystemTest.php @@ -0,0 +1,39 @@ +filesystem = new Filesystem(); + } + + /** + * @dataProvider providePathsForIsAbsolutePath + * @see Symfony\Component\Filesystem\Tests\FilesystemTest::testIsAbsolutePath + */ + public function testIsAbsolutePath($path, $expectedResult) + { + $result = $this->filesystem->isAbsolutePath($path); + + $this->assertEquals($expectedResult, $result); + } + + public function providePathsForIsAbsolutePath() + { + return [ + ['/var/lib', true], + ['c:\\\\var\\lib', true], + ['\\var\\lib', true], + ['var/lib', false], + ['../var/lib', false], + ['', false], + ]; + } +} From 13bffb86915415c25739dd98a0d4f836bb55acc7 Mon Sep 17 00:00:00 2001 From: Marc Jauvin Date: Wed, 1 Mar 2023 10:36:51 -0500 Subject: [PATCH 2/5] Prevent empty id attribute (#143) --- src/Html/FormBuilder.php | 4 +++ tests/Html/FormBuilderTest.php | 58 ++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) create mode 100644 tests/Html/FormBuilderTest.php diff --git a/src/Html/FormBuilder.php b/src/Html/FormBuilder.php index a671a558a..b3dbd9e0e 100644 --- a/src/Html/FormBuilder.php +++ b/src/Html/FormBuilder.php @@ -285,6 +285,10 @@ public function input(string $type, ?string $name = null, ?string $value = null, // when creating the HTML element. Then, we will return the entire input. $merge = compact('type', 'value', 'id'); + if ($id === "") { + unset($merge['id']); + } + $options = array_filter(array_merge($options, $merge), function ($item) { return !is_null($item); }); diff --git a/tests/Html/FormBuilderTest.php b/tests/Html/FormBuilderTest.php new file mode 100644 index 000000000..6bee1c4eb --- /dev/null +++ b/tests/Html/FormBuilderTest.php @@ -0,0 +1,58 @@ +formBuilder = new FormBuilder($htmlBuilder, $generator); + } + + public function testInputIdMissing() + { + $result = $this->formBuilder->input(type:"text", name:"my-name", value:"my value"); + $this->assertEquals('', $result); + } + + public function testInputIdEmpty() + { + $result = $this->formBuilder->input(type:"text", name:"my-name", value:"my value", options:["id"=>""]); + $this->assertEquals('', $result); + } + + public function testInputIdNull() + { + $result = $this->formBuilder->input(type:"text", name:"my-name", value:"my value", options:["id"=>null]); + $this->assertEquals('', $result); + } + + public function testInputIdFalse() + { + $result = $this->formBuilder->input(type:"text", name:"my-name", value:"my value", options:["id"=>false]); + $this->assertEquals('', $result); + } + + public function testInputIdZero() + { + $result = $this->formBuilder->input(type:"text", name:"my-name", value:"my value", options:["id"=>0]); + $this->assertEquals('', $result); + } + + public function testInputIdMissingWithAssociatedLabel() + { + $result = $this->formBuilder->label(name:"my-input", value:"my input label"); + $result = $this->formBuilder->input(type:"text", name:"my-input", value:"my value"); + $this->assertEquals('', $result); + } +} From 5b2dd605c20804070132159df8b8248e42ccf6b0 Mon Sep 17 00:00:00 2001 From: Ben Thomson Date: Thu, 2 Mar 2023 09:43:12 +0800 Subject: [PATCH 3/5] Fix recent static analysis errors --- src/Database/Model.php | 4 ++-- src/Database/Traits/Validation.php | 5 ++++- src/Extension/Extendable.php | 4 ++-- src/Extension/ExtendableTrait.php | 13 +++---------- src/Foundation/Http/Kernel.php | 22 +++++----------------- src/Network/Http.php | 4 ++-- src/Support/Facades/DB.php | 2 +- src/Support/Facades/File.php | 2 +- src/Support/helpers.php | 4 +++- 9 files changed, 23 insertions(+), 37 deletions(-) diff --git a/src/Database/Model.php b/src/Database/Model.php index 45031b2cb..d03442c5e 100644 --- a/src/Database/Model.php +++ b/src/Database/Model.php @@ -31,9 +31,9 @@ class Model extends EloquentModel implements ModelInterface use \Winter\Storm\Database\Traits\DeferredBinding; /** - * @var array Behaviors implemented by this model. + * @var string|array|null Extensions implemented by this class. */ - public $implement; + public $implement = null; /** * @var array Make the model's attributes public so behaviors can modify them. diff --git a/src/Database/Traits/Validation.php b/src/Database/Traits/Validation.php index 94dca33d1..a4f68504b 100644 --- a/src/Database/Traits/Validation.php +++ b/src/Database/Traits/Validation.php @@ -132,10 +132,13 @@ protected function getRelationValidationValue($relationName) * Instantiates the validator used by the validation process, depending if the class * is being used inside or outside of Laravel. Optional connection string to make * the validator use a different database connection than the default connection. - * @return \Illuminate\Validation\Validator + * + * @return \Illuminate\Contracts\Validation\Validator + * @phpstan-return \Illuminate\Validation\Validator */ protected static function makeValidator($data, $rules, $customMessages, $attributeNames, $connection = null) { + /** @var \Illuminate\Validation\Validator $validator */ $validator = Validator::make($data, $rules, $customMessages, $attributeNames); if ($connection !== null) { diff --git a/src/Extension/Extendable.php b/src/Extension/Extendable.php index 7d890e327..0ff0ffa89 100644 --- a/src/Extension/Extendable.php +++ b/src/Extension/Extendable.php @@ -18,9 +18,9 @@ class Extendable use ExtendableTrait; /** - * @var array Extensions implemented by this class. + * @var string|array|null Extensions implemented by this class. */ - public $implement; + public $implement = null; /** * Constructor diff --git a/src/Extension/ExtendableTrait.php b/src/Extension/ExtendableTrait.php index 4c1252df4..84f19847f 100644 --- a/src/Extension/ExtendableTrait.php +++ b/src/Extension/ExtendableTrait.php @@ -16,7 +16,6 @@ * * @author Alexey Bobkov, Samuel Georges */ - trait ExtendableTrait { /** @@ -72,18 +71,12 @@ public function extendableConstruct() /* * Apply extensions */ - if (!$this->implement) { - return; - } - if (is_string($this->implement)) { $uses = explode(',', $this->implement); - } - elseif (is_array($this->implement)) { + } elseif (is_array($this->implement)) { $uses = $this->implement; - } - else { - throw new Exception(sprintf('Class %s contains an invalid $implement value', get_class($this))); + } else { + return; } foreach ($uses as $use) { diff --git a/src/Foundation/Http/Kernel.php b/src/Foundation/Http/Kernel.php index 3d3e5bd24..1c4218cfc 100644 --- a/src/Foundation/Http/Kernel.php +++ b/src/Foundation/Http/Kernel.php @@ -5,9 +5,7 @@ class Kernel extends HttpKernel { /** - * The bootstrap classes for the application. - * - * @var string[] + * {@inheritDoc} */ protected $bootstrappers = [ \Winter\Storm\Foundation\Bootstrap\RegisterClassLoader::class, @@ -22,9 +20,7 @@ class Kernel extends HttpKernel ]; /** - * The application's global HTTP middleware stack. - * - * @var array + * {@inheritDoc} */ protected $middleware = [ \Winter\Storm\Foundation\Http\Middleware\CheckForTrustedHost::class, @@ -33,9 +29,7 @@ class Kernel extends HttpKernel ]; /** - * The application's route middleware. - * - * @var array + * {@inheritDoc} */ protected $routeMiddleware = [ // 'auth' => \Illuminate\Auth\Middleware\Authenticate::class, @@ -47,9 +41,7 @@ class Kernel extends HttpKernel ]; /** - * The application's route middleware groups. - * - * @var array + * {@inheritDoc} */ protected $middlewareGroups = [ 'web' => [ @@ -67,11 +59,7 @@ class Kernel extends HttpKernel ]; /** - * The priority-sorted list of middleware. - * - * Forces the listed middleware to always be in the given order. - * - * @var string[] + * {@inheritDoc} */ protected $middlewarePriority = [ \Illuminate\Session\Middleware\StartSession::class, diff --git a/src/Network/Http.php b/src/Network/Http.php index 011967fa3..fc9c4943e 100644 --- a/src/Network/Http.php +++ b/src/Network/Http.php @@ -111,7 +111,7 @@ class Http /** * @var array cURL Options. */ - public $requestOptions; + public $requestOptions = []; /** * @var array Request data. @@ -280,7 +280,7 @@ public function send() curl_setopt($curl, CURLOPT_MAXREDIRS, $this->maxRedirects); } - if ($this->requestOptions && is_array($this->requestOptions)) { + if (count($this->requestOptions)) { curl_setopt_array($curl, $this->requestOptions); } diff --git a/src/Support/Facades/DB.php b/src/Support/Facades/DB.php index f5738678c..d2c0bb8bf 100644 --- a/src/Support/Facades/DB.php +++ b/src/Support/Facades/DB.php @@ -4,7 +4,7 @@ /** * @method static \Doctrine\DBAL\Driver\PDOConnection getPdo() - * @method static \Illuminate\Database\ConnectionInterface connection(string $name = null) + * @method static \Illuminate\Database\Connection connection(string $name = null) * @method static \Winter\Storm\Database\QueryBuilder table(string $table, string $as = null) * @method static \Illuminate\Database\Query\Expression raw($value) * @method static array getQueryLog() diff --git a/src/Support/Facades/File.php b/src/Support/Facades/File.php index eeb25eb73..4442c87f1 100644 --- a/src/Support/Facades/File.php +++ b/src/Support/Facades/File.php @@ -4,7 +4,7 @@ /** * @method static bool exists(string $path) - * @method static string get(string $path, bool $lock) + * @method static string get(string $path, bool $lock = false) * @method static string sharedGet(string $path) * @method static mixed getRequire(string $path) * @method static mixed requireOnce(string $file) diff --git a/src/Support/helpers.php b/src/Support/helpers.php index 6863d796f..aa95f4af2 100644 --- a/src/Support/helpers.php +++ b/src/Support/helpers.php @@ -35,7 +35,9 @@ function e($value, $doubleEncode = false) */ function trans($id = null, $parameters = [], $locale = null) { - return app('translator')->trans($id, $parameters, $locale); + /** @var \Winter\Storm\Translation\Translator $translator */ + $translator = app('translator'); + return $translator->trans($id, $parameters, $locale); } } From 2f4a532eddff3583fcd5343d878b2a2382afb285 Mon Sep 17 00:00:00 2001 From: Ben Thomson Date: Thu, 2 Mar 2023 09:47:37 +0800 Subject: [PATCH 4/5] Remove unneeded test, run static analysis on PHP 8.1 --- .github/workflows/code-analysis.yaml | 4 ++-- tests/Extension/ExtendableTest.php | 8 -------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/.github/workflows/code-analysis.yaml b/.github/workflows/code-analysis.yaml index f4b411a25..b74a4fd33 100644 --- a/.github/workflows/code-analysis.yaml +++ b/.github/workflows/code-analysis.yaml @@ -28,7 +28,7 @@ jobs: id: extcache uses: shivammathur/cache-extensions@v1 with: - php-version: '8.0' + php-version: '8.1' extensions: ${{ env.extensions }} key: ${{ env.key }} @@ -42,7 +42,7 @@ jobs: - name: Install PHP uses: shivammathur/setup-php@v2 with: - php-version: '8.0' + php-version: '8.1' extensions: ${{ env.extensions }} coverage: none diff --git a/tests/Extension/ExtendableTest.php b/tests/Extension/ExtendableTest.php index 7390baa23..d4243a5cb 100644 --- a/tests/Extension/ExtendableTest.php +++ b/tests/Extension/ExtendableTest.php @@ -172,14 +172,6 @@ public function testAccessingProtectedStaticMethod() echo ExtendableTestExampleExtendableClass::protectedMars(); } - public function testInvalidImplementValue() - { - $this->expectException(Exception::class); - $this->expectExceptionMessage('Class ExtendableTestInvalidExtendableClass contains an invalid $implement value'); - - $result = new ExtendableTestInvalidExtendableClass; - } - public function testSoftImplementFake() { $result = $this->mockClassLoader(ExtendableTestExampleExtendableSoftImplementFakeClass::class); From 6c32cf1f245e72eeb4ae9e0b30b7b039a978eba6 Mon Sep 17 00:00:00 2001 From: Ben Thomson Date: Thu, 2 Mar 2023 10:16:21 +0800 Subject: [PATCH 5/5] Restrict PHPStan version, further analysis fixes --- composer.json | 2 +- phpstan-baseline.neon | 10 ---------- src/Foundation/Application.php | 4 ++-- 3 files changed, 3 insertions(+), 13 deletions(-) diff --git a/composer.json b/composer.json index 2d30d9adf..321d90e96 100644 --- a/composer.json +++ b/composer.json @@ -57,7 +57,7 @@ "php-parallel-lint/php-parallel-lint": "^1.0", "meyfa/phpunit-assert-gd": "^2.0.0|^3.0.0", "dms/phpunit-arraysubset-asserts": "^0.1.0|^0.2.1", - "nunomaduro/larastan": "^2.0.1", + "nunomaduro/larastan": "~2.2.9", "orchestra/testbench": "^7.1.0" }, "suggest": { diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 8336286cb..04673bf83 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -755,16 +755,6 @@ parameters: count: 1 path: src/Extension/Extendable.php - - - message: "#^Parameter \\#3 \\$replacement of method Illuminate\\\\Support\\\\Collection\\,Illuminate\\\\Support\\\\Collection\\<\\(int\\|string\\), mixed\\>\\>\\:\\:splice\\(\\) expects array\\\\>, array\\{array\\} given\\.$#" - count: 1 - path: src/Foundation/Application.php - - - - message: "#^Strict comparison using \\=\\=\\= between array\\|null and false will always evaluate to false\\.$#" - count: 1 - path: src/Foundation/Exception/Handler.php - - message: "#^Conditional return type uses subject type TCacheValue which is not part of PHPDoc @template tags\\.$#" count: 1 diff --git a/src/Foundation/Application.php b/src/Foundation/Application.php index 361f3c710..ea1028be4 100644 --- a/src/Foundation/Application.php +++ b/src/Foundation/Application.php @@ -312,7 +312,7 @@ public function make($abstract, array $parameters = []) */ public function before($callback) { - $this['router']->before($callback); + $this->make('router')->before($callback); } /** @@ -323,7 +323,7 @@ public function before($callback) */ public function after($callback) { - $this['router']->after($callback); + $this->make('router')->after($callback); } /**