Merge pull request #29024 from xedin/diagnose-shadowing

[Diagnostics] Port name shadowing diagnostics

Author: xedin

include/swift/AST/Decl.h

@@ -335,7 +335,7 @@ class alignas(1 << DeclAlignInBits) Decl {
     IsStatic : 1
   );
 
-  SWIFT_INLINE_BITFIELD(VarDecl, AbstractStorageDecl, 1+1+1+1+1+1+1,
+  SWIFT_INLINE_BITFIELD(VarDecl, AbstractStorageDecl, 1+1+1+1+1+1+1+1,
     /// Encodes whether this is a 'let' binding.
     Introducer : 1,
 
@@ -358,7 +358,10 @@ class alignas(1 << DeclAlignInBits) Decl {
     IsLazyStorageProperty : 1,
 
     /// Whether this is the backing storage for a property wrapper.
-    IsPropertyWrapperBackingProperty : 1
+    IsPropertyWrapperBackingProperty : 1,
+
+    /// Whether this is a lazily top-level global variable from the main file.
+    IsTopLevelGlobal : 1
   );
 
   SWIFT_INLINE_BITFIELD(ParamDecl, VarDecl, 1+2+NumDefaultArgumentKindBits,
@@ -5084,6 +5087,10 @@ class VarDecl : public AbstractStorageDecl {
     Bits.VarDecl.IsLazyStorageProperty = IsLazyStorage;
   }
 
+  /// True if this is a top-level global variable from the main source file.
+  bool isTopLevelGlobal() const { return Bits.VarDecl.IsTopLevelGlobal; }
+  void setTopLevelGlobal(bool b) { Bits.VarDecl.IsTopLevelGlobal = b; }
+  
   /// Retrieve the custom attributes that attach property wrappers to this
   /// property. The returned list contains all of the attached property wrapper attributes in source order,
   /// which means the outermost wrapper attribute is provided first.

include/swift/AST/DiagnosticsSema.def

@@ -1153,14 +1153,12 @@ NOTE(candidate_expected_different_labels,none,
      "incorrect labels for candidate (have: '%0', expected: '%1')",
      (StringRef, StringRef))
 
+ERROR(member_shadows_function,none,
+      "use of %0 refers to %1 rather than %2 %3",
+      (DeclNameRef, DescriptiveDeclKind, DescriptiveDeclKind, DeclName))
 ERROR(member_shadows_global_function,none,
-      "use of %0 refers to %1 %2 rather than %3 %4 in %5 %6",
-      (DeclNameRef, DescriptiveDeclKind, DeclName, DescriptiveDeclKind,
-       DeclName, DescriptiveDeclKind, DeclName))
-ERROR(member_shadows_global_function_near_match,none,
-      "use of %0 nearly matches %3 %4 in %5 %6 rather than %1 %2",
-      (DeclNameRef, DescriptiveDeclKind, DeclName, DescriptiveDeclKind,
-       DeclName, DescriptiveDeclKind, DeclName))
+      "use of %0 refers to %1 rather than %2 %3 in module %4",
+      (DeclNameRef, DescriptiveDeclKind, DescriptiveDeclKind, DeclName, DeclName))
 
 ERROR(instance_member_use_on_type,none,
       "instance member %1 cannot be used on type %0; "

lib/AST/Decl.cpp

@@ -5312,6 +5312,7 @@ VarDecl::VarDecl(DeclKind kind, bool isStatic, VarDecl::Introducer introducer,
   Bits.VarDecl.IsLazyStorageProperty = false;
   Bits.VarDecl.HasNonPatternBindingInit = false;
   Bits.VarDecl.IsPropertyWrapperBackingProperty = false;
+  Bits.VarDecl.IsTopLevelGlobal = false;
 }
 
 Type VarDecl::getType() const {
@@ -5410,11 +5411,7 @@ bool VarDecl::isLazilyInitializedGlobal() const {
 
   // Top-level global variables in the main source file and in the REPL are not
   // lazily initialized.
-  auto sourceFileContext = dyn_cast<SourceFile>(getDeclContext());
-  if (!sourceFileContext)
-    return true;
-
-  return !sourceFileContext->isScriptMode();
+  return !isTopLevelGlobal();
 }
 
 SourceRange VarDecl::getSourceRange() const {

lib/Parse/ParseDecl.cpp

@@ -5840,6 +5840,7 @@ Parser::parseDeclVar(ParseDeclOptions Flags,
       VD->setStatic(StaticLoc.isValid());
       VD->getAttrs() = Attributes;
       setLocalDiscriminator(VD);
+      VD->setTopLevelGlobal(topLevelDecl);
 
       // Set original declaration in `@differentiable` attributes.
       setOriginalDeclarationForDifferentiableAttributes(Attributes, VD);

lib/Sema/CSDiag.cpp

@@ -201,10 +201,6 @@ class FailureDiagnosis :public ASTVisitor<FailureDiagnosis, /*exprresult*/bool>{
                                          ContextualTypePurpose CTP,
                                          Type suggestedType = Type());
 
-  bool diagnoseImplicitSelfErrors(Expr *fnExpr, Expr *argExpr,
-                                  CalleeCandidateInfo &CCI,
-                                  ArrayRef<Identifier> argLabels);
-
 private:
   /// Validate potential contextual type for type-checking one of the
   /// sub-expressions, usually correct/valid types are the ones which
@@ -963,223 +959,6 @@ decomposeArgType(Type argType, ArrayRef<Identifier> argLabels) {
   return result;
 }
 
-bool FailureDiagnosis::diagnoseImplicitSelfErrors(
-    Expr *fnExpr, Expr *argExpr, CalleeCandidateInfo &CCI,
-    ArrayRef<Identifier> argLabels) {
-  // If candidate list is empty it means that problem is somewhere else,
-  // since we need to have candidates which might be shadowing other funcs.
-  if (CCI.empty() || !CCI[0].getDecl())
-    return false;
-
-  auto &ctx = CS.getASTContext();
-  // Call expression is formed as 'foo.bar' where 'foo' might be an
-  // implicit "Self" reference, such use wouldn't provide good diagnostics
-  // for situations where instance members have equal names to functions in
-  // Swift Standard Library e.g. min/max.
-  auto UDE = dyn_cast<UnresolvedDotExpr>(fnExpr);
-  if (!UDE)
-    return false;
-
-  auto baseExpr = dyn_cast<DeclRefExpr>(UDE->getBase());
-  if (!baseExpr)
-    return false;
-
-  auto baseDecl = baseExpr->getDecl();
-  if (!baseExpr->isImplicit() || baseDecl->getFullName() != ctx.Id_self)
-    return false;
-
-  // Our base expression is an implicit 'self.' reference e.g.
-  //
-  // extension Sequence {
-  //   func test() -> Int {
-  //     return max(1, 2)
-  //   }
-  // }
-  //
-  // In this example the Sequence class already has two methods named 'max'
-  // none of which accept two arguments, but there is a function in
-  // Swift Standard Library called 'max' which does accept two arguments,
-  // so user might have called that by mistake without realizing that
-  // compiler would add implicit 'self.' prefix to the call of 'max'.
-  auto argType = CS.getType(argExpr);
-  // If argument wasn't properly type-checked, let's retry without changing AST.
-  if (!argType || argType->hasUnresolvedType() || argType->hasTypeVariable() ||
-      argType->hasTypeParameter()) {
-    auto *argTuple = dyn_cast<TupleExpr>(argExpr);
-    if (!argTuple) {
-      // Bail out if we don't have a well-formed argument list.
-      return false;
-    }
-    
-    // Let's type check individual argument expressions without any
-    // contextual information to try to recover an argument type that
-    // matches what the user actually wrote instead of what the typechecker
-    // expects.
-    SmallVector<TupleTypeElt, 4> elts;
-    for (unsigned i = 0, e = argTuple->getNumElements(); i < e; ++i) {
-      ConcreteDeclRef ref = nullptr;
-      auto *el = argTuple->getElement(i);
-      auto typeResult =
-          TypeChecker::getTypeOfExpressionWithoutApplying(el, CS.DC, ref);
-      if (!typeResult)
-        return false;
-      auto flags = ParameterTypeFlags().withInOut(typeResult->is<InOutType>());
-      elts.push_back(TupleTypeElt(typeResult->getInOutObjectType(),
-                                  argTuple->getElementName(i),
-                                  flags));
-    }
-
-    argType = TupleType::get(elts, CS.getASTContext());
-  }
-
-  auto typeKind = argType->getKind();
-  if (typeKind != TypeKind::Tuple && typeKind != TypeKind::Paren)
-    return false;
-
-  // If argument type couldn't be properly resolved or has errors,
-  // we can't diagnose anything in here, it points to the different problem.
-  if (isUnresolvedOrTypeVarType(argType) || argType->hasError())
-    return false;
-
-  auto context = CS.DC;
-  using CandidateMap =
-      llvm::SmallDenseMap<ValueDecl *, llvm::SmallVector<OverloadChoice, 2>>;
-
-  auto getBaseKind = [](ValueDecl *base) -> DescriptiveDeclKind {
-    DescriptiveDeclKind kind = DescriptiveDeclKind::Module;
-    if (!base)
-      return kind;
-
-    auto context = base->getDeclContext();
-    do {
-      if (isa<ExtensionDecl>(context))
-        return DescriptiveDeclKind::Extension;
-
-      if (auto nominal = dyn_cast<NominalTypeDecl>(context)) {
-        kind = nominal->getDescriptiveKind();
-        break;
-      }
-
-      context = context->getParent();
-    } while (context);
-
-    return kind;
-  };
-
-  auto diagnoseShadowing = [&](ValueDecl *base,
-                               ArrayRef<OverloadChoice> candidates) -> bool {
-    CalleeCandidateInfo calleeInfo(base ? base->getInterfaceType() : nullptr,
-                                   candidates, CCI.hasTrailingClosure, CS,
-                                   base);
-
-    calleeInfo.filterListArgs(decomposeArgType(argType, argLabels));
-
-    auto diagnostic = diag::member_shadows_global_function_near_match;
-    switch (calleeInfo.closeness) {
-    case CC_Unavailable:
-    case CC_Inaccessible:
-    case CC_SelfMismatch:
-    case CC_ArgumentLabelMismatch:
-    case CC_ArgumentCountMismatch:
-    case CC_GeneralMismatch:
-      return false;
-
-    case CC_NonLValueInOut:
-    case CC_OneArgumentNearMismatch:
-    case CC_OneArgumentMismatch:
-    case CC_OneGenericArgumentNearMismatch:
-    case CC_OneGenericArgumentMismatch:
-    case CC_ArgumentNearMismatch:
-    case CC_ArgumentMismatch:
-    case CC_GenericNonsubstitutableMismatch:
-      break; // Near match cases
-
-    case CC_ExactMatch:
-      diagnostic = diag::member_shadows_global_function;
-      break;
-    }
-
-    auto choice = calleeInfo.candidates[0].getDecl();
-    auto baseKind = getBaseKind(base);
-    auto baseName = getBaseName(choice->getDeclContext());
-
-    auto origCandidate = CCI[0].getDecl();
-    ctx.Diags.diagnose(UDE->getLoc(), diagnostic, UDE->getName(),
-                      origCandidate->getDescriptiveKind(),
-                      origCandidate->getFullName(),
-                      choice->getDescriptiveKind(),
-                      choice->getFullName(), baseKind, baseName);
-
-    auto topLevelDiag = diag::fix_unqualified_access_top_level;
-    if (baseKind == DescriptiveDeclKind::Module)
-      topLevelDiag = diag::fix_unqualified_access_top_level_multi;
-
-    emitFixItForExplicitlyQualifiedReference(ctx.Diags, UDE, topLevelDiag,
-                                             baseName,
-                                             choice->getDescriptiveKind());
-
-    for (auto &candidate : calleeInfo.candidates) {
-      if (auto decl = candidate.getDecl())
-        ctx.Diags.diagnose(decl, diag::decl_declared_here, decl->getFullName());
-    }
-
-    return true;
-  };
-
-  // For each of the parent contexts, let's try to find any candidates
-  // which have the same name and the same number of arguments as callee.
-  while (context->getParent()) {
-    auto result =
-        TypeChecker::lookupUnqualified(context, UDE->getName(), UDE->getLoc());
-    context = context->getParent();
-
-    if (!result || result.empty())
-      continue;
-
-    CandidateMap candidates;
-    for (const auto &candidate : result) {
-      auto base = candidate.getBaseDecl();
-      auto decl = candidate.getValueDecl();
-      if ((base && base->isInvalid()) || decl->isInvalid())
-        continue;
-
-      // If base is present but it doesn't represent a valid nominal,
-      // we can't use current candidate as one of the choices.
-      if (base && !base->getInterfaceType()->getNominalOrBoundGenericNominal())
-        continue;
-
-      auto context = decl->getDeclContext();
-      // We are only interested in static or global functions, because
-      // there is no way to call anything else properly.
-      if (!decl->isStatic() && !context->isModuleScopeContext())
-        continue;
-
-      OverloadChoice choice(base ? base->getInterfaceType() : nullptr,
-                            decl, UDE->getFunctionRefKind());
-
-      if (base) { // Let's group all of the candidates have a common base.
-        candidates[base].push_back(choice);
-        continue;
-      }
-
-      // If there is no base, it means this is one of the global functions,
-      // let's try to diagnose its shadowing inline.
-      if (diagnoseShadowing(base, choice))
-        return true;
-    }
-
-    if (candidates.empty())
-      continue;
-
-    for (const auto &candidate : candidates) {
-      if (diagnoseShadowing(candidate.getFirst(), candidate.getSecond()))
-        return true;
-    }
-  }
-
-  return false;
-}
-
 // Extract expression for failed argument number
 static Expr *getFailedArgumentExpr(CalleeCandidateInfo CCI, Expr *argExpr) {
   if (auto *TE = dyn_cast<TupleExpr>(argExpr))
@@ -1201,10 +980,6 @@ static Expr *getFailedArgumentExpr(CalleeCandidateInfo CCI, Expr *argExpr) {
 bool FailureDiagnosis::diagnoseParameterErrors(CalleeCandidateInfo &CCI,
                                                Expr *fnExpr, Expr *argExpr,
                                                ArrayRef<Identifier> argLabels) {
-  // Try to diagnose errors related to the use of implicit self reference.
-  if (diagnoseImplicitSelfErrors(fnExpr, argExpr, CCI, argLabels))
-    return true;
-
   // If we have a failure where the candidate set differs on exactly one
   // argument, and where we have a consistent mismatch across the candidate set
   // (often because there is only one candidate in the set), then diagnose this

lib/Sema/CSDiagnostics.cpp

@@ -6137,3 +6137,45 @@ bool UnableToInferProtocolLiteralType::diagnoseAsError() {
 
   return true;
 }
+
+bool MissingQuialifierInMemberRefFailure::diagnoseAsError() {
+  auto selectedOverload = getOverloadChoiceIfAvailable(getLocator());
+  if (!selectedOverload)
+    return false;
+
+  auto *UDE = cast<UnresolvedDotExpr>(getRawAnchor());
+
+  auto baseType = getType(UDE->getBase());
+
+  auto methodKind = baseType->isAnyExistentialType()
+                        ? DescriptiveDeclKind::StaticMethod
+                        : DescriptiveDeclKind::Method;
+
+  auto choice = selectedOverload->choice.getDeclOrNull();
+  if (!choice)
+    return false;
+
+  auto *DC = choice->getDeclContext();
+  if (!(DC->isModuleContext() || DC->isModuleScopeContext())) {
+    emitDiagnostic(UDE->getLoc(), diag::member_shadows_function, UDE->getName(),
+                   methodKind, choice->getDescriptiveKind(),
+                   choice->getFullName());
+    return true;
+  }
+
+  auto qualifier = DC->getParentModule()->getName();
+
+  emitDiagnostic(UDE->getLoc(), diag::member_shadows_global_function,
+                 UDE->getName(), methodKind, choice->getDescriptiveKind(),
+                 choice->getFullName(), qualifier);
+
+  SmallString<32> namePlusDot = qualifier.str();
+  namePlusDot.push_back('.');
+
+  emitDiagnostic(UDE->getLoc(), diag::fix_unqualified_access_top_level_multi,
+                 namePlusDot, choice->getDescriptiveKind(), qualifier)
+      .fixItInsert(UDE->getStartLoc(), namePlusDot);
+
+  emitDiagnostic(choice, diag::decl_declared_here, choice->getFullName());
+  return true;
+}