[PATCH] D35186: [analyzer] Add annotation for functions taking user-facing strings
This revision was automatically updated to reflect the committed changes. Closed by commit rL308012: [analyzer] Add annotation for functions taking user-facing strings (authored by erikjv). Changed prior to commit: https://reviews.llvm.org/D35186?vs=105792&id=106605#toc Repository: rL LLVM https://reviews.llvm.org/D35186 Files: cfe/trunk/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp cfe/trunk/test/Analysis/localization-aggressive.m Index: cfe/trunk/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp === --- cfe/trunk/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp @@ -57,7 +57,7 @@ }; class NonLocalizedStringChecker -: public Checker> { @@ -79,9 +79,10 @@ void setNonLocalizedState(SVal S, CheckerContext &C) const; void setLocalizedState(SVal S, CheckerContext &C) const; - bool isAnnotatedAsLocalized(const Decl *D) const; - void reportLocalizationError(SVal S, const ObjCMethodCall &M, - CheckerContext &C, int argumentNumber = 0) const; + bool isAnnotatedAsReturningLocalized(const Decl *D) const; + bool isAnnotatedAsTakingLocalized(const Decl *D) const; + void reportLocalizationError(SVal S, const CallEvent &M, CheckerContext &C, + int argumentNumber = 0) const; int getLocalizedArgumentForSelector(const IdentifierInfo *Receiver, Selector S) const; @@ -97,6 +98,7 @@ void checkPreObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const; void checkPostObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const; void checkPostStmt(const ObjCStringLiteral *SL, CheckerContext &C) const; + void checkPreCall(const CallEvent &Call, CheckerContext &C) const; void checkPostCall(const CallEvent &Call, CheckerContext &C) const; }; @@ -644,7 +646,8 @@ /// Checks to see if the method / function declaration includes /// __attribute__((annotate("returns_localized_nsstring"))) -bool NonLocalizedStringChecker::isAnnotatedAsLocalized(const Decl *D) const { +bool NonLocalizedStringChecker::isAnnotatedAsReturningLocalized( +const Decl *D) const { if (!D) return false; return std::any_of( @@ -654,6 +657,19 @@ }); } +/// Checks to see if the method / function declaration includes +/// __attribute__((annotate("takes_localized_nsstring"))) +bool NonLocalizedStringChecker::isAnnotatedAsTakingLocalized( +const Decl *D) const { + if (!D) +return false; + return std::any_of( + D->specific_attr_begin(), + D->specific_attr_end(), [](const AnnotateAttr *Ann) { +return Ann->getAnnotation() == "takes_localized_nsstring"; + }); +} + /// Returns true if the given SVal is marked as Localized in the program state bool NonLocalizedStringChecker::hasLocalizedState(SVal S, CheckerContext &C) const { @@ -733,8 +749,7 @@ /// Reports a localization error for the passed in method call and SVal void NonLocalizedStringChecker::reportLocalizationError( -SVal S, const ObjCMethodCall &M, CheckerContext &C, -int argumentNumber) const { +SVal S, const CallEvent &M, CheckerContext &C, int argumentNumber) const { // Don't warn about localization errors in classes and methods that // may be debug code. @@ -832,7 +847,21 @@ } } - if (argumentNumber < 0) // There was no match in UIMethods + if (argumentNumber < 0) { // There was no match in UIMethods +if (const Decl *D = msg.getDecl()) { + if (const ObjCMethodDecl *OMD = dyn_cast_or_null(D)) { +auto formals = OMD->parameters(); +for (unsigned i = 0, ei = formals.size(); i != ei; ++i) { + if (isAnnotatedAsTakingLocalized(formals[i])) { +argumentNumber = i; +break; + } +} + } +} + } + + if (argumentNumber < 0) // Still no match return; SVal svTitle = msg.getArgSVal(argumentNumber); @@ -855,6 +884,25 @@ } } +void NonLocalizedStringChecker::checkPreCall(const CallEvent &Call, + CheckerContext &C) const { + const Decl *D = Call.getDecl(); + if (D && isa(D)) { +const FunctionDecl *FD = dyn_cast(D); +auto formals = FD->parameters(); +for (unsigned i = 0, + ei = std::min(unsigned(formals.size()), Call.getNumArgs()); + i != ei; ++i) { + if (isAnnotatedAsTakingLocalized(formals[i])) { +auto actual = Call.getArgSVal(i); +if (hasNonLocalizedState(actual, C)) { + reportLocalizationError(actual, Call, C, i + 1); +} + } +} + } +} + static inline bool isNSStringType(QualType T, ASTContext &Ctx) { const ObjCObjectPointerType *PT = T->getAs(); @@ -906,7 +954,7 @@ const IdentifierInfo *Identifier = Call.getCalleeIdentifier(); SVal sv = Call.
[PATCH] D35186: [analyzer] Add annotation for functions taking user-facing strings
dcoughlin accepted this revision. dcoughlin added a comment. This revision is now accepted and ready to land. Thanks for the patch! This looks very good to me. The one thing I would suggest is renaming 'isAnnotatedAsLocalized()' and 'isAnnotatedTakingLocalized()' to make it more clear what each does, now that there are two of them. How about: 'isAnnotatedAsReturningLocalized()' and 'isAnnotatedAsTakingLocalized()'? https://reviews.llvm.org/D35186 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35186: [analyzer] Add annotation for functions taking user-facing strings
erikjv updated this revision to Diff 105792. erikjv added a comment. Sorry, now with more context in the diff. https://reviews.llvm.org/D35186 Files: lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp test/Analysis/localization-aggressive.m Index: test/Analysis/localization-aggressive.m === --- test/Analysis/localization-aggressive.m +++ test/Analysis/localization-aggressive.m @@ -61,8 +61,16 @@ NSString *CFNumberFormatterCreateStringWithNumber(float x); + (NSString *)forceLocalized:(NSString *)str __attribute__((annotate("returns_localized_nsstring"))); ++ (NSString *)takesLocalizedString: +(NSString *)__attribute__((annotate("takes_localized_nsstring")))str; @end +NSString * +takesLocalizedString(NSString *str + __attribute__((annotate("takes_localized_nsstring" { + return str; +} + // Test cases begin here @implementation LocalizationTestSuite @@ -75,6 +83,8 @@ return str; } ++ (NSString *) takesLocalizedString:(NSString *)str { return str; } + // An ObjC method that returns a localized string + (NSString *)unLocalizedStringMethod { return @"UnlocalizedString"; @@ -269,4 +279,13 @@ NSString *string2 = POSSIBLE_FALSE_POSITIVE(@"Hello", @"Hello"); // no-warning } +- (void)testTakesLocalizedString { + NSString *localized = NSLocalizedString(@"Hello", @"World"); + NSString *alsoLocalized = [LocalizationTestSuite takesLocalizedString:localized]; // no-warning + NSString *stillLocalized = [LocalizationTestSuite takesLocalizedString:alsoLocalized]; // no-warning + takesLocalizedString(stillLocalized); // no-warning + + [LocalizationTestSuite takesLocalizedString:@"not localized"]; // expected-warning {{User-facing text should use localized string macro}} + takesLocalizedString(@"not localized"); // expected-warning {{User-facing text should use localized string macro}} +} @end Index: lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp === --- lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp +++ lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp @@ -57,7 +57,7 @@ }; class NonLocalizedStringChecker -: public Checker> { @@ -80,8 +80,9 @@ void setLocalizedState(SVal S, CheckerContext &C) const; bool isAnnotatedAsLocalized(const Decl *D) const; - void reportLocalizationError(SVal S, const ObjCMethodCall &M, - CheckerContext &C, int argumentNumber = 0) const; + bool isAnnotatedTakingLocalized(const Decl *D) const; + void reportLocalizationError(SVal S, const CallEvent &M, CheckerContext &C, + int argumentNumber = 0) const; int getLocalizedArgumentForSelector(const IdentifierInfo *Receiver, Selector S) const; @@ -97,6 +98,7 @@ void checkPreObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const; void checkPostObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const; void checkPostStmt(const ObjCStringLiteral *SL, CheckerContext &C) const; + void checkPreCall(const CallEvent &Call, CheckerContext &C) const; void checkPostCall(const CallEvent &Call, CheckerContext &C) const; }; @@ -654,6 +656,18 @@ }); } +/// Checks to see if the method / function declaration includes +/// __attribute__((annotate("takes_localized_nsstring"))) +bool NonLocalizedStringChecker::isAnnotatedTakingLocalized(const Decl *D) const { + if (!D) +return false; + return std::any_of( + D->specific_attr_begin(), + D->specific_attr_end(), [](const AnnotateAttr *Ann) { +return Ann->getAnnotation() == "takes_localized_nsstring"; + }); +} + /// Returns true if the given SVal is marked as Localized in the program state bool NonLocalizedStringChecker::hasLocalizedState(SVal S, CheckerContext &C) const { @@ -733,8 +747,7 @@ /// Reports a localization error for the passed in method call and SVal void NonLocalizedStringChecker::reportLocalizationError( -SVal S, const ObjCMethodCall &M, CheckerContext &C, -int argumentNumber) const { +SVal S, const CallEvent &M, CheckerContext &C, int argumentNumber) const { // Don't warn about localization errors in classes and methods that // may be debug code. @@ -832,7 +845,21 @@ } } - if (argumentNumber < 0) // There was no match in UIMethods + if (argumentNumber < 0) { // There was no match in UIMethods +if (const Decl *D = msg.getDecl()) { + if (const ObjCMethodDecl *OMD = dyn_cast_or_null(D)) { +auto formals = OMD->parameters(); +for (unsigned i = 0, ei = formals.size(); i != ei; ++i) { + if (isAnnotatedTakingLocalized(formals[i])) { +argumentNumber = i; +break; + } +} + } +} + } + + if (argumentNumber < 0) // Still no match return; SVal svT
[PATCH] D35186: [analyzer] Add annotation for functions taking user-facing strings
erikjv created this revision. Herald added a subscriber: xazax.hun. There was already a returns_localized_nsstring annotation to indicate that the return value could be passed to UIKit methods that would display them. However, those UIKit methods were hard-coded, and it was not possible to indicate that other classes/methods in a code-base would do the same. The takes_localized_nsstring annotation can be put on function parameters and selector parameters to indicate that those will also show the string to the user. https://reviews.llvm.org/D35186 Files: lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp test/Analysis/localization-aggressive.m Index: test/Analysis/localization-aggressive.m === --- test/Analysis/localization-aggressive.m +++ test/Analysis/localization-aggressive.m @@ -61,8 +61,16 @@ NSString *CFNumberFormatterCreateStringWithNumber(float x); + (NSString *)forceLocalized:(NSString *)str __attribute__((annotate("returns_localized_nsstring"))); ++ (NSString *)takesLocalizedString: +(NSString *)__attribute__((annotate("takes_localized_nsstring")))str; @end +NSString * +takesLocalizedString(NSString *str + __attribute__((annotate("takes_localized_nsstring" { + return str; +} + // Test cases begin here @implementation LocalizationTestSuite @@ -75,6 +83,8 @@ return str; } ++ (NSString *) takesLocalizedString:(NSString *)str { return str; } + // An ObjC method that returns a localized string + (NSString *)unLocalizedStringMethod { return @"UnlocalizedString"; @@ -269,4 +279,13 @@ NSString *string2 = POSSIBLE_FALSE_POSITIVE(@"Hello", @"Hello"); // no-warning } +- (void)testTakesLocalizedString { + NSString *localized = NSLocalizedString(@"Hello", @"World"); + NSString *alsoLocalized = [LocalizationTestSuite takesLocalizedString:localized]; // no-warning + NSString *stillLocalized = [LocalizationTestSuite takesLocalizedString:alsoLocalized]; // no-warning + takesLocalizedString(stillLocalized); // no-warning + + [LocalizationTestSuite takesLocalizedString:@"not localized"]; // expected-warning {{User-facing text should use localized string macro}} + takesLocalizedString(@"not localized"); // expected-warning {{User-facing text should use localized string macro}} +} @end Index: lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp === --- lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp +++ lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp @@ -57,7 +57,7 @@ }; class NonLocalizedStringChecker -: public Checker> { @@ -80,8 +80,9 @@ void setLocalizedState(SVal S, CheckerContext &C) const; bool isAnnotatedAsLocalized(const Decl *D) const; - void reportLocalizationError(SVal S, const ObjCMethodCall &M, - CheckerContext &C, int argumentNumber = 0) const; + bool isAnnotatedTakingLocalized(const Decl *D) const; + void reportLocalizationError(SVal S, const CallEvent &M, CheckerContext &C, + int argumentNumber = 0) const; int getLocalizedArgumentForSelector(const IdentifierInfo *Receiver, Selector S) const; @@ -97,6 +98,7 @@ void checkPreObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const; void checkPostObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const; void checkPostStmt(const ObjCStringLiteral *SL, CheckerContext &C) const; + void checkPreCall(const CallEvent &Call, CheckerContext &C) const; void checkPostCall(const CallEvent &Call, CheckerContext &C) const; }; @@ -654,6 +656,18 @@ }); } +/// Checks to see if the method / function declaration includes +/// __attribute__((annotate("takes_localized_nsstring"))) +bool NonLocalizedStringChecker::isAnnotatedTakingLocalized(const Decl *D) const { + if (!D) +return false; + return std::any_of( + D->specific_attr_begin(), + D->specific_attr_end(), [](const AnnotateAttr *Ann) { +return Ann->getAnnotation() == "takes_localized_nsstring"; + }); +} + /// Returns true if the given SVal is marked as Localized in the program state bool NonLocalizedStringChecker::hasLocalizedState(SVal S, CheckerContext &C) const { @@ -733,8 +747,7 @@ /// Reports a localization error for the passed in method call and SVal void NonLocalizedStringChecker::reportLocalizationError( -SVal S, const ObjCMethodCall &M, CheckerContext &C, -int argumentNumber) const { +SVal S, const CallEvent &M, CheckerContext &C, int argumentNumber) const { // Don't warn about localization errors in classes and methods that // may be debug code. @@ -832,7 +845,21 @@ } } - if (argumentNumber < 0) // There was no match in UIMethods + if (argumentNumber < 0) { // There was no match in UIMethods +if (const Decl *D = msg.ge