Creating UnnecessaryCoercion warning fix

Author: LucianoPAlmeida

lib/Sema/CSDiagnostics.cpp

@@ -4400,9 +4400,7 @@ bool InaccessibleMemberFailure::diagnoseAsError() {
     auto &cs = getConstraintSystem();
     auto *locator =
         cs.getConstraintLocator(baseExpr, ConstraintLocator::Member);
-    if (llvm::any_of(cs.getFixes(), [&](const ConstraintFix *fix) {
-          return fix->getLocator() == locator;
-        }))
+    if (cs.hasFixFor(locator))
       return false;
   }
 
@@ -5031,6 +5029,36 @@ bool ThrowingFunctionConversionFailure::diagnoseAsError() {
   return true;
 }
 
+bool UnnecessaryCoercionFailure::diagnoseAsError() {
+  auto expr = cast<CoerceExpr>(getAnchor());
+  auto sourceRange =
+      SourceRange(expr->getLoc(), expr->getCastTypeLoc().getSourceRange().End);
+
+  if (isa<TypeAliasType>(getFromType().getPointer()) &&
+      isa<TypeAliasType>(getToType().getPointer())) {
+    auto fromTypeAlias = cast<TypeAliasType>(getFromType().getPointer());
+    auto toTypeAlias = cast<TypeAliasType>(getToType().getPointer());
+    // If the typealias are different, we need a warning
+    // mentioning both types.
+    if (fromTypeAlias->getDecl() != toTypeAlias->getDecl()) {
+      emitDiagnostic(expr->getLoc(),
+                     diag::unnecessary_same_typealias_type_coercion,
+                     getFromType(), getToType())
+
+          .fixItRemove(sourceRange);
+    } else {
+      emitDiagnostic(expr->getLoc(), diag::unnecessary_same_type_coercion,
+                     getToType())
+          .fixItRemove(sourceRange);
+    }
+  } else {
+    emitDiagnostic(expr->getLoc(), diag::unnecessary_same_type_coercion,
+                   getToType())
+        .fixItRemove(sourceRange);
+  }
+  return true;
+}
+
 bool InOutConversionFailure::diagnoseAsError() {
   auto *anchor = getAnchor();
   auto *locator = getLocator();

lib/Sema/CSDiagnostics.h

@@ -156,7 +156,13 @@ class FailureDiagnostic {
   }
 
   /// \returns true is locator hasn't been simplified down to expression.
-  bool hasComplexLocator() const { return HasComplexLocator; }
+  bool hasComplexLocator() const {
+    bool isExplicitCoercion =
+        getLocator()
+            ->isLastElement<
+                ConstraintLocator::PathElement::ExplicitTypeCoercion>();
+    return HasComplexLocator && !isExplicitCoercion;
+  }
 
   /// \returns A parent expression if sub-expression is contained anywhere
   /// in the root expression or `nullptr` otherwise.
@@ -707,8 +713,8 @@ class ContextualFailure : public FailureDiagnostic {
   /// If we're trying to convert something to `nil`.
   bool diagnoseConversionToNil() const;
 
-  // If we're trying to convert something of type "() -> T" to T,
-  // then we probably meant to call the value.
+  /// If we're trying to convert something of type "() -> T" to T,
+  /// then we probably meant to call the value.
   bool diagnoseMissingFunctionCall() const;
 
   /// Produce a specialized diagnostic if this is an invalid conversion to Bool.
@@ -1460,11 +1466,11 @@ class MutatingMemberRefOnImmutableBase final : public FailureDiagnostic {
   bool diagnoseAsError() override;
 };
 
-// Diagnose an attempt to use AnyObject as the root type of a KeyPath
-//
-// ```swift
-// let keyPath = \AnyObject.bar
-// ```
+/// Diagnose an attempt to use AnyObject as the root type of a KeyPath
+///
+/// ```swift
+/// let keyPath = \AnyObject.bar
+/// ```
 class AnyObjectKeyPathRootFailure final : public FailureDiagnostic {
 
 public:
@@ -1774,6 +1780,27 @@ class ExpandArrayIntoVarargsFailure final : public ContextualFailure {
   void tryDropArrayBracketsFixIt(Expr *anchor) const;
 };
 
+/// Diagnose a situation where there is an explicit type coercion
+/// to the same type e.g.:
+///
+/// ```swift
+/// Double(1) as Double // redundant cast to 'Double' has no effect
+/// 1 as Double as Double // redundant cast to 'Double' has no effect
+/// let string = "String"
+/// let s = string as String // redundant cast to 'String' has no effect
+/// ```
+class UnnecessaryCoercionFailure final
+    : public ContextualFailure {
+      
+public:
+  UnnecessaryCoercionFailure(Expr *root, ConstraintSystem &cs,
+                             Type fromType, Type toType,
+                             ConstraintLocator *locator)
+      : ContextualFailure(root, cs, fromType, toType, locator) {}
+  
+  bool diagnoseAsError() override;
+};
+
 /// Diagnose a situation there is a mismatch between argument and parameter
 /// types e.g.:
 ///

lib/Sema/CSFix.cpp

@@ -857,6 +857,36 @@ IgnoreContextualType *IgnoreContextualType::create(ConstraintSystem &cs,
       IgnoreContextualType(cs, resultTy, specifiedTy, locator);
 }
 
+bool RemoveUnnecessaryCoercion::diagnose(Expr *root, bool asNote) const {
+  auto &cs = getConstraintSystem();
+  UnnecessaryCoercionFailure failure(root, cs, getFromType(), getToType(),
+                                     getLocator());
+  return failure.diagnose(asNote);
+}
+
+bool RemoveUnnecessaryCoercion::attempt(ConstraintSystem &cs, Type fromType,
+                                        Type toType,
+                                        ConstraintLocatorBuilder locator) {
+  if (auto last = locator.last()) {
+    if (last->is<LocatorPathElt::ExplicitTypeCoercion>()) {
+      auto expr = cast<CoerceExpr>(locator.getAnchor());
+      auto toTypeRepr = expr->getCastTypeLoc().getTypeRepr();
+
+      // only diagnosing for coercion where the left-side is a DeclRefExpr
+      // or a explicit/implicit coercion e.g. Double(1) as Double
+      if (!isa<ImplicitlyUnwrappedOptionalTypeRepr>(toTypeRepr) &&
+          (isa<DeclRefExpr>(expr->getSubExpr()) ||
+           isa<CoerceExpr>(expr->getSubExpr()))) {
+        auto *fix = new (cs.getAllocator()) RemoveUnnecessaryCoercion(
+            cs, fromType, toType, cs.getConstraintLocator(locator));
+
+        return cs.recordFix(fix);
+      }
+    }
+  }
+  return false;
+}
+
 bool IgnoreAssignmentDestinationType::diagnose(Expr *root, bool asNote) const {
   auto &cs = getConstraintSystem();
   auto *AE = cast<AssignExpr>(getAnchor());

lib/Sema/CSFix.h

@@ -196,6 +196,9 @@ enum class FixKind : uint8_t {
   /// Allow a single tuple parameter to be matched with N arguments
   /// by forming all of the given arguments into a single tuple.
   AllowTupleSplatForSingleParameter,
+  
+  /// Remove an unnecessary coercion ('as') if the types are already equal. e.g. Double(1) as Double
+  RemoveUnnecessaryCoercion,
 
   /// Allow a single argument type mismatch. This is the most generic
   /// failure related to argument-to-parameter conversions.
@@ -1340,6 +1343,24 @@ class IgnoreContextualType : public ContextualMismatch {
                                       ConstraintLocator *locator);
 };
 
+class RemoveUnnecessaryCoercion : public ContextualMismatch {
+protected:
+  RemoveUnnecessaryCoercion(ConstraintSystem &cs,
+                            Type fromType, Type toType,
+                            ConstraintLocator *locator)
+  : ContextualMismatch(cs, FixKind::RemoveUnnecessaryCoercion, fromType, toType,
+                       locator, /*isWarning*/ true) {}
+  
+public:
+  std::string getName() const override { return "remove unnecessary explicit type coercion"; }
+  
+  bool diagnose(Expr *root, bool asNote = false) const override;
+  
+  static bool attempt(ConstraintSystem &cs,
+                      Type fromType, Type toType,
+                      ConstraintLocatorBuilder locator);
+};
+
 class IgnoreAssignmentDestinationType final : public ContextualMismatch {
   IgnoreAssignmentDestinationType(ConstraintSystem &cs, Type sourceTy,
                                   Type destTy, ConstraintLocator *locator)

lib/Sema/ConstraintLocator.cpp

@@ -86,6 +86,7 @@ void ConstraintLocator::Profile(llvm::FoldingSetNodeID &id, Expr *anchor,
     case KeyPathRoot:
     case KeyPathValue:
     case KeyPathComponentResult:
+    case ExplicitTypeCoercion:
       auto numValues = numNumericValuesInPathElement(elt.getKind());
       for (unsigned i = 0; i < numValues; ++i)
         id.AddInteger(elt.getValue(i));
@@ -132,6 +133,7 @@ unsigned LocatorPathElt::getNewSummaryFlags() const {
   case ConstraintLocator::KeyPathRoot:
   case ConstraintLocator::KeyPathValue:
   case ConstraintLocator::KeyPathComponentResult:
+  case ConstraintLocator::ExplicitTypeCoercion:
     return 0;
 
   case ConstraintLocator::FunctionArgument:
@@ -456,6 +458,10 @@ void ConstraintLocator::dump(SourceManager *sm, raw_ostream &out) {
     case KeyPathComponentResult:
       out << "key path component result";
       break;
+        
+    case ExplicitTypeCoercion:
+      out << "type coercion";
+      break;
     }
   }
   out << ']';

lib/Sema/ConstraintLocator.h

@@ -89,6 +89,7 @@ class ConstraintLocator : public llvm::FoldingSetNode {
     case KeyPathRoot:
     case KeyPathValue:
     case KeyPathComponentResult:
+    case ExplicitTypeCoercion:
       return 0;
 
     case ContextualType:

lib/Sema/ConstraintLocatorPathElts.def

@@ -164,6 +164,9 @@ SIMPLE_LOCATOR_PATH_ELT(UnresolvedMember)
 /// The candidate witness during protocol conformance checking.
 CUSTOM_LOCATOR_PATH_ELT(Witness)
 
+/// An explicit type coercion.
+SIMPLE_LOCATOR_PATH_ELT(ExplicitTypeCoercion)
+
 #undef LOCATOR_PATH_ELT
 #undef CUSTOM_LOCATOR_PATH_ELT
 #undef SIMPLE_LOCATOR_PATH_ELT