Code review (spatie#60)

* nitpicks

* Apply fixes from StyleCI (spatie#59)

* cs

Author: freekmurze

src/Exceptions/InvalidFeedItem.php

@@ -8,7 +8,7 @@
 class InvalidFeedItem extends Exception
 {
     /** @var \Spatie\Feed\FeedItem|null */
-    protected $subject;
+    public $subject;
 
     public static function notFeedable($subject)
     {
@@ -25,11 +25,6 @@ public static function missingField(FeedItem $subject, $field)
         return (new static("Field `{$field}` is required"))->withSubject($subject);
     }
 
-    public function getSubject()
-    {
-        return $this->subject;
-    }
-
     protected function withSubject()
     {
         $this->subject;

src/Feed.php

@@ -3,6 +3,7 @@
 namespace Spatie\Feed;
 
 use Illuminate\Http\Response;
+use Illuminate\Support\Collection;
 use Spatie\Feed\Exceptions\InvalidFeedItem;
 use Illuminate\Contracts\Support\Responsable;
 
@@ -25,7 +26,7 @@ public function __construct($title, $url, $resolver)
         $this->items = $this->resolveItems($resolver);
     }
 
-    public function toResponse($request)
+    public function toResponse($request): Response
     {
         $meta = [
             'id' => url($this->url),
@@ -44,7 +45,7 @@ public function toResponse($request)
         ]);
     }
 
-    protected function resolveItems($resolver)
+    protected function resolveItems($resolver): Collection
     {
         $resolver = array_wrap($resolver);
 
@@ -57,7 +58,7 @@ protected function resolveItems($resolver)
         });
     }
 
-    protected function castToFeedItem($feedable)
+    protected function castToFeedItem($feedable): FeedItem
     {
         if (is_array($feedable)) {
             $feedable = new FeedItem($feedable);
@@ -84,7 +85,7 @@ protected function castToFeedItem($feedable)
         return $feedItem;
     }
 
-    protected function lastUpdated()
+    protected function lastUpdated(): string
     {
         if ($this->items->isEmpty()) {
             return '';

src/Feedable.php

@@ -5,7 +5,7 @@
 interface Feedable
 {
     /**
-     * @return \Spatie\Feed\FeedItem
+     * @return array|\Spatie\Feed\FeedItem
      */
     public function toFeedItem();
 }

src/Http/FeedController.php

@@ -14,9 +14,7 @@ public function __invoke()
 
         $feed = $feeds[$name] ?? null;
 
-        if (! $feed) {
-            abort(404);
-        }
+        abort_unless($feed, 404);
 
         return new Feed($feed['title'], request()->url(), $feed['items']);
     }

tests/DummyRepository.php

@@ -2,6 +2,8 @@
 
 namespace Spatie\Feed\Test;
 
+use Illuminate\Support\Collection;
+
 class DummyRepository
 {
     /** @var \Illuminate\Support\Collection */
@@ -18,7 +20,7 @@ public function __construct()
         ]);
     }
 
-    public function getAll()
+    public function getAll(): Collection
     {
         return $this->items;
     }

tests/FeedTest.php

@@ -4,23 +4,22 @@
 
 class FeedTest extends TestCase
 {
+    /** @var array */
     protected $feedNames = ['feed1', 'feed2', 'feed3'];
 
     /** @test */
     public function all_feeds_are_available_on_their_registered_routes()
     {
         collect($this->feedNames)->each(function (string $feedName) {
-            $response = $this->call('GET', "/feedBaseUrl/{$feedName}");
-
-            $this->assertEquals(200, $response->getStatusCode());
+            $this->get("/feedBaseUrl/{$feedName}")->assertStatus(200);
         });
     }
 
     /** @test */
     public function all_feed_items_have_expected_data()
     {
         collect($this->feedNames)->each(function (string $feedName) {
-            $generatedFeedContent = $this->call('GET', "/feedBaseUrl/{$feedName}")->getContent();
+            $generatedFeedContent = $this->get("/feedBaseUrl/{$feedName}")->getContent();
 
             $this->assertMatchesSnapshot($generatedFeedContent);
         });
@@ -29,7 +28,7 @@ public function all_feed_items_have_expected_data()
     /** @test */
     public function it_can_render_all_feed_links_via_a_view()
     {
-        $feedLinksHtml = $this->call('GET', '/test-route')->getContent();
+        $feedLinksHtml = $this->get('/test-route')->getContent();
 
         $this->assertMatchesSnapshot($feedLinksHtml);
     }

tests/TestCase.php

@@ -33,21 +33,21 @@ protected function getEnvironmentSetUp($app)
     {
         $feed = [
             [
-                'items'       => 'Spatie\Feed\Test\DummyRepository@getAll',
-                'url'         => '/feed1',
-                'title'       => 'Feed 1',
+                'items' => 'Spatie\Feed\Test\DummyRepository@getAll',
+                'url' => '/feed1',
+                'title' => 'Feed 1',
                 'description' => 'This is feed 1 from the unit tests',
             ],
             [
-                'items'       => 'Spatie\Feed\Test\DummyRepository@getAll',
-                'url'         => '/feed2',
-                'title'       => 'Feed 2',
+                'items' => 'Spatie\Feed\Test\DummyRepository@getAll',
+                'url' => '/feed2',
+                'title' => 'Feed 2',
                 'description' => 'This is feed 2 from the unit tests',
             ],
             [
-                'items'       => ['Spatie\Feed\Test\DummyRepository@getAllWithArguments', 'first'],
-                'url'         => '/feed3',
-                'title'       => 'Feed 3',
+                'items' => ['Spatie\Feed\Test\DummyRepository@getAllWithArguments', 'first'],
+                'url' => '/feed3',
+                'title' => 'Feed 3',
                 'description' => 'This is feed 3 from the unit tests',
             ],
         ];