Skip to content

Commit b1eccb5

Browse files
[Sema] Do not diagnose contextual type mismatches for malformed key path expressions (swiftlang#33230)
* [AST] Adding hasSingleInvalidComponent to key path expression * [Sema] Adding a new fix and failure to diagnose missing key path component * [Sema] Recording new fix for key path missing components and remove diagnose from pre-check * [tests] Adjusting key path missing component contextual tests * [Sema] Renaming missing component key path fix and failure * [Sema] Correcting comments typos
1 parent 1685b98 commit b1eccb5

File tree

9 files changed

+95
-9
lines changed

9 files changed

+95
-9
lines changed

include/swift/AST/Expr.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5657,7 +5657,13 @@ class KeyPathExpr : public Expr {
56575657
/// components from the argument array.
56585658
void resolveComponents(ASTContext &C,
56595659
ArrayRef<Component> resolvedComponents);
5660-
5660+
5661+
/// Indicates if the key path expression is composed by a single invalid
5662+
/// component. e.g. missing component `\Root`
5663+
bool hasSingleInvalidComponent() const {
5664+
return Components.size() == 1 && !Components.front().isValid();
5665+
}
5666+
56615667
/// Retrieve the string literal expression, which will be \c NULL prior to
56625668
/// type checking and a string literal after type checking for an
56635669
/// @objc key path.

lib/Sema/CSBindings.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1210,6 +1210,12 @@ bool TypeVariableBinding::attempt(ConstraintSystem &cs) const {
12101210
} else if (srcLocator->directlyAt<ObjectLiteralExpr>()) {
12111211
fix = SpecifyObjectLiteralTypeImport::create(cs, dstLocator);
12121212
} else if (srcLocator->isKeyPathRoot()) {
1213+
// If we recorded an invalid key path fix, let's skip this specify root
1214+
// type fix because it wouldn't produce a useful diagnostic.
1215+
auto *kpLocator = cs.getConstraintLocator(srcLocator->getAnchor());
1216+
if (cs.hasFixFor(kpLocator, FixKind::AllowKeyPathWithoutComponents))
1217+
return true;
1218+
12131219
fix = SpecifyKeyPathRootType::create(cs, dstLocator);
12141220
}
12151221

lib/Sema/CSDiagnostics.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6760,3 +6760,19 @@ void TrailingClosureRequiresExplicitLabel::fixIt(
67606760
diagnostic.fixItInsert(newRParenLoc,
67616761
isExpr<SubscriptExpr>(anchor) ? "]" : ")");
67626762
}
6763+
6764+
bool InvalidEmptyKeyPathFailure::diagnoseAsError() {
6765+
auto *KPE = getAsExpr<KeyPathExpr>(getAnchor());
6766+
assert(KPE && KPE->hasSingleInvalidComponent() &&
6767+
"Expected a malformed key path expression");
6768+
6769+
// If we have a string interpolation represented as key path expressions
6770+
// e.g. \(x), \(x, a: 1). Let's skip it because this would be already
6771+
// diagnosed and it is not the case for an extra empty key path diagnostic.
6772+
auto *root = KPE->getParsedRoot();
6773+
if (root && (isa<ParenExpr>(root) || isa<TupleExpr>(root)))
6774+
return true;
6775+
6776+
emitDiagnostic(diag::expr_swift_keypath_empty);
6777+
return true;
6778+
}

lib/Sema/CSDiagnostics.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2248,6 +2248,20 @@ class TrailingClosureRequiresExplicitLabel final : public FailureDiagnostic {
22482248
const FunctionArgApplyInfo &info) const;
22492249
};
22502250

2251+
/// Diagnose situations where we have a key path with no components.
2252+
///
2253+
/// \code
2254+
/// let _ : KeyPath<A, B> = \A
2255+
/// \endcode
2256+
class InvalidEmptyKeyPathFailure final : public FailureDiagnostic {
2257+
public:
2258+
InvalidEmptyKeyPathFailure(const Solution &solution,
2259+
ConstraintLocator *locator)
2260+
: FailureDiagnostic(solution, locator) {}
2261+
2262+
bool diagnoseAsError() override;
2263+
};
2264+
22512265
} // end namespace constraints
22522266
} // end namespace swift
22532267

lib/Sema/CSFix.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1539,3 +1539,15 @@ SpecifyLabelToAssociateTrailingClosure::create(ConstraintSystem &cs,
15391539
return new (cs.getAllocator())
15401540
SpecifyLabelToAssociateTrailingClosure(cs, locator);
15411541
}
1542+
1543+
bool AllowKeyPathWithoutComponents::diagnose(const Solution &solution,
1544+
bool asNote) const {
1545+
InvalidEmptyKeyPathFailure failure(solution, getLocator());
1546+
return failure.diagnose(asNote);
1547+
}
1548+
1549+
AllowKeyPathWithoutComponents *
1550+
AllowKeyPathWithoutComponents::create(ConstraintSystem &cs,
1551+
ConstraintLocator *locator) {
1552+
return new (cs.getAllocator()) AllowKeyPathWithoutComponents(cs, locator);
1553+
}

lib/Sema/CSFix.h

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,13 +266,16 @@ enum class FixKind : uint8_t {
266266

267267
/// Specify key path root type when it cannot be infered from context.
268268
SpecifyKeyPathRootType,
269-
269+
270270
/// Unwrap optional base on key path application.
271271
UnwrapOptionalBaseKeyPathApplication,
272272

273273
/// Explicitly specify a label to match trailing closure to a certain
274274
/// parameter in the call.
275275
SpecifyLabelToAssociateTrailingClosure,
276+
277+
/// Allow key path expressions with no components.
278+
AllowKeyPathWithoutComponents,
276279
};
277280

278281
class ConstraintFix {
@@ -1958,6 +1961,25 @@ class SpecifyLabelToAssociateTrailingClosure final : public ConstraintFix {
19581961
create(ConstraintSystem &cs, ConstraintLocator *locator);
19591962
};
19601963

1964+
/// Diagnose situations where we have a key path with no components.
1965+
///
1966+
/// \code
1967+
/// let _ : KeyPath<A, B> = \A
1968+
/// \endcode
1969+
class AllowKeyPathWithoutComponents final : public ConstraintFix {
1970+
AllowKeyPathWithoutComponents(ConstraintSystem &cs,
1971+
ConstraintLocator *locator)
1972+
: ConstraintFix(cs, FixKind::AllowKeyPathWithoutComponents, locator) {}
1973+
1974+
public:
1975+
std::string getName() const override { return "key path missing component"; }
1976+
1977+
bool diagnose(const Solution &solution, bool asNote = false) const override;
1978+
1979+
static AllowKeyPathWithoutComponents *create(ConstraintSystem &cs,
1980+
ConstraintLocator *locator);
1981+
};
1982+
19611983
} // end namespace constraints
19621984
} // end namespace swift
19631985

lib/Sema/CSSimplify.cpp

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8127,6 +8127,14 @@ ConstraintSystem::simplifyKeyPathConstraint(
81278127
if (keyPathTy->isHole())
81288128
return SolutionKind::Solved;
81298129

8130+
// If we have a malformed KeyPathExpr e.g. let _: KeyPath<A, C> = \A
8131+
// let's record a AllowKeyPathMissingComponent fix.
8132+
if (keyPath->hasSingleInvalidComponent()) {
8133+
auto *fix = AllowKeyPathWithoutComponents::create(
8134+
*this, getConstraintLocator(locator));
8135+
return recordFix(fix) ? SolutionKind::Error : SolutionKind::Solved;
8136+
}
8137+
81308138
// If the root type has been bound to a hole, we cannot infer it.
81318139
if (getFixedTypeRecursive(rootTy, /*wantRValue*/ true)->isHole())
81328140
return SolutionKind::Solved;
@@ -9988,7 +9996,8 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
99889996
case FixKind::UnwrapOptionalBaseKeyPathApplication:
99899997
case FixKind::AllowCoercionToForceCast:
99909998
case FixKind::SpecifyKeyPathRootType:
9991-
case FixKind::SpecifyLabelToAssociateTrailingClosure: {
9999+
case FixKind::SpecifyLabelToAssociateTrailingClosure:
10000+
case FixKind::AllowKeyPathWithoutComponents: {
999210001
return recordFix(fix) ? SolutionKind::Error : SolutionKind::Solved;
999310002
}
999410003

lib/Sema/TypeCheckConstraints.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1900,7 +1900,6 @@ void PreCheckExpression::resolveKeyPathExpr(KeyPathExpr *KPE) {
19001900

19011901
// Key paths must be spelled with at least one component.
19021902
if (components.empty()) {
1903-
DE.diagnose(KPE->getLoc(), diag::expr_swift_keypath_empty);
19041903
// Passes further down the pipeline expect keypaths to always have at least
19051904
// one component, so stuff an invalid component in the AST for recovery.
19061905
components.push_back(KeyPathExpr::Component());

test/expr/unary/keypath/keypath.swift

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ struct A: Hashable {
3636
func hash(into hasher: inout Hasher) { fatalError() }
3737
}
3838
struct B {}
39-
struct C<T> { // expected-note 3 {{'T' declared as parameter to type 'C'}}
39+
struct C<T> { // expected-note 4 {{'T' declared as parameter to type 'C'}}
4040
var value: T
4141
subscript() -> T { get { return value } }
4242
subscript(sub: Sub) -> T { get { return value } set { } }
@@ -224,15 +224,17 @@ func testKeyPathInGenericContext<H: Hashable, X>(hashable: H, anything: X) {
224224
}
225225

226226
func testDisembodiedStringInterpolation(x: Int) {
227-
\(x) // expected-error{{string interpolation}} expected-error{{}}
228-
\(x, radix: 16) // expected-error{{string interpolation}} expected-error{{}}
227+
\(x) // expected-error{{string interpolation can only appear inside a string literal}}
228+
\(x, radix: 16) // expected-error{{string interpolation can only appear inside a string literal}}
229229
}
230230

231231
func testNoComponents() {
232232
let _: KeyPath<A, A> = \A // expected-error{{must have at least one component}}
233-
let _: KeyPath<C, A> = \C // expected-error{{must have at least one component}} expected-error{{}}
233+
let _: KeyPath<C, A> = \C // expected-error{{must have at least one component}}
234234
// expected-error@-1 {{generic parameter 'T' could not be inferred}}
235-
// expected-error@-2 {{cannot convert value of type 'KeyPath<Root, Value>' to specified type 'KeyPath<C<T>, A>'}}
235+
let _: KeyPath<A, C> = \A // expected-error{{must have at least one component}}
236+
// expected-error@-1 {{generic parameter 'T' could not be inferred}}
237+
_ = \A // expected-error {{key path must have at least one component}}
236238
}
237239

238240
struct TupleStruct {

0 commit comments

Comments
 (0)