Fix some static analyzer warnings

realm · Jun 12, 2023 · eb472ea · eb472ea
1 parent 42852fe
commit eb472ea
Show file tree

Hide file tree

Showing 19 changed files with 82 additions and 101 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -125,6 +125,12 @@ supported version.
   attempting to connect to the old URL and getting redirected rather than only
   the first connection after the deployment location changed.
   ([Core #6630](https://github.com/realm/realm-core/issues/6630), since v10.38.2)
+* `-[RLMAsymmetricObject createObject:withValue:]` was marked as having a
+  non-null return value despite always returning `nil` (since v10.29.0).
+* Eliminate several clang static analyzer warnings which did not report actual
+  bugs.
+
+<!-- ### Breaking Changes - ONLY INCLUDE FOR NEW MAJOR version -->
 
 ### Compatibility
 

diff --git a/Realm/ObjectServerTests/RLMObjectServerTests.mm b/Realm/ObjectServerTests/RLMObjectServerTests.mm
@@ -1176,7 +1176,7 @@ - (void)testLogBackInSameRealmDownload {
         // Log out the user.
         [self logOutUser:user];
         // Log the user back in.
-        user = [self logInUserForCredentials:credentials];
+        [self logInUserForCredentials:credentials];
 
         RLMRunChildAndWait();
 
@@ -1205,7 +1205,7 @@ - (void)testLogBackInDeferredRealmUpload {
         [self addPersonsToRealm:realm persons:@[[Person john]]];
         CHECK_COUNT(1, Person, realm);
 
-        user = [self logInUserForCredentials:credentials];
+        [self logInUserForCredentials:credentials];
         [self addPersonsToRealm:realm
                         persons:@[[Person john], [Person paul], [Person ringo]]];
         [self waitForUploadsForRealm:realm];

diff --git a/Realm/ObjectServerTests/RLMSyncTestCase.h b/Realm/ObjectServerTests/RLMSyncTestCase.h
@@ -118,17 +118,14 @@ RLM_HEADER_AUDIT_BEGIN(nullability, sendability)
 
 /// Wait for downloads to complete; drop any error.
 - (void)waitForDownloadsForRealm:(RLMRealm *)realm;
-- (void)waitForDownloadsForRealm:(RLMRealm *)realm error:(NSError **)error;
 
 /// Wait for uploads to complete; drop any error.
 - (void)waitForUploadsForRealm:(RLMRealm *)realm;
-- (void)waitForUploadsForRealm:(RLMRealm *)realm error:(NSError **)error;
 
 /// Wait for downloads to complete while spinning the runloop. This method uses expectations.
 - (void)waitForDownloadsForUser:(RLMUser *)user
                  partitionValue:(NSString *)partitionValue
-                    expectation:(nullable XCTestExpectation *)expectation
-                          error:(NSError **)error;
+                    expectation:(nullable XCTestExpectation *)expectation;
 
 /// Manually set the access token for a user. Used for testing invalid token conditions.
 - (void)manuallySetAccessTokenForUser:(RLMUser *)user value:(NSString *)tokenValue;

diff --git a/Realm/ObjectServerTests/RLMSyncTestCase.mm b/Realm/ObjectServerTests/RLMSyncTestCase.mm
@@ -159,7 +159,7 @@ - (void)waitForDownloadsForUser:(RLMUser *)user
     NSAssert(realms.count == counts.count && realms.count == partitionValues.count,
              @"Test logic error: all array arguments must be the same size.");
     for (NSUInteger i = 0; i < realms.count; i++) {
-        [self waitForDownloadsForUser:user partitionValue:partitionValues[i] expectation:nil error:nil];
+        [self waitForDownloadsForUser:user partitionValue:partitionValues[i] expectation:nil];
         [realms[i] refresh];
         CHECK_COUNT([counts[i] integerValue], Person, realms[i]);
     }
@@ -372,71 +372,51 @@ - (RLMCredentials *)jwtCredentialWithAppId:(NSString *)appId {
     return [RLMCredentials credentialsWithJWT:[self createJWTWithAppId:appId]];
 }
 
-- (void)waitForDownloadsForRealm:(RLMRealm *)realm {
-    [self waitForDownloadsForRealm:realm error:nil];
-}
-
-- (void)waitForUploadsForRealm:(RLMRealm *)realm {
-    [self waitForUploadsForRealm:realm error:nil];
-}
-
 - (void)waitForDownloadsForUser:(RLMUser *)user
                  partitionValue:(NSString *)partitionValue
-                    expectation:(XCTestExpectation *)expectation
-                          error:(NSError **)error {
+                    expectation:(XCTestExpectation *)expectation {
     RLMSyncSession *session = [user sessionForPartitionValue:partitionValue];
     NSAssert(session, @"Cannot call with invalid partition value");
     XCTestExpectation *ex = expectation ?: [self expectationWithDescription:@"Wait for download completion"];
-    __block NSError *theError = nil;
     BOOL queued = [session waitForDownloadCompletionOnQueue:dispatch_get_global_queue(0, 0) callback:^(NSError *err) {
-        theError = err;
+        XCTAssertNil(err);
         [ex fulfill];
     }];
     if (!queued) {
         XCTFail(@"Download waiter did not queue; session was invalid or errored out.");
         return;
     }
     [self waitForExpectations:@[ex] timeout:60.0];
-    if (error) {
-        *error = theError;
-    }
 }
 
-- (void)waitForUploadsForRealm:(RLMRealm *)realm error:(NSError **)error {
+- (void)waitForUploadsForRealm:(RLMRealm *)realm {
     RLMSyncSession *session = realm.syncSession;
     NSAssert(session, @"Cannot call with invalid Realm");
     XCTestExpectation *ex = [self expectationWithDescription:@"Wait for upload completion"];
-    __block NSError *completionError;
     BOOL queued = [session waitForUploadCompletionOnQueue:dispatch_get_global_queue(0, 0) callback:^(NSError *error) {
-        completionError = error;
+        XCTAssertNil(error);
         [ex fulfill];
     }];
     if (!queued) {
         XCTFail(@"Upload waiter did not queue; session was invalid or errored out.");
         return;
     }
     [self waitForExpectations:@[ex] timeout:60.0];
-    if (error)
-        *error = completionError;
 }
 
-- (void)waitForDownloadsForRealm:(RLMRealm *)realm error:(NSError **)error {
+- (void)waitForDownloadsForRealm:(RLMRealm *)realm {
     RLMSyncSession *session = realm.syncSession;
     NSAssert(session, @"Cannot call with invalid Realm");
     XCTestExpectation *ex = [self expectationWithDescription:@"Wait for download completion"];
-    __block NSError *completionError;
     BOOL queued = [session waitForDownloadCompletionOnQueue:nil callback:^(NSError *error) {
-        completionError = error;
+        XCTAssertNil(error);
         [ex fulfill];
     }];
     if (!queued) {
         XCTFail(@"Download waiter did not queue; session was invalid or errored out.");
         return;
     }
     [self waitForExpectations:@[ex] timeout:60.0];
-    if (error) {
-        *error = completionError;
-    }
     [realm refresh];
 }
 

diff --git a/Realm/RLMAsymmetricObject.h b/Realm/RLMAsymmetricObject.h
@@ -76,7 +76,7 @@ RLM_HEADER_AUDIT_BEGIN(nullability, sendability)
 
  @return  Returns `nil`
  */
-+ (instancetype)createInRealm:(RLMRealm *)realm withValue:(id)value;
++ (nullable instancetype)createInRealm:(RLMRealm *)realm withValue:(id)value;
 
 #pragma mark - Properties
 

diff --git a/Realm/RLMMigration.mm b/Realm/RLMMigration.mm
@@ -38,6 +38,8 @@
 using namespace realm;
 
 @implementation RLMMigration {
+    RLMRealm *_oldRealm;
+    RLMRealm *_realm;
     realm::Schema *_schema;
 }
 
@@ -52,11 +54,11 @@ - (instancetype)initWithRealm:(RLMRealm *)realm oldRealm:(RLMRealm *)oldRealm sc
 }
 
 - (RLMSchema *)oldSchema {
-    return self.oldRealm.schema;
+    return _oldRealm.schema;
 }
 
 - (RLMSchema *)newSchema {
-    return self.realm.schema;
+    return _realm.schema;
 }
 
 - (void)enumerateObjects:(NSString *)className block:(__attribute__((noescape)) RLMObjectMigrationBlock)block {

diff --git a/Realm/RLMMigration_Private.h b/Realm/RLMMigration_Private.h
@@ -27,14 +27,8 @@ namespace realm {
 RLM_HEADER_AUDIT_BEGIN(nullability, sendability)
 
 @interface RLMMigration ()
-
-@property (nonatomic, strong) RLMRealm *oldRealm;
-@property (nonatomic, strong) RLMRealm *realm;
-
 - (instancetype)initWithRealm:(RLMRealm *)realm oldRealm:(RLMRealm *)oldRealm schema:(realm::Schema &)schema;
-
 - (void)execute:(RLMMigrationBlock)block objectClass:(_Nullable Class)cls;
-
 @end
 
 RLM_HEADER_AUDIT_END(nullability, sendability)
diff --git a/Realm/RLMQueryUtil.mm b/Realm/RLMQueryUtil.mm
@@ -731,16 +731,14 @@ Query make_diacritic_insensitive_constraint(NSPredicateOperatorType operatorType
 void QueryBuilder::add_diacritic_sensitive_string_constraint(NSPredicateOperatorType operatorType,
                                                              NSComparisonPredicateOptions predicateOptions,
                                                              C&& column, T&& value) {
-    if constexpr (is_any_v<C, Columns<Dictionary>>) {
-        // This nesting isnt pretty but without it the compiler will complain about `T` having no known
-        // conversion from Columns<StringData> to Mixed. This is due to the fact that all values on a
-        // dictionary column are boxed in Mixed.
-        if constexpr (is_any_v<T, Mixed, BinaryData, StringData>) {
-            do_add_diacritic_sensitive_string_constraint(operatorType, predicateOptions, std::forward<C>(column), value);
-        }
+
+    if constexpr (is_any_v<C, Columns<Dictionary>> && is_any_v<T, Columns<StringData>, Columns<BinaryData>>) {
+        // Core only implements these for Columns<Mixed> due to Dictionary being Mixed internall
+        throwException(@"Unsupported predicate",
+                       @"String comparisons on a Dictionary and another property are only implemented for AnyRealmValue properties.");
     }
     else {
-        do_add_diacritic_sensitive_string_constraint(operatorType, predicateOptions, std::forward<C>(column), value);
+        do_add_diacritic_sensitive_string_constraint(operatorType, predicateOptions, std::forward<C>(column), std::forward<T>(value));
     }
 }
 
@@ -749,7 +747,7 @@ Query make_diacritic_insensitive_constraint(NSPredicateOperatorType operatorType
                                          NSComparisonPredicateOptions predicateOptions,
                                          C&& column, T&& value) {
     if (!(predicateOptions & NSDiacriticInsensitivePredicateOption)) {
-        add_diacritic_sensitive_string_constraint(operatorType, predicateOptions, std::forward<C>(column), std::move(value));
+        add_diacritic_sensitive_string_constraint(operatorType, predicateOptions, std::forward<C>(column), std::forward<T>(value));
         return;
     }
 
@@ -998,7 +996,7 @@ void convert_null(T&& value, Fn&& fn) {
         fn(null());
     }
     else {
-        fn(value);
+        fn(std::forward<T>(value));
     }
 }
 
@@ -1382,15 +1380,15 @@ auto collection_operation_expr(CollectionOperation operation) {
                                                        CollectionOperation const& collectionOperation, R&& rhs,
                                                        NSComparisonPredicateOptions options)
 {
-    convert_null(rhs, [&](auto&& rhs) {
+    convert_null(std::forward<R>(rhs), [&]<typename T>(T&& rhs) {
         if (collectionOperation.link_column().is_link()) {
-            add_collection_operation_constraint<Operation, true, false>(operatorType, collectionOperation, std::move(rhs), options);
+            add_collection_operation_constraint<Operation, true, false>(operatorType, collectionOperation, std::forward<T>(rhs), options);
         }
         else if (collectionOperation.column().property().dictionary) {
-            add_collection_operation_constraint<Operation, false, true>(operatorType, collectionOperation, std::move(rhs), options);
+            add_collection_operation_constraint<Operation, false, true>(operatorType, collectionOperation, std::forward<T>(rhs), options);
         }
         else {
-            add_collection_operation_constraint<Operation, false, false>(operatorType, collectionOperation, std::move(rhs), options);
+            add_collection_operation_constraint<Operation, false, false>(operatorType, collectionOperation, std::forward<T>(rhs), options);
         }
     });
 }
@@ -1420,8 +1418,8 @@ void get_collection_type(__unsafe_unretained RLMProperty *prop, Fn&& fn) {
             auto& column = collectionOperation.link_column();
             RLMPropertyType type = column.type();
             auto rhsValue = value_of_type<Int>(rhs);
-            auto continuation = [&](auto t) {
-                add_numeric_constraint(type, operatorType, column.resolve<std::decay_t<decltype(*t)>>().size(), rhsValue);
+            auto continuation = [&]<typename T>(T *) {
+                add_numeric_constraint(type, operatorType, column.resolve<T>().size(), rhsValue);
             };
 
             switch (type) {
@@ -1453,16 +1451,16 @@ void get_collection_type(__unsafe_unretained RLMProperty *prop, Fn&& fn) {
             }
         }
         case CollectionOperation::Minimum:
-            add_collection_operation_constraint<CollectionOperation::Minimum>(operatorType, collectionOperation, std::move(rhs), comparisonOptions);
+            add_collection_operation_constraint<CollectionOperation::Minimum>(operatorType, collectionOperation, std::forward<R>(rhs), comparisonOptions);
             break;
         case CollectionOperation::Maximum:
-            add_collection_operation_constraint<CollectionOperation::Maximum>(operatorType, collectionOperation, std::move(rhs), comparisonOptions);
+            add_collection_operation_constraint<CollectionOperation::Maximum>(operatorType, collectionOperation, std::forward<R>(rhs), comparisonOptions);
             break;
         case CollectionOperation::Sum:
-            add_collection_operation_constraint<CollectionOperation::Sum>(operatorType, collectionOperation, std::move(rhs), comparisonOptions);
+            add_collection_operation_constraint<CollectionOperation::Sum>(operatorType, collectionOperation, std::forward<R>(rhs), comparisonOptions);
             break;
         case CollectionOperation::Average:
-            add_collection_operation_constraint<CollectionOperation::Average>(operatorType, collectionOperation, std::move(rhs), comparisonOptions);
+            add_collection_operation_constraint<CollectionOperation::Average>(operatorType, collectionOperation, std::forward<R>(rhs), comparisonOptions);
             break;
         case CollectionOperation::AllKeys: {
             // BETWEEN and IN are not supported by @allKeys as the parsing for collection

diff --git a/Realm/RLMRealm.mm b/Realm/RLMRealm.mm
@@ -215,18 +215,18 @@ bool copySeedFile(RLMRealmConfiguration *configuration, NSError **error) {
     if (!configuration.seedFilePath) {
         return false;
     }
+    NSError *copyError;
+    bool didCopySeed = false;
     @autoreleasepool {
-        bool didCopySeed = false;
-        NSError *copyError;
         DB::call_with_lock(configuration.path, [&](auto const&) {
             didCopySeed = [[NSFileManager defaultManager] copyItemAtURL:configuration.seedFilePath
                                                                   toURL:configuration.fileURL
                                                                   error:&copyError];
         });
-        if (!didCopySeed && copyError != nil && copyError.code != NSFileWriteFileExistsError) {
-            RLMSetErrorOrThrow(copyError, error);
-            return true;
-        }
+    }
+    if (!didCopySeed && copyError && copyError.code != NSFileWriteFileExistsError) {
+        RLMSetErrorOrThrow(copyError, error);
+        return true;
     }
     return false;
 }

diff --git a/Realm/RLMSyncConfiguration.h b/Realm/RLMSyncConfiguration.h
@@ -145,7 +145,7 @@ typedef void(^RLMClientResetAfterBlock)(RLMRealm * _Nonnull beforeFrozen, RLMRea
  Atlas App Services. All classes with a property with this value will be synchronized to the
  Realm.
  */
-@property (nonatomic, readonly) id<RLMBSON> partitionValue;
+@property (nonatomic, readonly, nullable) id<RLMBSON> partitionValue;
 
 /**
  An enum which determines file recovery behavior in the event of a client reset.

diff --git a/Realm/RLMSyncSubscription.mm b/Realm/RLMSyncSubscription.mm
@@ -127,7 +127,9 @@ @interface RLMSyncSubscriptionEnumerator() {
 }
 @end
 
-@implementation RLMSyncSubscriptionEnumerator
+@implementation RLMSyncSubscriptionEnumerator {
+    RLMSyncSubscriptionSet *_subscriptionSet;
+}
 
 - (instancetype)initWithSubscriptionSet:(RLMSyncSubscriptionSet *)subscriptionSet {
     if (self = [super init]) {

diff --git a/Realm/RLMSyncSubscription_Private.h b/Realm/RLMSyncSubscription_Private.h
@@ -37,14 +37,10 @@ RLM_HEADER_AUDIT_BEGIN(nullability, sendability)
 #pragma mark - SubscriptionSet
 
 @interface RLMSyncSubscriptionEnumerator : NSObject
-
-@property (nonatomic, readonly) RLMSyncSubscriptionSet *subscriptionSet;
-
 - (NSUInteger)countByEnumeratingWithState:(NSFastEnumerationState *)state
                                     count:(NSUInteger)len;
 
 - (instancetype)initWithSubscriptionSet:(RLMSyncSubscriptionSet *)subscriptionSet;
-
 @end
 
 @interface RLMSyncSubscriptionSet ()

diff --git a/Realm/RLMUtil.mm b/Realm/RLMUtil.mm
@@ -412,10 +412,10 @@ BOOL RLMIsRunningInPlayground() {
         RLMAccessorContext c{objBase->_info ? *objBase->_info : realm->_info[objBase->_objectSchema.className]};
         auto obj = c.unbox<realm::Obj>(v, createPolicy);
         return obj.is_valid() ? realm::Mixed(obj) : realm::Mixed();
-    }, [&](auto t) {
+    }, [&]<typename T>(T *) {
         RLMStatelessAccessorContext c;
-        return realm::Mixed(c.unbox<std::decay_t<decltype(*t)>>(v));
-    }, [&](realm::Mixed*) -> realm::Mixed {
+        return realm::Mixed(c.unbox<T>(v));
+    }, [](realm::Mixed*) -> realm::Mixed {
         REALM_UNREACHABLE();
     }});
 }

diff --git a/Realm/Tests/AsyncTests.mm b/Realm/Tests/AsyncTests.mm
@@ -437,6 +437,7 @@ - (void)testCancellationTokenKeepsSubscriptionAlive {
         }];
     }
     [self waitForExpectationsWithTimeout:2.0 handler:nil];
+    [token invalidate];
 }
 
 - (void)testCancellationTokenPreventsOpeningRealmWithMismatchedConfig {

diff --git a/Realm/Tests/MigrationTests.mm b/Realm/Tests/MigrationTests.mm
@@ -1603,6 +1603,10 @@ - (void)testConvertToEmbeddedAddingMoreLinks {
             XCTAssertNotNil(oldObject);
             XCTAssertNotNil(newObject);
             RLMObject *childObject = newObject[@"object"];
+            XCTAssertNotNil(childObject);
+            if (childObject == nil) {
+                return;
+            }
             [migration createObject:EmbeddedIntParentObject.className withValue:@[@43, childObject, NSNull.null]];
         }];
     }];