[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cfe-commits
Subject:    Re: [PATCH] D11572: [Static Analyzer] Checker for OS X / iOS localizability issues
From:       Anna Zaks <zaks.anna () gmail ! com>
Date:       2015-07-30 0:50:35
Message-ID: 2620c561b34195693c124c24b688de0b () localhost ! localdomain
[Download RAW message or body]

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




_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits


[prev in list] [next in list] [prev in thread] [next in thread]