benhamilton added inline comments.
================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:52
/// FIXME: provide fix for snake_case to snakeCase
-FixItHint generateFixItHint(const ObjCPropertyDecl *Decl) {
- if (isupper(Decl->getName()[0])) {
- auto NewName = Decl->getName().str();
- NewName[0] = tolower(NewName[0]);
- return FixItHint::CreateReplacement(
- CharSourceRange::getTokenRange(SourceRange(Decl->getLocation())),
- llvm::StringRef(NewName));
+FixItHint generateFixItHint(const ObjCPropertyDecl *Decl, bool isCategory) {
+ auto Name = Decl->getName();
----------------
Instead of bool isCategory, how about an enum NamingStyle:
```
enum NamingStyle {
StandardProperty = 1,
CategoryProperty = 2,
}
```
================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:102
+bool hasCategoryPropertyPrefix(const llvm::StringRef &PropertyName) {
+ for (size_t i = 0; i < PropertyName.size() - 1; ++i) {
+ if (PropertyName[i] == '_') {
----------------
I think this is an off-by-one error, right? Change:
i < PropertyName.size() - 1
to
i < PropertyName.size()
================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:103-105
+ if (PropertyName[i] == '_') {
+ return true;
+ }
----------------
Do we really want a leading _ to count? I think this might need to be a regular
expression instead, something like:
^[a-zA-Z][a-zA-Z0-9]+_[a-zA-Z0-9][a-zA-Z0-9_]*$
================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:120
+ auto RegexExp = llvm::Regex(
+ llvm::StringRef(validPropertyNameRegex(Acronyms).replace(0, 2, "^")));
+ return RegexExp.match(llvm::StringRef(PropertyName.substr(Start + 1)));
----------------
I don't understand what this is doing. Why are we replacing things in a regex?
I don't think this is very maintainable. Can you rewrite it for legibility,
please?
================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:147
+ auto NodeKind = std::string(ParentContext->getDeclKindName());
+ if (NodeKind == "ObjCCategory" &&
+ hasCategoryPropertyPrefix(MatchedDecl->getName())) {
----------------
Is a class extension treated as a category? If so, we probably need to treat it
specially.
e.g.:
Foo.h:
```
@interface Foo
@end
```
Foo.m:
```
@interface Foo ()
@property (nonatomic, readonly) int foo_bar;
@end
```
should not be allowed.
Can you handle this and add tests, please?
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
