Skip to content

[CSDiagnostics] Improving the fix-it for defining computed properties #23500

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 6 commits into from
Mar 25, 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
2 changes: 2 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -1522,6 +1522,8 @@ ERROR(extension_specialization,none,
"type %0 with constraints specified by a 'where' clause", (Identifier))
ERROR(extension_stored_property,none,
"extensions must not contain stored properties", ())
NOTE(extension_stored_property_fixit,none,
"Remove '=' to make %0 a computed property", (Identifier))
ERROR(extension_nongeneric_trailing_where,none,
"trailing 'where' clause for extension of non-generic type %0",
(DeclName))
Expand Down
13 changes: 9 additions & 4 deletions lib/Sema/CSDiag.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2315,16 +2315,21 @@ bool FailureDiagnosis::diagnoseContextualConversionError(
exprType->isEqual(contextualType)) {
return false;
}

// If we're trying to convert something of type "() -> T" to T, then we
// probably meant to call the value.
if (auto srcFT = exprType->getAs<AnyFunctionType>()) {
if (srcFT->getParams().empty() &&
!isUnresolvedOrTypeVarType(srcFT->getResult()) &&
CS.TC.isConvertibleTo(srcFT->getResult(), contextualType, CS.DC)) {
diagnose(expr->getLoc(), diag::missing_nullary_call, srcFT->getResult())
.highlight(expr->getSourceRange())
.fixItInsertAfter(expr->getEndLoc(), "()");

auto locator =
CS.getConstraintLocator(expr, ConstraintLocator::ContextualType);
ContextualFailure failure =
ContextualFailure(nullptr, CS, srcFT, contextualType, locator);
auto diagnosed = failure.diagnoseAsError();
assert(diagnosed && "Failed to produce contextual failure diagnostic");
(void)diagnosed;
return true;
}
}
Expand Down
51 changes: 51 additions & 0 deletions lib/Sema/CSDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "swift/AST/Decl.h"
#include "swift/AST/Expr.h"
#include "swift/AST/GenericSignature.h"
#include "swift/AST/Initializer.h"
#include "swift/AST/ParameterList.h"
#include "swift/AST/Pattern.h"
#include "swift/AST/ProtocolConformance.h"
Expand Down Expand Up @@ -1348,6 +1349,9 @@ bool ContextualFailure::diagnoseMissingFunctionCall() const {
srcFT->getResult())
.highlight(anchor->getSourceRange())
.fixItInsertAfter(anchor->getEndLoc(), "()");

tryComputedPropertyFixIts(anchor);

return true;
}

Expand Down Expand Up @@ -1378,6 +1382,53 @@ bool ContextualFailure::trySequenceSubsequenceFixIts(InFlightDiagnostic &diag,
return false;
}

void ContextualFailure::tryComputedPropertyFixIts(Expr *expr) const {
if (!isa<ClosureExpr>(expr))
return;

// It is possible that we're looking at a stored property being
// initialized with a closure. Something like:
//
// var foo: Int = { return 0 }
//
// Let's offer another fix-it to remove the '=' to turn the stored
// property into a computed property. If the variable is immutable, then
// replace the 'let' with a 'var'.

PatternBindingDecl *PBD = nullptr;

if (auto TLCD = dyn_cast<TopLevelCodeDecl>(getDC())) {
if (TLCD->getBody()->isImplicit()) {
if (auto decl = TLCD->getBody()->getElement(0).dyn_cast<Decl *>()) {
if (auto binding = dyn_cast<PatternBindingDecl>(decl)) {
PBD = binding;
}
}
}
} else if (auto PBI = dyn_cast<PatternBindingInitializer>(getDC())) {
PBD = PBI->getBinding();
}

if (PBD) {
if (auto VD = PBD->getSingleVar()) {
auto entry = PBD->getPatternEntryForVarDecl(VD);

if (!VD->isStatic() &&
!VD->getAttrs().getAttribute<DynamicReplacementAttr>() &&
entry.getInit() && isa<ClosureExpr>(entry.getInit())) {
auto diag = emitDiagnostic(expr->getLoc(),
diag::extension_stored_property_fixit,
VD->getName());
diag.fixItRemove(entry.getEqualLoc());

if (VD->isLet()) {
diag.fixItReplace(PBD->getStartLoc(), getTokenText(tok::kw_var));
}
}
}
}
}

bool AutoClosureForwardingFailure::diagnoseAsError() {
auto path = getLocator()->getPath();
assert(!path.empty());
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 final : public FailureDiagnostic {
}
return type;
}

/// Try to add a fix-it to convert a stored property into a computed
/// property
void tryComputedPropertyFixIts(Expr *expr) const;
};

/// Diagnose situations when @autoclosure argument is passed to @autoclosure
Expand Down
11 changes: 11 additions & 0 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1989,6 +1989,17 @@ repairFailures(ConstraintSystem &cs, Type lhs, Type rhs,
break;
}

case ConstraintLocator::ContextualType: {
if (lhs->is<FunctionType>() && !rhs->is<AnyFunctionType>() &&
isa<ClosureExpr>(anchor)) {
auto *fix = ContextualMismatch::create(cs, lhs, rhs,
cs.getConstraintLocator(locator));
conversionsOrFixes.push_back(fix);
}

break;
}

default:
return;
}
Expand Down
29 changes: 29 additions & 0 deletions test/decl/var/properties.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1277,3 +1277,32 @@ let sr8811b: Never = fatalError() // Ok
let sr8811c = (16, fatalError()) // expected-warning {{constant 'sr8811c' inferred to have type '(Int, Never)', which contains an enum with no cases}} expected-note {{add an explicit type annotation to silence this warning}}

let sr8811d: (Int, Never) = (16, fatalError()) // Ok

// SR-9267

class SR_9267 {}
extension SR_9267 {
var foo: String = { // expected-error {{extensions must not contain stored properties}} // expected-error {{function produces expected type 'String'; did you mean to call it with '()'?}} // expected-note {{Remove '=' to make 'foo' a computed property}}{{19-21=}}
return "Hello"
}
}

enum SR_9267_E {
var SR_9267_prop: String = { // expected-error {{enums must not contain stored properties}} // expected-error {{function produces expected type 'String'; did you mean to call it with '()'?}} // expected-note {{Remove '=' to make 'SR_9267_prop' a computed property}}{{28-30=}}
return "Hello"
}
}

var SR_9267_prop_1: Int = { // expected-error {{function produces expected type 'Int'; did you mean to call it with '()'?}} // expected-note {{Remove '=' to make 'SR_9267_prop_1' a computed property}}{{25-27=}}
return 0
}

class SR_9267_C {
var SR_9267_prop_2: String = { // expected-error {{function produces expected type 'String'; did you mean to call it with '()'?}} // expected-note {{Remove '=' to make 'SR_9267_prop_2' a computed property}}{{30-32=}}
return "Hello"
}
}

class SR_9267_C2 {
let SR_9267_prop_3: Int = { return 0 } // expected-error {{function produces expected type 'Int'; did you mean to call it with '()'?}} // expected-note {{Remove '=' to make 'SR_9267_prop_3' a computed property}}{{3-6=var}}{{27-29=}}
}
3 changes: 1 addition & 2 deletions test/expr/closure/closures.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ var closure5 : (Double) -> Int = {

var closure6 = $0 // expected-error {{anonymous closure argument not contained in a closure}}

var closure7 : Int =
{ 4 } // expected-error {{function produces expected type 'Int'; did you mean to call it with '()'?}} {{9-9=()}}
var closure7 : Int = { 4 } // expected-error {{function produces expected type 'Int'; did you mean to call it with '()'?}} {{27-27=()}} // expected-note {{Remove '=' to make 'closure7' a computed property}}{{20-22=}}

var capturedVariable = 1
var closure8 = { [capturedVariable] in
Expand Down