Skip to content

Commit 39dd630

Browse files
xedinxedin
committed
[Diagnostics] Refactor missing optional unwrap diagnostic
Instead of passing all of the information available in the diagnostic to static functions, let's bring "default value" and "force unwrap" fix-it logic under "missing optional unwrap diagnostic" umbrella.
1 parent a5485d8 commit 39dd630

File tree

2 files changed

+63
-61
lines changed

2 files changed

+63
-61
lines changed

‎lib/Sema/CSDiagnostics.cpp‎

Lines changed: 58 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -549,16 +549,16 @@ bool MemberAccessOnOptionalBaseFailure::diagnoseAsError() {
549549
resultIsOptional, SourceRange());
550550
}
551551

552-
// Suggest a default value via ?? <default value>
553-
static void offerDefaultValueUnwrapFixit(TypeChecker &TC, DeclContext *DC, Expr *expr, Expr *rootExpr) {
554-
auto diag =
555-
TC.diagnose(expr->getLoc(), diag::unwrap_with_default_value);
552+
void MissingOptionalUnwrapFailure::offerDefaultValueUnwrapFixIt(
553+
DeclContext *DC, Expr *expr) const {
554+
auto diag = emitDiagnostic(expr->getLoc(), diag::unwrap_with_default_value);
556555

556+
auto &TC = getTypeChecker();
557557
// Figure out what we need to parenthesize.
558558
bool needsParensInside =
559-
exprNeedsParensBeforeAddingNilCoalescing(TC, DC, expr);
559+
exprNeedsParensBeforeAddingNilCoalescing(TC, DC, expr);
560560
bool needsParensOutside =
561-
exprNeedsParensAfterAddingNilCoalescing(TC, DC, expr, rootExpr);
561+
exprNeedsParensAfterAddingNilCoalescing(TC, DC, expr, getParentExpr());
562562

563563
llvm::SmallString<2> insertBefore;
564564
llvm::SmallString<32> insertAfter;
@@ -580,8 +580,8 @@ static void offerDefaultValueUnwrapFixit(TypeChecker &TC, DeclContext *DC, Expr
580580
}
581581

582582
// Suggest a force-unwrap.
583-
static void offerForceUnwrapFixit(ConstraintSystem &CS, Expr *expr) {
584-
auto diag = CS.TC.diagnose(expr->getLoc(), diag::unwrap_with_force_value);
583+
void MissingOptionalUnwrapFailure::offerForceUnwrapFixIt(Expr *expr) const {
584+
auto diag = emitDiagnostic(expr->getLoc(), diag::unwrap_with_force_value);
585585

586586
// If expr is optional as the result of an optional chain and this last
587587
// dot isn't a member returning optional, then offer to force the last
@@ -590,7 +590,7 @@ static void offerForceUnwrapFixit(ConstraintSystem &CS, Expr *expr) {
590590
if (auto dotExpr =
591591
dyn_cast<UnresolvedDotExpr>(optionalChain->getSubExpr())) {
592592
auto bind = dyn_cast<BindOptionalExpr>(dotExpr->getBase());
593-
if (bind && !CS.getType(dotExpr)->getOptionalObjectType()) {
593+
if (bind && !getType(dotExpr)->getOptionalObjectType()) {
594594
diag.fixItReplace(SourceRange(bind->getLoc()), "!");
595595
return;
596596
}
@@ -601,7 +601,7 @@ static void offerForceUnwrapFixit(ConstraintSystem &CS, Expr *expr) {
601601
diag.fixItInsertAfter(expr->getEndLoc(), "!");
602602
} else {
603603
diag.fixItInsert(expr->getStartLoc(), "(")
604-
.fixItInsertAfter(expr->getEndLoc(), ")!");
604+
.fixItInsertAfter(expr->getEndLoc(), ")!");
605605
}
606606
}
607607

@@ -622,8 +622,38 @@ class VarDeclMultipleReferencesChecker : public ASTWalker {
622622
int referencesCount() { return count; }
623623
};
624624

625-
static bool diagnoseUnwrap(ConstraintSystem &CS, Expr *expr, Expr *rootExpr, Type baseType,
626-
Type unwrappedType) {
625+
bool MissingOptionalUnwrapFailure::diagnoseAsError() {
626+
if (hasComplexLocator())
627+
return false;
628+
629+
auto *anchor = getAnchor();
630+
631+
if (auto assignExpr = dyn_cast<AssignExpr>(anchor))
632+
anchor = assignExpr->getSrc();
633+
634+
auto *unwrappedExpr = anchor->getValueProvidingExpr();
635+
636+
if (auto *tryExpr = dyn_cast<OptionalTryExpr>(unwrappedExpr)) {
637+
bool isSwift5OrGreater = getASTContext().isSwiftVersionAtLeast(5);
638+
auto subExprType = getType(tryExpr->getSubExpr());
639+
bool subExpressionIsOptional = (bool)subExprType->getOptionalObjectType();
640+
641+
if (isSwift5OrGreater && subExpressionIsOptional) {
642+
// Using 'try!' won't change the type for a 'try?' with an optional
643+
// sub-expr under Swift 5+, so just report that a missing unwrap can't be
644+
// handled here.
645+
return false;
646+
}
647+
648+
emitDiagnostic(tryExpr->getTryLoc(), diag::missing_unwrap_optional_try,
649+
getType(anchor)->getRValueType())
650+
.fixItReplace({tryExpr->getTryLoc(), tryExpr->getQuestionLoc()},
651+
"try!");
652+
return true;
653+
}
654+
655+
auto baseType = getBaseType();
656+
auto unwrappedType = getUnwrappedType();
627657

628658
assert(!baseType->hasTypeVariable() &&
629659
"Base type must not be a type variable");
@@ -633,15 +663,14 @@ static bool diagnoseUnwrap(ConstraintSystem &CS, Expr *expr, Expr *rootExpr, Typ
633663
if (!baseType->getOptionalObjectType())
634664
return false;
635665

636-
CS.TC.diagnose(expr->getLoc(), diag::optional_not_unwrapped, baseType,
637-
unwrappedType);
666+
emitDiagnostic(unwrappedExpr->getLoc(), diag::optional_not_unwrapped,
667+
baseType, unwrappedType);
638668

639669
// If the expression we're unwrapping is the only reference to a
640670
// local variable whose type isn't explicit in the source, then
641671
// offer unwrapping fixits on the initializer as well.
642-
if (auto declRef = dyn_cast<DeclRefExpr>(expr)) {
672+
if (auto declRef = dyn_cast<DeclRefExpr>(unwrappedExpr)) {
643673
if (auto varDecl = dyn_cast<VarDecl>(declRef->getDecl())) {
644-
645674
bool singleUse = false;
646675
AbstractFunctionDecl *AFD = nullptr;
647676
if (auto contextDecl = varDecl->getDeclContext()->getAsDecl()) {
@@ -658,69 +687,37 @@ static bool diagnoseUnwrap(ConstraintSystem &CS, Expr *expr, Expr *rootExpr, Typ
658687

659688
Expr *initializer = varDecl->getParentInitializer();
660689
if (auto declRefExpr = dyn_cast<DeclRefExpr>(initializer)) {
661-
if (declRefExpr->getDecl()->getAttrs().hasAttribute<ImplicitlyUnwrappedOptionalAttr>()) {
662-
CS.TC.diagnose(declRefExpr->getLoc(), diag::unwrap_iuo_initializer,
690+
if (declRefExpr->getDecl()
691+
->getAttrs()
692+
.hasAttribute<ImplicitlyUnwrappedOptionalAttr>()) {
693+
emitDiagnostic(declRefExpr->getLoc(), diag::unwrap_iuo_initializer,
663694
baseType);
664695
}
665696
}
666697

667698
auto fnTy = AFD->getInterfaceType()->castTo<AnyFunctionType>();
668-
bool voidReturn = fnTy->getResult()->isEqual(TupleType::getEmpty(CS.DC->getASTContext()));
699+
bool voidReturn =
700+
fnTy->getResult()->isEqual(TupleType::getEmpty(getASTContext()));
669701

670-
auto diag = CS.TC.diagnose(varDecl->getLoc(), diag::unwrap_with_guard);
702+
auto diag = emitDiagnostic(varDecl->getLoc(), diag::unwrap_with_guard);
671703
diag.fixItInsert(binding->getStartLoc(), "guard ");
672704
if (voidReturn) {
673705
diag.fixItInsertAfter(binding->getEndLoc(), " else { return }");
674706
} else {
675707
diag.fixItInsertAfter(binding->getEndLoc(), " else { return <"
676-
"#default value#" "> }");
708+
"#default value#"
709+
"> }");
677710
}
678711
diag.flush();
679712

680-
offerDefaultValueUnwrapFixit(CS.TC, varDecl->getDeclContext(),
681-
initializer, rootExpr);
682-
offerForceUnwrapFixit(CS, initializer);
713+
offerDefaultValueUnwrapFixIt(varDecl->getDeclContext(), initializer);
714+
offerForceUnwrapFixIt(initializer);
683715
}
684716
}
685717
}
686718

687-
offerDefaultValueUnwrapFixit(CS.TC, CS.DC, expr, rootExpr);
688-
offerForceUnwrapFixit(CS, expr);
689-
return true;
690-
}
691-
692-
bool MissingOptionalUnwrapFailure::diagnoseAsError() {
693-
if (hasComplexLocator())
694-
return false;
695-
696-
auto *anchor = getAnchor();
697-
auto *rootExpr = getParentExpr();
698-
699-
if (auto assignExpr = dyn_cast<AssignExpr>(anchor))
700-
anchor = assignExpr->getSrc();
701-
702-
auto *unwrapped = anchor->getValueProvidingExpr();
703-
auto type = getType(anchor)->getRValueType();
704-
705-
auto *tryExpr = dyn_cast<OptionalTryExpr>(unwrapped);
706-
if (!tryExpr) {
707-
return diagnoseUnwrap(getConstraintSystem(), unwrapped, rootExpr, getBaseType(),
708-
getUnwrappedType());
709-
}
710-
711-
bool isSwift5OrGreater = getTypeChecker().getLangOpts().isSwiftVersionAtLeast(5);
712-
auto subExprType = getType(tryExpr->getSubExpr());
713-
bool subExpressionIsOptional = (bool)subExprType->getOptionalObjectType();
714-
715-
716-
if (isSwift5OrGreater && subExpressionIsOptional) {
717-
// Using 'try!' won't change the type for a 'try?' with an optional sub-expr
718-
// under Swift 5+, so just report that a missing unwrap can't be handled here.
719-
return false;
720-
}
721-
722-
emitDiagnostic(tryExpr->getTryLoc(), diag::missing_unwrap_optional_try, type)
723-
.fixItReplace({tryExpr->getTryLoc(), tryExpr->getQuestionLoc()}, "try!");
719+
offerDefaultValueUnwrapFixIt(getDC(), unwrappedExpr);
720+
offerForceUnwrapFixIt(unwrappedExpr);
724721
return true;
725722
}
726723

‎lib/Sema/CSDiagnostics.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -565,6 +565,11 @@ class MissingOptionalUnwrapFailure final : public FailureDiagnostic {
565565
Type getUnwrappedType() const {
566566
return resolveType(UnwrappedType, /*reconstituteSugar=*/true);
567567
}
568+
569+
/// Suggest a default value via `?? <default value>`
570+
void offerDefaultValueUnwrapFixIt(DeclContext *DC, Expr *expr) const;
571+
/// Suggest a force optional unwrap via `!`
572+
void offerForceUnwrapFixIt(Expr *expr) const;
568573
};
569574

570575
/// Diagnose errors associated with rvalues in positions

0 commit comments

Comments
 (0)