Suggest objc_method_family(none) for a property named -newFoo or similar.

As mentioned in the previous commit, if a property (declared with @property)
has a name that matches a special Objective-C method family, the getter picks
up that family despite being declared by the property. The most correct way
to solve this problem is to add the 'objc_method_family' attribute to the
getter with an argument of 'none', which unfortunately requires an explicit
declaration of the getter.

This commit adds a note to the existing error (ARC) or warning (MRR) for
such a poorly-named property that suggests the solution; if there's already
a declaration of the getter, it even includes a fix-it.

llvm-svn: 226339

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -797,6 +797,12 @@ def warn_cocoa_naming_owned_rule : Warning<
   "property follows Cocoa naming"
   " convention for returning 'owned' objects">,
   InGroup<DiagGroup<"objc-property-matches-cocoa-ownership-rule">>;
+def err_cocoa_naming_owned_rule : Error<
+  "property follows Cocoa naming"
+  " convention for returning 'owned' objects">;
+def note_cocoa_naming_declare_family : Note<
+  "explicitly declare getter %objcinstance0 with '%1' to return an 'unowned' "
+  "object">;
 def warn_auto_synthesizing_protocol_property :Warning<
   "auto property synthesis will not synthesize property %0"
   " declared in protocol %1">,
@@ -828,9 +834,6 @@ def warn_property_getter_owning_mismatch : Warning<
 def error_property_setter_ambiguous_use : Error<
   "synthesized properties %0 and %1 both claim setter %2 -"
   " use of this setter will cause unexpected behavior">;
-def err_cocoa_naming_owned_rule : Error<
-  "property follows Cocoa naming"
-  " convention for returning 'owned' objects">;
 def warn_default_atomic_custom_getter_setter : Warning<
   "atomic by default property %0 has a user defined %select{getter|setter}1 "
   "(property should be marked 'atomic' if this is intended)">,

clang/lib/Sema/SemaObjCProperty.cpp

@@ -19,6 +19,7 @@
 #include "clang/AST/ExprObjC.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Lexer.h"
+#include "clang/Lex/Preprocessor.h"
 #include "clang/Sema/Initialization.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/SmallString.h"
@@ -1854,6 +1855,39 @@ void Sema::DiagnoseOwningPropertyGetterSynthesis(const ObjCImplementationDecl *D
           Diag(PD->getLocation(), diag::err_cocoa_naming_owned_rule);
         else
           Diag(PD->getLocation(), diag::warn_cocoa_naming_owned_rule);
+
+        // Look for a getter explicitly declared alongside the property.
+        // If we find one, use its location for the note.
+        SourceLocation noteLoc = PD->getLocation();
+        SourceLocation fixItLoc;
+        for (auto *getterRedecl : method->redecls()) {
+          if (getterRedecl->isImplicit())
+            continue;
+          if (getterRedecl->getDeclContext() != PD->getDeclContext())
+            continue;
+          noteLoc = getterRedecl->getLocation();
+          fixItLoc = getterRedecl->getLocEnd();
+        }
+
+        Preprocessor &PP = getPreprocessor();
+        TokenValue tokens[] = {
+          tok::kw___attribute, tok::l_paren, tok::l_paren,
+          PP.getIdentifierInfo("objc_method_family"), tok::l_paren,
+          PP.getIdentifierInfo("none"), tok::r_paren,
+          tok::r_paren, tok::r_paren
+        };
+        StringRef spelling = "__attribute__((objc_method_family(none)))";
+        StringRef macroName = PP.getLastMacroWithSpelling(noteLoc, tokens);
+        if (!macroName.empty())
+          spelling = macroName;
+
+        auto noteDiag = Diag(noteLoc, diag::note_cocoa_naming_declare_family)
+            << method->getDeclName() << spelling;
+        if (fixItLoc.isValid()) {
+          SmallString<64> fixItText(" ");
+          fixItText += spelling;
+          noteDiag << FixItHint::CreateInsertion(fixItLoc, fixItText);
+        }
       }
     }
   }

clang/test/SemaObjC/arc-decls.m

@@ -54,13 +54,13 @@ void func()
 // rdar://15757510
 
 @interface J
-@property (retain) id newFoo; // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}}
-@property (strong) id copyBar;  // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}}
-@property (copy) id allocBaz; // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}}
+@property (retain) id newFoo; // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}} expected-note{{explicitly declare getter '-newFoo' with '__attribute__((objc_method_family(none)))' to return an 'unowned' object}}
+@property (strong) id copyBar;  // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}} expected-note{{explicitly declare getter '-copyBar' with '__attribute__((objc_method_family(none)))' to return an 'unowned' object}}
+@property (copy) id allocBaz; // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}} expected-note{{explicitly declare getter '-allocBaz' with '__attribute__((objc_method_family(none)))' to return an 'unowned' object}}
 @property (copy, nonatomic) id new;
-@property (retain) id newDFoo; // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}}
-@property (strong) id copyDBar; // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}}
-@property (copy) id allocDBaz; // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}}
+@property (retain) id newDFoo; // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}} expected-note{{explicitly declare getter '-newDFoo' with '__attribute__((objc_method_family(none)))' to return an 'unowned' object}}
+@property (strong) id copyDBar; // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}} expected-note{{explicitly declare getter '-copyDBar' with '__attribute__((objc_method_family(none)))' to return an 'unowned' object}}
+@property (copy) id allocDBaz; // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}} expected-note{{explicitly declare getter '-allocDBaz' with '__attribute__((objc_method_family(none)))' to return an 'unowned' object}}
 @end
 
 @implementation J
@@ -76,6 +76,29 @@ void func()
 @end
 
 
+@interface MethodFamilyDiags
+@property (retain) id newFoo; // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}}
+- (id)newFoo; // expected-note {{explicitly declare getter '-newFoo' with '__attribute__((objc_method_family(none)))' to return an 'unowned' object}}
+
+#define OBJC_METHOD_FAMILY_NONE __attribute__((objc_method_family(none)))
+- (id)newBar; // expected-note {{explicitly declare getter '-newBar' with 'OBJC_METHOD_FAMILY_NONE' to return an 'unowned' object}}
+@property (retain) id newBar; // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}}
+
+@property (retain) id newBaz; // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}} expected-note {{explicitly declare getter '-newBaz' with 'OBJC_METHOD_FAMILY_NONE' to return an 'unowned' object}}
+#undef OBJC_METHOD_FAMILY_NONE
+
+@property (retain, readonly) id newGarply; // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}} expected-note {{explicitly declare getter '-newGarply' with '__attribute__((objc_method_family(none)))' to return an 'unowned' object}}
+@end
+
+@interface MethodFamilyDiags (Redeclarations)
+- (id)newGarply; // no note here
+@end
+
+@implementation MethodFamilyDiags
+@synthesize newGarply;
+@end
+
+
 // rdar://10187884
 @interface Super
 - (void)bar:(id)b; // expected-note {{parameter declared here}}