hokein added inline comments.
================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:28
+enum NamingStyle {
+ StandardProperty = 1,
+ CategoryProperty = 2,
----------------
Please add documentation describing what these properties are.
================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:54
/// For now we will only fix 'CamelCase' property to
/// 'camelCase'. For other cases the users need to
----------------
Do we need to update the documentation here? it seems stale now?
================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:84
-std::string validPropertyNameRegex(const std::vector<std::string> &Acronyms) {
+std::string validPropertyNameRegex(const std::vector<std::string> &Acronyms,
+ bool UsedInMatcher) {
----------------
Also add documentation for the method as well as parameters. It is a bit hard
to follow the logic.
================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:93
+ [](const std::string &s) { return llvm::Regex::escape(s); });
// Allow any of these names:
// foo
----------------
Does the comment still make sense? Seems you changed the regex below.
================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:115
+
+bool prefixedPropertyNameMatches(const llvm::StringRef &PropertyName,
+ const std::vector<std::string> &Acronyms) {
----------------
no need to pass const reference of `llvm::StringRef`. `StringRef` is cheap.
================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:116
+bool prefixedPropertyNameMatches(const llvm::StringRef &PropertyName,
+ const std::vector<std::string> &Acronyms) {
+ size_t Start = 0;
----------------
nit: use `llvm::ArrayRef<std::string>` would save a lot of keystroke.
================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:118
+ size_t Start = 0;
+ while (PropertyName[Start] != '_') {
+ Start++;
----------------
we can use `find_first_not_of` or something similar.
================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:151
+ hasCategoryPropertyPrefix(MatchedDecl->getName())) {
+ const auto *CategoryDecl = (const ObjCCategoryDecl *)(DeclContext);
+ if (!prefixedPropertyNameMatches(MatchedDecl->getName(), SpecialAcronyms)
||
----------------
Consider using `const auto* CategoryDecl =
dyn_cast<ObjCCategoryDecl*>(DeclContext)`, we can get rid of this cast, and
`NodeKind` variable.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42464
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
