diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 36db72a8acf52..9c32d337738db 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -2612,10 +2612,17 @@ NOTE(availability_conformance_introduced_here, sema_avail, none, // Variable usage diagnostics //------------------------------------------------------------------------------ +WARNING(pbd_never_used_stmtcond, sema_varusage, none, + "value %0 was defined but never used; consider replacing " + "with boolean test", + (Identifier)) + WARNING(pbd_never_used, sema_varusage, none, "initialization of %select{variable|immutable value}1 %0 was never used" "; consider replacing with assignment to '_' or removing it", (Identifier, unsigned)) + + WARNING(capture_never_used, sema_varusage, none, "capture %0 was never used", (Identifier)) diff --git a/lib/Sema/MiscDiagnostics.cpp b/lib/Sema/MiscDiagnostics.cpp index ea8839fd63b50..b3bebad27edcd 100644 --- a/lib/Sema/MiscDiagnostics.cpp +++ b/lib/Sema/MiscDiagnostics.cpp @@ -1216,6 +1216,11 @@ class VarDeclUsageChecker : public ASTWalker { /// This is a mapping from an OpaqueValue to the expression that initialized /// it. llvm::SmallDenseMap OpaqueValueMap; + + /// This is a mapping from VarDecls to the if/while/guard statement that they + /// occur in, when they are in a pattern in a StmtCondition. + llvm::SmallDenseMap StmtConditionForVD; + bool sawError = false; VarDeclUsageChecker(const VarDeclUsageChecker &) = delete; @@ -1340,6 +1345,18 @@ class VarDeclUsageChecker : public ASTWalker { // for them. if (auto *ICS = dyn_cast(S)) handleIfConfig(ICS); + + // Keep track of an association between vardecls and the StmtCondition that + // they are bound in for IfStmt, GuardStmt, WhileStmt, etc. + if (auto LCS = dyn_cast(S)) { + for (auto &cond : LCS->getCond()) + if (auto pat = cond.getPatternOrNull()) { + pat->forEachVariable([&](VarDecl *VD) { + StmtConditionForVD[VD] = LCS; + }); + } + } + return { true, S }; } @@ -1409,6 +1426,60 @@ VarDeclUsageChecker::~VarDeclUsageChecker() { continue; } + // If the variable is defined in a pattern in an if/while/guard statement, + // see if we can produce a tuned fixit. When we have something like: + // + // if let x = { + // + // we prefer to rewrite it to: + // + // if != nil { + // + if (auto SC = StmtConditionForVD[var]) { + // We only handle the "if let" case right now, since it is vastly the + // most common situation that people run into. + if (SC->getCond().size() == 1) { + auto pattern = SC->getCond()[0].getPattern(); + if (auto OSP = dyn_cast(pattern)) + if (auto LP = dyn_cast(OSP->getSubPattern())) + if (isa(LP->getSubPattern())) { + auto initExpr = SC->getCond()[0].getInitializer(); + auto beforeExprLoc = + initExpr->getStartLoc().getAdvancedLocOrInvalid(-1); + if (beforeExprLoc.isValid()) { + unsigned noParens = initExpr->canAppendCallParentheses(); + + // If the subexpr is an "as?" cast, we can rewrite it to + // be an "is" test. + bool isIsTest = false; + if (isa(initExpr) && + !initExpr->isImplicit()) { + noParens = isIsTest = true; + } + + auto diagIF = TC.diagnose(var->getLoc(), + diag::pbd_never_used_stmtcond, + var->getName()); + auto introducerLoc = SC->getCond()[0].getIntroducerLoc(); + diagIF.fixItReplace(SourceRange(introducerLoc, beforeExprLoc), + &"("[noParens]); + + if (isIsTest) { + // If this was an "x as? T" check, rewrite it to "x is T". + auto CCE = cast(initExpr); + diagIF.fixItReplace(SourceRange(CCE->getLoc(), + CCE->getQuestionLoc()), + "is"); + } else { + diagIF.fixItInsertAfter(initExpr->getEndLoc(), + &") != nil"[noParens]); + } + continue; + } + } + } + } + // Otherwise, this is something more complex, perhaps // let (a,b) = foo() // Just rewrite the one variable with a _. diff --git a/test/decl/var/usage.swift b/test/decl/var/usage.swift index be07f83088546..5ce87625960e8 100644 --- a/test/decl/var/usage.swift +++ b/test/decl/var/usage.swift @@ -219,3 +219,28 @@ func testFixitsInStatementsWithPatterns(a : Int?) { _ = b } } + + +// QoI: "never used" in an "if let" should rewrite expression to use != nil +func test(a : Int?, b : Any) { + if true == true, let x = a { // expected-warning {{immutable value 'x' was never used; consider replacing with '_' or removing it}} {{24-25=_}} + } + if let x = a, y = a { // expected-warning {{immutable value 'x' was never used; consider replacing with '_' or removing it}} {{10-11=_}} + _ = y + } + + // Simple case, insert a comparison with nil. + if let x = a { // expected-warning {{value 'x' was defined but never used; consider replacing with boolean test}} {{6-14=}} {{15-15= != nil}} + } + + // General case, need to insert parentheses. + if let x = a ?? a {} // expected-warning {{value 'x' was defined but never used; consider replacing with boolean test}} {{6-14=(}} {{20-20=) != nil}} + + // Special case, we can turn this into an 'is' test. + if let x = b as? Int { // expected-warning {{value 'x' was defined but never used; consider replacing with boolean test}} {{6-14=}} {{16-19=is}} + } + + +} + +