Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix some static analyzer warnings #8249

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you tag the issue which reported this?

bugs.

<!-- ### Breaking Changes - ONLY INCLUDE FOR NEW MAJOR version -->

### Compatibility

Expand Down
4 changes: 2 additions & 2 deletions Realm/ObjectServerTests/RLMObjectServerTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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];
Expand Down
5 changes: 1 addition & 4 deletions Realm/ObjectServerTests/RLMSyncTestCase.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
34 changes: 7 additions & 27 deletions Realm/ObjectServerTests/RLMSyncTestCase.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
}
Expand Down Expand Up @@ -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];
}

Expand Down
2 changes: 1 addition & 1 deletion Realm/RLMAsymmetricObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 4 additions & 2 deletions Realm/RLMMigration.mm
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
using namespace realm;

@implementation RLMMigration {
RLMRealm *_oldRealm;
RLMRealm *_realm;
realm::Schema *_schema;
}

Expand All @@ -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 {
Expand Down
6 changes: 0 additions & 6 deletions Realm/RLMMigration_Private.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
38 changes: 18 additions & 20 deletions Realm/RLMQueryUtil.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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>>) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just check if T is mixed and throw otherwise

// Core only implements these for Columns<Mixed> due to Dictionary being Mixed internall
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Core only implements these for Columns<Mixed> due to Dictionary being Mixed internall
// Core only implements these for Columns<Mixed> due to Dictionary being Mixed internal

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));
}
}

Expand All @@ -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;
}

Expand Down Expand Up @@ -998,7 +996,7 @@ void convert_null(T&& value, Fn&& fn) {
fn(null());
}
else {
fn(value);
fn(std::forward<T>(value));
}
}

Expand Down Expand Up @@ -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);
}
});
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions Realm/RLMRealm.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion Realm/RLMSyncConfiguration.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 3 additions & 1 deletion Realm/RLMSyncSubscription.mm
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,9 @@ @interface RLMSyncSubscriptionEnumerator() {
}
@end

@implementation RLMSyncSubscriptionEnumerator
@implementation RLMSyncSubscriptionEnumerator {
RLMSyncSubscriptionSet *_subscriptionSet;
}

- (instancetype)initWithSubscriptionSet:(RLMSyncSubscriptionSet *)subscriptionSet {
if (self = [super init]) {
Expand Down
4 changes: 0 additions & 4 deletions Realm/RLMSyncSubscription_Private.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 ()
Expand Down
6 changes: 3 additions & 3 deletions Realm/RLMUtil.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}});
}
Expand Down
1 change: 1 addition & 0 deletions Realm/Tests/AsyncTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,7 @@ - (void)testCancellationTokenKeepsSubscriptionAlive {
}];
}
[self waitForExpectationsWithTimeout:2.0 handler:nil];
[token invalidate];
}

- (void)testCancellationTokenPreventsOpeningRealmWithMismatchedConfig {
Expand Down
4 changes: 4 additions & 0 deletions Realm/Tests/MigrationTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1603,6 +1603,10 @@ - (void)testConvertToEmbeddedAddingMoreLinks {
XCTAssertNotNil(oldObject);
XCTAssertNotNil(newObject);
RLMObject *childObject = newObject[@"object"];
XCTAssertNotNil(childObject);
if (childObject == nil) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the above assertion fails, is this even run?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. The rest of the test continues running even if an XCTest assertion fails.

return;
}
[migration createObject:EmbeddedIntParentObject.className withValue:@[@43, childObject, NSNull.null]];
}];
}];
Expand Down