Skip to content

Commit 149e57a

Browse files
xedinxedin
authored
Merge pull request #60893 from calda/cal--warn-redundant-comparison-to-optional.none-case
Add warning when comparing a non-optional value to `Optional.none`
2 parents 8f0dd56 + d9be4a7 commit 149e57a

File tree

5 files changed

+43
-13
lines changed

5 files changed

+43
-13
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4036,8 +4036,8 @@ WARNING(use_of_qq_on_non_optional_value,none,
40364036
"left side of nil coalescing operator '?""?' has non-optional type %0, "
40374037
"so the right side is never used", (Type))
40384038
WARNING(nonoptional_compare_to_nil,none,
4039-
"comparing non-optional value of type %0 to 'nil' always returns"
4040-
" %select{false|true}1", (Type, bool))
4039+
"comparing non-optional value of type %0 to '%select{Optional.none|nil}1' always returns"
4040+
" %select{false|true}2", (Type, bool, bool))
40414041
WARNING(optional_check_nonoptional,none,
40424042
"non-optional expression of type %0 used in a check for optionals",
40434043
(Type))

‎lib/Sema/MiscDiagnostics.cpp

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1306,9 +1306,12 @@ static void diagSyntacticUseRestrictions(const Expr *E, const DeclContext *DC,
13061306
/// Return true if this is a 'nil' literal. This looks
13071307
/// like this if the type is Optional<T>:
13081308
///
1309-
/// (dot_syntax_call_expr implicit type='Int?'
1310-
/// (declref_expr implicit decl=Optional.none)
1311-
/// (type_expr type=Int?))
1309+
/// (dot_syntax_call_expr type='String?'
1310+
/// (declref_expr type='(Optional<String>.Type) -> Optional<String>'
1311+
/// decl=Swift.(file).Optional.none function_ref=unapplied)
1312+
/// (argument_list implicit
1313+
/// (argument
1314+
/// (type_expr implicit type='String?.Type' typerepr='String?'))))
13121315
///
13131316
/// Or like this if it is any other ExpressibleByNilLiteral type:
13141317
///
@@ -1317,13 +1320,10 @@ static void diagSyntacticUseRestrictions(const Expr *E, const DeclContext *DC,
13171320
bool isTypeCheckedOptionalNil(Expr *E) {
13181321
if (dyn_cast<NilLiteralExpr>(E)) return true;
13191322

1320-
auto CE = dyn_cast<ApplyExpr>(E->getSemanticsProvidingExpr());
1321-
if (!CE || !CE->isImplicit())
1322-
return false;
1323-
1324-
// First case -- Optional.none
1325-
if (auto DRE = dyn_cast<DeclRefExpr>(CE->getSemanticFn()))
1326-
return DRE->getDecl() == Ctx.getOptionalNoneDecl();
1323+
if (auto *DSCE = dyn_cast_or_null<DotSyntaxCallExpr>(E->getSemanticsProvidingExpr())) {
1324+
if (auto *DRE = dyn_cast<DeclRefExpr>(DSCE->getSemanticFn()))
1325+
return DRE->getDecl() == Ctx.getOptionalNoneDecl();
1326+
}
13271327

13281328
return false;
13291329
}
@@ -1371,9 +1371,10 @@ static void diagSyntacticUseRestrictions(const Expr *E, const DeclContext *DC,
13711371
(isTypeCheckedOptionalNil(lhs) &&
13721372
(subExpr = isImplicitPromotionToOptional(rhs)))) {
13731373
bool isTrue = calleeName == "!=" || calleeName == "!==";
1374+
bool isNilLiteral = isa<NilLiteralExpr>(lhs) || isa<NilLiteralExpr>(rhs);
13741375

13751376
Ctx.Diags.diagnose(DRE->getLoc(), diag::nonoptional_compare_to_nil,
1376-
subExpr->getType(), isTrue)
1377+
subExpr->getType(), isNilLiteral, isTrue)
13771378
.highlight(lhs->getSourceRange())
13781379
.highlight(rhs->getSourceRange());
13791380
return;

‎test/Constraints/diagnostics.swift

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -746,9 +746,13 @@ class C_44203 {
746746
_ = (i === nil) // expected-error {{value of type 'Int?' cannot be compared by reference; did you mean to compare by value?}} {{12-15===}}
747747
_ = (bytes === nil) // expected-error {{type 'UnsafeMutablePointer<Int>' is not optional, value can never be nil}}
748748
_ = (self === nil) // expected-warning {{comparing non-optional value of type 'AnyObject' to 'nil' always returns false}}
749+
_ = (self === .none) // expected-warning {{comparing non-optional value of type 'AnyObject' to 'Optional.none' always returns false}}
750+
_ = (self === Optional.none) // expected-warning {{comparing non-optional value of type 'AnyObject' to 'Optional.none' always returns false}}
749751
_ = (i !== nil) // expected-error {{value of type 'Int?' cannot be compared by reference; did you mean to compare by value?}} {{12-15=!=}}
750752
_ = (bytes !== nil) // expected-error {{type 'UnsafeMutablePointer<Int>' is not optional, value can never be nil}}
751753
_ = (self !== nil) // expected-warning {{comparing non-optional value of type 'AnyObject' to 'nil' always returns true}}
754+
_ = (self !== .none) // expected-warning {{comparing non-optional value of type 'AnyObject' to 'Optional.none' always returns true}}
755+
_ = (self !== Optional.none) // expected-warning {{comparing non-optional value of type 'AnyObject' to 'Optional.none' always returns true}}
752756
}
753757
}
754758

@@ -757,6 +761,11 @@ func nilComparison(i: Int, o: AnyObject) {
757761
_ = nil == i // expected-warning {{comparing non-optional value of type 'Int' to 'nil' always returns false}}
758762
_ = i != nil // expected-warning {{comparing non-optional value of type 'Int' to 'nil' always returns true}}
759763
_ = nil != i // expected-warning {{comparing non-optional value of type 'Int' to 'nil' always returns true}}
764+
765+
_ = i == Optional.none // expected-warning {{comparing non-optional value of type 'Int' to 'Optional.none' always returns false}}
766+
_ = Optional.none == i // expected-warning {{comparing non-optional value of type 'Int' to 'Optional.none' always returns false}}
767+
_ = i != Optional.none // expected-warning {{comparing non-optional value of type 'Int' to 'Optional.none' always returns true}}
768+
_ = Optional.none != i // expected-warning {{comparing non-optional value of type 'Int' to 'Optional.none' always returns true}}
760769

761770
// FIXME(integers): uncomment these tests once the < is no longer ambiguous
762771
// _ = i < nil // _xpected-error {{type 'Int' is not optional, value can never be nil}}

‎test/expr/cast/nil_value_to_optional.swift

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ func markUsed<T>(_ t: T) {}
77

88
markUsed(t != nil) // expected-warning {{comparing non-optional value of type 'Bool' to 'nil' always returns true}}
99
markUsed(f != nil) // expected-warning {{comparing non-optional value of type 'Bool' to 'nil' always returns true}}
10+
markUsed(t != Optional.none) // expected-warning {{comparing non-optional value of type 'Bool' to 'Optional.none' always returns true}}
11+
markUsed(f != Optional.none) // expected-warning {{comparing non-optional value of type 'Bool' to 'Optional.none' always returns true}}
1012

1113
class C : Equatable {}
1214

@@ -16,6 +18,9 @@ func == (lhs: C, rhs: C) -> Bool {
1618

1719
func test(_ c: C) {
1820
if c == nil {} // expected-warning {{comparing non-optional value of type 'C' to 'nil' always returns false}}
21+
if c == .none {} // expected-warning {{comparing non-optional value of type 'C' to 'Optional.none' always returns false}}
22+
if c == Optional.none {} // expected-warning {{comparing non-optional value of type 'C' to 'Optional.none' always returns false}}
23+
if c == C?.none {} // expected-warning {{comparing non-optional value of type 'C' to 'Optional.none' always returns false}}
1924
}
2025

2126
class D {}
@@ -30,3 +35,13 @@ _ = dopt == nil
3035
_ = diuopt == nil
3136
_ = diuopt is ExpressibleByNilLiteral // expected-warning {{'is' test is always true}}
3237
_ = produceD() is ExpressibleByNilLiteral // expected-warning {{'is' test is always true}}
38+
39+
enum E {
40+
case none
41+
}
42+
func test(_ e: E) {
43+
_ = e == .none
44+
_ = e == E.none
45+
_ = e == Optional.none // expected-warning {{comparing non-optional value of type 'E' to 'Optional.none' always returns false}}
46+
_ = e == E?.none // expected-warning {{comparing non-optional value of type 'E' to 'Optional.none' always returns false}}
47+
}

‎test/expr/expressions.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -768,6 +768,11 @@ _ = nil == Int.self // expected-warning {{comparing non-optional value of type
768768
_ = Int.self != nil // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'nil' always returns true}}
769769
_ = nil != Int.self // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'nil' always returns true}}
770770

771+
_ = Int.self == .none // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'Optional.none' always returns false}}
772+
_ = .none == Int.self // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'Optional.none' always returns false}}
773+
_ = Int.self != .none // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'Optional.none' always returns true}}
774+
_ = .none != Int.self // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'Optional.none' always returns true}}
775+
771776
// <rdar://problem/19032294> Disallow postfix ? when not chaining
772777
func testOptionalChaining(_ a : Int?, b : Int!, c : Int??) {
773778
_ = a? // expected-error {{optional chain has no effect, expression already produces 'Int?'}} {{8-9=}}

0 commit comments

Comments
 (0)