NEW: Code cleanup (tests, linting).

.scrutinizer.yml

@@ -0,0 +1,11 @@
+checks:
+  php: true
+
+build:
+  nodes:
+    analysis:
+      tests:
+        override: [php-scrutinizer-run]
+
+filter:
+  paths: ["src/*", "tests/*"]

.travis.yml

@@ -1,51 +1,9 @@
-language: php
-
-dist: trusty
+version: ~> 1.0
 
 env:
   global:
-    - COMPOSER_ROOT_VERSION=dev-master
-
-matrix:
-  fast_finish: true
-  include:
-    - php: '7.2'
-      env: DB=MYSQL PHPUNIT_TEST=1
-
-    - php: '7.3'
-      env: DB=MYSQL PHPUNIT_TEST=1
-
-    - php: '7.4'
-      env: DB=MYSQL PHPUNIT_TEST=1
-
-    - php: *min-version
-      env: DB=MYSQL PHPUNIT_COVERAGE_TEST=1 PHPCS_TEST=1
-
-#    - php: *min-version
-#      env: DB=PGSQL PHPUNIT_TEST=1
-
-    - php: *min-version
-      env: DB=MYSQL PDO=1 PHPUNIT_TEST=1
-
-before_script:
-# Init PHP
-  - printf "\n" | pecl install imagick
-  - phpenv rehash
-  - phpenv config-rm xdebug.ini
-  - echo 'memory_limit = 2048M' >> ~/.phpenv/versions/$(phpenv version-name)/etc/conf.d/travis.ini
-
-# Install composer
-  - composer validate
-  - composer require silverstripe/recipe-core:4.5.x-dev silverstripe/recipe-cms:4.5.x-dev dnadesign/silverstripe-elemental:4.3.x-dev --prefer-dist --no-update
-# silverstripe/recipe-cms requires versioned 1.4.x-dev
-  - composer require silverstripe/versioned:"1.5.x-dev" --prefer-dist --no-update
-  - if [[ $DB == PGSQL ]]; then composer require silverstripe/postgresql:2.1.x-dev --prefer-dist --no-update; fi
-  - composer install --prefer-dist
-
-script:
-  - if [[ $PHPUNIT_TEST ]]; then vendor/bin/phpunit; fi
-  - if [[ $PHPCS_TEST ]]; then composer run-script lint; fi
-  - if [[ $PHPUNIT_COVERAGE_TEST ]]; then phpdbg -qrr vendor/bin/phpunit --coverage-clover=coverage.xml; fi
+    - COMPOSER_ROOT_VERSION="4.x-dev"
+    - REQUIRE_RECIPE="4.x-dev"
 
-after_success:
-  - if [[ $PHPUNIT_COVERAGE_TEST ]]; then bash <(curl -s https://codecov.io/bash) -f coverage.xml; fi
+import:
+  - silverstripe/silverstripe-travis-shared:config/provision/standard-jobs-fixed.yml

composer.json

@@ -19,18 +19,23 @@
             "homepage": "http://silverstripe.org"
         }
     ],
+    "extra": {
+        "branch-alias": {
+            "dev-master": "2.x-dev",
+            "dev-1": "1.x-dev"
+        }
+    },
     "require": {
-        "php": ">=7.1.0",
-        "silverstripe/framework": "^4.5.1",
+        "php": "^7.4 || ^8",
+        "silverstripe/framework": "^4.7",
         "silverstripe/versioned": "^1",
         "silverstripe/vendor-plugin": "^1",
         "silverstripe/cms-events": "dev-master"
     },
     "require-dev": {
-        "phpunit/phpunit": "^5.7",
+        "silverstripe/recipe-testing": "^1 || ^2",
         "silverstripe/versioned": "^1",
-        "squizlabs/php_codesniffer": "^3",
-        "sminnee/phpunit-mock-objects": "^3.4.9"
+        "dnadesign/silverstripe-elemental": "^4"
     },
     "conflict": {
         "dnadesign/elemental": "< 3.2.1",

phpcs.xml.dist

@@ -1,24 +1,26 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <ruleset name="SilverStripe">
-	<description>CodeSniffer ruleset for SilverStripe coding conventions.</description>
+    <description>CodeSniffer ruleset for SilverStripe coding conventions.</description>
+
+    <file>src</file>
+    <file>tests</file>
 
     <rule ref="Generic.Arrays.DisallowLongArraySyntax" />
 
-	<!-- base rules are PSR-2 -->
-	<rule ref="PSR2" >
-		<!-- Current exclusions -->
-		<exclude name="PSR1.Methods.CamelCapsMethodName" />
-		<exclude name="PSR1.Files.SideEffects.FoundWithSymbols" />
-		<exclude name="PSR2.Classes.PropertyDeclaration" />
-		<exclude name="PSR2.ControlStructures.SwitchDeclaration" /> <!-- causes php notice while linting -->
-		<exclude name="PSR2.ControlStructures.SwitchDeclaration.WrongOpenercase" />
-		<exclude name="PSR2.ControlStructures.SwitchDeclaration.WrongOpenerdefault" />
-		<exclude name="PSR2.ControlStructures.SwitchDeclaration.TerminatingComment" />
-		<exclude name="PSR2.Methods.MethodDeclaration.Underscore" />
-		<exclude name="Squiz.Scope.MethodScope" />
-		<exclude name="Squiz.Classes.ValidClassName.NotCamelCaps" />
-		<exclude name="Generic.Files.LineLength.TooLong" />
-		<exclude name="PEAR.Functions.ValidDefaultValue.NotAtEnd" />
-	</rule>
+    <!-- base rules are PSR-2 -->
+    <rule ref="PSR2" >
+        <!-- Current exclusions -->
+        <exclude name="PSR1.Methods.CamelCapsMethodName" />
+        <exclude name="PSR1.Files.SideEffects.FoundWithSymbols" />
+        <exclude name="PSR2.Classes.PropertyDeclaration" />
+        <exclude name="PSR2.ControlStructures.SwitchDeclaration" /> <!-- causes php notice while linting -->
+        <exclude name="PSR2.ControlStructures.SwitchDeclaration.WrongOpenercase" />
+        <exclude name="PSR2.ControlStructures.SwitchDeclaration.WrongOpenerdefault" />
+        <exclude name="PSR2.ControlStructures.SwitchDeclaration.TerminatingComment" />
+        <exclude name="PSR2.Methods.MethodDeclaration.Underscore" />
+        <exclude name="Squiz.Scope.MethodScope" />
+        <exclude name="Squiz.Classes.ValidClassName.NotCamelCaps" />
+        <exclude name="Generic.Files.LineLength.TooLong" />
+        <exclude name="PEAR.Functions.ValidDefaultValue.NotAtEnd" />
+    </rule>
 </ruleset>
-

src/ActivityEntry.php

@@ -2,10 +2,14 @@
 
 namespace SilverStripe\Snapshots;
 
+use Exception;
+use SilverStripe\ORM\DataObject;
 use SilverStripe\Versioned\Versioned;
 use SilverStripe\View\ArrayData;
-use Exception;
 
+/**
+ * @property DataObject $Subject
+ */
 class ActivityEntry extends ArrayData
 {
     const MODIFIED = 'MODIFIED';
@@ -22,16 +26,19 @@ class ActivityEntry extends ArrayData
 
     const UNPUBLISHED = 'UNPUBLISHED';
 
-    public static function createFromSnapshotItem(SnapshotItem $item)
+    public static function createFromSnapshotItem(SnapshotItem $item): self
     {
+        /** @var DataObject|Versioned $itemObj */
         $itemObj = $item->getItem();
 
-        if ($itemObj !== null && $itemObj instanceof SnapshotEvent) {
+        if ($itemObj instanceof SnapshotEvent) {
             $flag = null;
         } elseif ($item->WasPublished) {
             $flag = self::PUBLISHED;
         } elseif ($item->Parent()->exists()) {
-            $flag = $item->WasDeleted ? self::REMOVED : self::ADDED;
+            $flag = $item->WasDeleted
+                ? self::REMOVED
+                : self::ADDED;
         } elseif ($item->WasDeleted) {
             $flag = self::DELETED;
         } elseif ($item->WasUnpublished) {
@@ -45,10 +52,11 @@ public static function createFromSnapshotItem(SnapshotItem $item)
         // If the items been deleted then we want to get the last version of it
         if ($itemObj === null || $itemObj->WasDeleted) {
             // This gets all versions except for the deleted version so we just get the latest one
+            /** @var DataObject|Versioned $previousVersion */
             $previousVersion = Versioned::get_all_versions($item->ObjectClass, $item->ObjectID)
-                ->filter(['WasDeleted' => 0])
                 ->sort('Version', 'DESC')
-                ->first();
+                ->find('WasDeleted', 0);
+
             if ($previousVersion && $previousVersion->exists()) {
                 $itemObj = $item->getItem($previousVersion->Version);
             // This is to deal with the case in which there is no previous version
@@ -66,7 +74,7 @@ public static function createFromSnapshotItem(SnapshotItem $item)
             ));
         }
 
-        return new static([
+        return static::create([
             'Subject' => $itemObj,
             'Action' => $flag,
             'Owner' => null,

src/CsvBulkLoader/SaveListener.php

@@ -1,16 +1,23 @@
 <?php
 
-
 namespace SilverStripe\Snapshots\CsvBulkLoader;
 
+use SilverStripe\Dev\CsvBulkLoader;
 use SilverStripe\EventDispatcher\Dispatch\Dispatcher;
 use SilverStripe\EventDispatcher\Symfony\Event;
 use SilverStripe\ORM\DataExtension;
 use SilverStripe\ORM\DataObject;
 
 class SaveListener extends DataExtension
 {
-    public function onAfterProcessRecord(DataObject $obj, $preview, $isChanged)
+    /**
+     * Extension point in @see CsvBulkLoader::processRecord()
+     *
+     * @param DataObject $obj
+     * @param mixed $preview
+     * @param mixed $isChanged
+     */
+    public function onAfterProcessRecord(DataObject $obj, $preview, $isChanged): void
     {
         // No need tracking previews, since we don't expect any writes
         if ($preview) {

src/Elemental/SaveListener.php

@@ -1,20 +1,25 @@
 <?php
 
-
 namespace SilverStripe\Snapshots\Elemental;
 
 use DNADesign\Elemental\Forms\ElementalAreaField;
+use Psr\Container\NotFoundExceptionInterface;
 use SilverStripe\EventDispatcher\Dispatch\Dispatcher;
 use SilverStripe\EventDispatcher\Symfony\Event;
 use SilverStripe\ORM\DataExtension;
 
 class SaveListener extends DataExtension
 {
-    public function onSaveInto(array $elements)
+    /**
+     * @param array $elements
+     * @throws NotFoundExceptionInterface
+     */
+    public function onSaveInto(array $elements): void
     {
-        /* @var ElementalAreaField $owner */
+        /** @var ElementalAreaField $owner */
         $owner = $this->getOwner();
         $page = $owner->getArea()->getOwnerPage();
+
         Dispatcher::singleton()->trigger(
             'elementalAreaUpdated',
             Event::create(

src/Handler/CMSMain/Handler.php

@@ -1,6 +1,5 @@
 <?php
 
-
 namespace SilverStripe\Snapshots\Handler\CMSMain;
 
 use SilverStripe\CMS\Model\SiteTree;
@@ -10,7 +9,6 @@
 use SilverStripe\ORM\ValidationException;
 use SilverStripe\Snapshots\Handler\HandlerAbstract;
 use SilverStripe\Snapshots\Snapshot;
-use SilverStripe\Snapshots\SnapshotEvent;
 
 class Handler extends HandlerAbstract
 {
@@ -22,15 +20,18 @@ class Handler extends HandlerAbstract
     protected function createSnapshot(EventContextInterface $context): ?Snapshot
     {
         $action = $context->getAction();
+
         if ($action === null) {
             return null;
         }
 
-        /* @var HTTPResponse $result */
+        /** @var HTTPResponse $result */
         $result = $context->get('result');
+
         if (!$result instanceof HTTPResponse) {
             return null;
         }
+
         if ((int) $result->getStatusCode() !== 200) {
             return null;
         }

src/Handler/CsvBulkLoader/Handler.php

@@ -1,9 +1,10 @@
 <?php
 
-
 namespace SilverStripe\Snapshots\Handler\CsvBulkLoader;
 
+use InvalidArgumentException;
 use SilverStripe\EventDispatcher\Event\EventContextInterface;
+use SilverStripe\ORM\ValidationException;
 use SilverStripe\Snapshots\Handler\HandlerAbstract;
 use SilverStripe\Snapshots\Snapshot;
 
@@ -19,7 +20,7 @@ protected function createSnapshot(EventContextInterface $context): ?Snapshot
         $obj = $context->get('record');
 
         if (!$obj) {
-            throw new \InvalidArgumentException('Requires "record" in context');
+            throw new InvalidArgumentException('Requires "record" in context');
         }
 
         // Create an individual snapshot for each object to ensure they're all captured.

src/Handler/Elemental/ArchiveElementHandler.php

@@ -1,44 +1,49 @@
 <?php
 
-
 namespace SilverStripe\Snapshots\Handler\Elemental;
 
 use DNADesign\Elemental\Models\BaseElement;
 use SilverStripe\EventDispatcher\Event\EventContextInterface;
 use SilverStripe\Snapshots\Handler\GraphQL\Middleware\Handler;
 use SilverStripe\Snapshots\Snapshot;
 use SilverStripe\Snapshots\SnapshotHasher;
-use SilverStripe\Snapshots\SnapshotItem;
 use SilverStripe\Snapshots\SnapshotPublishable;
-use SilverStripe\Versioned\Versioned;
 
 class ArchiveElementHandler extends Handler
 {
+
     use SnapshotHasher;
 
     protected function createSnapshot(EventContextInterface $context): ?Snapshot
     {
         $action = $context->getAction();
+
         if ($action === null) {
             return null;
         }
 
         // GraphQL 4 ?? GraphQL 3
         $params = $context->get('variables') ?? $context->get('params');
+
         if (!$params) {
             return null;
         }
+
         $blockID = $params['blockId'] ?? null;
+
         if (!$blockID) {
             return null;
         }
+
         $block = BaseElement::get()->byID($blockID);
 
         // Ensure the block is gone
         if ($block) {
             return null;
         }
+
         $archivedBlock = SnapshotPublishable::get_at_last_snapshot(BaseElement::class, $blockID);
+
         if (!$archivedBlock) {
             return null;
         }