Add IGListExperimentSkipNilIndexPathFiltering to opt into nil-index-path-aware visible cell filtering

Summary:
There's a subtle bug in `-[IGListAdapter visibleCellsForSectionController:]` and `-[IGListAdapter fullyVisibleCellsForSectionController:]`... both methods filter `[collectionView visibleCells]` by `[collectionView indexPathForCell:cell].section == section`, but `indexPathForCell:` is allowed to return `nil` for cells in transitional states — mid-`prepareForReuse`, mid-detach during a `performBatchUpdates`, or in flight between dequeue and final attachment. When that happens, sending `-section` to a `nil` `NSIndexPath` returns `0` in Objective-C, so any visible cell with a cleared index path is silently treated as belonging to **section 0**.

For most consumers this is harmless or invisible. But callers that walk these results and then read layout / window-relative geometry from each returned cell can end up touching cells whose model has been swapped out. We've seen this surface as crashes when a consumer triggers a layout pass on a cell whose backing data is mid-replacement (for example, text typesetting on an `NSAttributedString` that's being swapped out as the cell prepares for reuse). It's also a logical correctness issue: section 0 is the only section that can ever observe transitional cells from other sections leaking in.

The fix itself is essentially one line: skip cells where `indexPathForCell:` returned `nil` before doing the section comparison. The interesting question is how to roll it out.

Given how far-reaching this could be for code depending on IGListKit, I've introduced this as an opt-in experiment so we can effectively test this in Instagram and Threads before iterating or making an informed decision.

The fix is strictly subtractive — it can only narrow the result set, never expand it. Most consumers will just get more correct results. But these methods are called from a wide variety of places across multiple apps that depend on IGListKit, and we can't realistically audit every caller for code that may have implicitly leaned on the section-0 leak (for example, a state machine that flips the moment any cell becomes visible). Once it's been baked in across enough downstream consumers we can revisit and consider flipping the default or removing the flag entirely.

If you'd like to opt in yourself, you can set:

```objc
adapter.experiments |= IGListExperimentSkipNilIndexPathFiltering;
```

Differential Revision: D106699470

fbshipit-source-id: 966934aa7b1eefee3289d7d06c9a34e5a2ef0d35

Author: camroth

Source/IGListDiffKit/IGListExperiments.h

@@ -28,6 +28,12 @@ typedef NS_OPTIONS (NSInteger, IGListExperiment) {
     IGListExperimentRemoveDataSourceChangeEarlyExit = 1 << 4,
     /// Avoids creating off-screen cells
     IGListExperimentFixPreferredFocusedView = 1 << 5,
+    /// Excludes cells whose `[UICollectionView indexPathForCell:]` returns nil from
+    /// `-visibleCellsForSectionController:` and `-fullyVisibleCellsForSectionController:`.
+    /// Without this experiment, transitional cells (mid-prepareForReuse, mid-detach during
+    /// batch updates, or in flight between dequeue and final attachment) are mis-attributed
+    /// to section 0 because `-section` sent to a nil `NSIndexPath` returns 0.
+    IGListExperimentSkipNilIndexPathFiltering = 1 << 6,
 };
 
 /**

Source/IGListKit/IGListAdapter.m

@@ -34,17 +34,27 @@
 /// Returns the cells from `[collectionView visibleCells]` whose index path is in `section`.
 /// When `fullyVisibleOnly` is YES, additionally requires the cell's frame to be fully contained
 /// within the collection view's content-inset-adjusted bounds.
+///
+/// When `skipNilIndexPath` is YES, cells whose `-indexPathForCell:` returns nil are skipped.
+/// Such cells are in a transitional state (mid-prepareForReuse, mid-detach during batch updates,
+/// or in flight between dequeue and final attachment). Without that guard, `-section` sent to a
+/// nil index path returns 0, which mis-attributes transitional cells to section 0.
 static NSArray<UICollectionViewCell *> *IGListAdapterCellsInSection(UICollectionView *collectionView,
                                                                      NSInteger section,
-                                                                     BOOL fullyVisibleOnly) {
+                                                                     BOOL fullyVisibleOnly,
+                                                                     BOOL skipNilIndexPath) {
     NSMutableArray<UICollectionViewCell *> *const cells = [NSMutableArray new];
     NSArray<UICollectionViewCell *> *const visibleCells = [collectionView visibleCells];
     const CGRect insetBounds = fullyVisibleOnly
         ? UIEdgeInsetsInsetRect(collectionView.bounds, collectionView.contentInset)
         : CGRectZero;
 
     for (UICollectionViewCell *cell in visibleCells) {
-        if ([collectionView indexPathForCell:cell].section != section) {
+        NSIndexPath *const cellIndexPath = [collectionView indexPathForCell:cell];
+        if (skipNilIndexPath && cellIndexPath == nil) {
+            continue;
+        }
+        if (cellIndexPath.section != section) {
             continue;
         }
         if (fullyVisibleOnly) {
@@ -1167,7 +1177,8 @@ - (nullable UICollectionViewLayoutAttributes *)layoutAttributesForItemAtIndex:(N
         // The section controller is not in the map, which can happen if the associated object was deleted or after a full reload.
         return @[];
     }
-    return IGListAdapterCellsInSection(self.collectionView, section, /*fullyVisibleOnly=*/YES);
+    const BOOL skipNilIndexPath = IGListExperimentEnabled(self.experiments, IGListExperimentSkipNilIndexPathFiltering);
+    return IGListAdapterCellsInSection(self.collectionView, section, /*fullyVisibleOnly=*/YES, skipNilIndexPath);
 }
 
 - (NSArray<UICollectionViewCell *> *)visibleCellsForSectionController:(IGListSectionController *)sectionController {
@@ -1176,7 +1187,8 @@ - (nullable UICollectionViewLayoutAttributes *)layoutAttributesForItemAtIndex:(N
         // The section controller is not in the map, which can happen if the associated object was deleted or after a full reload.
         return @[];
     }
-    return IGListAdapterCellsInSection(self.collectionView, section, /*fullyVisibleOnly=*/NO);
+    const BOOL skipNilIndexPath = IGListExperimentEnabled(self.experiments, IGListExperimentSkipNilIndexPathFiltering);
+    return IGListAdapterCellsInSection(self.collectionView, section, /*fullyVisibleOnly=*/NO, skipNilIndexPath);
 }
 
 - (NSArray<NSIndexPath *> *)visibleIndexPathsForSectionController:(IGListSectionController *) sectionController {

Tests/IGListAdapterTests.m

@@ -2345,6 +2345,68 @@ - (void)test_whenSectionControllerRemoved_thatVisibleCellsIsEmpty {
     XCTAssertEqual(cells.count, 0);
 }
 
+// Regression test for IGListExperimentSkipNilIndexPathFiltering. When a cell's index path has
+// been cleared (mid-prepareForReuse, mid-detach during batch updates, or in flight between
+// dequeue and final attachment), `-[NSIndexPath section]` sent to nil returns 0, so without
+// the experiment flag the filter erroneously includes any visible cell with a nil index path
+// in section 0's result. With the flag, those cells are skipped.
+- (void)test_whenSkipNilIndexPathExperimentEnabled_andCellHasNilIndexPath_thatVisibleCellsForSection0DoesNotIncludeIt {
+    self.adapter.experiments |= IGListExperimentSkipNilIndexPathFiltering;
+
+    // Set up two sections so we have cells in section 1 that should never appear in section 0.
+    self.dataSource.objects = @[@1, @2];
+    [self.adapter reloadDataWithCompletion:nil];
+    [self.collectionView layoutIfNeeded];
+
+    IGListSectionController *const section0Controller = [self.adapter sectionControllerForObject:@1];
+    IGListSectionController *const section1Controller = [self.adapter sectionControllerForObject:@2];
+    UICollectionViewCell *const section1Cell = [self.adapter cellForItemAtIndex:0 sectionController:section1Controller];
+    XCTAssertNotNil(section1Cell, @"Setup precondition: a real section-1 cell must exist.");
+    XCTAssertTrue([[self.collectionView visibleCells] containsObject:section1Cell],
+                  @"Setup precondition: section-1 cell must be in the collection view's visible cells.");
+
+    // Force [collectionView indexPathForCell:section1Cell] to return nil, simulating a cell that
+    // is mid-prepareForReuse / mid-detach. Other cells continue to return their real index paths.
+    id mockCollectionView = OCMPartialMock(self.collectionView);
+    OCMStub([mockCollectionView indexPathForCell:section1Cell]).andReturn(nil);
+
+    NSArray<UICollectionViewCell *> *const section0VisibleCells = [self.adapter visibleCellsForSectionController:section0Controller];
+    XCTAssertFalse([section0VisibleCells containsObject:section1Cell],
+                   @"With experiment enabled, cell with nil index path must not be mis-attributed to section 0 by visibleCellsForSectionController:.");
+
+    NSArray<UICollectionViewCell *> *const section0FullyVisibleCells = [self.adapter fullyVisibleCellsForSectionController:section0Controller];
+    XCTAssertFalse([section0FullyVisibleCells containsObject:section1Cell],
+                   @"With experiment enabled, cell with nil index path must not be mis-attributed to section 0 by fullyVisibleCellsForSectionController:.");
+
+    [mockCollectionView stopMocking];
+}
+
+// Counter-test: when the experiment flag is OFF (default), the legacy mis-attribution behavior
+// is preserved. Verifies the experiment flag is observably gating the behavior change.
+- (void)test_whenSkipNilIndexPathExperimentDisabled_andCellHasNilIndexPath_thatVisibleCellsForSection0IncludesIt_legacyBehavior {
+    XCTAssertFalse(IGListExperimentEnabled(self.adapter.experiments, IGListExperimentSkipNilIndexPathFiltering),
+                   @"Setup precondition: experiment must default to off.");
+
+    self.dataSource.objects = @[@1, @2];
+    [self.adapter reloadDataWithCompletion:nil];
+    [self.collectionView layoutIfNeeded];
+
+    IGListSectionController *const section0Controller = [self.adapter sectionControllerForObject:@1];
+    IGListSectionController *const section1Controller = [self.adapter sectionControllerForObject:@2];
+    UICollectionViewCell *const section1Cell = [self.adapter cellForItemAtIndex:0 sectionController:section1Controller];
+    XCTAssertNotNil(section1Cell);
+
+    id mockCollectionView = OCMPartialMock(self.collectionView);
+    OCMStub([mockCollectionView indexPathForCell:section1Cell]).andReturn(nil);
+
+    // Legacy buggy behavior: [nil section] == 0, so the section-1 cell is mis-attributed to section 0.
+    NSArray<UICollectionViewCell *> *const section0VisibleCells = [self.adapter visibleCellsForSectionController:section0Controller];
+    XCTAssertTrue([section0VisibleCells containsObject:section1Cell],
+                  @"Without experiment, legacy mis-attribution must be preserved (cell appears in section 0).");
+
+    [mockCollectionView stopMocking];
+}
+
 - (void)test_whenSectionControllerRemoved_thatVisibleIndexPathIsEmpty {
     self.dataSource.objects = @[@1];
     [self.adapter performUpdatesAnimated:NO completion:nil];