Merge pull request swiftlang#4925 from jrose-apple/ignore-access-when…

…-you-find-nothing Add "IgnoreAccessControl" to UnqualifiedLookup. rdar://problem/27663403
athas · Sep 23, 2016 · 3170c65 · 3170c65
2 parents 1cffaed + b76dea8
commit 3170c65
Show file tree

Hide file tree

Showing 8 changed files with 93 additions and 31 deletions.
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
@@ -443,7 +443,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;
@@ -630,6 +631,8 @@ UnqualifiedLookup::UnqualifiedLookup(DeclName Name, DeclContext *DC,
           options |= NL_ProtocolMembers;
         if (IsTypeLookup)
           options |= NL_OnlyTypes;
+        if (IgnoreAccessControl)
+          options |= NL_IgnoreAccessibility;
 
         SmallVector<ValueDecl *, 4> lookup;
         dc->lookupQualified(lookupType, Name, options, TypeResolver, lookup);
@@ -815,6 +818,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
@@ -183,11 +183,13 @@ namespace {
 LookupResult TypeChecker::lookupUnqualified(DeclContext *dc, DeclName name,
                                             SourceLoc loc,
                                             NameLookupOptions options) {
-  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;
   LookupResultBuilder builder(*this, result, dc, options,

diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp
@@ -747,7 +747,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
@@ -75,7 +75,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}}
   }
 }

diff --git a/test/attr/accessibility_multifile.swift b/test/attr/accessibility_multifile.swift
@@ -8,12 +8,12 @@ private protocol P  {
 }
 public class C : P {
   public init() {}
-  fileprivate func privMethod() {}
+  fileprivate func privMethod() {} // expected-note {{declared here}}
 }
 
 // BEGIN file2.swift
 extension C {
   public func someFunc() {
-    privMethod() // expected-error {{use of unresolved identifier 'privMethod'}}
+    privMethod() // expected-error {{'privMethod' is inaccessible due to 'fileprivate' protection level}}
   }
 }