Merge pull request #12 from ingenerator/2.x-class-typing

[BREAKING] Add strict typing, switch from array result to a DTO, bump PHPUnit and drop 8.2

Author: acoulton

.gitattributes

@@ -27,3 +27,10 @@ README  text eol=lf
 *.swf    binary
 *.zip    binary
 *.sqlite binary
+
+# Ignore paths that should not be included in an archive (eg for a distribution version)
+/.github export-ignore
+/docs export-ignore
+/test export-ignore
+phpunit.xml export-ignore
+koharness.php export-ignore

.github/workflows/test.yaml

@@ -14,13 +14,10 @@ jobs:
       fail-fast: false
       matrix:
         php_version:
-        - '8.2'
         - '8.3'
         dependencies:
         - 'default'
         include:
-        - php_version: '8.2'
-          dependencies: 'lowest'
         - php_version: '8.3'
           dependencies: 'lowest'
     steps:

CHANGELOG.md

@@ -1,6 +1,13 @@
 ### Unreleased
 
 * [BREAKING] Use doctrine attributes instead of annotations for entity metadata
+* [BREAKING] Add hard parameter and return typehints to all classes
+* [BREAKING] ContentSnippetContentFilter is now an interface, use HtmlPurifierContentFilter for 
+  a runtime implementation. The ->filterContent now returns a ContentFilterResult DTO instead of
+  a plain array.
+* Drop support for PHP 8.2
+* Internal test & config files are now excluded from distribution archives
+* Upgraded to PHPUnit 11 for internal tests
 
 ### v1.5.0 (2024-10-01)
 

composer.json

@@ -14,15 +14,14 @@
   "require": {
     "composer/installers": "^1.9",
     "ezyang/htmlpurifier": "^4.15",
-    "php": "~8.2.0 || ~8.3.0",
+    "php": "~8.3.0",
     "ingenerator/kohana-core": "^4.7",
     "ingenerator/kohana-extras": "^2.0 || ^3.0",
     "ingenerator/kohana-view": "^4.4"
   },
   "require-dev": {
     "kohana/koharness": "dev-master",
-    "phpunit/phpunit": "^9.5.5",
-    "johnkary/phpunit-speedtrap": "^3.3"
+    "phpunit/phpunit": "^11"
   },
   "repositories": [
     {

phpunit.xml

@@ -3,28 +3,20 @@
 <phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
          bootstrap="test/bootstrap.php"
          colors="true"
-         convertErrorsToExceptions="true"
-         convertNoticesToExceptions="true"
-         convertWarningsToExceptions="true"
-         convertDeprecationsToExceptions="true"
-         xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd">
+         failOnDeprecation="true"
+         failOnNotice="true"
+         failOnWarning="true"
+         failOnRisky="true"
+         displayDetailsOnIncompleteTests="true"
+         displayDetailsOnSkippedTests="true"
+         displayDetailsOnTestsThatTriggerDeprecations="true"
+         displayDetailsOnTestsThatTriggerErrors="true"
+         displayDetailsOnTestsThatTriggerNotices="true"
+         displayDetailsOnTestsThatTriggerWarnings="true"
+         xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/11.5/phpunit.xsd">
     <testsuites>
         <testsuite name="Unit">
             <directory>test/unit</directory>
         </testsuite>
     </testsuites>
-    <listeners>
-        <listener class="JohnKary\PHPUnit\Listener\SpeedTrapListener">
-            <arguments>
-                <array>
-                    <element key="slowThreshold">
-                        <integer>200</integer>
-                    </element>
-                    <element key="reportLength">
-                        <integer>20</integer>
-                    </element>
-                </array>
-            </arguments>
-        </listener>
-    </listeners>
 </phpunit>

src/BehatContexts/ContentSnippetsContext.php

@@ -9,62 +9,54 @@
 
 use Behat\Gherkin\Node\PyStringNode;
 use Behat\MinkExtension\Context\RawMinkContext;
+use Behat\Step\Given;
 use Ingenerator\ContentSnippets\Repository\ContentSnippetRepository;
+use PHPUnit\Framework\Assert;
+use function trim;
 
 class ContentSnippetsContext extends RawMinkContext
 {
 
-    /**
-     * @var \Ingenerator\ContentSnippets\Repository\ContentSnippetRepository
-     */
-    protected $repo;
+    protected ContentSnippetRepository $repo;
 
     public function __construct(ContentSnippetRepository $repo)
     {
         $this->repo = $repo;
     }
 
-    /**
-     * @Given /^the (?P<slug>[^ ]+) snippet has the following content:$/
-     */
-    public function givenContent($slug, PyStringNode $content)
+    #[Given('/^the (?P<slug>[^ ]+) snippet has the following content:$/')]
+    public function givenContent(string $slug, PyStringNode $content): void
     {
         $snippet = $this->repo->load($slug);
         $snippet->setContent($content->getRaw());
         $this->repo->save($snippet);
     }
 
-    /**
-     * @Given /^the (?P<slug>[^ ]+) snippet is empty$/
-     */
-    public function givenEmpty($slug)
+    #[Given('/^the (?P<slug>[^ ]+) snippet is empty$/')]
+    public function givenEmpty(string $slug): void
     {
         $snippet = $this->repo->load($slug);
         $snippet->setContent(NULL);
         $this->repo->save($snippet);
     }
 
-    /**
-     * @When /^I try to update the "(?P<display_name>[^"]+)" snippet with:$/
-     */
-    public function tryToUpdateContent($display_name, PyStringNode $new_content)
+    #[When('/^I try to update the "(?P<display_name>[^"]+)" snippet with:$/')]
+    public function tryToUpdateContent(string $display_name, PyStringNode $new_content): void
     {
         $assert = $this->assertSession();
-        $table  = $assert->elementExists('css', '[data-content-snippets-list]');
+        $table = $assert->elementExists('css', '[data-content-snippets-list]');
         $table->clickLink('Edit '.$display_name);
         $page = $this->getSession()->getPage();
         $page->fillField('Content', $new_content->getRaw());
         $page->pressButton('Save changes');
     }
 
-    /**
-     * @Then /^the (?P<selector>.+?) element should have this exact HTML:$/
-     */
-    public function assertElementExactHtml($selector, PyStringNode $expect)
+    #[Then('/^the (?P<selector>.+?) element should have this exact HTML:$/')]
+    public function assertElementExactHtml(string $selector, PyStringNode $expect): void
     {
         $element = $this->assertSession()->elementExists('css', $selector);
-        $actual  = \trim($element->getHtml());
-        \PHPUnit\Framework\Assert::assertEquals($expect->getRaw(), $actual);
+        $actual = trim($element->getHtml());
+        Assert::assertEquals($expect->getRaw(), $actual);
     }
 
 }

src/ContentFilter/HtmlPurifierContentFilter.php

@@ -0,0 +1,52 @@
+<?php
+
+namespace Ingenerator\ContentSnippets\ContentFilter;
+
+use HTMLPurifier;
+use Ingenerator\ContentSnippets\ContentFilterResult;
+use Ingenerator\ContentSnippets\ContentSnippetContentFilter;
+use Ingenerator\ContentSnippets\Entity\ContentSnippet;
+
+final class HtmlPurifierContentFilter implements ContentSnippetContentFilter
+{
+
+    public function __construct(
+        private readonly HTMLPurifier $purifier
+    ) {
+    }
+
+    public function filterContent(ContentSnippet $snippet, ?string $new_content): ContentFilterResult
+    {
+        if (ContentSnippet::isHtmlString($new_content)) {
+            if ($snippet->allowsHtml()) {
+                $cleaned = $this->cleanHtmlContent($new_content);
+
+                return new ContentFilterResult(
+                    cleaned_content: $cleaned,
+                    is_valid: TRUE,
+                    error_msg: NULL,
+                    was_cleaned: ($cleaned !== $new_content),
+                );
+            } else {
+                return new ContentFilterResult(
+                    cleaned_content: $new_content,
+                    is_valid: FALSE,
+                    error_msg: self::MSG_NO_HTML,
+                    was_cleaned: FALSE
+                );
+            }
+        } else {
+            return new ContentFilterResult(
+                cleaned_content: $new_content,
+                is_valid: TRUE,
+                error_msg: NULL,
+                was_cleaned: FALSE
+            );
+        }
+    }
+
+    protected function cleanHtmlContent(?string $new_content): ?string
+    {
+        return $this->purifier->purify($new_content);
+    }
+}

src/ContentFilterResult.php

@@ -0,0 +1,17 @@
+<?php
+
+namespace Ingenerator\ContentSnippets;
+
+readonly final class ContentFilterResult
+{
+
+    public function __construct(
+        public ?string $cleaned_content,
+        public bool $is_valid,
+        public ?string $error_msg,
+        public bool $was_cleaned,
+    ) {
+
+    }
+
+}

src/ContentSnippetContentFilter.php

@@ -9,57 +9,10 @@
 
 use Ingenerator\ContentSnippets\Entity\ContentSnippet;
 
-class ContentSnippetContentFilter
+interface ContentSnippetContentFilter
 {
-    const MSG_NO_HTML = 'This snippet does not accept HTML content, please use plain text.';
+    public const string MSG_NO_HTML = 'This snippet does not accept HTML content, please use plain text.';
 
-    /**
-     * @var \HTMLPurifier
-     */
-    private $purifier;
+    public function filterContent(ContentSnippet $snippet, ?string $new_content): ContentFilterResult;
 
-    /**
-     * ContentSnippetContentFilter constructor.
-     *
-     * @param \HTMLPurifier $purifier
-     */
-    public function __construct(\HTMLPurifier $purifier)
-    {
-        $this->purifier = $purifier;
-    }
-
-    public function filterContent(ContentSnippet $snippet, $new_content)
-    {
-        if (ContentSnippet::isHtmlString($new_content)) {
-            if ($snippet->allowsHtml()) {
-                $cleaned = $this->cleanHtmlContent($new_content);
-
-                return [
-                    'cleaned_content' => $cleaned,
-                    'is_valid'        => TRUE,
-                    'error_msg'       => NULL,
-                    'was_cleaned'     => ($cleaned !== $new_content),
-                ];
-            } else {
-                return [
-                    'cleaned_content' => $new_content,
-                    'is_valid'        => FALSE,
-                    'error_msg'       => static::MSG_NO_HTML,
-                    'was_cleaned'     => FALSE,
-                ];
-            }
-        } else {
-            return [
-                'cleaned_content' => $new_content,
-                'is_valid'        => TRUE,
-                'error_msg'       => NULL,
-                'was_cleaned'     => FALSE,
-            ];
-        }
-    }
-
-    protected function cleanHtmlContent($new_content)
-    {
-        return $this->purifier->purify($new_content);
-    }
 }