[PATCH] D11572: [Static Analyzer] Checker for OS X / iOS localizability issues

[Anna Zaks]

zaks.anna added inline comments.

================
Comment at: lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp:14
@@ +13,3 @@
+//  2) A syntactic checker that warns against the bad practice of
+//     not including a comment in NSLocalizedString macros.
+//
----------------
My quick Google search did not show any mention of this :(

================
Comment at: lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp:202
@@ +201,3 @@
+void NonLocalizedStringChecker::reportLocalizationError(
+    SVal S, const ObjCMethodCall &M, CheckerContext &C,
+    int argumentNumber) const {
----------------
Adding a new transition is more debugging friendly.

================
Comment at: lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp:334
@@ +333,3 @@
+  SVal sv = Call.getReturnValue();
+  if (isAnnotatedAsLocalized(D) || LSF.find(IdentifierName) != LSF.end()) {
+    setLocalizedState(sv, C);
----------------
Ah, I see!

================
Comment at: lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp:366
@@ +365,3 @@
+  Selector S = msg.getSelector();
+  StringRef SelectorName = S.getAsString();
+  assert(!SelectorName.empty());
----------------
I am not sure if we are on the same page.. I would not store/mark this information in the state, but just use the code as part of the check.

================
Comment at: lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp:451
@@ +450,3 @@
+/// checking for (comment) is not used and thus not present in the AST,
+/// so we use Lexer on the original macro call and retrieve the value of
+/// the comment. If it's empty or nil, we raise a warning.
----------------
The point is that this macro can be used inside another, user defined macro where you'd no longer know which argument corresponds to "comment".  The only way around that that I see is to only warn when these macros are called directly.


http://reviews.llvm.org/D11572