Comment diagnostics: for unresolved parameters, do not suggest parameter fixit

with parameter that is documented.

Fixes PR13670, <rdar://problem/12155840>.


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@162570 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/AST/CommentSema.h

@@ -46,13 +46,6 @@ class Sema {
   /// Information about the declaration this comment is attached to.
   DeclInfo *ThisDeclInfo;
 
-  /// Comment AST nodes that correspond to \c ParamVars for which we have
-  /// found a \\param command or NULL if no documentation was found so far.
-  ///
-  /// Has correct size and contains valid values if \c DeclInfo->IsFilled is
-  /// true.
-  llvm::SmallVector<ParamCommandComment *, 8> ParamVarDocs;
-
   /// Comment AST nodes that correspond to parameter names in
   /// \c TemplateParameters.
   ///
@@ -190,6 +183,10 @@ public:
   /// used only once per comment, e.g., \\brief and \\returns.
   void checkBlockCommandDuplicate(const BlockCommandComment *Command);
 
+  /// Resolve parameter names to parameter indexes in function declaration.
+  /// Emit diagnostics about unknown parametrs.
+  void resolveParamCommandIndexes(const FullComment *FC);
+
   bool isFunctionDecl();
   bool isTemplateOrSpecialization();
 

lib/AST/CommentSema.cpp

@@ -142,56 +142,6 @@ void Sema::actOnParamCommandParamNameArg(ParamCommandComment *Command,
                                                      ArgLocEnd),
                                          Arg);
   Command->setArgs(llvm::makeArrayRef(A, 1));
-
-  if (!isFunctionDecl()) {
-    // We already warned that this \\param is not attached to a function decl.
-    return;
-  }
-
-  ArrayRef<const ParmVarDecl *> ParamVars = getParamVars();
-
-  // Check that referenced parameter name is in the function decl.
-  const unsigned ResolvedParamIndex = resolveParmVarReference(Arg, ParamVars);
-  if (ResolvedParamIndex != ParamCommandComment::InvalidParamIndex) {
-    Command->setParamIndex(ResolvedParamIndex);
-    if (ParamVarDocs[ResolvedParamIndex]) {
-      SourceRange ArgRange(ArgLocBegin, ArgLocEnd);
-      Diag(ArgLocBegin, diag::warn_doc_param_duplicate)
-        << Arg << ArgRange;
-      ParamCommandComment *PrevCommand = ParamVarDocs[ResolvedParamIndex];
-      Diag(PrevCommand->getLocation(), diag::note_doc_param_previous)
-        << PrevCommand->getParamNameRange();
-    }
-    ParamVarDocs[ResolvedParamIndex] = Command;
-    return;
-  }
-
-  SourceRange ArgRange(ArgLocBegin, ArgLocEnd);
-  Diag(ArgLocBegin, diag::warn_doc_param_not_found)
-    << Arg << ArgRange;
-
-  // No parameters -- can't suggest a correction.
-  if (ParamVars.size() == 0)
-    return;
-
-  unsigned CorrectedParamIndex = ParamCommandComment::InvalidParamIndex;
-  if (ParamVars.size() == 1) {
-    // If function has only one parameter then only that parameter
-    // can be documented.
-    CorrectedParamIndex = 0;
-  } else {
-    // Do typo correction.
-    CorrectedParamIndex = correctTypoInParmVarReference(Arg, ParamVars);
-  }
-  if (CorrectedParamIndex != ParamCommandComment::InvalidParamIndex) {
-    const ParmVarDecl *CorrectedPVD = ParamVars[CorrectedParamIndex];
-    if (const IdentifierInfo *CorrectedII = CorrectedPVD->getIdentifier())
-      Diag(ArgLocBegin, diag::note_doc_param_name_suggestion)
-        << CorrectedII->getName()
-        << FixItHint::CreateReplacement(ArgRange, CorrectedII->getName());
-  }
-
-  return;
 }
 
 void Sema::actOnParamCommandFinish(ParamCommandComment *Command,
@@ -445,7 +395,9 @@ HTMLEndTagComment *Sema::actOnHTMLEndTag(SourceLocation LocBegin,
 
 FullComment *Sema::actOnFullComment(
                               ArrayRef<BlockContentComment *> Blocks) {
-  return new (Allocator) FullComment(Blocks, ThisDeclInfo);
+  FullComment *FC = new (Allocator) FullComment(Blocks, ThisDeclInfo);
+  resolveParamCommandIndexes(FC);
+  return FC;
 }
 
 void Sema::checkBlockCommandEmptyParagraph(BlockCommandComment *Command) {
@@ -529,6 +481,91 @@ void Sema::checkBlockCommandDuplicate(const BlockCommandComment *Command) {
         << Name;
 }
 
+void Sema::resolveParamCommandIndexes(const FullComment *FC) {
+  if (!isFunctionDecl()) {
+    // We already warned that \\param commands are not attached to a function
+    // decl.
+    return;
+  }
+
+  llvm::SmallVector<ParamCommandComment *, 8> UnresolvedParamCommands;
+
+  // Comment AST nodes that correspond to \c ParamVars for which we have
+  // found a \\param command or NULL if no documentation was found so far.
+  llvm::SmallVector<ParamCommandComment *, 8> ParamVarDocs;
+
+  ArrayRef<const ParmVarDecl *> ParamVars = getParamVars();
+  ParamVarDocs.resize(ParamVars.size(), NULL);
+
+  // First pass over all \\param commands: resolve all parameter names.
+  for (Comment::child_iterator I = FC->child_begin(), E = FC->child_end();
+       I != E; ++I) {
+    ParamCommandComment *PCC = dyn_cast<ParamCommandComment>(*I);
+    if (!PCC || !PCC->hasParamName())
+      continue;
+    StringRef ParamName = PCC->getParamName();
+
+    // Check that referenced parameter name is in the function decl.
+    const unsigned ResolvedParamIndex = resolveParmVarReference(ParamName,
+                                                                ParamVars);
+    if (ResolvedParamIndex == ParamCommandComment::InvalidParamIndex) {
+      UnresolvedParamCommands.push_back(PCC);
+      continue;
+    }
+    PCC->setParamIndex(ResolvedParamIndex);
+    if (ParamVarDocs[ResolvedParamIndex]) {
+      SourceRange ArgRange = PCC->getParamNameRange();
+      Diag(ArgRange.getBegin(), diag::warn_doc_param_duplicate)
+        << ParamName << ArgRange;
+      ParamCommandComment *PrevCommand = ParamVarDocs[ResolvedParamIndex];
+      Diag(PrevCommand->getLocation(), diag::note_doc_param_previous)
+        << PrevCommand->getParamNameRange();
+    }
+    ParamVarDocs[ResolvedParamIndex] = PCC;
+  }
+
+  // Find parameter declarations that have no corresponding \\param.
+  llvm::SmallVector<const ParmVarDecl *, 8> OrphanedParamDecls;
+  for (unsigned i = 0, e = ParamVarDocs.size(); i != e; ++i) {
+    if (!ParamVarDocs[i])
+      OrphanedParamDecls.push_back(ParamVars[i]);
+  }
+
+  // Second pass over unresolved \\param commands: do typo correction.
+  // Suggest corrections from a set of parameter declarations that have no
+  // corresponding \\param.
+  for (unsigned i = 0, e = UnresolvedParamCommands.size(); i != e; ++i) {
+    const ParamCommandComment *PCC = UnresolvedParamCommands[i];
+
+    SourceRange ArgRange = PCC->getParamNameRange();
+    StringRef ParamName = PCC->getParamName();
+    Diag(ArgRange.getBegin(), diag::warn_doc_param_not_found)
+      << ParamName << ArgRange;
+
+    // All parameters documented -- can't suggest a correction.
+    if (OrphanedParamDecls.size() == 0)
+      continue;
+
+    unsigned CorrectedParamIndex = ParamCommandComment::InvalidParamIndex;
+    if (OrphanedParamDecls.size() == 1) {
+      // If one parameter is not documented then that parameter is the only
+      // possible suggestion.
+      CorrectedParamIndex = 0;
+    } else {
+      // Do typo correction.
+      CorrectedParamIndex = correctTypoInParmVarReference(ParamName,
+                                                          OrphanedParamDecls);
+    }
+    if (CorrectedParamIndex != ParamCommandComment::InvalidParamIndex) {
+      const ParmVarDecl *CorrectedPVD = OrphanedParamDecls[CorrectedParamIndex];
+      if (const IdentifierInfo *CorrectedII = CorrectedPVD->getIdentifier())
+        Diag(ArgRange.getBegin(), diag::note_doc_param_name_suggestion)
+          << CorrectedII->getName()
+          << FixItHint::CreateReplacement(ArgRange, CorrectedII->getName());
+    }
+  }
+}
+
 bool Sema::isFunctionDecl() {
   if (!ThisDeclInfo)
     return false;
@@ -553,7 +590,6 @@ ArrayRef<const ParmVarDecl *> Sema::getParamVars() {
 
 void Sema::inspectThisDecl() {
   ThisDeclInfo->fill();
-  ParamVarDocs.resize(ThisDeclInfo->ParamVars.size(), NULL);
 }
 
 unsigned Sema::resolveParmVarReference(StringRef Name,

test/Sema/warn-documentation.cpp

@@ -218,9 +218,32 @@ int test_param12(int a);
 /// \param aab Blah blah.
 int test_param13(int aaa, int bbb);
 
+// expected-warning@+2 {{parameter 'aab' not found in the function declaration}} expected-note@+2 {{did you mean 'bbb'?}}
+/// \param aaa Blah blah.
+/// \param aab Blah blah.
+int test_param14(int aaa, int bbb);
+
 // expected-warning@+1 {{parameter 'aab' not found in the function declaration}}
 /// \param aab Blah blah.
-int test_param14(int bbb, int ccc);
+int test_param15(int bbb, int ccc);
+
+// expected-warning@+1 {{parameter 'aab' not found in the function declaration}}
+/// \param aab Ccc.
+/// \param aaa Aaa.
+/// \param bbb Bbb.
+int test_param16(int aaa, int bbb);
+
+// expected-warning@+2 {{parameter 'aab' not found in the function declaration}}
+/// \param aaa Aaa.
+/// \param aab Ccc.
+/// \param bbb Bbb.
+int test_param17(int aaa, int bbb);
+
+// expected-warning@+3 {{parameter 'aab' not found in the function declaration}}
+/// \param aaa Aaa.
+/// \param bbb Bbb.
+/// \param aab Ccc.
+int test_param18(int aaa, int bbb);
 
 class C {
   // expected-warning@+1 {{parameter 'aaa' not found in the function declaration}}
@@ -229,50 +252,51 @@ class C {
 
   // expected-warning@+1 {{parameter 'aaa' not found in the function declaration}}
   /// \param aaa Blah blah.
- int test_param15(int bbb, int ccc);
+ int test_param19(int bbb, int ccc);
 };
 
 // expected-warning@+1 {{parameter 'aab' not found in the function declaration}}
 /// \param aab Blah blah.
 template<typename T>
-void test_param16(int bbb, int ccc);
+void test_param20(int bbb, int ccc);
 
 // expected-warning@+3 {{parameter 'a' is already documented}}
 // expected-note@+1 {{previous documentation}}
 /// \param a Aaa.
 /// \param a Aaa.
-int test_param17(int a);
+int test_param21(int a);
 
 // expected-warning@+4 {{parameter 'x2' is already documented}}
 // expected-note@+2 {{previous documentation}}
 /// \param x1 Aaa.
 /// \param x2 Bbb.
 /// \param x2 Ccc.
-int test_param18(int x1, int x2, int x3);
+int test_param22(int x1, int x2, int x3);
 
-// expected-warning@+2 {{parameter 'bbb' not found in the function declaration}} expected-note@+2 {{did you mean 'aaa'?}}
+// expected-warning@+2 {{parameter 'bbb' not found in the function declaration}} expected-note@+2 {{did you mean 'ccc'?}}
 /// \param aaa Meow.
 /// \param bbb Bbb.
 /// \returns aaa.
-typedef int test_param19(int aaa);
+typedef int test_param23(int aaa, int ccc);
 
-// expected-warning@+2 {{parameter 'bbb' not found in the function declaration}} expected-note@+2 {{did you mean 'aaa'?}}
+// expected-warning@+2 {{parameter 'bbb' not found in the function declaration}} expected-note@+2 {{did you mean 'ccc'?}}
 /// \param aaa Meow.
 /// \param bbb Bbb.
 /// \returns aaa.
-typedef int (*test_param20)(int aaa);
+typedef int (*test_param24)(int aaa, int ccc);
 
-// expected-warning@+2 {{parameter 'bbb' not found in the function declaration}} expected-note@+2 {{did you mean 'aaa'?}}
+// expected-warning@+2 {{parameter 'bbb' not found in the function declaration}} expected-note@+2 {{did you mean 'ccc'?}}
 /// \param aaa Meow.
 /// \param bbb Bbb.
 /// \returns aaa.
-typedef int (* const test_param21)(int aaa);
+typedef int (* const test_param25)(int aaa, int ccc);
 
-// expected-warning@+2 {{parameter 'bbb' not found in the function declaration}} expected-note@+2 {{did you mean 'aaa'?}}
+// expected-warning@+2 {{parameter 'bbb' not found in the function declaration}} expected-note@+2 {{did you mean 'ccc'?}}
 /// \param aaa Meow.
 /// \param bbb Bbb.
 /// \returns aaa.
-typedef int (C::*test_param22)(int aaa);
+typedef int (C::*test_param26)(int aaa, int ccc);
+
 
 // expected-warning@+1 {{'\tparam' command used in a comment that is not attached to a template declaration}}
 /// \tparam T Aaa

test/Sema/warn-documentation.m

@@ -91,9 +91,9 @@ int b;
 - (void)test2:(NSString *)aaa;
 @end
 
-// expected-warning@+2 {{parameter 'bbb' not found in the function declaration}} expected-note@+2 {{did you mean 'aaa'?}}
+// expected-warning@+2 {{parameter 'bbb' not found in the function declaration}} expected-note@+2 {{did you mean 'ccc'?}}
 /// \param aaa Meow.
 /// \param bbb Bbb.
 /// \returns aaa.
-typedef int (^test_param1)(int aaa);
+typedef int (^test_param1)(int aaa, int ccc);