Skip to content

Commit 6e657e8

Browse files
author
Nathan Hawes
committed
[CodeCompletion] Fix incorrect type relations in complete_enum_elements test.
We were reporting methods that return function types that return void (rather than returning void directly) as being invalid in contexts that expect non-void expressions and testing for that incorrect behavior.
1 parent 8d5c94f commit 6e657e8

7 files changed

+52
-28
lines changed

‎include/swift/IDE/CodeCompletion.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -870,7 +870,7 @@ class CodeCompletionContext {
870870
/// but should not hide any results.
871871
SingleExpressionBody,
872872

873-
/// There are known contextual types.
873+
/// There are known contextual types, or there aren't but a nonvoid type is expected.
874874
Required,
875875
};
876876

‎include/swift/Sema/IDETypeChecking.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ namespace swift {
5858
class CompletionCollector {
5959
public:
6060
virtual void sawSolution(const constraints::Solution &solution) = 0;
61-
virtual bool isApplicable(Expr *E) { return false; }
6261
virtual ~CompletionCollector() {}
6362
};
6463

@@ -68,8 +67,9 @@ namespace swift {
6867
Type Ty;
6968
ValueDecl* ReferencedDecl;
7069
SmallVector<Type, 4> ExpectedTypes;
70+
bool ExpectsNonVoid;
7171
bool BaseIsStaticMetaType;
72-
bool isSingleExpressionClosure;
72+
bool IsSingleExpressionClosure;
7373
};
7474

7575
SourceLoc DotLoc;

‎lib/IDE/CodeCompletion.cpp

+23-9
Original file line numberDiff line numberDiff line change
@@ -1162,7 +1162,7 @@ static CodeCompletionResult::ExpectedTypeRelation calculateTypeRelation(
11621162
static CodeCompletionResult::ExpectedTypeRelation
11631163
calculateTypeRelationForDecl(const Decl *D, Type ExpectedType,
11641164
bool IsImplicitlyCurriedInstanceMethod,
1165-
bool UseFuncResultType = true) {
1165+
bool UseFuncResultType){
11661166
auto VD = dyn_cast<ValueDecl>(D);
11671167
auto DC = D->getDeclContext();
11681168
if (!VD)
@@ -1197,9 +1197,9 @@ calculateTypeRelationForDecl(const Decl *D, Type ExpectedType,
11971197
}
11981198

11991199
static CodeCompletionResult::ExpectedTypeRelation
1200-
calculateMaxTypeRelationForDecl(
1201-
const Decl *D, const ExpectedTypeContext &typeContext,
1202-
bool IsImplicitlyCurriedInstanceMethod = false) {
1200+
calculateMaxTypeRelationForDecl(const Decl *D,
1201+
const ExpectedTypeContext &typeContext,
1202+
bool IsImplicitlyCurriedInstanceMethod) {
12031203
if (typeContext.empty())
12041204
return CodeCompletionResult::ExpectedTypeRelation::Unknown;
12051205

@@ -1218,7 +1218,8 @@ calculateMaxTypeRelationForDecl(
12181218
continue;
12191219

12201220
Result = std::max(Result, calculateTypeRelationForDecl(
1221-
D, Type, IsImplicitlyCurriedInstanceMethod));
1221+
D, Type, IsImplicitlyCurriedInstanceMethod,
1222+
/*UseFuncResultTy*/true));
12221223

12231224
// Map invalid -> unrelated when in a single-expression body, since the
12241225
// input may be incomplete.
@@ -1334,9 +1335,12 @@ CodeCompletionResult *CodeCompletionResultBuilder::takeResult() {
13341335
}
13351336

13361337
auto typeRelation = ExpectedTypeRelation;
1338+
// FIXME: we don't actually have enough info to compute
1339+
// IsImplicitlyCurriedInstanceMethod here.
13371340
if (typeRelation == CodeCompletionResult::Unknown)
13381341
typeRelation =
1339-
calculateMaxTypeRelationForDecl(AssociatedDecl, declTypeContext);
1342+
calculateMaxTypeRelationForDecl(AssociatedDecl, declTypeContext,
1343+
/*IsImplicitlyCurriedMethod*/false);
13401344

13411345
return new (*Sink.Allocator) CodeCompletionResult(
13421346
SemanticContext, NumBytesToErase, CCS, AssociatedDecl, ModuleName,
@@ -2012,8 +2016,10 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
20122016
IsStaticMetatype = value;
20132017
}
20142018

2015-
void setExpectedTypes(ArrayRef<Type> Types, bool isSingleExpressionBody) {
2019+
void setExpectedTypes(ArrayRef<Type> Types, bool isSingleExpressionBody,
2020+
bool preferNonVoid = false) {
20162021
expectedTypeContext.isSingleExpressionBody = isSingleExpressionBody;
2022+
expectedTypeContext.preferNonVoid = preferNonVoid;
20172023
expectedTypeContext.possibleTypes.clear();
20182024
expectedTypeContext.possibleTypes.reserve(Types.size());
20192025
for (auto T : Types)
@@ -2026,7 +2032,7 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
20262032
}
20272033

20282034
CodeCompletionContext::TypeContextKind typeContextKind() const {
2029-
if (expectedTypeContext.empty()) {
2035+
if (expectedTypeContext.empty() && !expectedTypeContext.preferNonVoid) {
20302036
return CodeCompletionContext::TypeContextKind::None;
20312037
} else if (expectedTypeContext.isSingleExpressionBody) {
20322038
return CodeCompletionContext::TypeContextKind::SingleExpressionBody;
@@ -3044,6 +3050,12 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
30443050

30453051
if (isUnresolvedMemberIdealType(ResultType))
30463052
Builder.setSemanticContext(SemanticContextKind::ExpressionSpecific);
3053+
3054+
if (!IsImplicitlyCurriedInstanceMethod &&
3055+
expectedTypeContext.requiresNonVoid() &&
3056+
ResultType->isVoid()) {
3057+
Builder.setExpectedTypeRelation(CodeCompletionResult::Invalid);
3058+
}
30473059
};
30483060

30493061
if (!AFT || IsImplicitlyCurriedInstanceMethod) {
@@ -5924,7 +5936,9 @@ void DotExprLookup::performLookup(ide::CodeCompletionContext &CompletionCtx,
59245936
for (auto &Solution: Solutions) {
59255937
Lookup.setIsStaticMetatype(Solution.BaseIsStaticMetaType);
59265938
Lookup.getPostfixKeywordCompletions(Solution.Ty, BaseExpr);
5927-
Lookup.setExpectedTypes(Solution.ExpectedTypes, Solution.isSingleExpressionClosure);
5939+
Lookup.setExpectedTypes(Solution.ExpectedTypes,
5940+
Solution.IsSingleExpressionClosure,
5941+
Solution.ExpectsNonVoid);
59285942
if (isDynamicLookup(Solution.Ty))
59295943
Lookup.setIsDynamicLookup();
59305944
Lookup.getValueExprCompletions(Solution.Ty, Solution.ReferencedDecl);

‎lib/IDE/CodeCompletionResultBuilder.h

+12
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,20 @@ struct ExpectedTypeContext {
4949
/// Since the input may be incomplete, we take into account that the types are
5050
/// only a hint.
5151
bool isSingleExpressionBody = false;
52+
bool preferNonVoid = false;
5253

5354
bool empty() const { return possibleTypes.empty(); }
55+
bool requiresNonVoid() const {
56+
if (isSingleExpressionBody)
57+
return false;
58+
if (preferNonVoid)
59+
return true;
60+
if (possibleTypes.empty())
61+
return false;
62+
return std::all_of(possibleTypes.begin(), possibleTypes.end(), [](Type Ty) {
63+
return !Ty->isVoid();
64+
});
65+
}
5466

5567
ExpectedTypeContext() = default;
5668
ExpectedTypeContext(ArrayRef<Type> types, bool isSingleExpressionBody)

‎lib/Sema/TypeCheckCodeCompletion.cpp

+9-2
Original file line numberDiff line numberDiff line change
@@ -884,8 +884,11 @@ void DotExprLookup::sawSolution(const constraints::Solution &S) {
884884
if (Type BaseTy = GetType(ParsedExpr)) {
885885
auto *Locator = CS.getConstraintLocator(SemanticExpr);
886886
Type ExpectedTy = GetType(CompletionExpr);
887-
if (!CS.getParentExpr(CompletionExpr))
887+
bool DisallowVoid = true;
888+
if (!CS.getParentExpr(CompletionExpr)) {
888889
ExpectedTy = CS.getContextualType(CompletionExpr);
890+
DisallowVoid = CS.getContextualTypePurpose(CompletionExpr) != CTP_Unused;
891+
}
889892

890893
auto *CalleeLocator = S.getCalleeLocator(Locator);
891894
ValueDecl *ReferencedDecl = nullptr;
@@ -904,7 +907,11 @@ void DotExprLookup::sawSolution(const constraints::Solution &S) {
904907
ISEC = Parent->getSingleExpressionBody() == CompletionExpr;
905908
}
906909
}
907-
Solutions.push_back({BaseTy, ReferencedDecl, {ExpectedTy}, ISDMT, ISEC});
910+
Solutions.push_back({
911+
BaseTy, ReferencedDecl, {}, DisallowVoid, ISDMT, ISEC
912+
});
913+
if (ExpectedTy)
914+
Solutions.back().ExpectedTypes.push_back(ExpectedTy);
908915
}
909916
}
910917
}

‎lib/Sema/TypeCheckStmt.cpp

-9
Original file line numberDiff line numberDiff line change
@@ -1608,15 +1608,6 @@ void StmtChecker::typeCheckASTNode(ASTNode &node) {
16081608
options |= TypeCheckExprFlags::AllowUnresolvedTypeVariables;
16091609
}
16101610

1611-
// if (ctx.CompletionCallback) {
1612-
// TypeChecker::typeCheckForCodeCompletion(E, DC, Type(), CTP_Unused,
1613-
// [&](const constraints::Solution &solution) {
1614-
// ctx.CompletionCallback->sawSolution(solution);
1615-
// });
1616-
// node = E;
1617-
// return;
1618-
// }
1619-
16201611
auto resultTy =
16211612
TypeChecker::typeCheckExpression(E, DC, Type(), CTP_Unused, options);
16221613

‎test/IDE/complete_enum_elements.swift

+5-5
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ enum FooEnum: CaseIterable {
133133
// FOO_ENUM_DOT_INVALID-NEXT: Decl[EnumElement]/CurrNominal: Foo1[#FooEnum#]{{; name=.+$}}
134134
// FOO_ENUM_DOT_INVALID-NEXT: Decl[EnumElement]/CurrNominal: Foo2[#FooEnum#]{{; name=.+$}}
135135
// FOO_ENUM_DOT_INVALID-NEXT: Decl[StaticVar]/CurrNominal: alias1[#FooEnum#]{{; name=.+$}}
136-
// FOO_ENUM_DOT_INVALID-NEXT: Decl[InstanceMethod]/CurrNominal/TypeRelation[Invalid]: hash({#(self): FooEnum#})[#(into: inout Hasher) -> Void#]{{; name=.+$}}
136+
// FOO_ENUM_DOT_INVALID-NEXT: Decl[InstanceMethod]/CurrNominal: hash({#(self): FooEnum#})[#(into: inout Hasher) -> Void#]{{; name=.+$}}
137137
// FOO_ENUM_DOT_INVALID-NEXT: Decl[TypeAlias]/CurrNominal: AllCases[#[FooEnum]#]{{; name=.+$}}
138138
// FOO_ENUM_DOT_INVALID-NEXT: Decl[StaticVar]/CurrNominal: allCases[#[FooEnum]#]{{; name=.+$}}
139139
// FOO_ENUM_DOT_INVALID-NEXT: End completions
@@ -224,7 +224,7 @@ enum BarEnum {
224224
// BAR_ENUM_DOT-NEXT: Decl[EnumElement]/CurrNominal: Bar10({#Int#}, {#Float#})[#BarEnum#]{{; name=.+$}}
225225
// BAR_ENUM_DOT-NEXT: Decl[EnumElement]/CurrNominal: Bar11({#Int#}, {#(Float)#})[#BarEnum#]{{; name=.+$}}
226226
// BAR_ENUM_DOT-NEXT: Decl[EnumElement]/CurrNominal: Bar12({#Int#}, {#(Float, Double)#})[#BarEnum#]{{; name=.+$}}
227-
// BAR_ENUM_DOT-NEXT: Decl[InstanceMethod]/CurrNominal/TypeRelation[Invalid]: barInstanceFunc({#(self): &BarEnum#})[#() -> Void#]{{; name=.+$}}
227+
// BAR_ENUM_DOT-NEXT: Decl[InstanceMethod]/CurrNominal: barInstanceFunc({#(self): &BarEnum#})[#() -> Void#]{{; name=.+$}}
228228
// BAR_ENUM_DOT-NEXT: Decl[StaticVar]/CurrNominal: staticVar[#Int#]{{; name=.+$}}
229229
// BAR_ENUM_DOT-NEXT: Decl[StaticMethod]/CurrNominal/TypeRelation[Invalid]: barStaticFunc()[#Void#]{{; name=.+$}}
230230
// BAR_ENUM_DOT-NEXT: End completions
@@ -272,7 +272,7 @@ enum BazEnum<T> {
272272
// BAZ_INT_ENUM_DOT-NEXT: Keyword/CurrNominal: Type[#BazEnum<Int>.Type#]; name=Type
273273
// BAZ_INT_ENUM_DOT-NEXT: Decl[EnumElement]/CurrNominal: Baz1[#BazEnum<Int>#]{{; name=.+$}}
274274
// BAZ_INT_ENUM_DOT-NEXT: Decl[EnumElement]/CurrNominal: Baz2({#Int#})[#BazEnum<Int>#]{{; name=.+$}}
275-
// BAZ_INT_ENUM_DOT-NEXT: Decl[InstanceMethod]/CurrNominal/TypeRelation[Invalid]: bazInstanceFunc({#(self): &BazEnum<Int>#})[#() -> Void#]{{; name=.+$}}
275+
// BAZ_INT_ENUM_DOT-NEXT: Decl[InstanceMethod]/CurrNominal: bazInstanceFunc({#(self): &BazEnum<Int>#})[#() -> Void#]{{; name=.+$}}
276276
// BAZ_INT_ENUM_DOT-NEXT: Decl[StaticVar]/CurrNominal: staticVar[#Int#]{{; name=.+$}}
277277
// BAZ_INT_ENUM_DOT-NEXT: Decl[StaticVar]/CurrNominal: staticVarT[#Int#]{{; name=.+$}}
278278
// BAZ_INT_ENUM_DOT-NEXT: Decl[StaticMethod]/CurrNominal/TypeRelation[Invalid]: bazStaticFunc()[#Void#]{{; name=.+$}}
@@ -286,7 +286,7 @@ enum BazEnum<T> {
286286
// BAZ_T_ENUM_DOT-NEXT: Decl[InstanceMethod]/CurrNominal: bazInstanceFunc({#(self): &BazEnum<_>#})[#() -> Void#]{{; name=.+$}}
287287
// BAZ_T_ENUM_DOT-NEXT: Decl[StaticVar]/CurrNominal: staticVar[#Int#]{{; name=.+$}}
288288
// BAZ_T_ENUM_DOT-NEXT: Decl[StaticVar]/CurrNominal: staticVarT[#_#]{{; name=.+$}}
289-
// BAZ_T_ENUM_DOT-NEXT: Decl[StaticMethod]/CurrNominal: bazStaticFunc()[#Void#]{{; name=.+$}}
289+
// BAZ_T_ENUM_DOT-NEXT: Decl[StaticMethod]/CurrNominal/TypeRelation[Invalid]: bazStaticFunc()[#Void#]{{; name=.+$}}
290290
// BAZ_T_ENUM_DOT-NEXT: End completions
291291

292292
enum QuxEnum : Int {
@@ -316,7 +316,7 @@ enum QuxEnum : Int {
316316
// QUX_ENUM_DOT-NEXT: Decl[EnumElement]/CurrNominal: Qux2[#QuxEnum#]{{; name=.+$}}
317317
// QUX_ENUM_DOT-NEXT: Decl[TypeAlias]/CurrNominal: RawValue[#Int#]{{; name=.+$}}
318318
// QUX_ENUM_DOT-NEXT: Decl[Constructor]/CurrNominal: init({#rawValue: Int#})[#QuxEnum?#]{{; name=.+$}}
319-
// QUX_ENUM_DOT-NEXT: Decl[InstanceMethod]/Super/IsSystem/TypeRelation[Invalid]: hash({#(self): QuxEnum#})[#(into: inout Hasher) -> Void#]{{; name=.+$}}
319+
// QUX_ENUM_DOT-NEXT: Decl[InstanceMethod]/Super/IsSystem: hash({#(self): QuxEnum#})[#(into: inout Hasher) -> Void#]{{; name=.+$}}
320320
// QUX_ENUM_DOT-NEXT: End completions
321321

322322
func freeFunc() {}

0 commit comments

Comments
 (0)