[Sema] Diagnose use of ambiguous property wrappers (swiftlang#33688)

Author: theblixguy

include/swift/AST/DiagnosticsCommon.def

@@ -188,5 +188,19 @@ ERROR(scanner_find_cycle, none,
 ERROR(scanner_arguments_invalid, none,
       "dependencies scanner cannot be configured with arguments: '%0'", (StringRef))
 
+//------------------------------------------------------------------------------
+// MARK: custom attribute diagnostics
+//------------------------------------------------------------------------------
+
+ERROR(ambiguous_custom_attribute_ref,none,
+      "ambiguous use of attribute %0", (Identifier))
+NOTE(ambiguous_custom_attribute_ref_fix,none,
+     "use '%0.' to reference the attribute %1 in module %2",
+     (StringRef, Identifier, Identifier))
+NOTE(found_attribute_candidate,none,
+     "found this attribute", ())
+ERROR(unknown_attribute,none,
+      "unknown attribute '%0'", (StringRef))
+
 #define UNDEFINE_DIAGNOSTIC_MACROS
 #include "DefineDiagnosticMacros.h"

include/swift/AST/DiagnosticsParse.def

@@ -1324,8 +1324,6 @@ ERROR(replace_equal_with_colon_for_value,none,
       "'=' has been replaced with ':' in attribute arguments", ())
 ERROR(expected_attribute_name,none,
       "expected an attribute name", ())
-ERROR(unknown_attribute,none,
-      "unknown attribute '%0'", (StringRef))
 ERROR(unexpected_lparen_in_attribute,none,
       "unexpected '(' in attribute '%0'", (StringRef))
 ERROR(duplicate_attribute,none,

lib/AST/NameLookup.cpp

@@ -2434,6 +2434,53 @@ CustomAttrNominalRequest::evaluate(Evaluator &evaluator,
     }
   }
 
+  // If we have more than one attribute declaration, we have an ambiguity.
+  // So, emit an ambiguity diagnostic.
+  if (auto typeRepr = attr->getTypeRepr()) {
+    if (nominals.size() > 1) {
+      SmallVector<NominalTypeDecl *, 4> ambiguousCandidates;
+      // Filter out declarations that cannot be attributes.
+      for (auto decl : nominals) {
+        if (isa<ProtocolDecl>(decl)) {
+          continue;
+        }
+        ambiguousCandidates.push_back(decl);
+      }
+      if (ambiguousCandidates.size() > 1) {
+        auto attrName = nominals.front()->getName();
+        ctx.Diags.diagnose(typeRepr->getLoc(),
+                           diag::ambiguous_custom_attribute_ref, attrName);
+        for (auto candidate : ambiguousCandidates) {
+          ctx.Diags.diagnose(candidate->getLoc(),
+                             diag::found_attribute_candidate);
+          // If the candidate is a top-level attribute, let's suggest
+          // adding module name to resolve the ambiguity.
+          if (candidate->getDeclContext()->isModuleScopeContext()) {
+            auto moduleName = candidate->getParentModule()->getName();
+            ctx.Diags
+                .diagnose(typeRepr->getLoc(),
+                          diag::ambiguous_custom_attribute_ref_fix,
+                          moduleName.str(), attrName, moduleName)
+                .fixItInsert(typeRepr->getLoc(), moduleName.str().str() + ".");
+          }
+        }
+        return nullptr;
+      }
+    }
+  }
+
+  // There is no nominal type with this name, so complain about this being
+  // an unknown attribute.
+  std::string typeName;
+  if (auto typeRepr = attr->getTypeRepr()) {
+    llvm::raw_string_ostream out(typeName);
+    typeRepr->print(out);
+  } else {
+    typeName = attr->getType().getString();
+  }
+
+  ctx.Diags.diagnose(attr->getLocation(), diag::unknown_attribute, typeName);
+
   return nullptr;
 }
 

lib/Sema/TypeCheckAttr.cpp

@@ -2901,18 +2901,7 @@ void AttributeChecker::visitCustomAttr(CustomAttr *attr) {
   auto nominal = evaluateOrDefault(
     Ctx.evaluator, CustomAttrNominalRequest{attr, dc}, nullptr);
 
-  // If there is no nominal type with this name, complain about this being
-  // an unknown attribute.
   if (!nominal) {
-    std::string typeName;
-    if (auto typeRepr = attr->getTypeRepr()) {
-      llvm::raw_string_ostream out(typeName);
-      typeRepr->print(out);
-    } else {
-      typeName = attr->getType().getString();
-    }
-
-    diagnose(attr->getLocation(), diag::unknown_attribute, typeName);
     attr->setInvalid();
     return;
   }

test/NameLookup/Inputs/custom_attrs_A.swift

@@ -0,0 +1,13 @@
+@propertyWrapper
+public struct Wrapper<Value> {
+  public var wrappedValue: Value
+
+  public init(wrappedValue: Value) {
+    self.wrappedValue = wrappedValue
+  }
+}
+
+@_functionBuilder
+public struct Builder {
+  static func buildBlock<T>(_ component: T) -> T { component }
+}

test/NameLookup/Inputs/custom_attrs_B.swift

@@ -0,0 +1,13 @@
+@propertyWrapper
+public struct Wrapper<Value> {
+  public var wrappedValue: Value
+
+  public init(wrappedValue: Value) {
+    self.wrappedValue = wrappedValue
+  }
+}
+
+@_functionBuilder
+public struct Builder {
+  static func buildBlock<T>(_ component: T) -> T { component }
+}

test/NameLookup/custom_attrs_ambiguous.swift

@@ -0,0 +1,21 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -emit-module -I %t -o %t %S/Inputs/custom_attrs_A.swift
+// RUN: %target-swift-frontend -emit-module -I %t -o %t %S/Inputs/custom_attrs_B.swift
+// RUN: %target-swift-frontend -typecheck -verify -verify-ignore-unknown -I %t %s
+import custom_attrs_A
+import custom_attrs_B
+
+// SR-13470
+
+struct Test {
+  @Wrapper var x: Int = 17
+  // expected-error@-1 {{ambiguous use of attribute 'Wrapper'}}
+  // expected-note@-2 {{use 'custom_attrs_A.' to reference the attribute 'Wrapper' in module 'custom_attrs_A'}} {{4-4=custom_attrs_A.}}
+  // expected-note@-3 {{use 'custom_attrs_B.' to reference the attribute 'Wrapper' in module 'custom_attrs_B'}} {{4-4=custom_attrs_B.}}
+
+  init(@Builder closure: () -> Int) {}
+  // expected-error@-1 {{ambiguous use of attribute 'Builder'}}
+  // expected-note@-2 {{use 'custom_attrs_A.' to reference the attribute 'Builder' in module 'custom_attrs_A'}} {{9-9=custom_attrs_A.}}
+  // expected-note@-3 {{use 'custom_attrs_B.' to reference the attribute 'Builder' in module 'custom_attrs_B'}} {{9-9=custom_attrs_B.}}
+}
+