Skip to content

[CSDiagnostics] Offer protocol conformance fix-it in ContextualFailure #27026

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Sep 5, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,9 @@ ERROR(cannot_convert_coerce_nil,none,
ERROR(cannot_convert_assign,none,
"cannot assign value of type %0 to type %1",
(Type,Type))
NOTE(assign_protocol_conformance_fix_it,none,
"add missing conformance to %0 to %1 %2",
(Type, DescriptiveDeclKind, Type))
ERROR(cannot_convert_assign_protocol,none,
"value of type %0 does not conform to %1 in assignment",
(Type, Type))
Expand Down
88 changes: 88 additions & 0 deletions lib/Sema/CSDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "TypoCorrection.h"
#include "swift/AST/ASTContext.h"
#include "swift/AST/Decl.h"
#include "swift/AST/ExistentialLayout.h"
#include "swift/AST/Expr.h"
#include "swift/AST/GenericSignature.h"
#include "swift/AST/Initializer.h"
Expand Down Expand Up @@ -2006,6 +2007,9 @@ void ContextualFailure::tryFixIts(InFlightDiagnostic &diagnostic) const {
if (tryIntegerCastFixIts(diagnostic))
return;

if (tryProtocolConformanceFixIt(diagnostic))
return;

if (tryTypeCoercionFixIt(diagnostic))
return;
}
Expand Down Expand Up @@ -2430,6 +2434,90 @@ bool ContextualFailure::tryTypeCoercionFixIt(
return false;
}

bool ContextualFailure::tryProtocolConformanceFixIt(
InFlightDiagnostic &diagnostic) const {
auto innermostTyCtx = getDC()->getInnermostTypeContext();
if (!innermostTyCtx)
return false;

auto nominal = innermostTyCtx->getSelfNominalTypeDecl();
if (!nominal)
return false;

// We need to get rid of optionals and parens as it's not relevant when
// printing the diagnostic and the fix-it.
auto unwrappedToType =
ToType->lookThroughAllOptionalTypes()->getWithoutParens();

// If the protocol requires a class & we don't have one (maybe the context
// is a struct), then bail out instead of offering a broken fix-it later on.
auto requiresClass = false;
ExistentialLayout layout;
if (unwrappedToType->isExistentialType()) {
layout = unwrappedToType->getExistentialLayout();
requiresClass = layout.requiresClass();
}

if (requiresClass && !FromType->is<ClassType>()) {
return false;
}

// We can only offer a fix-it if we're assigning to a protocol type and
// the type we're assigning is the same as the innermost type context.
bool shouldOfferFixIt = nominal->getSelfTypeInContext()->isEqual(FromType) &&
unwrappedToType->isExistentialType();
if (!shouldOfferFixIt)
return false;

diagnostic.flush();

// Let's build a list of protocols that the context does not conform to.
SmallVector<std::string, 8> missingProtoTypeStrings;
for (auto protocol : layout.getProtocols()) {
if (!getTypeChecker().conformsToProtocol(
FromType, protocol->getDecl(), getDC(),
ConformanceCheckFlags::InExpression)) {
missingProtoTypeStrings.push_back(protocol->getString());
}
}

// If we have a protocol composition type and we don't conform to all
// the protocols of the composition, then store the composition directly.
// This is because we need to append 'Foo & Bar' instead of 'Foo, Bar' in
// order to match the written type.
if (auto compositionTy = unwrappedToType->getAs<ProtocolCompositionType>()) {
if (compositionTy->getMembers().size() == missingProtoTypeStrings.size()) {
missingProtoTypeStrings = {compositionTy->getString()};
}
}

assert(!missingProtoTypeStrings.empty() &&
"type already conforms to all the protocols?");

// Combine all protocol names together, separated by commas.
std::string protoString = llvm::join(missingProtoTypeStrings, ", ");

// Emit a diagnostic to inform the user that they need to conform to the
// missing protocols.
//
// TODO: Maybe also insert the requirement stubs?
auto conformanceDiag = emitDiagnostic(
getAnchor()->getLoc(), diag::assign_protocol_conformance_fix_it,
unwrappedToType, nominal->getDescriptiveKind(), FromType);
if (nominal->getInherited().size() > 0) {
auto lastInherited = nominal->getInherited().back().getLoc();
auto lastInheritedEndLoc =
Lexer::getLocForEndOfToken(getASTContext().SourceMgr, lastInherited);
conformanceDiag.fixItInsert(lastInheritedEndLoc, ", " + protoString);
} else {
auto nameEndLoc = Lexer::getLocForEndOfToken(getASTContext().SourceMgr,
nominal->getNameLoc());
conformanceDiag.fixItInsert(nameEndLoc, ": " + protoString);
}

return true;
}

void ContextualFailure::tryComputedPropertyFixIts(Expr *expr) const {
if (!isa<ClosureExpr>(expr))
return;
Expand Down
4 changes: 4 additions & 0 deletions lib/Sema/CSDiagnostics.h
Original file line number Diff line number Diff line change
Expand Up @@ -681,6 +681,10 @@ class ContextualFailure : public FailureDiagnostic {
/// conversion is possible.
bool tryTypeCoercionFixIt(InFlightDiagnostic &diagnostic) const;

/// Try to add a fix-it to conform the decl context (if it's a type) to the
/// protocol
bool tryProtocolConformanceFixIt(InFlightDiagnostic &diagnostic) const;

/// Check whether this contextual failure represents an invalid
/// conversion from array literal to dictionary.
static bool isInvalidDictionaryConversion(ConstraintSystem &cs, Expr *anchor,
Expand Down
68 changes: 68 additions & 0 deletions test/decl/protocol/protocols.swift
Original file line number Diff line number Diff line change
Expand Up @@ -511,3 +511,71 @@ extension LetThereBeCrash {
init() { x = 1 }
// expected-error@-1 {{'let' property 'x' may not be initialized directly; use "self.init(...)" or "self = ..." instead}}
}

// SR-11412
// Offer fix-it to conform type of context to the missing protocols

protocol SR_11412_P1 {}
protocol SR_11412_P2 {}
protocol SR_11412_P3 {}
protocol SR_11412_P4: AnyObject {}

class SR_11412_C0 {
var foo1: SR_11412_P1?
var foo2: (SR_11412_P1 & SR_11412_P2)?
weak var foo3: SR_11412_P4?
}

// Context has no inherited types and does not conform to protocol //

class SR_11412_C1 {
let c0 = SR_11412_C0()

func conform() {
c0.foo1 = self // expected-error {{cannot assign value of type 'SR_11412_C1' to type 'SR_11412_P1?'}}
// expected-note@-1 {{add missing conformance to 'SR_11412_P1' to class 'SR_11412_C1'}}{{18-18=: SR_11412_P1}}
}
}

// Context has no inherited types and does not conform to protocol composition //

class SR_11412_C2 {
let c0 = SR_11412_C0()

func conform() {
c0.foo2 = self // expected-error {{cannot assign value of type 'SR_11412_C2' to type '(SR_11412_P1 & SR_11412_P2)?'}}
// expected-note@-1 {{add missing conformance to 'SR_11412_P1 & SR_11412_P2' to class 'SR_11412_C2'}}{{18-18=: SR_11412_P1 & SR_11412_P2}}
}
}

// Context already has an inherited type, but does not conform to protocol //

class SR_11412_C3: SR_11412_P3 {
let c0 = SR_11412_C0()

func conform() {
c0.foo1 = self // expected-error {{cannot assign value of type 'SR_11412_C3' to type 'SR_11412_P1?'}}
// expected-note@-1 {{add missing conformance to 'SR_11412_P1' to class 'SR_11412_C3'}}{{31-31=, SR_11412_P1}}
}
}

// Context conforms to only one protocol in the protocol composition //

class SR_11412_C4: SR_11412_P1 {
let c0 = SR_11412_C0()

func conform() {
c0.foo2 = self // expected-error {{cannot assign value of type 'SR_11412_C4' to type '(SR_11412_P1 & SR_11412_P2)?'}}
// expected-note@-1 {{add missing conformance to 'SR_11412_P1 & SR_11412_P2' to class 'SR_11412_C4'}}{{31-31=, SR_11412_P2}}
}
}

// Context is a value type, but protocol requires class //

struct SR_11412_S0 {
let c0 = SR_11412_C0()

func conform() {
c0.foo3 = self // expected-error {{cannot assign value of type 'SR_11412_S0' to type 'SR_11412_P4?'}}
}
}