Use of NS_FORMAT_ARGUMENT is wrong

[newacct]

SLLocalization.h declares -pluralizedStringWithKey:defaultValue:table:pluralValue: with an "NS_FORMAT_ARGUMENT(1)". However, that is wrong because that macro is only used for variadic functions where arguments are used as arguments to a format string, which is not the case here.

[macdrevx]

This was likely patterned after -[NSBundle localizedStringForKey:value:table:] which has the same attribute. I think that the idea is correct in that (as discussed in these GCC docs):

The format_arg attribute specifies that a function takes a format string for a printf, scanf, strftime or strfmon style function and modifies it (for example, to translate it into another language), so the result can be passed to a printf, scanf, strftime or strfmon style function (with the remaining arguments to the format function the same as they would have been for the unmodified string).

This allows the compiler to check the format specifiers in the argument to the annotated function against the format arguments passed to the printf-style function. I did a little test to see how this works with LLVM 5.0:

NSString *pluralizedString(NSString *key, NSString *defaultValue, NSString *tableName, float pluralValue) NS_FORMAT_ARGUMENT(1);

NSString *pluralizedString(NSString *key, NSString *defaultValue, NSString *tableName, float pluralValue)
{
    return key;
}

@interface AppDelegate ()

- (NSString *)pluralizedStringWithKey:(NSString *)key defaultValue:(NSString *)defaultValue table:(NSString *)tableName pluralValue:(float)pluralValue NS_FORMAT_ARGUMENT(1);

@end

@implementation AppDelegate

- (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions
{
    self.window = [[UIWindow alloc] initWithFrame:[[UIScreen mainScreen] bounds]];
    self.window.rootViewController = [UIViewController new];
    [self.window makeKeyAndVisible];

    float foo = 10.0;

    // Test 1
    NSLog([self pluralizedStringWithKey:@"foo bar %d" defaultValue:nil table:nil pluralValue:5.0f], foo);

    // Test 2
    NSLog([self pluralizedStringWithKey:@"foo bar %f" defaultValue:nil table:nil pluralValue:5.0f], foo);

    // Test 3
    NSLog(pluralizedString(@"foo bar %d", nil, nil, 5.0f), foo);

    // Test 4
    NSLog(pluralizedString(@"foo bar %f", nil, nil, 5.0f), foo);

    return YES;
}

- (NSString *)pluralizedStringWithKey:(NSString *)key defaultValue:(NSString *)defaultValue table:(NSString *)tableName pluralValue:(float)pluralValue
{
    return key;
}

@end

As expected, I received a warning in Test 3:

Format specifies type 'int' but the argument has type 'float'

Also as expected, no warning was generated for tests 2 and 4.

Unexpectedly, no error is generated for Test 1.

As an alternative, I tried changing the attribute argument to 3 for the method variant (just in case the compiler considered self as argument 1 and cmd as argument 2). Still no warning.

I have not tested this with GCC, but I am interested to know whether GCC handled this attribute for objc methods. That could be an explanation of why it is present on the method on NSBundle.

So in summary, I think that conceptually, the usage of NS_FORMAT_ARGUMENT in SLLocalization.h is correct, but it seems like it currently makes no difference in terms of LLVM's format specifier validation.

[paiv]

@macdrevx Hi Andrew, thank you for investigating this. See this definition and spec for format_arg:

#define NS_FORMAT_ARGUMENT(A) __attribute__ ((format_arg(A)))

http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html

format_arg (string-index) The format_arg attribute specifies that a function takes a format string for a printf, scanf, strftime or strfmon style function and modifies it (for example, to translate it into another language), so the result can be passed to a printf, scanf, strftime or strfmon style function

In the case of SLPluralizedString and NSLocalizedString -- they take 'key' to localized dictionary, and commonly this key is format string itself. SLPluralizedString and NSLocalizedString return format string, therefore this check is useful.