Refactor part2

Author: aschwaighofer

include/swift/AST/Module.h

@@ -990,9 +990,7 @@ class SourceFile final : public FileUnit {
 
   void addImports(ArrayRef<ImportedModuleDesc> IM);
 
-  bool hasTestableImport(const ModuleDecl *module) const;
-
-  bool hasPrivateImport(AccessLevel accessLevel, const ValueDecl *ofDecl) const;
+  bool hasTestableOrPrivateImport(AccessLevel accessLevel, const ValueDecl *ofDecl) const;
 
   void clearLookupCache();
 

lib/AST/Decl.cpp

@@ -2455,12 +2455,8 @@ static AccessLevel getAdjustedFormalAccess(const ValueDecl *VD,
     // @testable/@_private import attributes.
     auto *useSF = dyn_cast<SourceFile>(useDC->getModuleScopeContext());
     if (!useSF) return access;
-    if (useSF->hasPrivateImport(access, VD))
+    if (useSF->hasTestableOrPrivateImport(access, VD))
       return getTestableOrPrivateImportsAccess(VD);
-    if ((access == AccessLevel::Internal || access == AccessLevel::Public) &&
-        useSF->hasTestableImport(VD->getModuleContext())) {
-      return getTestableOrPrivateImportsAccess(VD);
-    }
   }
 
   return access;
@@ -2677,15 +2673,15 @@ static bool checkAccess(const DeclContext *useDC, const ValueDecl *VD,
   case AccessLevel::Private:
     if (useDC != sourceDC) {
       auto *useSF = dyn_cast<SourceFile>(useDC->getModuleScopeContext());
-      if (useSF && useSF->hasPrivateImport(access, VD))
+      if (useSF && useSF->hasTestableOrPrivateImport(access, VD))
         return true;
     }
     return (useDC == sourceDC ||
       AccessScope::allowsPrivateAccess(useDC, sourceDC));
   case AccessLevel::FilePrivate:
     if (useDC->getModuleScopeContext() != sourceDC->getModuleScopeContext()) {
       auto *useSF = dyn_cast<SourceFile>(useDC->getModuleScopeContext());
-      if (useSF && useSF->hasPrivateImport(access, VD))
+      if (useSF && useSF->hasTestableOrPrivateImport(access, VD))
         return true;
       return false;
     }
@@ -2697,10 +2693,8 @@ static bool checkAccess(const DeclContext *useDC, const ValueDecl *VD,
       return true;
     auto *useSF = dyn_cast<SourceFile>(useFile);
     if (!useSF) return false;
-    if (useSF->hasTestableImport(sourceModule))
+    if (useSF->hasTestableOrPrivateImport(access, sourceModule))
       return true;
-    if (useSF->hasPrivateImport(access, VD))
-        return true;
     return false;
   }
   case AccessLevel::Public:
@@ -3638,7 +3632,8 @@ bool EnumDecl::isFormallyExhaustive(const DeclContext *useDC) const {
   // Testably imported enums are exhaustive, on the grounds that only the author
   // of the original library can import it testably.
   if (auto *useSF = dyn_cast<SourceFile>(useDC->getModuleScopeContext()))
-    if (useSF->hasTestableImport(containingModule))
+    if (useSF->hasTestableOrPrivateImport(AccessLevel::Internal,
+                                          containingModule))
       return true;
 
   // Otherwise, the enum is non-exhaustive.

lib/AST/Module.cpp

@@ -1388,30 +1388,22 @@ void SourceFile::addImports(ArrayRef<ImportedModuleDesc> IM) {
   Imports = newBuf;
 }
 
-bool SourceFile::hasTestableImport(const swift::ModuleDecl *module) const {
-  return std::any_of(
-      Imports.begin(), Imports.end(),
-      [module](ImportedModuleDesc desc) -> bool {
-        return desc.module.second == module &&
-               desc.importOptions.contains(ImportFlags::Testable);
-      });
-}
-
-bool SourceFile::hasPrivateImport(AccessLevel accessLevel,
-                                  const swift::ValueDecl *ofDecl) const {
+bool SourceFile::hasTestableOrPrivateImport(
+    AccessLevel accessLevel, const swift::ValueDecl *ofDecl) const {
   auto *module = ofDecl->getModuleContext();
   switch (accessLevel) {
   case AccessLevel::Internal:
   case AccessLevel::Public:
     // internal/public access only needs an import marked as @_private. The
     // filename does not need to match (and we don't serialize it for such
     // decls).
-    return std::any_of(Imports.begin(), Imports.end(),
-                       [module](ImportedModuleDesc desc) -> bool {
-                         return desc.module.second == module &&
-                                desc.importOptions.contains(
-                                    ImportFlags::PrivateImport);
-                       });
+    return std::any_of(
+        Imports.begin(), Imports.end(),
+        [module](ImportedModuleDesc desc) -> bool {
+          return desc.module.second == module &&
+                 (desc.importOptions.contains(ImportFlags::PrivateImport) ||
+                  desc.importOptions.contains(ImportFlags::Testable));
+        });
   case AccessLevel::Open:
     return true;
   case AccessLevel::FilePrivate:

lib/IDE/CodeCompletion.cpp

@@ -5943,10 +5943,13 @@ void CodeCompletionCallbacksImpl::doneParsing() {
       // module loading, for example, the module file is corrupted.
       if (!ModuleFilename.empty()) {
         auto &Ctx = TheModule->getASTContext();
-        CodeCompletionCache::Key K{ModuleFilename, TheModule->getName().str(),
-                                   AccessPath, Request.NeedLeadingDot,
-                                   SF.hasTestableImport(TheModule),
-                                   Ctx.LangOpts.CodeCompleteInitsInPostfixExpr};
+        CodeCompletionCache::Key K{
+            ModuleFilename,
+            TheModule->getName().str(),
+            AccessPath,
+            Request.NeedLeadingDot,
+            SF.hasTestableOrPrivateImport(AccessLevel::Internal, TheModule),
+            Ctx.LangOpts.CodeCompleteInitsInPostfixExpr};
 
         using PairType = llvm::DenseSet<swift::ide::CodeCompletionCache::Key,
             llvm::DenseMapInfo<CodeCompletionCache::Key>>::iterator;

lib/Sema/TypeCheckProtocol.cpp

@@ -1264,13 +1264,14 @@ bool WitnessChecker::checkWitnessAccess(AccessScope &requiredAccessScope,
     if (auto parentFile = dyn_cast<SourceFile>(DC->getModuleScopeContext())) {
       const ModuleDecl *witnessModule = witness->getModuleContext();
       if (parentFile->getParentModule() != witnessModule &&
-          parentFile->hasTestableImport(witnessModule) &&
+          parentFile->hasTestableOrPrivateImport(AccessLevel::Internal,
+                                                 witnessModule) &&
           witness->isAccessibleFrom(parentFile)) {
         actualScopeToCheck = parentFile;
       // Same with @_private(sourceFile:) import.
       } else if (parentFile->getParentModule() != witnessModule &&
-                 parentFile->hasPrivateImport(witness->getFormalAccess(),
-                                              witness) &&
+                 parentFile->hasTestableOrPrivateImport(
+                     witness->getFormalAccess(), witness) &&
                  witness->isAccessibleFrom(parentFile)) {
         actualScopeToCheck = parentFile;
       }