From dfb52ea701f40dcc23a42d670b21550123efc660 Mon Sep 17 00:00:00 2001 From: Tim Oliver Date: Mon, 17 Apr 2023 20:41:49 -0700 Subject: [PATCH] Refactor code syntax in public classes to be easier to provide code coverage Summary: While testing the existing code coverage, I discovered several code patterns in some of IGListKit's public classes that were literally impossible to cover. These included things like bailing out of blocks when `weakSelf` is `nil`, or exiting early inside `switch` statements. This diff reimplements some of the existing logic in several of the public facing IGListKit classes, so that all of these classes can be 100% covered by unit tests. Reviewed By: candance Differential Revision: D45002889 fbshipit-source-id: 6b87ea6338b9f33bed7955d6cc797d116b533085 --- Source/IGListKit/IGListAdapter.m | 49 +++++++------------ Source/IGListKit/IGListAdapterUpdater.m | 5 +- Source/IGListKit/IGListCollectionView.m | 2 +- .../IGListKit/IGListCollectionViewLayout.mm | 29 ++++------- .../Internal/IGListAdapterUpdaterHelpers.h | 2 + .../Internal/IGListAdapterUpdaterHelpers.m | 8 +++ 6 files changed, 41 insertions(+), 54 deletions(-) diff --git a/Source/IGListKit/IGListAdapter.m b/Source/IGListKit/IGListAdapter.m index 11500480..4984fde1 100644 --- a/Source/IGListKit/IGListAdapter.m +++ b/Source/IGListKit/IGListAdapter.m @@ -244,8 +244,8 @@ typedef struct OffsetRange { case UICollectionViewScrollPositionCenteredHorizontally: { const CGFloat insets = (contentInset.left - contentInset.right) / 2.0; contentOffset.x = offsetMid - collectionViewWidth / 2.0 - insets; - break; } + break; case UICollectionViewScrollPositionLeft: case UICollectionViewScrollPositionNone: case UICollectionViewScrollPositionTop: @@ -269,8 +269,8 @@ typedef struct OffsetRange { case UICollectionViewScrollPositionCenteredVertically: { const CGFloat insets = (contentInset.top - contentInset.bottom) / 2.0; contentOffset.y = offsetMid - collectionViewHeight / 2.0 - insets; - break; } + break; case UICollectionViewScrollPositionTop: case UICollectionViewScrollPositionNone: case UICollectionViewScrollPositionLeft: @@ -397,22 +397,22 @@ typedef struct OffsetRange { __weak __typeof__(self) weakSelf = self; IGListTransitionDataBlock sectionDataBlock = ^IGListTransitionData *{ __typeof__(self) strongSelf = weakSelf; - if (strongSelf == nil) { - return nil; + IGListTransitionData *transitionData = nil; + if (strongSelf) { + NSArray *toObjects = objectsWithDuplicateIdentifiersRemoved([dataSource objectsForListAdapter:strongSelf]); + transitionData = [strongSelf _generateTransitionDataWithObjects:toObjects dataSource:dataSource]; } - NSArray *toObjects = objectsWithDuplicateIdentifiersRemoved([dataSource objectsForListAdapter:strongSelf]); - return [strongSelf _generateTransitionDataWithObjects:toObjects dataSource:dataSource]; + return transitionData; }; IGListTransitionDataApplyBlock applySectionDataBlock = ^void(IGListTransitionData *data) { __typeof__(self) strongSelf = weakSelf; - if (strongSelf == nil) { - return; + if (strongSelf) { + // temporarily capture the item map that we are transitioning from in case + // there are any item deletes at the same + strongSelf.previousSectionMap = [strongSelf.sectionMap copy]; + [strongSelf _updateWithData:data]; } - // temporarily capture the item map that we are transitioning from in case - // there are any item deletes at the same - strongSelf.previousSectionMap = [strongSelf.sectionMap copy]; - [strongSelf _updateWithData:data]; }; IGListUpdaterCompletion outerCompletionBlock = ^(BOOL finished){ @@ -474,12 +474,12 @@ typedef struct OffsetRange { // use the item map based on whether or not we're in an update block IGListSectionMap *map = [self _sectionMapUsingPreviousIfInUpdateBlock:YES]; - for (id object in objects) { + [objects enumerateObjectsUsingBlock:^(id object, NSUInteger idx, BOOL *stop) { // look up the item using the map's lookup function. might not be the same item const NSInteger section = [map sectionForObject:object]; const BOOL notFound = section == NSNotFound; if (notFound) { - continue; + return; } [sections addIndex:section]; @@ -488,11 +488,10 @@ typedef struct OffsetRange { [map updateObject:object]; [[map sectionControllerForSection:section] didUpdateToObject:object]; } - } + }]; UICollectionView *collectionView = self.collectionView; IGAssert(collectionView != nil, @"Tried to reload the adapter without a collection view"); - [self.updater reloadCollectionView:collectionView sections:sections]; } @@ -691,7 +690,7 @@ typedef struct OffsetRange { // for IGListSectionController subclasses after calling [super init] IGListSectionControllerPushThread(self.viewController, self); - for (id object in objects) { + [objects enumerateObjectsUsingBlock:^(id object, NSUInteger idx, BOOL *stop) { // infra checks to see if a controller exists IGListSectionController *sectionController = [map sectionControllerForObject:object]; @@ -703,7 +702,7 @@ typedef struct OffsetRange { if (sectionController == nil) { IGLKLog(@"WARNING: Ignoring nil section controller returned by data source %@ for object %@.", dataSource, object); - continue; + return; } // in case the section controller was created outside of -listAdapter:sectionControllerForObject: @@ -712,7 +711,7 @@ typedef struct OffsetRange { [sectionControllers addObject:sectionController]; [validObjects addObject:object]; - } + }]; #if DEBUG IGAssert([NSSet setWithArray:sectionControllers].count == sectionControllers.count, @@ -874,17 +873,7 @@ typedef struct OffsetRange { - (nullable IGListSectionController *)_sectionControllerForCell:(UICollectionViewCell *)cell { IGAssertMainThread(); - if (IGListExperimentEnabled(_experiments, IGListExperimentSkipViewSectionControllerMap)) { - NSIndexPath *indexPath = [_collectionView indexPathForCell:cell]; - if (indexPath != nil) { - NSInteger section = indexPath.section; - return [self.sectionMap sectionControllerForSection:section]; - } else { - return nil; - } - } else { - return [_viewSectionControllerMap objectForKey:cell]; - } + return [_viewSectionControllerMap objectForKey:cell]; } - (void)removeMapForView:(UICollectionReusableView *)view { diff --git a/Source/IGListKit/IGListAdapterUpdater.m b/Source/IGListKit/IGListAdapterUpdater.m index 72697ef5..62463a79 100644 --- a/Source/IGListKit/IGListAdapterUpdater.m +++ b/Source/IGListKit/IGListAdapterUpdater.m @@ -313,11 +313,8 @@ static NSUInteger IGListIdentifierHash(const void *item, NSUInteger (*size)(cons id delegate = self.delegate; - NSMutableIndexSet *visibleSections = [NSMutableIndexSet new]; NSArray *visibleIndexPaths = [collectionView indexPathsForVisibleItems]; - for (NSIndexPath *visibleIndexPath in visibleIndexPaths) { - [visibleSections addIndex:visibleIndexPath.section]; - } + NSIndexSet *visibleSections = IGListSectionIndexFromIndexPaths(visibleIndexPaths); [delegate listAdapterUpdater:self willReloadSections:visibleSections collectionView:collectionView]; diff --git a/Source/IGListKit/IGListCollectionView.m b/Source/IGListKit/IGListCollectionView.m index e309e5c1..5c5948a6 100644 --- a/Source/IGListKit/IGListCollectionView.m +++ b/Source/IGListKit/IGListCollectionView.m @@ -33,7 +33,7 @@ return nil; } -#pragma mark - Overides reloads +#pragma mark - Overrides reloads - (void)reloadItemsAtIndexPaths:(NSArray *)indexPaths { [self _didModifyIndexPaths:indexPaths]; diff --git a/Source/IGListKit/IGListCollectionViewLayout.mm b/Source/IGListKit/IGListCollectionViewLayout.mm index ba098507..6678b835 100644 --- a/Source/IGListKit/IGListCollectionViewLayout.mm +++ b/Source/IGListKit/IGListCollectionViewLayout.mm @@ -128,16 +128,10 @@ static void adjustZIndexForAttributes(UICollectionViewLayoutAttributes *attribut const NSInteger maxZIndexPerSection = 1000; const NSInteger baseZIndex = attributes.indexPath.section * maxZIndexPerSection; - switch (attributes.representedElementCategory) { - case UICollectionElementCategoryCell: - attributes.zIndex = baseZIndex + attributes.indexPath.item; - break; - case UICollectionElementCategorySupplementaryView: - attributes.zIndex = baseZIndex + maxZIndexPerSection - 1; - break; - case UICollectionElementCategoryDecorationView: - attributes.zIndex = baseZIndex - 1; - break; + if (attributes.representedElementCategory == UICollectionElementCategoryCell) { + attributes.zIndex = baseZIndex + attributes.indexPath.item; + } else if (attributes.representedElementCategory == UICollectionElementCategorySupplementaryView) { + attributes.zIndex = baseZIndex + maxZIndexPerSection - 1; } } @@ -373,16 +367,13 @@ static void adjustZIndexForAttributes(UICollectionViewLayoutAttributes *attribut UICollectionView *collectionView = self.collectionView; const UIEdgeInsets contentInset = collectionView.ig_contentInset; switch (self.scrollDirection) { - case UICollectionViewScrollDirectionVertical: { - const CGFloat height = CGRectGetMaxY(section.bounds) + section.insets.bottom; - return CGSizeMake(CGRectGetWidth(collectionView.bounds) - contentInset.left - contentInset.right, height); - } - case UICollectionViewScrollDirectionHorizontal: { - const CGFloat width = CGRectGetMaxX(section.bounds) + section.insets.right; - return CGSizeMake(width, CGRectGetHeight(collectionView.bounds) - contentInset.top - contentInset.bottom); - } + case UICollectionViewScrollDirectionVertical: + return CGSizeMake(CGRectGetWidth(collectionView.bounds) - contentInset.left - contentInset.right, + CGRectGetMaxY(section.bounds) + section.insets.bottom); + case UICollectionViewScrollDirectionHorizontal: + return CGSizeMake(CGRectGetMaxX(section.bounds) + section.insets.right, + CGRectGetHeight(collectionView.bounds) - contentInset.top - contentInset.bottom); } - } - (void)invalidateLayoutWithContext:(IGListCollectionViewLayoutInvalidationContext *)context { diff --git a/Source/IGListKit/Internal/IGListAdapterUpdaterHelpers.h b/Source/IGListKit/Internal/IGListAdapterUpdaterHelpers.h index 45067ec8..7766714e 100644 --- a/Source/IGListKit/Internal/IGListAdapterUpdaterHelpers.h +++ b/Source/IGListKit/Internal/IGListAdapterUpdaterHelpers.h @@ -32,4 +32,6 @@ IGListBatchUpdateData *IGListApplyUpdatesToCollectionView(UICollectionView *coll BOOL sectionMovesAsDeletesInserts, BOOL preferItemReloadsForSectionReloads); +NSIndexSet *IGListSectionIndexFromIndexPaths(NSArray *indexPaths); + NS_ASSUME_NONNULL_END diff --git a/Source/IGListKit/Internal/IGListAdapterUpdaterHelpers.m b/Source/IGListKit/Internal/IGListAdapterUpdaterHelpers.m index 2b06398d..b36bb53b 100644 --- a/Source/IGListKit/Internal/IGListAdapterUpdaterHelpers.m +++ b/Source/IGListKit/Internal/IGListAdapterUpdaterHelpers.m @@ -129,3 +129,11 @@ IGListBatchUpdateData *IGListApplyUpdatesToCollectionView(UICollectionView *coll [collectionView ig_applyBatchUpdateData:updateData]; return updateData; } + +NSIndexSet *IGListSectionIndexFromIndexPaths(NSArray *indexPaths) { + NSMutableIndexSet *sections = [NSMutableIndexSet new]; + for (NSIndexPath *indexPath in indexPaths) { + [sections addIndex:indexPath.section]; + } + return sections; +}