Re-apply: Warning for framework headers using double quote includes

Introduce -Wquoted-include-in-framework-header, which should fire a warning
whenever a quote include appears in a framework header and suggest a fix-it.
For instance, for header A.h added in the tests, this is how the warning looks
like:

./A.framework/Headers/A.h:2:10: warning: double-quoted include "A0.h" in framework header, expected angle-bracketed instead [-Wquoted-include-in-framework-header]
#include "A0.h"
         ^~~~~~
         <A/A0.h>
./A.framework/Headers/A.h:3:10: warning: double-quoted include "B.h" in framework header, expected angle-bracketed instead [-Wquoted-include-in-framework-header]
#include "B.h"
         ^~~~~
         <B.h>

This helps users to prevent frameworks from using local headers when in fact
they should be targetting system level ones.

The warning is off by default.

Differential Revision: https://reviews.llvm.org/D47157

rdar://problem/37077034

llvm-svn: 335375

1 files changed, 64 insertions, 1 deletions

diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index 1f2c339ea3c..757cd097e4e 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -621,6 +621,59 @@ static const char *copyString(StringRef Str, llvm::BumpPtrAllocator &Alloc) {
   return CopyStr;
 }
 
+static bool isFrameworkStylePath(StringRef Path,
+                                 SmallVectorImpl<char> &FrameworkName) {
+  using namespace llvm::sys;
+  path::const_iterator I = path::begin(Path);
+  path::const_iterator E = path::end(Path);
+
+  // Detect different types of framework style paths:
+  //
+  //   ...Foo.framework/{Headers,PrivateHeaders}
+  //   ...Foo.framework/Versions/{A,Current}/{Headers,PrivateHeaders}
+  //   ...Foo.framework/Frameworks/Nested.framework/{Headers,PrivateHeaders}
+  //   ...<other variations with 'Versions' like in the above path>
+  //
+  // and some other variations among these lines.
+  int FoundComp = 0;
+  while (I != E) {
+    if (I->endswith(".framework")) {
+      FrameworkName.append(I->begin(), I->end());
+      ++FoundComp;
+    }
+    if (*I == "Headers" || *I == "PrivateHeaders")
+      ++FoundComp;
+    ++I;
+  }
+
+  return FoundComp >= 2;
+}
+
+static void
+diagnoseFrameworkInclude(DiagnosticsEngine &Diags, SourceLocation IncludeLoc,
+                         StringRef Includer, StringRef IncludeFilename,
+                         const FileEntry *IncludeFE, bool isAngled = false,
+                         bool FoundByHeaderMap = false) {
+  SmallString<128> FromFramework, ToFramework;
+  if (!isFrameworkStylePath(Includer, FromFramework))
+    return;
+  bool IsIncludeeInFramework =
+      isFrameworkStylePath(IncludeFE->getName(), ToFramework);
+
+  if (!isAngled && !FoundByHeaderMap) {
+    SmallString<128> NewInclude("<");
+    if (IsIncludeeInFramework) {
+      NewInclude += StringRef(ToFramework).drop_back(10); // drop .framework
+      NewInclude += "/";
+    }
+    NewInclude += IncludeFilename;
+    NewInclude += ">";
+    Diags.Report(IncludeLoc, diag::warn_quoted_include_in_framework_header)
+        << IncludeFilename
+        << FixItHint::CreateReplacement(IncludeLoc, NewInclude);
+  }
+}
+
 /// LookupFile - Given a "foo" or \<foo> reference, look up the indicated file,
 /// return null on failure.  isAngled indicates whether the file reference is
 /// for system \#include's or not (i.e. using <> instead of ""). Includers, if
@@ -722,8 +775,12 @@ const FileEntry *HeaderSearch::LookupFile(
           RelativePath->clear();
           RelativePath->append(Filename.begin(), Filename.end());
         }
-        if (First)
+        if (First) {
+          diagnoseFrameworkInclude(Diags, IncludeLoc,
+                                   IncluderAndDir.second->getName(), Filename,
+                                   FE);
           return FE;
+        }
 
         // Otherwise, we found the path via MSVC header search rules.  If
         // -Wmsvc-include is enabled, we have to keep searching to see if we
@@ -834,6 +891,12 @@ const FileEntry *HeaderSearch::LookupFile(
       return MSFE;
     }
 
+    bool FoundByHeaderMap = !IsMapped ? false : *IsMapped;
+    if (!Includers.empty())
+      diagnoseFrameworkInclude(Diags, IncludeLoc,
+                               Includers.front().second->getName(), Filename,
+                               FE, isAngled, FoundByHeaderMap);
+
     // Remember this location for the next lookup we do.
     CacheLookup.HitIdx = i;
     return FE;