Merge pull request swiftlang#32220 from gregomni/unintended_generic_param

[QoI] Detect unintended generic params and provide a note and fixit.

Author: gregomni

include/swift/AST/DiagnosticsSema.def

@@ -94,6 +94,9 @@ ERROR(could_not_find_enum_case,none,
 
 NOTE(did_you_mean_raw_type,none,
      "did you mean to specify a raw type on the enum declaration?", ())
+     
+NOTE(did_you_mean_generic_param_as_conformance,none,
+     "did you mean to declare %0 as a protocol conformance for %1?", (DeclName, Type))
 
 NOTE(any_as_anyobject_fixit, none,
      "cast 'Any' to 'AnyObject' or use 'as!' to force downcast to a more specific type to access members", ())

lib/Sema/CSDiagnostics.cpp

@@ -3335,6 +3335,30 @@ bool MissingMemberFailure::diagnoseInLiteralCollectionContext() const {
   return false;
 }
 
+bool UnintendedExtraGenericParamMemberFailure::diagnoseAsError() {
+  MissingMemberFailure::diagnoseAsError();
+
+  auto baseType = resolveType(getBaseType())->getWithoutSpecifierType();
+  auto archetype = baseType->getMetatypeInstanceType()->castTo<ArchetypeType>();
+  auto genericTy =
+      archetype->mapTypeOutOfContext()->castTo<GenericTypeParamType>();
+  SourceLoc loc = genericTy->getDecl()->getSourceRange().End;
+  StringRef replacement;
+
+  if (archetype->getConformsTo().size()) {
+    loc = loc.getAdvancedLoc(
+        archetype->getConformsTo().back()->getName().getLength());
+    replacement = " &";
+  } else {
+    loc = loc.getAdvancedLoc(archetype->getName().getLength());
+    replacement = ":";
+  }
+  emitDiagnosticAt(loc, diag::did_you_mean_generic_param_as_conformance,
+                   ParamName, archetype)
+      .fixItReplaceChars(loc, loc.getAdvancedLoc(1), replacement);
+  return true;
+}
+
 bool InvalidMemberRefOnExistential::diagnoseAsError() {
   auto anchor = getRawAnchor();
 

lib/Sema/CSDiagnostics.h

@@ -1039,7 +1039,7 @@ class InvalidMemberRefFailure : public FailureDiagnostic {
 ///   let _: Int = s.foo(1, 2) // expected type is `(Int, Int) -> Int`
 /// }
 /// ```
-class MissingMemberFailure final : public InvalidMemberRefFailure {
+class MissingMemberFailure : public InvalidMemberRefFailure {
 public:
   MissingMemberFailure(const Solution &solution, Type baseType,
                        DeclNameRef memberName, ConstraintLocator *locator)
@@ -1068,6 +1068,22 @@ class MissingMemberFailure final : public InvalidMemberRefFailure {
                                           DeclNameRef memberName);
 };
 
+class UnintendedExtraGenericParamMemberFailure final
+    : public MissingMemberFailure {
+  Identifier ParamName;
+
+public:
+  UnintendedExtraGenericParamMemberFailure(const Solution &solution,
+                                           Type baseType,
+                                           DeclNameRef memberName,
+                                           Identifier paramName,
+                                           ConstraintLocator *locator)
+      : MissingMemberFailure(solution, baseType, memberName, locator),
+        ParamName(paramName) {}
+
+  bool diagnoseAsError() override;
+};
+
 /// Diagnose cases where a member only accessible on generic constraints
 /// requiring conformance to a protocol is used on a value of the
 /// existential protocol type e.g.

lib/Sema/CSFix.cpp

@@ -531,6 +531,23 @@ DefineMemberBasedOnUse::create(ConstraintSystem &cs, Type baseType,
       DefineMemberBasedOnUse(cs, baseType, member, alreadyDiagnosed, locator);
 }
 
+bool DefineMemberBasedOnUnintendedGenericParam::diagnose(
+    const Solution &solution, bool asNote) const {
+  UnintendedExtraGenericParamMemberFailure failure(solution, BaseType, Name,
+                                                   ParamName, getLocator());
+  return failure.diagnose(asNote);
+}
+
+DefineMemberBasedOnUnintendedGenericParam *
+DefineMemberBasedOnUnintendedGenericParam::create(ConstraintSystem &cs,
+                                                  Type baseType,
+                                                  DeclNameRef member,
+                                                  Identifier paramName,
+                                                  ConstraintLocator *locator) {
+  return new (cs.getAllocator()) DefineMemberBasedOnUnintendedGenericParam(
+      cs, baseType, member, paramName, locator);
+}
+
 AllowMemberRefOnExistential *
 AllowMemberRefOnExistential::create(ConstraintSystem &cs, Type baseType,
                                     ValueDecl *member, DeclNameRef memberName,

lib/Sema/CSFix.h

@@ -884,6 +884,33 @@ class DefineMemberBasedOnUse final : public ConstraintFix {
   }
 };
 
+class DefineMemberBasedOnUnintendedGenericParam final : public ConstraintFix {
+  Type BaseType;
+  DeclNameRef Name;
+  Identifier ParamName;
+
+  DefineMemberBasedOnUnintendedGenericParam(ConstraintSystem &cs, Type baseType,
+                                            DeclNameRef member,
+                                            Identifier paramName,
+                                            ConstraintLocator *locator)
+      : ConstraintFix(cs, FixKind::DefineMemberBasedOnUse, locator),
+        BaseType(baseType), Name(member), ParamName(paramName) {}
+
+public:
+  std::string getName() const override {
+    llvm::SmallVector<char, 16> scratch;
+    auto memberName = Name.getString(scratch);
+    return "allow access to invalid member '" + memberName.str() +
+           "' on archetype presumed intended to conform to protocol";
+  }
+
+  bool diagnose(const Solution &solution, bool asNote = false) const override;
+
+  static DefineMemberBasedOnUnintendedGenericParam *
+  create(ConstraintSystem &cs, Type baseType, DeclNameRef member,
+         Identifier paramName, ConstraintLocator *locator);
+};
+
 class AllowInvalidMemberRef : public ConstraintFix {
   Type BaseType;
   ValueDecl *Member;

lib/Sema/CSSimplify.cpp

@@ -15,13 +15,14 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "CSFix.h"
 #include "CSDiagnostics.h"
+#include "CSFix.h"
 #include "ConstraintSystem.h"
 #include "swift/AST/ExistentialLayout.h"
 #include "swift/AST/GenericEnvironment.h"
 #include "swift/AST/GenericSignature.h"
 #include "swift/AST/Initializer.h"
+#include "swift/AST/NameLookupRequests.h"
 #include "swift/AST/ParameterList.h"
 #include "swift/AST/PropertyWrappers.h"
 #include "swift/AST/ProtocolConformance.h"
@@ -6932,6 +6933,54 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyMemberConstraint(
       return recordFix(fix) ? SolutionKind::Error : SolutionKind::Solved;
     }
 
+    // If base is an archetype or metatype of archetype, check for an unintended
+    // extra generic parameter.
+    if (auto archetype =
+            baseTy->getMetatypeInstanceType()->getAs<ArchetypeType>()) {
+      if (auto genericTy =
+              archetype->mapTypeOutOfContext()->getAs<GenericTypeParamType>()) {
+        for (auto param :
+             archetype->getGenericEnvironment()->getGenericParams()) {
+          // Find a param at the same depth and one index past the type we're
+          // dealing with
+          if (param->getDepth() != genericTy->getDepth() ||
+              param->getIndex() != genericTy->getIndex() + 1)
+            continue;
+          auto paramDecl = param->getDecl();
+          if (!paramDecl)
+            continue;
+
+          auto descriptor = UnqualifiedLookupDescriptor(
+              DeclNameRef(param->getName()),
+              paramDecl->getDeclContext()->getParentForLookup(),
+              paramDecl->getStartLoc(),
+              UnqualifiedLookupFlags::KnownPrivate |
+                  UnqualifiedLookupFlags::TypeLookup);
+          auto lookup = evaluateOrDefault(
+              Context.evaluator, UnqualifiedLookupRequest{descriptor}, {});
+          for (auto &result : lookup) {
+            if (auto proto =
+                    dyn_cast_or_null<ProtocolDecl>(result.getValueDecl())) {
+              auto result =
+                  baseTy->is<MetatypeType>()
+                      ? solveWithNewBaseOrName(ExistentialMetatypeType::get(
+                                                   proto->getDeclaredType()),
+                                               member)
+                      : solveWithNewBaseOrName(proto->getDeclaredType(),
+                                               member);
+              if (result == SolutionKind::Solved)
+                return recordFix(
+                           DefineMemberBasedOnUnintendedGenericParam::create(
+                               *this, baseTy, member, param->getName(),
+                               locator))
+                           ? SolutionKind::Error
+                           : SolutionKind::Solved;
+            }
+          }
+        }
+      }
+    }
+
     if (auto *funcType = baseTy->getAs<FunctionType>()) {
       // We can't really suggest anything useful unless
       // function takes no arguments, otherwise it

test/Sema/diag_unintended_generic_param.swift

@@ -0,0 +1,24 @@
+// RUN: %target-typecheck-verify-swift
+
+protocol Proto {
+  init?(string: String)
+  func foo()
+}
+
+struct TypedFoo<T: Hashable, Proto> {
+  // expected-note@-1 {{did you mean to declare 'Proto' as a protocol conformance for 'T'?}} {{28-29= &}}
+  func foo(value: String) {
+    if let str = T.init(string:value) { // expected-error {{type 'T' has no member 'init'}}
+      print(str)
+    }
+  }
+}
+
+func bar<T, Proto>(arg: T) { // expected-error {{generic parameter 'Proto' is not used in function signature}}
+  // expected-note@-1 {{did you mean to declare 'Proto' as a protocol conformance for 'T'?}} {{11-12=:}}
+  arg.foo() // expected-error {{value of type 'T' has no member 'foo'}}
+}
+
+func baz<T: Hashable, Proto>(arg: T) { // expected-error {{generic parameter 'Proto' is not used in function signature}}
+  arg.ugh() // expected-error {{value of type 'T' has no member 'ugh'}}
+}