Switch previously implicitly declared properties to public visibility

[iansltx]

This resolves the bugged behavior from earlier today. Specifically:

1. As part of code cleanup to hit PHPStan Level 1, Configuration file for PHPStan level 1 #735 explicitly declared properties that had been implicitly set before. One catch: implicit properties are public, but this declaration was private.
2. Talk link building relied on the talk ID property (one of the implicitly set ones) to be public. When it wasn't, links that would have included talk ID included '' instead. For example, /talks//comments instead of /talks/1234/comments.
3. web2 used the talk comments link handed back from the API to grab talk comments on talk page load. Since this was not, in fact, a valid URL (even after some middlebox along the way dropped the extra slash), the API threw back a 405.
4. web2, which assumes that the talk comments endpoint will always return successfully, tried to grab the comments element from what was actually a 405 error response, then iterate over that nonexistent value to show comments, fatal'ing in the process.

So, while a bunch of pieces in this chain were brittle, the quick fix for now is to switch properties back to being public, matching their behavior from implicit declaration. Creating a follow-on issue to get the properties back to private once we've made sure that every external or descendant consumer of these properties does the consumption via a getter (or updates via a setter).

[localheinz]

What do you think about adding tests to prevent these kinds of regressions, perhaps something along the lines of:

diff --git a/tests/Controller/TalkLinkControllerTest.php b/tests/Controller/TalkLinkControllerTest.php
index 710aeb1..da9fbb3 100644
--- a/tests/Controller/TalkLinkControllerTest.php
+++ b/tests/Controller/TalkLinkControllerTest.php
@@ -10,6 +10,14 @@ use Teapot\StatusCode\Http;

 class TalkLinkControllerTest extends TalkBase
 {
+    public function testDefaults(): void
+    {
+        $controller = new TalkLinkController();
+
+        self::assertNull($controller->talk_mapper);
+        self::assertNull($controller->user_mapper);
+    }
+
     /**
      * Test sending delete link where the link id is not found
      */
diff --git a/tests/Model/ClientModelTest.php b/tests/Model/ClientModelTest.php
new file mode 100644
index 0000000..af51bad
--- /dev/null
+++ b/tests/Model/ClientModelTest.php
@@ -0,0 +1,21 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Joindin\Api\Test\Model;
+
+use Joindin\Api\Model\ClientModel;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @covers \Joindin\Api\Model\ClientModel
+ */
+final class ClientModelTest extends TestCase
+{
+    public function testDefaults(): void
+    {
+        $model = new ClientModel([]);
+
+        self::assertNull($model->id);
+    }
+}
diff --git a/tests/Model/EventCommentReportModelTest.php b/tests/Model/EventCommentReportModelTest.php
new file mode 100644
index 0000000..c1bbd3d
--- /dev/null
+++ b/tests/Model/EventCommentReportModelTest.php
@@ -0,0 +1,22 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Joindin\Api\Test\Model;
+
+use Joindin\Api\Model\EventCommentReportModel;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @covers \Joindin\Api\Model\EventCommentReportModel
+ */
+final class EventCommentReportModelTest extends TestCase
+{
+    public function testDefaults(): void
+    {
+        $model = new EventCommentReportModel([]);
+
+        self::assertNull($model->event_id);
+        self::assertNull($model->reporting_user_id);
+    }
+}
diff --git a/tests/Model/TalkCommentReportModelTest.php b/tests/Model/TalkCommentReportModelTest.php
new file mode 100644
index 0000000..4702c08
--- /dev/null
+++ b/tests/Model/TalkCommentReportModelTest.php
@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Joindin\Api\Test\Model;
+
+use Joindin\Api\Model\TalkCommentReportModel;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @covers \Joindin\Api\Model\EventCommentReportModel
+ */
+final class TalkCommentReportModelTest extends TestCase
+{
+    public function testDefaults(): void
+    {
+        $model = new TalkCommentReportModel([]);
+
+        self::assertNull($model->event_id);
+        self::assertNull($model->reporting_user_id);
+        self::assertNull($model->talk_id);
+    }
+}
diff --git a/tests/Model/TalkModelTest.php b/tests/Model/TalkModelTest.php
new file mode 100644
index 0000000..883ae93
--- /dev/null
+++ b/tests/Model/TalkModelTest.php
@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Joindin\Api\Test\Model;
+
+use Joindin\Api\Model\TalkModel;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @covers \Joindin\Api\Model\TalkModel
+ */
+final class TalkModelTest extends TestCase
+{
+    public function testDefaults(): void
+    {
+        $model = new TalkModel([]);
+
+        self::assertNull($model->event_id);
+        self::assertNull($model->ID);
+        self::assertNull($model->stub);
+    }
+}
diff --git a/tests/Model/TokenModelTest.php b/tests/Model/TokenModelTest.php
new file mode 100644
index 0000000..c1a5aa6
--- /dev/null
+++ b/tests/Model/TokenModelTest.php
@@ -0,0 +1,21 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Joindin\Api\Test\Model;
+
+use Joindin\Api\Model\TokenModel;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @covers \Joindin\Api\Model\TokenModel
+ */
+final class TokenModelTest extends TestCase
+{
+    public function testDefaults(): void
+    {
+        $model = new TokenModel([]);
+
+        self::assertNull($model->id);
+    }
+}
diff --git a/tests/Model/TwitterRequestTokenModelTest.php b/tests/Model/TwitterRequestTokenModelTest.php
new file mode 100644
index 0000000..001b86b
--- /dev/null
+++ b/tests/Model/TwitterRequestTokenModelTest.php
@@ -0,0 +1,21 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Joindin\Api\Test\Model;
+
+use Joindin\Api\Model\TwitterRequestTokenModel;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @covers \Joindin\Api\Model\TwitterRequestTokenModel
+ */
+final class TwitterRequestTokenModelTest extends TestCase
+{
+    public function testDefaults(): void
+    {
+        $model = new TwitterRequestTokenModel([]);
+
+        self::assertNull($model->ID);
+    }
+}

[iansltx]

@localheinz Not sure how those tests would prevent regressions. The regression here was that required properties became inaccessible, not the inverse. The contract between e.g. models and consumers using model properties was being honored by both sides, even if it was poorly documented.

[iansltx]

So, this didn't solve the problem, and I just figured out why: models have magic getters, and store their state inside a $data property. Throwing props at the top level breaks the read side of the equation, since the top-level props would shadow stuff that normally hits the magic getters.

So the solution here is to drop props back out of models and let them do their thing. For now, anyway...I'm not a huge magic getters fan, particularly as we can't just throw an IDE helper at this to get @Property annotations to see what's going on.