Fix symbol mappings to relative includes

If a symbol was ultimately mapped to a header which was (or should be)
included via a ""-style relative include then that would not be handled
correctly.

Update the symbol mappings to be handled in the same way as the include
mappings so that this should no longer be an issue.

8 files changed, 115 insertions, 35 deletions

diff --git a/iwyu_include_picker.cc b/iwyu_include_picker.cc
index 5129066..dcb06a8 100644
--- a/iwyu_include_picker.cc
+++ b/iwyu_include_picker.cc
@@ -1329,18 +1329,50 @@ string IncludePicker::MaybeGetIncludeNameAsWritten(
   return value ? *value : "";
 }
 
-vector<string> IncludePicker::GetCandidateHeadersForSymbol(
-    const string& symbol) const {
-  CHECK_(has_called_finalize_added_include_lines_ && "Must finalize includes");
-  vector<MappedInclude> mapped_includes =
-      GetPublicValues(symbol_include_map_, symbol);
+vector<string> IncludePicker::BestQuotedIncludesForIncluder(
+    const vector<MappedInclude>& includes,
+    const string& including_filepath) const {
+  // Convert each MappedInclude to a quoted include, according to the
+  // following priorities:
+  // 1. If the file is already included, use whatever name it's already
+  //    included via.  This is better to use than ConvertToQuotedInclude
+  //    because it avoids trouble when the same file is accessible via
+  //    different include search-paths, or is accessed via a symlink.
+  // 2. If the quoted include in the MappedInclude object is an absolute path,
+  //    that's unlikely to be what's wanted.  Try to convert it to a relative
+  //    include via ConvertToQuotedInclude.
+  // 3. Otherwise, use the quoted include present in the MappedInclude.
+  const string including_path =
+      MakeAbsolutePath(GetParentPath(including_filepath));
   vector<string> retval;
-  for (const MappedInclude& mapped_include : mapped_includes) {
-    retval.push_back(mapped_include.quoted_include);
+  for (const MappedInclude& mapped_include : includes) {
+    const string& quoted_include_as_written =
+        MaybeGetIncludeNameAsWritten(including_filepath, mapped_include.path);
+    if (!quoted_include_as_written.empty()) {
+      retval.push_back(quoted_include_as_written);
+    } else if (mapped_include.HasAbsoluteQuotedInclude() &&
+               !mapped_include.path.empty()) {
+      retval.push_back(ConvertToQuotedInclude(mapped_include.path,
+                                              including_path));
+    } else {
+      retval.push_back(mapped_include.quoted_include);
+    }
   }
   return retval;
 }
 
+vector<MappedInclude> IncludePicker::GetCandidateHeadersForSymbol(
+    const string& symbol) const {
+  CHECK_(has_called_finalize_added_include_lines_ && "Must finalize includes");
+  return GetPublicValues(symbol_include_map_, symbol);
+}
+
+vector<string> IncludePicker::GetCandidateHeadersForSymbolUsedFrom(
+    const string& symbol, const string& including_filepath) const {
+  return BestQuotedIncludesForIncluder(
+      GetCandidateHeadersForSymbol(symbol), including_filepath);
+}
+
 vector<MappedInclude> IncludePicker::GetCandidateHeadersForFilepath(
     const string& filepath, const string& including_filepath) const {
   CHECK_(has_called_finalize_added_include_lines_ && "Must finalize includes");
@@ -1397,31 +1429,7 @@ vector<string> IncludePicker::GetCandidateHeadersForFilepathIncludedFrom(
     }
   }
 
-  // Now convert each MappedInclude to a quoted include.  Each contains
-  // a default quoted include but sometimes we can do better.
-  // For each path, the iwyu-preprocessor may have stored the quoted-include
-  // as written in including_filepath.  This is better to use than
-  // ConvertToQuotedInclude because it avoids trouble when the same
-  // file is accessible via different include search-paths, or is
-  // accessed via a symlink.
-  // We also try to recalculate any quoted includes using absolute paths,
-  // because they can likely be converted to an include relative to the
-  // current file.
-  vector<string> retval;
-  for (MappedInclude& mapped_include : mapped_includes) {
-    const string& quoted_include_as_written =
-      MaybeGetIncludeNameAsWritten(including_filepath, mapped_include.path);
-    if (!quoted_include_as_written.empty()) {
-      retval.push_back(quoted_include_as_written);
-    } else if (mapped_include.HasAbsoluteQuotedInclude() &&
-               !mapped_include.path.empty()) {
-      retval.push_back(ConvertToQuotedInclude(mapped_include.path,
-                                              including_path));
-    } else {
-      retval.push_back(mapped_include.quoted_include);
-    }
-  }
-  return retval;
+  return BestQuotedIncludesForIncluder(mapped_includes, including_filepath);
 }
 
 bool IncludePicker::HasMapping(const string& map_from_filepath,
diff --git a/iwyu_include_picker.h b/iwyu_include_picker.h
index 75b135e..69b6094 100644
--- a/iwyu_include_picker.h
+++ b/iwyu_include_picker.h
@@ -136,7 +136,14 @@ class IncludePicker {
   // important (which is why we return a vector, not a set): all else
   // being equal, the first element of the vector is the "best" (or
   // most standard) header for the symbol.
-  vector<string> GetCandidateHeadersForSymbol(const string& symbol) const;
+  vector<MappedInclude> GetCandidateHeadersForSymbol(
+      const string& symbol) const;
+
+  // As above, but given a specific including header it is possible to convert
+  // mapped includes to quoted include strings (because we can for example know
+  // the correct relative path for ""-style includes).
+  vector<string> GetCandidateHeadersForSymbolUsedFrom(
+      const string& symbol, const string& including_filepath) const;
 
   // Returns the set of all public header files that a given header
   // file -- specified as a full path -- would map to, as a set of
@@ -234,6 +241,11 @@ class IncludePicker {
   string MaybeGetIncludeNameAsWritten(const string& includer_filepath,
                                       const string& includee_filepath) const;
 
+  // Given a collection of MappedIncludes, and a path that might include them,
+  // choose the best quoted include form for each MappedInclude.
+  vector<string> BestQuotedIncludesForIncluder(
+      const vector<MappedInclude>&, const string& including_filepath) const;
+
   // From symbols to includes.
   IncludeMap symbol_include_map_;
 
diff --git a/iwyu_output.cc b/iwyu_output.cc
index 3337df3..a4c578e 100644
--- a/iwyu_output.cc
+++ b/iwyu_output.cc
@@ -295,11 +295,13 @@ void OneUse::SetPublicHeaders() {
   // who we map to.
   CHECK_(suggested_header_.empty() && "Should not need a public header here");
   const IncludePicker& picker = GlobalIncludePicker();   // short alias
+  const string use_path = GetFilePath(use_loc_);
   // If the symbol has a special mapping, use it, otherwise map its file.
-  public_headers_ = picker.GetCandidateHeadersForSymbol(symbol_name_);
+  public_headers_ = picker.GetCandidateHeadersForSymbolUsedFrom(
+      symbol_name_, use_path);
   if (public_headers_.empty())
     public_headers_ = picker.GetCandidateHeadersForFilepathIncludedFrom(
-        decl_filepath_, GetFilePath(use_loc_));
+        decl_filepath_, use_path);
   if (public_headers_.empty())
     public_headers_.push_back(ConvertToQuotedInclude(decl_filepath_));
 }
diff --git a/run_iwyu_tests.py b/run_iwyu_tests.py
index 5ee8824..24c8f02 100755
--- a/run_iwyu_tests.py
+++ b/run_iwyu_tests.py
@@ -88,6 +88,8 @@ class OneIwyuTest(unittest.TestCase):
       'prefix_header_includes_remove.cc': ['--prefix_header_includes=remove'],
       'prefix_header_operator_new.cc': ['--prefix_header_includes=remove'],
       'quoted_includes_first.cc': ['--pch_in_code', '--quoted_includes_first'],
+      'relative_exported_mapped_include.cc':
+          [self.MappingFile('relative_exported_mapped_include.imp')],
       'cxx17ns.cc': ['--cxx17ns'],
     }
     prefix_headers = [self.Include('prefix_header_includes-d1.h'),
@@ -184,6 +186,7 @@ class OneIwyuTest(unittest.TestCase):
       'range_for.cc': ['.'],
       're_fwd_decl.cc': ['.'],
       'redecls.cc': ['.'],
+      'relative_exported_mapped_include.cc': ['tests/cxx/subdir'],
       'remove_fwd_decl_when_including.cc': ['.'],
       'self_include.cc': ['.'],
       'sizeof_reference.cc': ['.'],
diff --git a/tests/cxx/relative_exported_mapped_include-d1.h b/tests/cxx/relative_exported_mapped_include-d1.h
new file mode 100644
index 0000000..2ec3d97
--- /dev/null
+++ b/tests/cxx/relative_exported_mapped_include-d1.h
@@ -0,0 +1,15 @@
+//===--- relative_exported_mapped_include-d1.h - test input file for iwyu -===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef INCLUDE_WHAT_YOU_USE_TESTS_CXX_RELATIVE_EXPORTED_MAPPED_INCLUDE_D1_H_
+#define INCLUDE_WHAT_YOU_USE_TESTS_CXX_RELATIVE_EXPORTED_MAPPED_INCLUDE_D1_H_
+
+#include <relative_exported_mapped_include-i1.h> // IWYU pragma: export
+
+#endif // INCLUDE_WHAT_YOU_USE_TESTS_CXX_RELATIVE_EXPORTED_MAPPED_INCLUDE_D1_H_
diff --git a/tests/cxx/relative_exported_mapped_include.cc b/tests/cxx/relative_exported_mapped_include.cc
new file mode 100644
index 0000000..b11428e
--- /dev/null
+++ b/tests/cxx/relative_exported_mapped_include.cc
@@ -0,0 +1,22 @@
+//===--- relative_exported_mapped_include.cc - test input file for iwyu ---===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// Ensure that when an include is added which is the public mapping of a
+// symbol, that header can be added as a relative include rather than using a
+// full path.
+
+#include "relative_exported_mapped_include-d1.h"
+
+MappedToExportedHeader x;
+
+/**** IWYU_SUMMARY
+
+(tests/cxx/relative_exported_mapped_include.cc has correct #includes/fwd-decls)
+
+***** IWYU_SUMMARY */
diff --git a/tests/cxx/relative_exported_mapped_include.imp b/tests/cxx/relative_exported_mapped_include.imp
new file mode 100644
index 0000000..cacde17
--- /dev/null
+++ b/tests/cxx/relative_exported_mapped_include.imp
@@ -0,0 +1,3 @@
+[
+    { symbol: ["MappedToExportedHeader", "private", "<relative_exported_mapped_include-i1.h>", "public"] },
+]
diff --git a/tests/cxx/subdir/relative_exported_mapped_include-i1.h b/tests/cxx/subdir/relative_exported_mapped_include-i1.h
new file mode 100644
index 0000000..b2ed95c
--- /dev/null
+++ b/tests/cxx/subdir/relative_exported_mapped_include-i1.h
@@ -0,0 +1,15 @@
+//===--- relative_exported_mapped_include-i1.h - test input file for iwyu ===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef INCLUDE_WHAT_YOU_USE_TESTS_CXX_RELATIVE_EXPORTED_MAPPED_INCLUDE_I1_H_
+#define INCLUDE_WHAT_YOU_USE_TESTS_CXX_RELATIVE_EXPORTED_MAPPED_INCLUDE_I1_H_
+
+enum MappedToExportedHeader {};
+
+#endif // INCLUDE_WHAT_YOU_USE_TESTS_CXX_RELATIVE_EXPORTED_MAPPED_INCLUDE_I1_H_