[analyzer] Add annotation for functions taking user-facing strings

[erikjv]

Summary

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.

[erikjv]

Sorry, now with more context in the diff.

[dcoughlin]

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()'?

cfe/trunk/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp

public:
   // Overload the == operator
   bool operator==(const LocalizedState &X) const { return K == X.K; }
 
   // LLVMs equivalent of a hash function
   void Profile(llvm::FoldingSetNodeID &ID) const { ID.AddInteger(K); }
 };
 
 class NonLocalizedStringChecker
-    : public Checker<check::PostCall, check::PreObjCMessage,
+    : public Checker<check::PreCall, check::PostCall, check::PreObjCMessage,
                      check::PostObjCMessage,
                      check::PostStmt<ObjCStringLiteral>> {
 
   mutable std::unique_ptr<BugType> BT;
 
   // Methods that require a localized string
   mutable llvm::DenseMap<const IdentifierInfo *,
                          llvm::DenseMap<Selector, uint8_t>> UIMethods;
   // Methods that return a localized string
   mutable llvm::SmallSet<std::pair<const IdentifierInfo *, Selector>, 12> LSM;
   // C Functions that return a localized string
   mutable llvm::SmallSet<const IdentifierInfo *, 5> LSF;
 
   void initUIMethods(ASTContext &Ctx) const;
   void initLocStringsMethods(ASTContext &Ctx) const;
 
   bool hasNonLocalizedState(SVal S, CheckerContext &C) const;
   bool hasLocalizedState(SVal S, CheckerContext &C) const;
   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;
 
 public:
   NonLocalizedStringChecker();
 
   // When this parameter is set to true, the checker assumes all
   // methods that return NSStrings are unlocalized. Thus, more false
   // positives will be reported.
   DefaultBool IsAggressive;
 
   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;
 };
 
 } // end anonymous namespace
 
 REGISTER_MAP_WITH_PROGRAMSTATE(LocalizedMemMap, const MemRegion *,
                                LocalizedState)
 
void NonLocalizedStringChecker::initLocStringsMethods(ASTContext &Ctx) const {
 
   LSF_INSERT("CFDateFormatterCreateStringWithDate");
   LSF_INSERT("CFDateFormatterCreateStringWithAbsoluteTime");
   LSF_INSERT("CFNumberFormatterCreateStringWithNumber");
 }
 
 /// 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(
       D->specific_attr_begin<AnnotateAttr>(),
       D->specific_attr_end<AnnotateAttr>(), [](const AnnotateAttr *Ann) {
         return Ann->getAnnotation() == "returns_localized_nsstring";
       });
 }
 
+/// 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<AnnotateAttr>(),
+      D->specific_attr_end<AnnotateAttr>(), [](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 {
   const MemRegion *mt = S.getAsRegion();
   if (mt) {
     const LocalizedState *LS = C.getState()->get<LocalizedMemMap>(mt);
     if (LS && LS->isLocalized())
       return true;
static bool isDebuggingContext(CheckerContext &C) {
   }
 
   return false;
 }
 
 
 /// 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.
   if (isDebuggingContext(C))
     return;
 
   ExplodedNode *ErrNode = C.getPredecessor();
   static CheckerProgramPointTag Tag("NonLocalizedStringChecker",
for (const auto *P : OD->all_referenced_protocols()) {
         break;
     }
     if (argumentNumber < 0) {
       OD = OD->getSuperClass();
       argumentNumber = getLocalizedArgumentForSelector(OD->getIdentifier(), S);
     }
   }
 
-  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<ObjCMethodDecl>(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);
 
   if (const ObjCStringRegion *SR =
           dyn_cast_or_null<ObjCStringRegion>(svTitle.getAsRegion())) {
     StringRef stringValue =
         SR->getObjCStringLiteral()->getString()->getString();
     if ((stringValue.trim().size() == 0 && stringValue.size() > 0) ||
         stringValue.empty())
       return;
     if (!IsAggressive && llvm::sys::unicode::columnWidthUTF8(stringValue) < 2)
       return;
   }
 
   bool isNonLocalized = hasNonLocalizedState(svTitle, C);
 
   if (isNonLocalized) {
     reportLocalizationError(svTitle, msg, C, argumentNumber + 1);
   }
 }
 
+void NonLocalizedStringChecker::checkPreCall(const CallEvent &Call,
+                                             CheckerContext &C) const {
+  const Decl *D = Call.getDecl();
+  if (D && isa<FunctionDecl>(D)) {
+    const FunctionDecl *FD = dyn_cast<FunctionDecl>(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<ObjCObjectPointerType>();
   if (!PT)
     return false;
 
   ObjCInterfaceDecl *Cls = PT->getObjectType()->getInterface();
   if (!Cls)
void NonLocalizedStringChecker::checkPostCall(const CallEvent &Call,
 
   const Decl *D = Call.getDecl();
   if (!D)
     return;
 
   const IdentifierInfo *Identifier = Call.getCalleeIdentifier();
 
   SVal sv = Call.getReturnValue();
-  if (isAnnotatedAsLocalized(D) || LSF.count(Identifier) != 0) {
+  if (isAnnotatedAsReturningLocalized(D) || LSF.count(Identifier) != 0) {
     setLocalizedState(sv, C);
   } else if (isNSStringType(RT, C.getASTContext()) &&
              !hasLocalizedState(sv, C)) {
     if (IsAggressive) {
       setNonLocalizedState(sv, C);
     } else {
       const SymbolicRegion *SymReg =
           dyn_cast_or_null<SymbolicRegion>(sv.getAsRegion());
if (!OD)
     return;
   const IdentifierInfo *odInfo = OD->getIdentifier();
 
   Selector S = msg.getSelector();
   std::string SelectorName = S.getAsString();
 
   std::pair<const IdentifierInfo *, Selector> MethodDescription = {odInfo, S};
 
-  if (LSM.count(MethodDescription) || isAnnotatedAsLocalized(msg.getDecl())) {
+  if (LSM.count(MethodDescription) ||
+      isAnnotatedAsReturningLocalized(msg.getDecl())) {
     SVal sv = msg.getReturnValue();
     setLocalizedState(sv, C);
   }
 }
 
 /// Marks all empty string literals as localized
 void NonLocalizedStringChecker::checkPostStmt(const ObjCStringLiteral *SL,
                                               CheckerContext &C) const {

cfe/trunk/test/Analysis/localization-aggressive.m

 NSString *ForceLocalized(NSString *str)
     __attribute__((annotate("returns_localized_nsstring")));
 CGPoint CGPointMake(CGFloat x, CGFloat y);
 int random();
 // This next one is a made up API
 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
 
 // A C-Funtion that returns a localized string because it has the
 // "returns_localized_nsstring" annotation
 NSString *ForceLocalized(NSString *str) { return str; }
 // An ObjC method that returns a localized string because it has the
 // "returns_localized_nsstring" annotation
 + (NSString *)forceLocalized:(NSString *)str {
   return str;
 }
 
++ (NSString *) takesLocalizedString:(NSString *)str { return str; }
+
 // An ObjC method that returns a localized string
 + (NSString *)unLocalizedStringMethod {
   return @"UnlocalizedString";
 }
 
 - (void)testLocalizationErrorDetectedOnPathway {
   UILabel *testLabel = [[UILabel alloc] init];
   NSString *bar = NSLocalizedString(@"Hello", @"Comment");
- (void)testNoWarningForNilCommentPassedIntoOtherMacro {
   NSString *string3 = KCLocalizedString(@"Hello",@"Comment"); // no-warning
 }
 
 - (void)testPossibleFalsePositiveSituationAbove {
   NSString *string = POSSIBLE_FALSE_POSITIVE(@"Hello", nil); // no-warning
   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