Skip to content

Teach the VarDeclUsageChecker About Variables Bound in Patterns #33033

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 1 commit into from
Jul 21, 2020
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
29 changes: 27 additions & 2 deletions lib/Sema/MiscDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2743,7 +2743,7 @@ VarDeclUsageChecker::~VarDeclUsageChecker() {
// let x = foo()
// ->
// _ = foo()
if (auto *pbd = var->getParentPatternBinding())
if (auto *pbd = var->getParentPatternBinding()) {
if (pbd->getSingleVar() == var && pbd->getInit(0) != nullptr &&
!isa<TypedPattern>(pbd->getPattern(0))) {
unsigned varKind = var->isLet();
Expand All @@ -2755,7 +2755,8 @@ VarDeclUsageChecker::~VarDeclUsageChecker() {
.fixItReplace(replaceRange, "_");
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:
//
Expand Down Expand Up @@ -2808,6 +2809,30 @@ VarDeclUsageChecker::~VarDeclUsageChecker() {
}
}
}

// If the variable is defined in a pattern that isn't one of the usual
// conditional statements, try to detect and rewrite "simple" binding
// patterns:
// case .pattern(let x):
// ->
// case .pattern(_):
if (auto *pattern = var->getParentPattern()) {
BindingPattern *foundVP = nullptr;
pattern->forEachNode([&](Pattern *P) {
if (auto *VP = dyn_cast<BindingPattern>(P))
if (VP->getSingleVar() == var)
foundVP = VP;
});

if (foundVP) {
unsigned varKind = var->isLet();
Diags
.diagnose(var->getLoc(), diag::variable_never_used,
var->getName(), varKind)
.fixItReplace(foundVP->getSourceRange(), "_");
continue;
}
}

// Otherwise, this is something more complex, perhaps
// let (a,b) = foo()
Expand Down
2 changes: 1 addition & 1 deletion test/Constraints/patterns.swift
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ default:

// <rdar://problem/19382878> Introduce new x? pattern
switch Optional(42) {
case let x?: break // expected-warning{{immutable value 'x' was never used; consider replacing with '_' or removing it}}
case let x?: break // expected-warning{{immutable value 'x' was never used; consider replacing with '_' or removing it}} {{10-11=_}}
case nil: break
}

Expand Down
2 changes: 1 addition & 1 deletion test/Parse/switch.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func parseError4(x: Int) {

func parseError5(x: Int) {
switch x {
case let z // expected-error {{expected ':' after 'case'}} expected-warning {{immutable value 'z' was never used}} {{12-13=_}}
case let z // expected-error {{expected ':' after 'case'}} expected-warning {{immutable value 'z' was never used}} {{8-13=_}}
}
}

Expand Down
52 changes: 50 additions & 2 deletions test/decl/var/usage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -304,9 +304,9 @@ func testFixitsInStatementsWithPatterns(_ a : Int?) {

// <rdar://22774938> 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 true == true, let x = a { // expected-warning {{immutable value 'x' was never used; consider replacing with '_' or removing it}} {{20-25=_}}
}
if let x = a, let y = a { // expected-warning {{immutable value 'x' was never used; consider replacing with '_' or removing it}} {{10-11=_}}
if let x = a, let y = a { // expected-warning {{immutable value 'x' was never used; consider replacing with '_' or removing it}} {{6-11=_}}
_ = y
}

Expand Down Expand Up @@ -447,3 +447,51 @@ func testForwardReferenceCapture() {
innerFunc()
print(x)
}

// rdar://47240768 Expand the definition of "simple" pattern to variables bound in patterns
func testVariablesBoundInPatterns() {
enum StringB {
case simple(b: Bool)
case tuple(b: (Bool, Bool))
case optional(b: Bool?)
}

// Because Swift enables all kinds of creative binding forms, make sure that
// variable patterns occuring directly under a `let` or `var` have that
// introducer stripped by the fixit. All other cases are currently too
// complicated for the VarDeclUsageChecker.
switch StringB.simple(b: true) {
case .simple(b: let b) where false: // expected-warning {{immutable value 'b' was never used; consider replacing with '_' or removing it}} {{19-24=_}}
break
case .simple(b: var b) where false: // expected-warning {{variable 'b' was never used; consider replacing with '_' or removing it}} {{19-24=_}}
break
case var .simple(b: b): // expected-warning {{variable 'b' was never used; consider replacing with '_' or removing it}} {{23-24=_}}
break
case .tuple(b: let (b1, b2)) where false:
// expected-warning@-1 {{immutable value 'b1' was never used; consider replacing with '_' or removing it}} {{23-25=_}}
// expected-warning@-2 {{immutable value 'b2' was never used; consider replacing with '_' or removing it}} {{27-29=_}}
break
case .tuple(b: (let b1, let b2)) where false:
// expected-warning@-1 {{immutable value 'b1' was never used; consider replacing with '_' or removing it}} {{19-25=_}}
// expected-warning@-2 {{immutable value 'b2' was never used; consider replacing with '_' or removing it}} {{27-33=_}}
break
case .tuple(b: (var b1, var b2)) where false:
// expected-warning@-1 {{variable 'b1' was never used; consider replacing with '_' or removing it}} {{19-25=_}}
// expected-warning@-2 {{variable 'b2' was never used; consider replacing with '_' or removing it}} {{27-33=_}}
break
case var .tuple(b: (b1, b2)) where false:
// expected-warning@-1 {{variable 'b1' was never used; consider replacing with '_' or removing it}} {{23-25=_}}
// expected-warning@-2 {{variable 'b2' was never used; consider replacing with '_' or removing it}} {{27-29=_}}
break
case .tuple(b: let b): // expected-warning {{immutable value 'b' was never used; consider replacing with '_' or removing it}} {{18-23=_}}
break
case .optional(b: let x?) where false: // expected-warning {{immutable value 'x' was never used; consider replacing with '_' or removing it}} {{25-26=_}}
break
case .optional(b: let .some(x)) where false: // expected-warning {{immutable value 'x' was never used; consider replacing with '_' or removing it}} {{31-32=_}}
break
case let .optional(b: x?): // expected-warning {{immutable value 'x' was never used; consider replacing with '_' or removing it}} {{25-26=_}}
break
case let .optional(b: .none): // expected-warning {{'let' pattern has no effect; sub-pattern didn't bind any variables}} {{8-12=}}
break
}
}