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

Add support for Laravel 9 #213

Merged
merged 33 commits into from
May 2, 2022
Merged
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
b8913dc
run test suite with Laravel 9
Nielsvanpach Jan 19, 2022
538e1cb
update packages in laravel test script
Nielsvanpach Jan 19, 2022
6096cf0
use --with on composer
Nielsvanpach Jan 19, 2022
c7a4457
allow illuminate v8
Nielsvanpach Feb 14, 2022
8bf5d3f
require instead of update
Nielsvanpach Feb 14, 2022
7ba59e0
fix malformed yaml
Nielsvanpach Feb 14, 2022
33a46af
ignore unsupported matches
Nielsvanpach Feb 14, 2022
95c9c06
define laravel/framework version
Nielsvanpach Feb 14, 2022
5fb7f5a
require a higher version of barryvdh/laravel-ide-helper
Nielsvanpach Feb 14, 2022
08e9b38
drop support for 7.3, 7.4 and Illuminate v6
Nielsvanpach Feb 14, 2022
74347de
add typehinting according to the contract
Nielsvanpach Feb 14, 2022
3465c35
wip: require v5 of codeception
Nielsvanpach Feb 14, 2022
0892cd0
remove dev dependency
Nielsvanpach Mar 20, 2022
3f0479a
fix psalm issues
Nielsvanpach Mar 20, 2022
9d685ce
use dev version of codeception
Nielsvanpach Mar 20, 2022
df524d6
add missing dev package for codeception
Nielsvanpach Mar 20, 2022
11cb4da
add remaining dev packages for codeception
Nielsvanpach Mar 20, 2022
ce996c6
Merge branch 'master' into laravel-9
Nielsvanpach Mar 21, 2022
3ddc65b
fixes codeception to actually work
tm1000 Apr 7, 2022
0dc88d3
Merge pull request #1 from tm1000/laravel-9-codeception-fixes
Nielsvanpach Apr 7, 2022
a0d98b6
fix failing tests
tm1000 Apr 7, 2022
813874b
Merge branch 'master' into laravel-9-test-fixes
tm1000 Apr 7, 2022
1c9e193
bump up illuminate for 8.1
tm1000 Apr 7, 2022
48b4fd7
fix version string
tm1000 Apr 7, 2022
e50f7f0
Merge pull request #2 from tm1000/laravel-9-test-fixes
Nielsvanpach Apr 7, 2022
48d1c56
Fix failing tests for firstOrCreate and firstOrNew in Laravel 9
mzur Apr 26, 2022
fad8f22
Fix failing tests for Eloquent relation types in Laravel 9
mzur Apr 26, 2022
906d80f
Add require-dev for phpoption/phpoption version supporting PHP 8.1
mzur Apr 26, 2022
de5f936
Add require-dev for symfony/http-foundation version supporting PHP 8.1
mzur Apr 26, 2022
5fba336
Add require-dev for ramsey/collection version supporting PHP 8.1
mzur Apr 26, 2022
07f342d
Merge pull request #3 from mzur/laravel-9-patch-1
Nielsvanpach Apr 27, 2022
6c3c9b5
Fix accidental script declaration in composer.json
mzur Apr 27, 2022
76a53f5
Merge pull request #4 from mzur/laravel-9-patch-2
Nielsvanpach Apr 27, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
37 changes: 4 additions & 33 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,40 +8,11 @@ jobs:
strategy:
fail-fast: false
matrix:
php: [7.3, 7.4, 8.0, 8.1]
illuminate_version: [6.*, 8.*]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be great if we could support all versions in the same major release (1.x) -- Could you help me understand what might be a breaking change not allowing us to support these still? (I want to continue supporting laravel 6 til EOL, which is in september).

Copy link
Contributor

@tm1000 tm1000 Apr 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it' possible to add Laravel 9 support without dropping 7.3 and 7.4 support.

Laravel 6 won't work well on php 8. It's certainly not supported. That's why I believe laravel 6 support was dropped.

Is there a major issue with just issuing a 2.0 release for this PR? I don't understand the hesitation at doing a split release as has been already mentioned in this PR previously. See: #213 (comment)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Larastan (Laravel plugin for PHPStan) also released a new major version for Laravel 9.

Main reason we have a major version increase is we dropped support for Laravel versions older than 9. Because now all the Collection stubs Larastan had are moved to the Laravel core!

Copy link
Collaborator

@mr-feek mr-feek Apr 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My hesitation is just more maintenance overhead -- I would think we could solve that by adjusting the test matrix to not run laravel 6 on php 8 (like we are already doing with other tests) .

It's not a blocker, would just like to understand what exactly is causing the need for multiple versions

php: [8.0, 8.1]
illuminate_version: [8.67.*, 9.7.*]
stability: [prefer-lowest, prefer-stable]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--prefer-stable is not the opposite of --prefer-lowest, it also makes sense in combination.

Actually, I think that --prefer-stable is even the default, but I'm not totally sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, both can be used in combination I think. prefer-stable will check for packages with minimum-stability set to stable. But I don't think it's the default (and if so, it's still explicit this way). It's just an extra check to see if this package is easily installable, feel free to improve if possible!

exclude:
- illuminate_version: 6.*
php: 7.3
stability: prefer-stable

- illuminate_version: 6.*
php: 7.4
stability: prefer-stable

- illuminate_version: 6.*
php: 8.0
stability: prefer-stable

# Exclude unsupported combination
# https://laravel.com/docs/8.x/releases#support-policy
- illuminate_version: 6.*
php: 8.1

# "Added PHP 8.1 Support from v8.67.0"
# https://github.com/laravel/framework/blob/8.x/CHANGELOG-8.x.md#v8670-2021-10-22
# see also `matrix.include` section
- illuminate_version: 8.*
php: 8.1

include:
# "Added PHP 8.1 Support from v8.67.0"
- illuminate_version: ^8.67.0
php: 8.1
stability: 'prefer-stable'

name: ${{ matrix.php }} | Illuminate ${{ matrix.illuminate_version }} | ${{ matrix.stability }}
name: P${{ matrix.php }} | I ${{ matrix.illuminate_version }} | ${{ matrix.stability }}

steps:
- name: Checkout code
Expand All @@ -56,7 +27,7 @@ jobs:

- name: Install dependencies
run: |
composer require "illuminate/container:${{ matrix.illuminate_version }}" "illuminate/contracts:${{ matrix.illuminate_version }}" "illuminate/database:${{ matrix.illuminate_version }}" "illuminate/http:${{ matrix.illuminate_version }}" "illuminate/support:${{ matrix.illuminate_version }}" --no-interaction --no-progress --prefer-dist --${{ matrix.stability }}
composer require "laravel/framework:${{ matrix.illuminate_version }}" "illuminate/container:${{ matrix.illuminate_version }}" "illuminate/contracts:${{ matrix.illuminate_version }}" "illuminate/database:${{ matrix.illuminate_version }}" "illuminate/http:${{ matrix.illuminate_version }}" "illuminate/support:${{ matrix.illuminate_version }}" --no-interaction --no-progress --prefer-dist --${{ matrix.stability }}

- name: Run Tests
run: composer test
39 changes: 22 additions & 17 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,28 +11,32 @@
}
],
"require": {
"php": "^7.3|^8.0",
"php": "^8.0",
"ext-simplexml": "*",
"illuminate/config": "^6.0 || ^8.0",
mr-feek marked this conversation as resolved.
Show resolved Hide resolved
"illuminate/container": "^6.0 || ^8.0",
"illuminate/contracts": "^6.0 || ^8.0",
"illuminate/database": "^6.0 || ^8.0",
"illuminate/events": "^6.0 || ^8.0",
"illuminate/http": "^6.0 || ^8.0",
"illuminate/routing": "^6.0 || ^8.0",
"illuminate/support": "^6.0 || ^8.0",
"illuminate/view": "^6.0 || ^8.0",
"illuminate/config": "^8.0 || ^9.0",
"illuminate/container": "^8.0 || ^9.0",
"illuminate/contracts": "^8.0 || ^9.0",
"illuminate/database": "^8.0 || ^9.0",
"illuminate/events": "^8.0 || ^9.0",
"illuminate/http": "^8.0 || ^9.0",
"illuminate/routing": "^8.0 || ^9.0",
"illuminate/support": "^8.0 || ^9.0",
"illuminate/view": "^8.0 || ^9.0",
"vimeo/psalm": "^4.8.1",
"orchestra/testbench": "^3.8 || ^4.0 || ^5.0 || ^6.22 || ^7.0",
"barryvdh/laravel-ide-helper": ">=2.8.0"
"orchestra/testbench": "^6.22 || ^7.0",
"barryvdh/laravel-ide-helper": "^2.10"
},
"require-dev": {
"codeception/codeception": "^4.1.6",
"codeception/module-phpbrowser": "^1.0.0",
"codeception/module-asserts": "^1.0.0",
"weirdan/codeception-psalm-module": "^0.13.1",
"codeception/codeception": "^5.0",
"codeception/module-asserts": "*@dev",
"codeception/module-cli": "^2.0",
"codeception/module-filesystem": "^3.0",
"codeception/module-phpbrowser": "*@dev",
"slevomat/coding-standard": "^6.2",
"squizlabs/php_codesniffer": "*",
"slevomat/coding-standard": "^6.2"
"phpoption/phpoption": "^1.8.0",
"symfony/http-foundation": "^5.3.7 || ^6.0",
"ramsey/collection": "^1.2.0"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious what we need this for? I don't see it referenced in the code

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that if ramsey/collection is less than 1.2.0 it blows up on PHP 8.1 tests because Return Type will Change errors.

Copy link
Contributor

@mzur mzur Apr 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These were all the packages that didn't work with PHP 8.1 at prefer-lowest. I didn't know any better than to require the versions compatible with PHP 8.1 to fix the failing tests. The main issue is the breaking change of return type checks in the minor release of PHP and all these packages claiming to work with PHP ^8.0 although they are not.

},
"autoload": {
"psr-4": {
Expand All @@ -46,6 +50,7 @@
},
"scripts": {
"analyze": "psalm",
"analyse": "psalm",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks accidental

"lint": "phpcs -n",
"lint-fix": "phpcbf -n",
"test": "codecept run --skip-group skip",
Expand Down
1 change: 1 addition & 0 deletions phpcs.xml
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,5 @@
<file>tests</file>
<exclude-pattern>src/cache/</exclude-pattern>
<exclude-pattern>tests/_support/</exclude-pattern>
<exclude-pattern>tests/Support/</exclude-pattern>
</ruleset>
7 changes: 6 additions & 1 deletion psalm-baseline.xml
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
<?xml version="1.0" encoding="UTF-8"?>
<files psalm-version="dev-master@b07de1fefde4714f29bca0b56c632bd82855fc64">
<files psalm-version="4.22.0@fc2c6ab4d5fa5d644d8617089f012f3bb84b8703">
<file src="src/Providers/ApplicationProvider.php">
<MissingFile occurrences="1">
<code>require $applicationPath</code>
</MissingFile>
</file>
<file src="src/Providers/ModelStubProvider.php">
<PossiblyUndefinedMethod occurrences="1">
<code>databasePath</code>
Expand Down
1 change: 0 additions & 1 deletion src/Fakes/FakeMetaCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ protected function registerClassAutoloadExceptions(): callable
// by default, the ide-helper throws exceptions when it cannot find a class. However it does not unregister that
// autoloader when it is done, and we certainly do not want to throw exceptions when we are simply checking if
// a certain class exists. We are instead changing this to be a noop.

return function () {
};
}
Expand Down
2 changes: 1 addition & 1 deletion src/Fakes/FakeModelsCommandLogic.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public function getModels(): array
*
* @param Model $model
*/
protected function getPropertiesFromTable($model): void
public function getPropertiesFromTable($model): void
{
$table_name = $model->getTable();

Expand Down
14 changes: 14 additions & 0 deletions stubs/EloquentBuilder.stubphp
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,20 @@ class Builder
*/
public function firstOr($columns = ['*'], Closure $callback = null) { }

/**
* @param array $attributes
* @param array $values
* @return TModel|self<TModel>
*/
public function firstOrNew(array $attributes = [], array $values = []) { }

/**
* @param array $attributes
* @param array $values
* @return TModel|self<TModel>
*/
public function firstOrCreate(array $attributes = [], array $values = []) { }

/**
* @param string $column
* @return mixed
Expand Down
16 changes: 16 additions & 0 deletions stubs/HasOneOrMany.stubphp
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,20 @@ abstract class HasOneOrMany extends Relation
* @psalm-return \Illuminate\Database\Eloquent\Collection<TRelatedModel>
*/
public function createMany(array $records) { }

/**
* @param array $attributes
* @param array $values
* @return \Illuminate\Database\Eloquent\Model
* @psalm-return TRelatedModel
*/
public function firstOrNew(array $attributes = [], array $values = []) { }

/**
* @param array $attributes
* @param array $values
* @return \Illuminate\Database\Eloquent\Model
* @psalm-return TRelatedModel
*/
public function firstOrCreate(array $attributes = [], array $values = []) { }
}