Merge pull request #4970 from jrose-apple/swift-3-ignore-access-when-you-find-nothing

tkremenek · web-flow · commit 485ccd6c6f41 · 2016-09-23T16:52:50.000-07:00
Add "IgnoreAccessControl" to UnqualifiedLookup, and use it for diagnostics
diff --git a/include/swift/AST/NameLookup.h b/include/swift/AST/NameLookup.h
@@ -68,7 +68,8 @@ class UnqualifiedLookup {
                     bool IsKnownPrivate = false,
                     SourceLoc Loc = SourceLoc(),
                     bool IsTypeLookup = false,
-                    bool AllowProtocolMembers = false);
+                    bool AllowProtocolMembers = false,
+                    bool IgnoreAccessControl = false);
 
   SmallVector<UnqualifiedLookupResult, 4> Results;
 
diff --git a/lib/AST/NameLookup.cpp b/lib/AST/NameLookup.cpp
@@ -399,7 +399,8 @@ UnqualifiedLookup::UnqualifiedLookup(DeclName Name, DeclContext *DC,
                                      LazyResolver *TypeResolver,
                                      bool IsKnownNonCascading,
                                      SourceLoc Loc, bool IsTypeLookup,
-                                     bool AllowProtocolMembers) {
+                                     bool AllowProtocolMembers,
+                                     bool IgnoreAccessControl) {
   Module &M = *DC->getParentModule();
   ASTContext &Ctx = M.getASTContext();
   const SourceManager &SM = Ctx.SourceMgr;
@@ -563,6 +564,8 @@ UnqualifiedLookup::UnqualifiedLookup(DeclName Name, DeclContext *DC,
           options |= NL_ProtocolMembers;
         if (IsTypeLookup)
           options |= NL_OnlyTypes;
+        if (IgnoreAccessControl)
+          options |= NL_IgnoreAccessibility;
 
         if (!ExtendedType)
           ExtendedType = ErrorType::get(Ctx);
diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp
@@ -425,10 +425,10 @@ resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE, DeclContext *DC) {
   SourceLoc Loc = UDRE->getLoc();
 
   // Perform standard value name lookup.
-  NameLookupOptions LookupOptions = defaultUnqualifiedLookupOptions;
+  NameLookupOptions lookupOptions = defaultUnqualifiedLookupOptions;
   if (isa<AbstractFunctionDecl>(DC))
-    LookupOptions |= NameLookupFlags::KnownPrivate;
-  auto Lookup = lookupUnqualified(DC, Name, Loc, LookupOptions);
+    lookupOptions |= NameLookupFlags::KnownPrivate;
+  auto Lookup = lookupUnqualified(DC, Name, Loc, lookupOptions);
 
   if (!Lookup) {
     // If we failed lookup of an operator, check to see it to see if it is
@@ -438,12 +438,36 @@ resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE, DeclContext *DC) {
       return new (Context) ErrorExpr(UDRE->getSourceRange());
     }
 
+    // Try ignoring access control.
+    NameLookupOptions relookupOptions = lookupOptions;
+    relookupOptions |= NameLookupFlags::KnownPrivate;
+    relookupOptions |= NameLookupFlags::IgnoreAccessibility;
+    LookupResult inaccessibleResults = lookupUnqualified(DC, Name, Loc,
+                                                         relookupOptions);
+    if (inaccessibleResults) {
+      // FIXME: What if the unviable candidates have different levels of access?
+      const ValueDecl *first = inaccessibleResults.front().Decl;
+      diagnose(Loc, diag::candidate_inaccessible,
+               Name, first->getFormalAccess());
+
+      // FIXME: If any of the candidates (usually just one) are in the same
+      // module we could offer a fix-it.
+      for (auto lookupResult : inaccessibleResults) {
+        diagnose(lookupResult.Decl, diag::decl_declared_here,
+                 lookupResult.Decl->getFullName());
+      }
+
+      // Don't try to recover here; we'll get more access-related diagnostics
+      // downstream if the type of the inaccessible decl is also inaccessible.
+      return new (Context) ErrorExpr(UDRE->getSourceRange());
+    }
+
     // TODO: Name will be a compound name if it was written explicitly as
     // one, but we should also try to propagate labels into this.
     DeclNameLoc nameLoc = UDRE->getNameLoc();
 
     performTypoCorrection(DC, UDRE->getRefKind(), Type(), Name, Loc,
-                          LookupOptions, Lookup);
+                          lookupOptions, Lookup);
 
     diagnose(Loc, diag::use_unresolved_identifier, Name, Name.isOperator())
       .highlight(UDRE->getSourceRange());
diff --git a/lib/Sema/TypeCheckNameLookup.cpp b/lib/Sema/TypeCheckNameLookup.cpp
@@ -199,11 +199,13 @@ LookupResult TypeChecker::lookupUnqualified(DeclContext *dc, DeclName name,
     }
   }
 
-  UnqualifiedLookup lookup(name, dc, this,
-                           options.contains(NameLookupFlags::KnownPrivate),
-                           loc,
-                           options.contains(NameLookupFlags::OnlyTypes),
-                           options.contains(NameLookupFlags::ProtocolMembers));
+  UnqualifiedLookup lookup(
+      name, dc, this,
+      options.contains(NameLookupFlags::KnownPrivate),
+      loc,
+      options.contains(NameLookupFlags::OnlyTypes),
+      options.contains(NameLookupFlags::ProtocolMembers),
+      options.contains(NameLookupFlags::IgnoreAccessibility));
 
   LookupResult result;
   bool considerProtocolMembers
diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp
@@ -745,7 +745,35 @@ static Type diagnoseUnknownType(TypeChecker &tc, DeclContext *dc,
       comp->setValue(nominal);
       return type;
     }
-    
+
+
+    // Try ignoring access control.
+    DeclContext *lookupDC = dc;
+    if (options.contains(TR_GenericSignature))
+      lookupDC = dc->getParent();
+
+    NameLookupOptions relookupOptions = lookupOptions;
+    relookupOptions |= NameLookupFlags::KnownPrivate;
+    relookupOptions |= NameLookupFlags::IgnoreAccessibility;
+    LookupResult inaccessibleResults =
+        tc.lookupUnqualified(lookupDC, comp->getIdentifier(), comp->getIdLoc(),
+                             relookupOptions);
+    if (inaccessibleResults) {
+      // FIXME: What if the unviable candidates have different levels of access?
+      auto first = cast<TypeDecl>(inaccessibleResults.front().Decl);
+      tc.diagnose(comp->getIdLoc(), diag::candidate_inaccessible,
+                  comp->getIdentifier(), first->getFormalAccess());
+
+      // FIXME: If any of the candidates (usually just one) are in the same
+      // module we could offer a fix-it.
+      for (auto lookupResult : inaccessibleResults)
+        tc.diagnose(lookupResult.Decl, diag::type_declared_here);
+
+      // Don't try to recover here; we'll get more access-related diagnostics
+      // downstream if we do.
+      return ErrorType::get(tc.Context);
+    }
+
     // Fallback.
     SourceLoc L = comp->getIdLoc();
     SourceRange R = SourceRange(comp->getIdLoc());
diff --git a/test/NameBinding/accessibility.swift b/test/NameBinding/accessibility.swift
@@ -66,7 +66,7 @@ class Sub : Base {
     // TESTABLE-NOT: :[[@LINE-3]]:{{[^:]+}}:
     // TESTABLE-NOT: :[[@LINE-3]]:{{[^:]+}}:
 
-    method() // expected-error {{use of unresolved identifier 'method'}}
+    method() // expected-error {{'method' is inaccessible due to 'internal' protection level}}
     self.method() // expected-error {{'method' is inaccessible due to 'internal' protection level}}
     super.method() // expected-error {{'method' is inaccessible due to 'internal' protection level}}
     // TESTABLE-NOT: :[[@LINE-3]]:{{[^:]+}}:
diff --git a/test/Sema/accessibility_private.swift b/test/Sema/accessibility_private.swift
@@ -15,7 +15,7 @@ class Container {
     _ = self.bar
     self.bar = 5
 
-    privateExtensionMethod() // FIXME expected-error {{use of unresolved identifier 'privateExtensionMethod'}}
+    privateExtensionMethod() // expected-error {{'privateExtensionMethod' is inaccessible due to 'private' protection level}}
     self.privateExtensionMethod() // expected-error {{'privateExtensionMethod' is inaccessible due to 'private' protection level}}
 
     _ = PrivateInner()
@@ -54,23 +54,23 @@ extension Container {
   private func privateExtensionMethod() {} // expected-note * {{declared here}}
 
   func extensionTest() {
-    foo() // FIXME expected-error {{use of unresolved identifier 'foo'}}
+    foo() // expected-error {{'foo' is inaccessible due to 'private' protection level}}
     self.foo() // expected-error {{'foo' is inaccessible due to 'private' protection level}}
 
-    _ = bar // FIXME expected-error {{use of unresolved identifier 'bar'}}
-    bar = 5 // FIXME expected-error {{use of unresolved identifier 'bar'}}
+    _ = bar // expected-error {{'bar' is inaccessible due to 'private' protection level}}
+    bar = 5 // expected-error {{'bar' is inaccessible due to 'private' protection level}}
     _ = self.bar // expected-error {{'bar' is inaccessible due to 'private' protection level}}
     self.bar = 5 // expected-error {{'bar' is inaccessible due to 'private' protection level}}
 
     privateExtensionMethod()
     self.privateExtensionMethod()
 
-    _ = PrivateInner() // FIXME expected-error {{use of unresolved identifier 'PrivateInner'}}
+    _ = PrivateInner() // expected-error {{'PrivateInner' is inaccessible due to 'private' protection level}}
     _ = Container.PrivateInner() // expected-error {{'PrivateInner' is inaccessible due to 'private' protection level}}
   }
 
   // FIXME: Why do these errors happen twice?
-  var extensionInner: PrivateInner? { return nil } // FIXME expected-error 2 {{use of undeclared type 'PrivateInner'}}
+  var extensionInner: PrivateInner? { return nil } // expected-error 2 {{'PrivateInner' is inaccessible due to 'private' protection level}}
   var extensionInnerQualified: Container.PrivateInner? { return nil } // expected-error 2 {{'PrivateInner' is inaccessible due to 'private' protection level}}
 }
 
@@ -81,33 +81,35 @@ extension Container.Inner {
     obj.bar = 5 // expected-error {{'bar' is inaccessible due to 'private' protection level}}
     obj.privateExtensionMethod() // expected-error {{'privateExtensionMethod' is inaccessible due to 'private' protection level}}
 
-    _ = PrivateInner() // FIXME expected-error {{use of unresolved identifier 'PrivateInner'}}
+    // FIXME: Unqualified lookup won't look into Container from here.
+    _ = PrivateInner() // expected-error {{use of unresolved identifier 'PrivateInner'}}
     _ = Container.PrivateInner() // expected-error {{'PrivateInner' is inaccessible due to 'private' protection level}}
   }
 
   // FIXME: Why do these errors happen twice?
-  var inner: PrivateInner? { return nil } // FIXME expected-error 2 {{use of undeclared type 'PrivateInner'}}
+  // FIXME: Unqualified lookup won't look into Container from here.
+  var inner: PrivateInner? { return nil } // expected-error 2 {{use of undeclared type 'PrivateInner'}}
   var innerQualified: Container.PrivateInner? { return nil } // expected-error 2 {{'PrivateInner' is inaccessible due to 'private' protection level}}
 }
 
 class Sub : Container {
   func subTest() {
-    foo() // FIXME expected-error {{use of unresolved identifier 'foo'}}
+    foo() // expected-error {{'foo' is inaccessible due to 'private' protection level}}
     self.foo() // expected-error {{'foo' is inaccessible due to 'private' protection level}}
 
-    _ = bar // FIXME expected-error {{use of unresolved identifier 'bar'}}
-    bar = 5 // FIXME expected-error {{use of unresolved identifier 'bar'}}
+    _ = bar // expected-error {{'bar' is inaccessible due to 'private' protection level}}
+    bar = 5 // expected-error {{'bar' is inaccessible due to 'private' protection level}}
     _ = self.bar // expected-error {{'bar' is inaccessible due to 'private' protection level}}
     self.bar = 5 // expected-error {{'bar' is inaccessible due to 'private' protection level}}
 
-    privateExtensionMethod() // FIXME expected-error {{use of unresolved identifier 'privateExtensionMethod'}}
+    privateExtensionMethod() // expected-error {{'privateExtensionMethod' is inaccessible due to 'private' protection level}}
     self.privateExtensionMethod() // expected-error {{'privateExtensionMethod' is inaccessible due to 'private' protection level}}
 
-    _ = PrivateInner() // FIXME expected-error {{use of unresolved identifier 'PrivateInner'}}
+    _ = PrivateInner() // expected-error {{'PrivateInner' is inaccessible due to 'private' protection level}}
     _ = Container.PrivateInner() // expected-error {{'PrivateInner' is inaccessible due to 'private' protection level}}
   }
 
-  var subInner: PrivateInner? // FIXME expected-error {{use of undeclared type 'PrivateInner'}}
+  var subInner: PrivateInner? // expected-error {{'PrivateInner' is inaccessible due to 'private' protection level}}
   var subInnerQualified: Container.PrivateInner? // expected-error {{'PrivateInner' is inaccessible due to 'private' protection level}}
 }
 
@@ -154,9 +156,9 @@ extension Container {
 }
 extension Container {
   func test() {
-    let a: ExtensionConflictingType? = nil // FIXME expected-error {{use of undeclared type 'ExtensionConflictingType'}}
+    let a: ExtensionConflictingType? = nil // expected-error {{'ExtensionConflictingType' is inaccessible due to 'private' protection level}}
     let b: Container.ExtensionConflictingType? = nil // expected-error {{'ExtensionConflictingType' is inaccessible due to 'private' protection level}}
-    _ = ExtensionConflictingType() // FIXME expected-error {{use of unresolved identifier 'ExtensionConflictingType'}}
+    _ = ExtensionConflictingType() // expected-error {{'ExtensionConflictingType' is inaccessible due to 'private' protection level}}
     _ = Container.ExtensionConflictingType() // expected-error {{'ExtensionConflictingType' is inaccessible due to 'private' protection level}}
   }
 }
