Skip to content

[Diagnostics] Check whether missing conformance could be fixed by using .rawValue #36445

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 4 commits into from
Mar 17, 2021
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
20 changes: 20 additions & 0 deletions lib/Sema/CSDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6936,6 +6936,26 @@ void MissingRawRepresentableInitFailure::fixIt(
}
}

bool MissingRawValueFailure::diagnoseAsError() {
auto *locator = getLocator();

if (locator->isLastElement<LocatorPathElt::AnyRequirement>()) {
MissingConformanceFailure failure(getSolution(), locator,
{RawReprType, ExpectedType});

auto diagnosed = failure.diagnoseAsError();
if (!diagnosed)
return false;

auto note = emitDiagnostic(diag::note_remapped_type, ".rawValue");
fixIt(note);

return true;
}

return AbstractRawRepresentableFailure::diagnoseAsError();
}

void MissingRawValueFailure::fixIt(InFlightDiagnostic &diagnostic) const {
auto *E = getAsExpr(getAnchor());
if (!E)
Expand Down
2 changes: 2 additions & 0 deletions lib/Sema/CSDiagnostics.h
Original file line number Diff line number Diff line change
Expand Up @@ -2273,6 +2273,8 @@ class MissingRawValueFailure final : public AbstractRawRepresentableFailure {
Type getFromType() const override { return RawReprType; }
Type getToType() const override { return ExpectedType; }

bool diagnoseAsError() override;

private:
void fixIt(InFlightDiagnostic &diagnostic) const override;
};
Expand Down
29 changes: 27 additions & 2 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3243,7 +3243,14 @@ repairViaOptionalUnwrap(ConstraintSystem &cs, Type fromType, Type toType,
// behind itself which we can use to better understand
// how many levels of optionality have to be unwrapped.
if (auto *OEE = dyn_cast<OptionalEvaluationExpr>(anchor)) {
auto type = cs.getType(OEE->getSubExpr());
auto *subExpr = OEE->getSubExpr();

// First, let's check whether it has been determined that
// it was incorrect to use `?` in this position.
if (cs.hasFixFor(cs.getConstraintLocator(subExpr), FixKind::RemoveUnwrap))
return true;

auto type = cs.getType(subExpr);
// If the type of sub-expression is optional, type of the
// `OptionalEvaluationExpr` could be safely ignored because
// it doesn't add any type information.
Expand Down Expand Up @@ -6013,6 +6020,14 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyConformsToConstraint(
}
}

if (auto rawValue = isRawRepresentable(*this, type)) {
if (!rawValue->isTypeVariableOrMember() &&
TypeChecker::conformsToProtocol(rawValue, protocol, DC)) {
auto *fix = UseRawValue::create(*this, type, protocolTy, loc);
return recordFix(fix) ? SolutionKind::Error : SolutionKind::Solved;
}
}

auto anchor = locator.getAnchor();

if (isExpr<UnresolvedMemberExpr>(anchor) &&
Expand Down Expand Up @@ -10751,7 +10766,6 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
case FixKind::AddPropertyWrapperAttribute:
case FixKind::ExpandArrayIntoVarargs:
case FixKind::UseRawValue:
case FixKind::ExplicitlyConstructRawRepresentable:
case FixKind::SpecifyBaseTypeForContextualMember:
case FixKind::CoerceToCheckedCast:
case FixKind::SpecifyObjectLiteralTypeImport:
Expand All @@ -10772,6 +10786,17 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
return recordFix(fix) ? SolutionKind::Error : SolutionKind::Solved;
}

case FixKind::ExplicitlyConstructRawRepresentable: {
// Let's increase impact of this fix for binary operators because
// it's possible to get both `.rawValue` and construction fixes for
// different overloads of a binary operator and `.rawValue` is a
// better fix because raw representable has a failable constructor.
return recordFix(fix,
/*impact=*/isExpr<BinaryExpr>(locator.getAnchor()) ? 2 : 1)
? SolutionKind::Error
: SolutionKind::Solved;
}

case FixKind::TreatRValueAsLValue: {
unsigned impact = 1;
// If this is an attempt to use result of a function/subscript call as
Expand Down
1 change: 1 addition & 0 deletions test/ClangImporter/newtype_conformance.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ func testNewTypeWrapper(x: NSNotification.Name, y: NSNotification.Name) {
acceptEquatable(x)
acceptHashable(x)
acceptComparable(x) // expected-error {{global function 'acceptComparable' requires that 'NSNotification.Name' conform to 'Comparable'}}
// expected-note@-1 {{did you mean to use '.rawValue'?}} {{19-19=.rawValue}}

_ = x == y
_ = x != y
Expand Down
2 changes: 1 addition & 1 deletion test/Constraints/fixes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -359,5 +359,5 @@ func testKeyPathSubscriptArgFixes(_ fn: @escaping () -> Int) {

func sr12426(a: Any, _ str: String?) {
a == str // expected-error {{binary operator '==' cannot be applied to operands of type 'Any' and 'String?'}}
// expected-note@-1 {{overloads for '==' exist with these partially matching parameter lists: (CodingUserInfoKey, CodingUserInfoKey), (String, String)}}
// expected-note@-1 {{overloads for '==' exist with these partially matching parameter lists: (String, String)}}
}
5 changes: 3 additions & 2 deletions test/Constraints/optional.swift
Original file line number Diff line number Diff line change
Expand Up @@ -425,8 +425,9 @@ func test_force_unwrap_not_being_too_eager() {

// rdar://problem/57097401
func invalidOptionalChaining(a: Any) {
a == "="? // expected-error {{binary operator '==' cannot be applied to operands of type 'Any' and 'String?'}}
// expected-note@-1 {{overloads for '==' exist with these partially matching parameter lists: (CodingUserInfoKey, CodingUserInfoKey), (FloatingPointSign, FloatingPointSign), (String, String), (Unicode.CanonicalCombiningClass, Unicode.CanonicalCombiningClass)}}
a == "="? // expected-error {{cannot use optional chaining on non-optional value of type 'String'}}
// expected-error@-1 {{protocol 'Any' as a type cannot conform to 'Equatable'}}
// expected-note@-2 {{requirement from conditional conformance of 'Any?' to 'Equatable'}} expected-note@-2 {{only concrete types such as structs, enums and classes can conform to protocols}}
}

// SR-12309 - Force unwrapping 'nil' compiles without warning
Expand Down
30 changes: 30 additions & 0 deletions validation-test/Sema/SwiftUI/rdar75367157.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// RUN: %target-typecheck-verify-swift -target x86_64-apple-macosx10.15 -swift-version 5
// REQUIRES: objc_interop
// REQUIRES: OS=macosx

import SwiftUI

enum E : String {
case a
case b

var description: String { get { "" } }
}

struct S : View {
var test: E = .a

func check(_: String) -> Bool {}

var body: some View {
List {
if test != E.b.description { // expected-error {{cannot convert value of type 'E' to expected argument type 'String'}} {{14-14=.rawValue}}
EmptyView()
}

if (check(self.test)) { // expected-error {{cannot convert value of type 'E' to expected argument type 'String'}} {{26-26=.rawValue}}
Spacer()
}
}
}
}