diff --git a/.github/workflows/phpstan.yml b/.github/workflows/analyse-php.yml similarity index 67% rename from .github/workflows/phpstan.yml rename to .github/workflows/analyse-php.yml index cbda75afd..191399d78 100644 --- a/.github/workflows/phpstan.yml +++ b/.github/workflows/analyse-php.yml @@ -1,21 +1,18 @@ -name: phpstan +name: analyse-php on: [push, pull_request] jobs: build: if: ${{ github.ref != 'refs/heads/l10n_development' }} - runs-on: ubuntu-20.04 - strategy: - matrix: - php: ['7.4'] + runs-on: ubuntu-22.04 steps: - uses: actions/checkout@v1 - name: Setup PHP uses: shivammathur/setup-php@v2 with: - php-version: ${{ matrix.php }} + php-version: 8.1 extensions: gd, mbstring, json, curl, xml, mysql, ldap - name: Get Composer Cache Directory @@ -24,13 +21,14 @@ jobs: echo "::set-output name=dir::$(composer config cache-files-dir)" - name: Cache composer packages - uses: actions/cache@v1 + uses: actions/cache@v2 with: path: ${{ steps.composer-cache.outputs.dir }} - key: ${{ runner.os }}-composer-${{ matrix.php }} + key: ${{ runner.os }}-composer-8.1 + restore-keys: ${{ runner.os }}-composer- - name: Install composer dependencies run: composer install --prefer-dist --no-interaction --ansi - - name: Run PHPStan - run: php${{ matrix.php }} ./vendor/bin/phpstan analyse --memory-limit=2G + - name: Run static analysis check + run: composer check-static diff --git a/.github/workflows/lint-php.yml b/.github/workflows/lint-php.yml new file mode 100644 index 000000000..75d18b60d --- /dev/null +++ b/.github/workflows/lint-php.yml @@ -0,0 +1,19 @@ +name: lint-php + +on: [push, pull_request] + +jobs: + build: + if: ${{ github.ref != 'refs/heads/l10n_development' }} + runs-on: ubuntu-22.04 + steps: + - uses: actions/checkout@v1 + + - name: Setup PHP + uses: shivammathur/setup-php@v2 + with: + php-version: 8.1 + tools: phpcs + + - name: Run formatting check + run: composer lint diff --git a/.github/workflows/test-migrations.yml b/.github/workflows/test-migrations.yml index 7fb05dca6..e9b66a0a6 100644 --- a/.github/workflows/test-migrations.yml +++ b/.github/workflows/test-migrations.yml @@ -5,7 +5,7 @@ on: [push, pull_request] jobs: build: if: ${{ github.ref != 'refs/heads/l10n_development' }} - runs-on: ubuntu-20.04 + runs-on: ubuntu-22.04 strategy: matrix: php: ['7.4', '8.0', '8.1'] @@ -24,10 +24,11 @@ jobs: echo "::set-output name=dir::$(composer config cache-files-dir)" - name: Cache composer packages - uses: actions/cache@v1 + uses: actions/cache@v2 with: path: ${{ steps.composer-cache.outputs.dir }} key: ${{ runner.os }}-composer-${{ matrix.php }} + restore-keys: ${{ runner.os }}-composer- - name: Start MySQL run: | diff --git a/.github/workflows/phpunit.yml b/.github/workflows/test-php.yml similarity index 91% rename from .github/workflows/phpunit.yml rename to .github/workflows/test-php.yml index 53812cb42..917038f59 100644 --- a/.github/workflows/phpunit.yml +++ b/.github/workflows/test-php.yml @@ -1,11 +1,11 @@ -name: phpunit +name: test-php on: [push, pull_request] jobs: build: if: ${{ github.ref != 'refs/heads/l10n_development' }} - runs-on: ubuntu-20.04 + runs-on: ubuntu-22.04 strategy: matrix: php: ['7.4', '8.0', '8.1'] @@ -24,10 +24,11 @@ jobs: echo "::set-output name=dir::$(composer config cache-files-dir)" - name: Cache composer packages - uses: actions/cache@v1 + uses: actions/cache@v2 with: path: ${{ steps.composer-cache.outputs.dir }} key: ${{ runner.os }}-composer-${{ matrix.php }} + restore-keys: ${{ runner.os }}-composer- - name: Start Database run: | @@ -48,5 +49,5 @@ jobs: php${{ matrix.php }} artisan migrate --force -n --database=mysql_testing php${{ matrix.php }} artisan db:seed --force -n --class=DummyContentSeeder --database=mysql_testing - - name: phpunit + - name: Run PHP tests run: php${{ matrix.php }} ./vendor/bin/phpunit diff --git a/app/Actions/Webhook.php b/app/Actions/Webhook.php index 72a67ad92..3ed5bb56f 100644 --- a/app/Actions/Webhook.php +++ b/app/Actions/Webhook.php @@ -22,10 +22,10 @@ use Illuminate\Database\Eloquent\Relations\HasMany; */ class Webhook extends Model implements Loggable { - protected $fillable = ['name', 'endpoint', 'timeout']; - use HasFactory; + protected $fillable = ['name', 'endpoint', 'timeout']; + protected $casts = [ 'last_called_at' => 'datetime', 'last_errored_at' => 'datetime', diff --git a/app/Actions/WebhookTrackedEvent.php b/app/Actions/WebhookTrackedEvent.php index 6289581a2..28b3a366d 100644 --- a/app/Actions/WebhookTrackedEvent.php +++ b/app/Actions/WebhookTrackedEvent.php @@ -12,7 +12,7 @@ use Illuminate\Database\Eloquent\Model; */ class WebhookTrackedEvent extends Model { - protected $fillable = ['event']; - use HasFactory; + + protected $fillable = ['event']; } diff --git a/app/Auth/Access/LdapService.php b/app/Auth/Access/LdapService.php index 2540fe2d8..359eeca2f 100644 --- a/app/Auth/Access/LdapService.php +++ b/app/Auth/Access/LdapService.php @@ -105,7 +105,7 @@ class LdapService 'name' => $this->getUserResponseProperty($user, $displayNameAttr, $userCn), 'dn' => $user['dn'], 'email' => $this->getUserResponseProperty($user, $emailAttr, null), - 'avatar'=> $thumbnailAttr ? $this->getUserResponseProperty($user, $thumbnailAttr, null) : null, + 'avatar' => $thumbnailAttr ? $this->getUserResponseProperty($user, $thumbnailAttr, null) : null, ]; if ($this->config['dump_user_details']) { diff --git a/app/Auth/Access/Oidc/OidcService.php b/app/Auth/Access/Oidc/OidcService.php index 4f5a3e1ac..c9c3cc511 100644 --- a/app/Auth/Access/Oidc/OidcService.php +++ b/app/Auth/Access/Oidc/OidcService.php @@ -2,7 +2,6 @@ namespace BookStack\Auth\Access\Oidc; -use function auth; use BookStack\Auth\Access\GroupSyncService; use BookStack\Auth\Access\LoginService; use BookStack\Auth\Access\RegistrationService; @@ -10,14 +9,11 @@ use BookStack\Auth\User; use BookStack\Exceptions\JsonDebugException; use BookStack\Exceptions\StoppedAuthenticationException; use BookStack\Exceptions\UserRegistrationException; -use function config; use Illuminate\Support\Arr; use Illuminate\Support\Facades\Cache; use League\OAuth2\Client\OptionProvider\HttpBasicAuthOptionProvider; use League\OAuth2\Client\Provider\Exception\IdentityProviderException; use Psr\Http\Client\ClientInterface as HttpClient; -use function trans; -use function url; /** * Class OpenIdConnectService diff --git a/app/Config/dompdf.php b/app/Config/dompdf.php index 16a527b3c..09dd91bcc 100644 --- a/app/Config/dompdf.php +++ b/app/Config/dompdf.php @@ -7,6 +7,7 @@ * Configuration should be altered via the `.env` file or environment variables. * Do not edit this file unless you're happy to maintain any changes yourself. */ + $dompdfPaperSizeMap = [ 'a4' => 'a4', 'letter' => 'letter', diff --git a/app/Config/snappy.php b/app/Config/snappy.php index 8ab10a39c..a87ce805f 100644 --- a/app/Config/snappy.php +++ b/app/Config/snappy.php @@ -7,6 +7,7 @@ * Configuration should be altered via the `.env` file or environment variables. * Do not edit this file unless you're happy to maintain any changes yourself. */ + $snappyPaperSizeMap = [ 'a4' => 'A4', 'letter' => 'Letter', diff --git a/app/Entities/Tools/PageEditActivity.php b/app/Entities/Tools/PageEditActivity.php index 2672de941..646b200f1 100644 --- a/app/Entities/Tools/PageEditActivity.php +++ b/app/Entities/Tools/PageEditActivity.php @@ -42,7 +42,7 @@ class PageEditActivity $userMessage = trans('entities.pages_draft_edit_active.start_b', ['userName' => $firstDraft->createdBy->name ?? '']); } - $timeMessage = trans('entities.pages_draft_edit_active.time_b', ['minCount'=> 60]); + $timeMessage = trans('entities.pages_draft_edit_active.time_b', ['minCount' => 60]); return trans('entities.pages_draft_edit_active.message', ['start' => $userMessage, 'time' => $timeMessage]); } diff --git a/app/Http/Controllers/Auth/ForgotPasswordController.php b/app/Http/Controllers/Auth/ForgotPasswordController.php index 5e73b232c..b345fad1c 100644 --- a/app/Http/Controllers/Auth/ForgotPasswordController.php +++ b/app/Http/Controllers/Auth/ForgotPasswordController.php @@ -20,7 +20,6 @@ class ForgotPasswordController extends Controller | your application to your users. Feel free to explore this trait. | */ - use SendsPasswordResetEmails; /** diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index f1a1a8bd6..1d6a36c5b 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -24,8 +24,9 @@ class LoginController extends Controller | to conveniently provide its functionality to your applications. | */ - - use AuthenticatesUsers { logout as traitLogout; } + use AuthenticatesUsers { + logout as traitLogout; + } /** * Redirection paths. @@ -112,8 +113,10 @@ class LoginController extends Controller // If the class is using the ThrottlesLogins trait, we can automatically throttle // the login attempts for this application. We'll key this by the username and // the IP address of the client making these requests into this application. - if (method_exists($this, 'hasTooManyLoginAttempts') && - $this->hasTooManyLoginAttempts($request)) { + if ( + method_exists($this, 'hasTooManyLoginAttempts') && + $this->hasTooManyLoginAttempts($request) + ) { $this->fireLockoutEvent($request); Activity::logFailedLogin($username); diff --git a/app/Http/Controllers/Auth/RegisterController.php b/app/Http/Controllers/Auth/RegisterController.php index b0aec1177..15ee78d50 100644 --- a/app/Http/Controllers/Auth/RegisterController.php +++ b/app/Http/Controllers/Auth/RegisterController.php @@ -27,7 +27,6 @@ class RegisterController extends Controller | provide this functionality without requiring any additional code. | */ - use RegistersUsers; protected SocialAuthService $socialAuthService; diff --git a/app/Http/Controllers/Auth/ResetPasswordController.php b/app/Http/Controllers/Auth/ResetPasswordController.php index a31529b11..9df010736 100644 --- a/app/Http/Controllers/Auth/ResetPasswordController.php +++ b/app/Http/Controllers/Auth/ResetPasswordController.php @@ -20,7 +20,6 @@ class ResetPasswordController extends Controller | explore this trait and override any methods you wish to tweak. | */ - use ResetsPasswords; protected $redirectTo = '/'; diff --git a/app/Http/Controllers/BookController.php b/app/Http/Controllers/BookController.php index a041267bb..cc2f6f534 100644 --- a/app/Http/Controllers/BookController.php +++ b/app/Http/Controllers/BookController.php @@ -147,7 +147,7 @@ class BookController extends Controller { $book = $this->bookRepo->getBySlug($slug); $this->checkOwnablePermission('book-update', $book); - $this->setPageTitle(trans('entities.books_edit_named', ['bookName'=>$book->getShortName()])); + $this->setPageTitle(trans('entities.books_edit_named', ['bookName' => $book->getShortName()])); return view('books.edit', ['book' => $book, 'current' => $book]); } diff --git a/app/Http/Controllers/BookSortController.php b/app/Http/Controllers/BookSortController.php index 8aac2b769..5d0577a74 100644 --- a/app/Http/Controllers/BookSortController.php +++ b/app/Http/Controllers/BookSortController.php @@ -28,7 +28,7 @@ class BookSortController extends Controller $bookChildren = (new BookContents($book))->getTree(false); - $this->setPageTitle(trans('entities.books_sort_named', ['bookName'=>$book->getShortName()])); + $this->setPageTitle(trans('entities.books_sort_named', ['bookName' => $book->getShortName()])); return view('books.sort', ['book' => $book, 'current' => $book, 'bookChildren' => $bookChildren]); } diff --git a/app/Http/Controllers/PageRevisionController.php b/app/Http/Controllers/PageRevisionController.php index c4d5fbc7b..89775a602 100644 --- a/app/Http/Controllers/PageRevisionController.php +++ b/app/Http/Controllers/PageRevisionController.php @@ -91,7 +91,7 @@ class PageRevisionController extends Controller // TODO - Refactor PageContent so we don't need to juggle this $page->html = $revision->html; $page->html = (new PageContent($page))->render(); - $this->setPageTitle(trans('entities.pages_revision_named', ['pageName'=>$page->getShortName()])); + $this->setPageTitle(trans('entities.pages_revision_named', ['pageName' => $page->getShortName()])); return view('pages.revision', [ 'page' => $page, diff --git a/app/Uploads/ImageService.php b/app/Uploads/ImageService.php index 7b8af3b84..0199c207e 100644 --- a/app/Uploads/ImageService.php +++ b/app/Uploads/ImageService.php @@ -315,7 +315,7 @@ class ImageService { try { $thumb = $this->imageTool->make($imageData); - } catch (ErrorException|NotSupportedException $e) { + } catch (ErrorException | NotSupportedException $e) { throw new ImageUploadException(trans('errors.cannot_create_thumbs')); } diff --git a/composer.json b/composer.json index 6d4bb734d..64630833d 100644 --- a/composer.json +++ b/composer.json @@ -50,6 +50,7 @@ "nunomaduro/collision": "^5.10", "nunomaduro/larastan": "^1.0", "phpunit/phpunit": "^9.5", + "squizlabs/php_codesniffer": "^3.7", "ssddanbrown/asserthtml": "^1.0" }, "autoload": { @@ -68,6 +69,10 @@ } }, "scripts": { + "check-static": "phpstan --memory-limit=2g", + "format": "phpcbf", + "lint": "phpcs", + "test": "phpunit", "post-autoload-dump": [ "Illuminate\\Foundation\\ComposerScripts::postAutoloadDump", "@php artisan package:discover --ansi" diff --git a/composer.lock b/composer.lock index 6f9ed25cc..b807fd577 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "4ab21f732b2380ed1c3dd1a4eca2ef1a", + "content-hash": "1d3bd88b99d07b5410ee4b245bece28e", "packages": [ { "name": "aws/aws-crt-php", @@ -10645,6 +10645,62 @@ }, "time": "2022-07-20T18:31:45+00:00" }, + { + "name": "squizlabs/php_codesniffer", + "version": "3.7.1", + "source": { + "type": "git", + "url": "https://github.com/squizlabs/PHP_CodeSniffer.git", + "reference": "1359e176e9307e906dc3d890bcc9603ff6d90619" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/squizlabs/PHP_CodeSniffer/zipball/1359e176e9307e906dc3d890bcc9603ff6d90619", + "reference": "1359e176e9307e906dc3d890bcc9603ff6d90619", + "shasum": "" + }, + "require": { + "ext-simplexml": "*", + "ext-tokenizer": "*", + "ext-xmlwriter": "*", + "php": ">=5.4.0" + }, + "require-dev": { + "phpunit/phpunit": "^4.0 || ^5.0 || ^6.0 || ^7.0" + }, + "bin": [ + "bin/phpcs", + "bin/phpcbf" + ], + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "3.x-dev" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "BSD-3-Clause" + ], + "authors": [ + { + "name": "Greg Sherwood", + "role": "lead" + } + ], + "description": "PHP_CodeSniffer tokenizes PHP, JavaScript and CSS files and detects violations of a defined set of coding standards.", + "homepage": "https://github.com/squizlabs/PHP_CodeSniffer", + "keywords": [ + "phpcs", + "standards" + ], + "support": { + "issues": "https://github.com/squizlabs/PHP_CodeSniffer/issues", + "source": "https://github.com/squizlabs/PHP_CodeSniffer", + "wiki": "https://github.com/squizlabs/PHP_CodeSniffer/wiki" + }, + "time": "2022-06-18T07:21:10+00:00" + }, { "name": "ssddanbrown/asserthtml", "version": "v1.0.1", diff --git a/phpcs.xml b/phpcs.xml new file mode 100644 index 000000000..8d4c6b702 --- /dev/null +++ b/phpcs.xml @@ -0,0 +1,35 @@ + + + The coding standard for BookStack + + app + bootstrap/app.php + database + public/index.php + routes + tests + + + + + + + + + + ./tests/* + + + + ./tests/* + + + + ./database/* + + + + ./app/Config/* + + + diff --git a/readme.md b/readme.md index 83986525e..245b113aa 100644 --- a/readme.md +++ b/readme.md @@ -3,8 +3,8 @@ [![GitHub release](https://img.shields.io/github/release/BookStackApp/BookStack.svg)](https://github.com/BookStackApp/BookStack/releases/latest) [![license](https://img.shields.io/badge/License-MIT-yellow.svg)](https://github.com/BookStackApp/BookStack/blob/development/LICENSE) [![Crowdin](https://badges.crowdin.net/bookstack/localized.svg)](https://crowdin.com/project/bookstack) -[![Build Status](https://github.com/BookStackApp/BookStack/workflows/phpunit/badge.svg)](https://github.com/BookStackApp/BookStack/actions) -[![StyleCI](https://github.styleci.io/repos/41589337/shield?style=flat)](https://github.styleci.io/repos/41589337) +[![Build Status](https://github.com/BookStackApp/BookStack/workflows/test-php/badge.svg)](https://github.com/BookStackApp/BookStack/actions) +[![Lint Status](https://github.com/BookStackApp/BookStack/workflows/lint-php/badge.svg)](https://github.com/BookStackApp/BookStack/actions) [![Maintainability](https://api.codeclimate.com/v1/badges/5551731994dd22fa1f4f/maintainability)](https://codeclimate.com/github/BookStackApp/BookStack/maintainability) [![Repo Stats](https://img.shields.io/static/v1?label=GitHub+project&message=stats&color=f27e3f)](https://gh-stats.bookstackapp.com/) @@ -115,12 +115,29 @@ php artisan migrate --database=mysql_testing php artisan db:seed --class=DummyContentSeeder --database=mysql_testing ``` -Once done you can run `php vendor/bin/phpunit` in the application root directory to run all tests. +Once done you can run `composer test` in the application root directory to run all tests. ### 📜 Code Standards -PHP code style is enforced automatically [using StyleCI](https://github.styleci.io/repos/41589337). -If submitting a PR, any formatting changes to be made will be automatically fixed after merging. +PHP code standards are managed by [using PHP_CodeSniffer](https://github.com/squizlabs/PHP_CodeSniffer). +Static analysis is in place using [PHPStan](https://phpstan.org/) & [Larastan](https://github.com/nunomaduro/larastan). +The below commands can be used to utilise these tools: + +```bash +# Run code linting using PHP_CodeSniffer +composer lint + +# As above, but show rule names in output +composer lint -- -s + +# Auto-fix formatting & lint issues via PHP_CodeSniffer phpcbf +composer format + +# Run static analysis via larastan/phpstan +composer check-static +``` + +If submitting a PR, formatting as per our project standards would help for clarity but don't worry too much about using/understanding these tools as we can always address issues at a later stage when they're picked up by our automated tools. ### 🐋 Development using Docker @@ -234,9 +251,9 @@ Note: This is not an exhaustive list of all libraries and projects that would be * [OneLogin's SAML PHP Toolkit](https://github.com/onelogin/php-saml) - _[MIT](https://github.com/onelogin/php-saml/blob/master/LICENSE)_ * [League/CommonMark](https://commonmark.thephpleague.com/) - _[BSD-3-Clause](https://github.com/thephpleague/commonmark/blob/2.2/LICENSE)_ * [League/Flysystem](https://flysystem.thephpleague.com) - _[MIT](https://github.com/thephpleague/flysystem/blob/3.x/LICENSE)_ -* [StyleCI](https://styleci.io/) - _Hosted Service_ * [pragmarx/google2fa](https://github.com/antonioribeiro/google2fa) - _[MIT](https://github.com/antonioribeiro/google2fa/blob/8.x/LICENSE.md)_ * [Bacon/BaconQrCode](https://github.com/Bacon/BaconQrCode) - _[BSD-2-Clause](https://github.com/Bacon/BaconQrCode/blob/master/LICENSE)_ * [phpseclib](https://github.com/phpseclib/phpseclib) - _[MIT](https://github.com/phpseclib/phpseclib/blob/master/LICENSE)_ * [Clockwork](https://github.com/itsgoingd/clockwork) - _[MIT](https://github.com/itsgoingd/clockwork/blob/master/LICENSE)_ * [PHPStan](https://phpstan.org/) & [Larastan](https://github.com/nunomaduro/larastan) - _[MIT](https://github.com/phpstan/phpstan/blob/master/LICENSE) and [MIT](https://github.com/nunomaduro/larastan/blob/master/LICENSE.md)_ +* [PHP_CodeSniffer](https://github.com/squizlabs/PHP_CodeSniffer) - _[BSD 3-Clause](https://github.com/squizlabs/PHP_CodeSniffer/blob/master/licence.txt)_ diff --git a/routes/api.php b/routes/api.php index 20e167d70..d350fd86b 100644 --- a/routes/api.php +++ b/routes/api.php @@ -1,5 +1,11 @@ $fileName, - 'uploaded_to'=> $page->id, + 'uploaded_to' => $page->id, 'extension' => 'txt', 'order' => 1, 'created_by' => $admin->id, diff --git a/tests/UrlTest.php b/tests/UrlTest.php index 13f9d9e0c..dd278c240 100644 --- a/tests/UrlTest.php +++ b/tests/UrlTest.php @@ -3,6 +3,7 @@ namespace Tests; use BookStack\Http\Request; + use function request; use function url;