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.

Sign up for GitHub

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

Implement NSArray descriptionWithLocale* methods #2256

Merged
merged 2 commits into from
Mar 20, 2017
Merged

Implement NSArray descriptionWithLocale* methods #2256

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ca854a9
Implement NSArray descriptionWithLocale* methods
aballway Mar 16, 2017
37d4c69
In retrospect it makes sense why that was called level but the docume…
aballway Mar 16, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 57 additions & 33 deletions Frameworks/Foundation/NSArray.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -749,31 +749,7 @@ - (void)encodeWithCoder:(NSCoder*)coder {
@Status Interoperable
*/
- (NSString*)description {
thread_local unsigned int indent = 0;
NSMutableString* s = [NSMutableString string];
for (unsigned int i = 0; i < indent; ++i) {
[s appendString:@" "];
}
[s appendString:@"(\n"];
{
++indent;
auto deferPop = wil::ScopeExit([]() { --indent; });
for (id val in self) {
for (unsigned int i = 0; i < indent; ++i) {
[s appendString:@" "];
}
[s appendFormat:@"%@,\n", val];
}

if ([self count] > 0) {
[s deleteCharactersInRange:{[s length] - 2, 1 }];
}
}
for (unsigned int i = 0; i < indent; ++i) {
[s appendString:@" "];
}
[s appendString:@")"];
return s;
return [self descriptionWithLocale:nil];
}

/**
Expand Down Expand Up @@ -947,21 +923,69 @@ + (NSArray*)arrayWithContentsOfURL:(NSURL*)aURL {
}

/**
@Status Stub
@Notes
@Status Interoperable
*/
- (NSString*)descriptionWithLocale:(id)locale {
UNIMPLEMENTED();
return StubReturn();
return [self descriptionWithLocale:locale indent:0];
}

/**
@Status Stub
@Notes
@Status Interoperable
*/
- (NSString*)descriptionWithLocale:(id)locale indent:(NSUInteger)level {
UNIMPLEMENTED();
return StubReturn();
thread_local unsigned int indent = 0;
NSMutableString* s = [NSMutableString string];
NSString* indentStr = (level == 0) ? @" " : @"\t";
Copy link

Choose a reason for hiding this comment

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

I'm torn on how this is supposed to work. Shouldn't it call the next one with the next level, and always make sure to indent itself to this level? The thread local becomes superfluous, and it's initialized with the wrong value anyway.

Copy link
Contributor Author

@aballway aballway Mar 16, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

https://developer.apple.com/reference/foundation/nsarray/1416257-descriptionwithlocale

UPDATE: this is possibly the most misleading documentation I have read 👎

for (unsigned int i = 0; i < indent; ++i) {
[s appendString:indentStr];
}

[s appendString:@"(\n"];

{
++indent;
auto deferPop = wil::ScopeExit([]() { --indent; });
for (id val in self) {
for (unsigned int i = 0; i < indent; ++i) {
[s appendString:indentStr];
}

// Documentation states order to determine what values are printed
NSString* valToWrite = nil;
if ([val isKindOfClass:[NSString class]]) {
// If val is an NSString, use it directly
valToWrite = val;
}

if (valToWrite.length == 0 && [val respondsToSelector:@selector(descriptionWithLocale:indent:)]) {
// If val is not a string but responds to descriptionWithLocale:indent, use that value
valToWrite = [val descriptionWithLocale:locale indent:level];
}

if (valToWrite.length == 0 && [val respondsToSelector:@selector(descriptionWithLocale:)]) {
// If not an NSString and doesn't respond to descriptionWithLocale:indent but does descriptionWithLocale:, use that
valToWrite = [val descriptionWithLocale:locale];
}

if (valToWrite.length == 0) {
// If all else fails, use description
valToWrite = [val description];
}

[s appendFormat:@"%@,\n", valToWrite];
}

if ([self count] > 0) {
[s deleteCharactersInRange:{[s length] - 2, 1 }];
}
}

for (unsigned int i = 0; i < indent; ++i) {
[s appendString:indentStr];
}

[s appendString:@")"];
return s;
}

/**
Expand Down
4 changes: 2 additions & 2 deletions include/Foundation/NSArray.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ FOUNDATION_EXPORT_CLASS
- (NSArray<ObjectType>*)sortedArrayWithOptions:(NSSortOptions)opts usingComparator:(NSComparator)cmptr;
- (NSString*)componentsJoinedByString:(NSString*)separator;
@property (readonly, copy) NSString* description;
- (NSString*)descriptionWithLocale:(id)locale STUB_METHOD;
- (NSString*)descriptionWithLocale:(id)locale indent:(NSUInteger)level STUB_METHOD;
- (NSString*)descriptionWithLocale:(id)locale;
- (NSString*)descriptionWithLocale:(id)locale indent:(NSUInteger)level;
- (BOOL)writeToFile:(NSString*)path atomically:(BOOL)flag;
- (BOOL)writeToURL:(NSURL*)aURL atomically:(BOOL)flag;
- (NSArray<NSString*>*)pathsMatchingExtensions:(NSArray<NSString*>*)filterTypes;
Expand Down
115 changes: 114 additions & 1 deletion tests/unittests/Foundation/NSArrayTests.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ - (void)verifyAndResetFlags:(unsigned int)expectedCalls {
NSArray* testArray = @[ @1, @2, @3 ];
EXPECT_OBJCEQ(@"(\n 1,\n 2,\n 3\n)", [testArray description]);

NSArray* testArray2 = [[NSArray new] autorelease];
NSArray* testArray2 = [NSArray array];
EXPECT_OBJCEQ(@"(\n)", [testArray2 description]);

NSArray* testArray3 = @[ @1 ];
Expand Down Expand Up @@ -257,6 +257,119 @@ - (void)verifyAndResetFlags:(unsigned int)expectedCalls {
EXPECT_OBJCEQ(testValue4, [testArray4 description]);
}

@interface NSArrayDescriptionTest : NSObject
+ (NSArrayDescriptionTest*)test;
- (NSString*)descriptionWithLocale:(id)locale indent:(NSUInteger)level;
- (NSString*)fakeDescriptionWithLocale:(id)locale indent:(NSUInteger)level;
- (NSString*)descriptionWithLocale:(id)locale;
- (NSString*)fakeDescriptionWithLocale:(id)locale;
- (NSString*)description;
@end

@implementation NSArrayDescriptionTest
+ (NSArrayDescriptionTest*)test {
return [[NSArrayDescriptionTest new] autorelease];
}

- (NSString*)descriptionWithLocale:(id)locale indent:(NSUInteger)level {
if (locale) {
if (level) {
return @"Both";
}

return @"JustLocale";
} else {
if (level) {
return @"JustLevel";
}

return @"Neither";
}
}
- (NSString*)fakeDescriptionWithLocale:(id)locale indent:(NSUInteger)level {
return nil;
}

- (NSString*)descriptionWithLocale:(id)locale {
if (locale) {
return @"Locale";
}

return @"NoLocale";
}

- (NSString*)fakeDescriptionWithLocale:(id)locale {
return nil;
}

- (NSString*)description {
return @"Description";
}
@end

// Unfortunately despite what documentation suggests [NSArray descriptionWithLocale:] and [NSArray descriptionWithLocale:indent:]
// do not actually call descriptionWithLocale:indent: and descriptionWithLocale: when available (and does not follow the ordering
// specified to check for availability) so we are opting to implement this correctly and not test on the reference platform
OSX_DISABLED_TEST(NSArray, DescriptionWithLocale) {
NSArray* testArray = @[ @"HI", [NSArrayDescriptionTest test] ];

// Should default to descriptionWithLocale:indent:
EXPECT_OBJCEQ(@"(\n HI,\n JustLocale\n)", [testArray descriptionWithLocale:[NSLocale currentLocale]]);
EXPECT_OBJCEQ(@"(\n HI,\n Neither\n)", [testArray descriptionWithLocale:nil]);

Method originalDescWithLocaleIndent = class_getInstanceMethod([NSArrayDescriptionTest class], @selector(descriptionWithLocale:indent:));
Method fakeDescWithLocaleIndent = class_getInstanceMethod([NSArrayDescriptionTest class], @selector(fakeDescriptionWithLocale:indent:));
method_exchangeImplementations(originalDescWithLocaleIndent, fakeDescWithLocaleIndent);

// Should that fail then to descriptionWithLocale:
EXPECT_OBJCEQ(@"(\n HI,\n Locale\n)", [testArray descriptionWithLocale:[NSLocale currentLocale]]);
EXPECT_OBJCEQ(@"(\n HI,\n NoLocale\n)", [testArray descriptionWithLocale:nil]);

Method originalDescWithLocale = class_getInstanceMethod([NSArrayDescriptionTest class], @selector(descriptionWithLocale:));
Method fakeDescWithLocale = class_getInstanceMethod([NSArrayDescriptionTest class], @selector(fakeDescriptionWithLocale:));
method_exchangeImplementations(originalDescWithLocale, fakeDescWithLocale);

// Should that fail then to description
EXPECT_OBJCEQ(@"(\n HI,\n Description\n)", [testArray descriptionWithLocale:[NSLocale currentLocale]]);
EXPECT_OBJCEQ(@"(\n HI,\n Description\n)", [testArray descriptionWithLocale:nil]);

method_exchangeImplementations(originalDescWithLocaleIndent, fakeDescWithLocaleIndent);
method_exchangeImplementations(originalDescWithLocale, fakeDescWithLocale);
}

OSX_DISABLED_TEST(NSArray, DescriptionWithLocaleIndent) {
NSArray* testArray = @[ @"HI", [NSArrayDescriptionTest test] ];

// Should default to descriptionWithLocale:indent:
EXPECT_OBJCEQ(@"(\n\tHI,\n\tBoth\n)", [testArray descriptionWithLocale:[NSLocale currentLocale] indent:1]);
EXPECT_OBJCEQ(@"(\n HI,\n JustLocale\n)", [testArray descriptionWithLocale:[NSLocale currentLocale] indent:0]);
EXPECT_OBJCEQ(@"(\n\tHI,\n\tJustLevel\n)", [testArray descriptionWithLocale:nil indent:1]);
EXPECT_OBJCEQ(@"(\n HI,\n Neither\n)", [testArray descriptionWithLocale:nil indent:0]);

Method originalDescWithLocaleIndent = class_getInstanceMethod([NSArrayDescriptionTest class], @selector(descriptionWithLocale:indent:));
Method fakeDescWithLocaleIndent = class_getInstanceMethod([NSArrayDescriptionTest class], @selector(fakeDescriptionWithLocale:indent:));
method_exchangeImplementations(originalDescWithLocaleIndent, fakeDescWithLocaleIndent);

// Should that fail then to descriptionWithLocale:
EXPECT_OBJCEQ(@"(\n HI,\n Locale\n)", [testArray descriptionWithLocale:[NSLocale currentLocale] indent:0]);
EXPECT_OBJCEQ(@"(\n\tHI,\n\tLocale\n)", [testArray descriptionWithLocale:[NSLocale currentLocale] indent:1]);
EXPECT_OBJCEQ(@"(\n HI,\n NoLocale\n)", [testArray descriptionWithLocale:nil indent:0]);
EXPECT_OBJCEQ(@"(\n\tHI,\n\tNoLocale\n)", [testArray descriptionWithLocale:nil indent:1]);

Method originalDescWithLocale = class_getInstanceMethod([NSArrayDescriptionTest class], @selector(descriptionWithLocale:));
Method fakeDescWithLocale = class_getInstanceMethod([NSArrayDescriptionTest class], @selector(fakeDescriptionWithLocale:));
method_exchangeImplementations(originalDescWithLocale, fakeDescWithLocale);

// Should that fail then to description
EXPECT_OBJCEQ(@"(\n HI,\n Description\n)", [testArray descriptionWithLocale:[NSLocale currentLocale] indent:0]);
EXPECT_OBJCEQ(@"(\n\tHI,\n\tDescription\n)", [testArray descriptionWithLocale:[NSLocale currentLocale] indent:1]);
EXPECT_OBJCEQ(@"(\n HI,\n Description\n)", [testArray descriptionWithLocale:nil indent:0]);
EXPECT_OBJCEQ(@"(\n\tHI,\n\tDescription\n)", [testArray descriptionWithLocale:nil indent:1]);

method_exchangeImplementations(originalDescWithLocaleIndent, fakeDescWithLocaleIndent);
method_exchangeImplementations(originalDescWithLocale, fakeDescWithLocale);
}

TEST(NSArray, Autorelease) {
NSArray* array;
NSObject* object = [NSObject new];
Expand Down