Warn about module name shadowing in interfaces

There is a known issue with module interfaces where a type with the same name as a module will disrupt references to types in that module. Fully fixing it will require a new language feature (SR-898) which is not yet available. In the meantime, module interfaces support a workaround flag (“-Xfrontend -module-interface-preserve-types-as-written”) which prints an alternate form that usually works. However, you have to know to add this flag, and it’s not obvious because nothing breaks until a compiler tries to consume the affected module interface (or sometimes even one of its clients).

This commit emits a warning during module interface emission whenever the module interface either imports a type with the same name as the module being built, or declares a type with the same name as a visible module. This lets the user know that the type may cause problems and they might need to implement a workaround.

lib/Frontend/ModuleInterfaceSupport.cpp

@@ -18,6 +18,7 @@
 #include "swift/AST/ExistentialLayout.h"
 #include "swift/AST/FileSystem.h"
 #include "swift/AST/Module.h"
+#include "swift/AST/ModuleNameLookup.h"
 #include "swift/AST/ProtocolConformance.h"
 #include "swift/Basic/STLExtras.h"
 #include "swift/Frontend/Frontend.h"
@@ -78,6 +79,87 @@ llvm::Regex swift::getSwiftInterfaceCompilerVersionRegex() {
                      ": (.+)$", llvm::Regex::Newline);
 }
 
+// MARK: Module name shadowing warnings (SR-898)
+//
+// When swiftc emits a module interface, it qualifies most types with their
+// module name. This usually makes the interface less ambiguous, but if a type
+// exists with the same name as a module, then references to that module will
+// incorrectly look inside the type instead. This breakage is not obvious until
+// someone tries to load the module interface, and may sometimes only occur in
+// clients' module interfaces.
+//
+// Truly fixing this will require a new module-qualification syntax which
+// completely ignores shadowing. In lieu of that, we detect and warn about three
+// common examples which are relatively actionable:
+//
+// 1. An `import` statement written into the module interface will
+//    (transitively) import a type with the module interface's name.
+//
+// 2. The module interface declares a type with the same name as the module the
+//    interface is for.
+//
+// 3. The module interface declares a type with the same name as a module it has
+//     (transitively) imported without `@_implementationOnly`.
+//
+// We do not check for shadowing between imported module names and imported
+// declarations; this is both much rarer and much more difficult to solve.
+// We silence these warnings if you use the temporary workaround flag,
+// '-module-interface-preserve-types-as-written'.
+
+/// Emit a warning explaining that \p shadowingDecl will interfere with
+/// references to types in \p shadowedModule in the module interfaces of
+/// \p brokenModule and its clients.
+static void
+diagnoseDeclShadowsModule(ModuleInterfaceOptions const &Opts,
+                          TypeDecl *shadowingDecl, ModuleDecl *shadowedModule,
+                          ModuleDecl *brokenModule) {
+  if (Opts.PreserveTypesAsWritten || shadowingDecl == shadowedModule)
+    return;
+
+  shadowingDecl->diagnose(
+      diag::warning_module_shadowing_may_break_module_interface,
+      shadowingDecl->getDescriptiveKind(),
+      FullyQualified<Type>(shadowingDecl->getDeclaredInterfaceType()),
+      shadowedModule, brokenModule);
+}
+
+/// Check whether importing \p importedModule will bring in any declarations
+/// that will shadow \p importingModule, and diagnose them if so.
+static void
+diagnoseIfModuleImportsShadowingDecl(ModuleInterfaceOptions const &Opts,
+                                     ModuleDecl *importedModule,
+                                     ModuleDecl *importingModule) {
+  using namespace namelookup;
+
+  SmallVector<ValueDecl *, 4> decls;
+  lookupInModule(importedModule, importingModule->getName(), decls,
+                 NLKind::UnqualifiedLookup, ResolutionKind::TypesOnly,
+                 importedModule,
+                 NL_UnqualifiedDefault | NL_IncludeUsableFromInline);
+  for (auto decl : decls)
+    diagnoseDeclShadowsModule(Opts, cast<TypeDecl>(decl), importingModule,
+                              importingModule);
+}
+
+/// Check whether \p D will shadow any modules imported by \p M, and diagnose
+/// them if so.
+static void diagnoseIfDeclShadowsKnownModule(ModuleInterfaceOptions const &Opts,
+                                             Decl *D, ModuleDecl *M) {
+  ASTContext &ctx = M->getASTContext();
+
+  // We only care about types (and modules, which are a subclass of TypeDecl);
+  // when the grammar expects a type name, it ignores non-types during lookup.
+  TypeDecl *TD = dyn_cast<TypeDecl>(D);
+  if (!TD)
+    return;
+
+  ModuleDecl *shadowedModule = ctx.getLoadedModule(TD->getName());
+  if (!shadowedModule || M->isImportedImplementationOnly(shadowedModule))
+    return;
+
+  diagnoseDeclShadowsModule(Opts, TD, shadowedModule, M);
+}
+
 // MARK: Import statements
 
 /// Diagnose any scoped imports in \p imports, i.e. those with a non-empty
@@ -175,6 +257,8 @@ static void printImports(raw_ostream &out,
     }
 
     out << "\n";
+
+    diagnoseIfModuleImportsShadowingDecl(Opts, importedModule, M);
   }
 }
 
@@ -596,6 +680,8 @@ bool swift::emitSwiftInterface(raw_ostream &out,
 
     D->print(out, printOptions);
     out << "\n";
+
+    diagnoseIfDeclShadowsKnownModule(Opts, const_cast<Decl *>(D), M);
   }
 
   // Print dummy extensions for any protocols that were indirectly conformed to.