avoid crashing when not subclassing IGListSectionController

Summary:
Currently, if you use `IGListSectionController` without a subclass, `UICollectionView` will crash when requesting the cell. That's because `[IGListSectionController numberOfItems]` defaults to 1, but returns no cell. There's a few issues with that:
1. It can be tough to debug, because the crash in within `UICollectionView`. We don't know the dataSource or object type responsible.
2. The crash only happens if the user scrolls by the missing cell, which can make it hard to repro.
3. If we don't know how to render a single section, we might not want to crash the entire app. Passing a plain `IGListSectionController` kind of feels like the dataSource is ok with just rendering nothing. It's not clear at all that it will crash.

Options:
1. Crash immediately when a plain `IGListSectionController` is passed to `IGListAdapter`, so we get a clear callstack and API feedback. If the dataSource doesn't want to crash, it must return `IGEmptySectionController` that has 0 `numberOfItems`.
2. Don't crash, but log a `IGFailAssert()`. If the dataSource wants to throw on a missing object-controller match, they can can, but it's not the IGListKit default.

I'm leaning on #2 for a few reasons. It's really not obvious that passing a plain `IGListSectionController` would crash and no one will know that `IGEmptySectionController` exists. We could have a linter, but that only works within Meta.

Reviewed By: DimaVartanian

Differential Revision: D52087286

fbshipit-source-id: 8b8754d56e66c0c2b00f8df3b9671a6fc2287aea

CHANGELOG.md

@@ -94,6 +94,8 @@ The changelog for `IGListKit`. Also see the [releases](https://github.com/instag
 
 ### Fixes
 
+- Don't crash if you use `IGListSectionController` without a subclass [Maxime Ollivier](https://github.com/maxolls) (tbd)
+
 - Testing crash fix when calling `-[IGListAdapter reloadObjects ...]` during an update [Maxime Ollivier](https://github.com/maxolls) (tbd)
 
 - Repaired Swift Package Manager support. [Petro Rovenskyy](https://github.com/3a4oT/) [(#1487)](https://github.com/Instagram/IGListKit/pull/1487)

Source/IGListKit/IGListAdapter.m

@@ -709,6 +709,15 @@ - (IGListTransitionData *)_generateTransitionDataWithObjects:(NSArray *)objects
                     dataSource, object);
             return;
         }
+
+        if ([sectionController isMemberOfClass:[IGListSectionController class]]) {
+            // If IGListSectionController is not subclassed, it could be a side effect of a problem. For example, nothing stops
+            // dataSource from returning a plain IGListSectionController if it doesn't recognize the object type, instead of throwing.
+            // Why not throw here then? Maybe we should, but in most cases, it feels like an over reaction. If we don't know how to render
+            // a single item, terminating the entire app might not be necessary. The dataSource should be the one who decides if throwing is appropriate.
+            IGFailAssert(@"Ignoring IGListSectionController that's not a subclass from data source %@ for object %@", NSStringFromClass([dataSource class]), NSStringFromClass([object class]));
+            return;
+        }
 
         // in case the section controller was created outside of -listAdapter:sectionControllerForObject:
         sectionController.collectionContext = self;

Tests/IGListAdapterE2ETests.m

@@ -16,6 +16,7 @@
 #import "IGListAdapterUpdater.h"
 #import "IGListAdapterUpdaterInternal.h"
 #import "IGListTestCase.h"
+#import "IGListTestCollectionViewLayout.h"
 #import "IGListTestHelpers.h"
 #import "IGListTestOffsettingLayout.h"
 #import "IGListUpdateTransactionBuilder.h"
@@ -2643,4 +2644,20 @@ - (void)test_whenCollectionViewSyncsBeforeTheAdapterDataSourceIsSet_thenScheduli
     [self waitForExpectationsWithTimeout:30 handler:nil];
 }
 
+- (void)test_whenSectionControllerNotSubclassed_thatDoesNotCrash {
+    // We need a custom layout that creates attributes for all cells, even the ones with size
+    // zero, so that the UICollectionView requests all cells. Using `UICollectionViewDelegateFlowLayout`
+    // doesn't crash, because it doesn't seem to return attributes where the size is zero.
+    self.collectionView.collectionViewLayout = [IGListTestCollectionViewLayout new];
+
+    [self setupWithObjects:@[kIGTestDelegateDataSourceNoSectionControllerSubclass]];
+
+    XCTestExpectation *expectation = genExpectation;
+    [self.adapter reloadDataWithCompletion:^(BOOL finished) {
+        [self.collectionView layoutIfNeeded];
+        [expectation fulfill];
+    }];
+    [self waitForExpectationsWithTimeout:30 handler:nil];
+}
+
 @end

Tests/Objects/IGListTestCollectionViewLayout.h

@@ -0,0 +1,17 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+#import <UIKit/UIKit.h>
+
+NS_ASSUME_NONNULL_BEGIN
+
+/// Layout that 1) creates all attributes regardless of size, and 2) positions them with origin (0,0)
+@interface IGListTestCollectionViewLayout : UICollectionViewLayout
+
+@end
+
+NS_ASSUME_NONNULL_END

Tests/Objects/IGListTestCollectionViewLayout.m

@@ -0,0 +1,56 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+#import "IGListTestCollectionViewLayout.h"
+
+@implementation IGListTestCollectionViewLayout {
+    NSDictionary<NSIndexPath *, UICollectionViewLayoutAttributes *> *_attributes;
+}
+
+- (void)prepareLayout {
+    UICollectionView *const collectionView = self.collectionView;
+
+    // Get the UICollectionViewDelegateFlowLayout for sizes
+    if (![collectionView.delegate conformsToProtocol:@protocol(UICollectionViewDelegateFlowLayout)]) {
+        _attributes = nil;
+        return;
+    }
+    const id<UICollectionViewDelegateFlowLayout> flowDelegate = (id<UICollectionViewDelegateFlowLayout>)collectionView.delegate;
+
+    // Create the attributes
+    NSMutableDictionary<NSIndexPath *, UICollectionViewLayoutAttributes *> *const attributes = [NSMutableDictionary new];
+    const NSInteger numberOfSections = collectionView.numberOfSections;
+
+    for (NSInteger section = 0; section < numberOfSections; section++) {
+        const NSInteger numberOfItems = [collectionView numberOfItemsInSection:section];
+        for (NSInteger item = 0; item < numberOfItems; item++) {
+            NSIndexPath *const indexPath = [NSIndexPath indexPathForItem:item inSection:section];
+            UICollectionViewLayoutAttributes *const attribute = [UICollectionViewLayoutAttributes layoutAttributesForCellWithIndexPath:indexPath];
+            const CGSize size = [flowDelegate collectionView:collectionView layout:self sizeForItemAtIndexPath:indexPath];
+            attribute.frame = CGRectMake(0, 0, size.width, size.height);
+            attributes[indexPath] = attribute;
+        }
+    }
+
+    _attributes = [attributes copy];
+}
+
+- (nullable NSArray<__kindof UICollectionViewLayoutAttributes *> *)layoutAttributesForElementsInRect:(CGRect)rect {
+    NSMutableArray<UICollectionViewLayoutAttributes *> *attributes = [NSMutableArray new];
+    [_attributes enumerateKeysAndObjectsUsingBlock:^(NSIndexPath *indexPath, UICollectionViewLayoutAttributes *attribute, BOOL* stop) {
+        if (CGRectIntersectsRect(attribute.frame, rect)) {
+            [attributes addObject:attribute];
+        }
+    }];
+    return [attributes copy];
+}
+
+- (nullable UICollectionViewLayoutAttributes *)layoutAttributesForItemAtIndexPath:(NSIndexPath *)indexPath {
+    return _attributes[indexPath];
+}
+
+@end

Tests/Objects/IGTestDelegateDataSource.h

@@ -15,6 +15,7 @@
 @class IGTestObject;
 
 extern NSObject *const kIGTestDelegateDataSourceSkipObject;
+extern NSObject *const kIGTestDelegateDataSourceNoSectionControllerSubclass;
 
 @interface IGTestDelegateDataSource : NSObject <IGListTestCaseDataSource>
 

Tests/Objects/IGTestDelegateDataSource.m

@@ -13,6 +13,7 @@
 #import "IGTestObject.h"
 
 NSObject *const kIGTestDelegateDataSourceSkipObject = @"kIGTestDelegateDataSourceSkipObject";
+NSObject *const kIGTestDelegateDataSourceNoSectionControllerSubclass = @"kIGTestDelegateDataSourceNoSectionControllerSubclass";
 
 @implementation IGTestDelegateDataSource
 
@@ -23,6 +24,8 @@ - (NSArray *)objectsForListAdapter:(IGListAdapter *)listAdapter {
 - (IGListSectionController *)listAdapter:(IGListAdapter *)listAdapter sectionControllerForObject:(id)object {
     if ([object isEqual:kIGTestDelegateDataSourceSkipObject]) {
         return nil;
+    } else if ([object isEqual:kIGTestDelegateDataSourceNoSectionControllerSubclass]) {
+        return [IGListSectionController new];
     }
     IGTestDelegateController *sectionController = [[IGTestDelegateController alloc] init];
     sectionController.cellConfigureBlock = self.cellConfigureBlock;