Skip to content

Commit f83af65

Browse files
DougGregorDougGregor
authored
Merge pull request #42396 from rintaro/5.7-ide-completion-rdar89051832
[5.7][CodeCompletion] Don't mark some undesirable imported default values
2 parents 3a920cd + db097aa commit f83af65

File tree

3 files changed

+147
-8
lines changed

3 files changed

+147
-8
lines changed

‎include/swift/IDE/CompletionLookup.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
356356

357357
static bool hasInterestingDefaultValue(const ParamDecl *param);
358358

359-
bool addItemWithoutDefaultArgs(const AbstractFunctionDecl *func);
359+
bool shouldAddItemWithoutDefaultArgs(const AbstractFunctionDecl *func);
360360

361361
/// Build argument patterns for calling. Returns \c true if any content was
362362
/// added to \p Builder. If \p declParams is non-empty, the size must match

‎lib/IDE/CompletionLookup.cpp‎

Lines changed: 77 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -872,19 +872,88 @@ void CompletionLookup::addVarDeclRef(const VarDecl *VD,
872872
Builder.addFlair(CodeCompletionFlairBit::ExpressionSpecific);
873873
}
874874

875+
/// Return whether \p param has a non-desirable default value for code
876+
/// completion.
877+
///
878+
/// 'ClangImporter::Implementation::inferDefaultArgument()' automatically adds
879+
/// default values for some parameters;
880+
/// * NS_OPTIONS enum type with the name '...Options'.
881+
/// * NSDictionary and labeled 'options', 'attributes', or 'userInfo'.
882+
///
883+
/// But sometimes, this behavior isn't really desirable. This function add a
884+
/// heuristic where if a parameter matches all the following condition, we
885+
/// consider the imported default value is _not_ desirable:
886+
/// * it is the first parameter,
887+
/// * it doesn't have an argument label, and
888+
/// * the imported function base name ends with those words
889+
/// For example, ClangImporter imports:
890+
///
891+
/// -(void)addAttributes:(NSDictionary *)attrs, options:(NSDictionary *)opts;
892+
///
893+
/// as:
894+
///
895+
/// func addAttributes(_ attrs: [AnyHashable:Any] = [:],
896+
/// options opts: [AnyHashable:Any] = [:])
897+
///
898+
/// In this case, we don't want 'attrs' defaulted because the function name have
899+
/// 'Attribute' in its name so calling 'value.addAttribute()' doesn't make
900+
/// sense, but we _do_ want to keep 'opts' defaulted.
901+
///
902+
/// Note that:
903+
///
904+
/// -(void)performWithOptions:(NSDictionary *) opts;
905+
///
906+
/// This doesn't match the condition because the base name of the function in
907+
/// Swift is 'peform':
908+
///
909+
/// func perform(options opts: [AnyHashable:Any] = [:])
910+
///
911+
bool isNonDesirableImportedDefaultArg(const ParamDecl *param) {
912+
auto kind = param->getDefaultArgumentKind();
913+
if (kind != DefaultArgumentKind::EmptyArray &&
914+
kind != DefaultArgumentKind::EmptyDictionary)
915+
return false;
916+
917+
if (!param->getArgumentName().empty())
918+
return false;
919+
920+
auto *func = dyn_cast<FuncDecl>(param->getDeclContext());
921+
if (!func->hasClangNode())
922+
return false;
923+
if (func->getParameters()->front() != param)
924+
return false;
925+
if (func->getBaseName().isSpecial())
926+
return false;
927+
928+
auto baseName = func->getBaseName().getIdentifier().str();
929+
switch (kind) {
930+
case DefaultArgumentKind::EmptyArray:
931+
return (baseName.endswith("Options"));
932+
case DefaultArgumentKind::EmptyDictionary:
933+
return (baseName.endswith("Options") || baseName.endswith("Attributes") ||
934+
baseName.endswith("UserInfo"));
935+
default:
936+
llvm_unreachable("unhandled DefaultArgumentKind");
937+
}
938+
}
939+
875940
bool CompletionLookup::hasInterestingDefaultValue(const ParamDecl *param) {
876941
if (!param)
877942
return false;
878943

879944
switch (param->getDefaultArgumentKind()) {
880945
case DefaultArgumentKind::Normal:
881946
case DefaultArgumentKind::NilLiteral:
882-
case DefaultArgumentKind::EmptyArray:
883-
case DefaultArgumentKind::EmptyDictionary:
884947
case DefaultArgumentKind::StoredProperty:
885948
case DefaultArgumentKind::Inherited:
886949
return true;
887950

951+
case DefaultArgumentKind::EmptyArray:
952+
case DefaultArgumentKind::EmptyDictionary:
953+
if (isNonDesirableImportedDefaultArg(param))
954+
return false;
955+
return true;
956+
888957
case DefaultArgumentKind::None:
889958
#define MAGIC_IDENTIFIER(NAME, STRING, SYNTAX_KIND) \
890959
case DefaultArgumentKind::NAME:
@@ -893,7 +962,7 @@ bool CompletionLookup::hasInterestingDefaultValue(const ParamDecl *param) {
893962
}
894963
}
895964

896-
bool CompletionLookup::addItemWithoutDefaultArgs(
965+
bool CompletionLookup::shouldAddItemWithoutDefaultArgs(
897966
const AbstractFunctionDecl *func) {
898967
if (!func || !Sink.addCallWithNoDefaultArgs)
899968
return false;
@@ -923,7 +992,8 @@ bool CompletionLookup::addCallArgumentPatterns(
923992
bool hasDefault = false;
924993
if (!declParams.empty()) {
925994
const ParamDecl *PD = declParams[i];
926-
hasDefault = PD->isDefaultArgument();
995+
hasDefault =
996+
PD->isDefaultArgument() && !isNonDesirableImportedDefaultArg(PD);
927997
// Skip default arguments if we're either not including them or they
928998
// aren't interesting
929999
if (hasDefault &&
@@ -1188,7 +1258,7 @@ void CompletionLookup::addFunctionCallPattern(
11881258
if (isImplicitlyCurriedInstanceMethod) {
11891259
addPattern({AFD->getImplicitSelfDecl()}, /*includeDefaultArgs=*/true);
11901260
} else {
1191-
if (addItemWithoutDefaultArgs(AFD))
1261+
if (shouldAddItemWithoutDefaultArgs(AFD))
11921262
addPattern(AFD->getParameters()->getArray(),
11931263
/*includeDefaultArgs=*/false);
11941264
addPattern(AFD->getParameters()->getArray(),
@@ -1384,7 +1454,7 @@ void CompletionLookup::addMethodCall(const FuncDecl *FD,
13841454
if (trivialTrailingClosure)
13851455
addMethodImpl(/*includeDefaultArgs=*/false,
13861456
/*trivialTrailingClosure=*/true);
1387-
if (addItemWithoutDefaultArgs(FD))
1457+
if (shouldAddItemWithoutDefaultArgs(FD))
13881458
addMethodImpl(/*includeDefaultArgs=*/false);
13891459
addMethodImpl(/*includeDefaultArgs=*/true);
13901460
}
@@ -1474,7 +1544,7 @@ void CompletionLookup::addConstructorCall(const ConstructorDecl *CD,
14741544
}
14751545
};
14761546

1477-
if (ConstructorType && addItemWithoutDefaultArgs(CD))
1547+
if (ConstructorType && shouldAddItemWithoutDefaultArgs(CD))
14781548
addConstructorImpl(/*includeDefaultArgs=*/false);
14791549
addConstructorImpl();
14801550
}

‎test/IDE/complete_default_arguments_rdar89051832.swift‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
2+
// RUN: %empty-directory(%t)
3+
// RUN: split-file %s %t
4+
5+
// RUN: %target-swift-ide-test(mock-sdk: %clang-importer-sdk) -batch-code-completion -source-filename %t/test.swift -filecheck %raw-FileCheck -completion-output-dir %t/out -code-completion-annotate-results -import-objc-header %t/ObjC.h -enable-objc-interop %t/Lib.swift
6+
// REQUIRES: objc_interop
7+
8+
9+
//--- ObjC.h
10+
@import Foundation;
11+
12+
typedef NS_OPTIONS(NSInteger, MyOptions) {
13+
MyOptOne = 1 << 0,
14+
MyOptTwo = 1 << 1,
15+
};
16+
17+
@interface MyObj : NSObject
18+
// 'opt' should not be defaulted.
19+
// FIXME: Currently this is considered defaulted because the base name is 'store'.
20+
- (void)storeOptions:(MyOptions)opts;
21+
22+
// 'opts' should not be defaulted.
23+
- (void)addOptions:(NSDictionary*)opts;
24+
25+
// 'attrs' should not be defaulted.
26+
- (void)addAttributes:(NSDictionary *)attrs;
27+
28+
// 'info' should not be defaulted but 'opts' should be.
29+
- (void)addUserInfo:(NSDictionary *)info options:(MyOptions)opts;
30+
31+
// 'opts' should be defaulted because the base name is 'run'.
32+
- (void)runWithOptions:(MyOptions)opts;
33+
34+
// 'attrs' should be defaulted because the base name is 'execute'.
35+
- (void)executeWithAttributes:(NSDictionary *)attrs;
36+
@end
37+
38+
//--- Lib.swift
39+
extension MyObj {
40+
// 'attrs' should not be defaulted because this is explicitly written in Swift.
41+
func swift_addAttributes(_ attrs : [AnyHashable:Any]! = [:]) {}
42+
}
43+
44+
//--- test.swift
45+
func test(value: MyObj) {
46+
value.#^COMPLETE^#
47+
// COMPLETE: Begin completions
48+
// COMPLETE-NOT: name=addOptions()
49+
// COMPLETE-NOT: name=addAttributes()
50+
51+
// FIXME: we don't want to suggest 'store()'.
52+
// COMPLETE-DAG: Decl[InstanceMethod]/CurrNominal: <name>store</name>(); typename=<typeid.sys>Void</typeid.sys>; name=store()
53+
// COMPLETE-DAG: Decl[InstanceMethod]/CurrNominal: <name>store</name>(<callarg><callarg.label>_</callarg.label> <callarg.param>opts</callarg.param>: <callarg.type><typeid.user>MyOptions</typeid.user></callarg.type><callarg.default/></callarg>); typename=<typeid.sys>Void</typeid.sys>; name=store(:)
54+
// COMPLETE-DAG: Decl[InstanceMethod]/CurrNominal: <name>addOptions</name>(<callarg><callarg.label>_</callarg.label> <callarg.param>opts</callarg.param>: <callarg.type>[<typeid.sys>AnyHashable</typeid.sys> : <keyword>Any</keyword>]!</callarg.type></callarg>); typename=<typeid.sys>Void</typeid.sys>; name=addOptions(:)
55+
// COMPLETE-DAG: Decl[InstanceMethod]/CurrNominal: <name>addAttributes</name>(<callarg><callarg.label>_</callarg.label> <callarg.param>attrs</callarg.param>: <callarg.type>[<typeid.sys>AnyHashable</typeid.sys> : <keyword>Any</keyword>]!</callarg.type></callarg>); typename=<typeid.sys>Void</typeid.sys>; name=addAttributes(:)
56+
// COMPLETE-DAG: Decl[InstanceMethod]/CurrNominal: <name>addUserInfo</name>(<callarg><callarg.label>_</callarg.label> <callarg.param>info</callarg.param>: <callarg.type>[<typeid.sys>AnyHashable</typeid.sys> : <keyword>Any</keyword>]!</callarg.type></callarg>); typename=<typeid.sys>Void</typeid.sys>; name=addUserInfo(:)
57+
// COMPLETE-DAG: Decl[InstanceMethod]/CurrNominal: <name>addUserInfo</name>(<callarg><callarg.label>_</callarg.label> <callarg.param>info</callarg.param>: <callarg.type>[<typeid.sys>AnyHashable</typeid.sys> : <keyword>Any</keyword>]!</callarg.type></callarg>, <callarg><callarg.label>options</callarg.label> <callarg.param>opts</callarg.param>: <callarg.type><typeid.user>MyOptions</typeid.user></callarg.type><callarg.default/></callarg>); typename=<typeid.sys>Void</typeid.sys>; name=addUserInfo(:options:)
58+
// COMPLETE-DAG: Decl[InstanceMethod]/CurrNominal: <name>run</name>(); typename=<typeid.sys>Void</typeid.sys>; name=run()
59+
// COMPLETE-DAG: Decl[InstanceMethod]/CurrNominal: <name>run</name>(<callarg><callarg.label>options</callarg.label> <callarg.param>opts</callarg.param>: <callarg.type><typeid.user>MyOptions</typeid.user></callarg.type><callarg.default/></callarg>); typename=<typeid.sys>Void</typeid.sys>; name=run(options:)
60+
// COMPLETE-DAG: Decl[InstanceMethod]/CurrNominal: <name>execute</name>(); typename=<typeid.sys>Void</typeid.sys>; name=execute()
61+
// COMPLETE-DAG: Decl[InstanceMethod]/CurrNominal: <name>execute</name>(<callarg><callarg.label>attributes</callarg.label> <callarg.param>attrs</callarg.param>: <callarg.type>[<typeid.sys>AnyHashable</typeid.sys> : <keyword>Any</keyword>]!</callarg.type><callarg.default/></callarg>); typename=<typeid.sys>Void</typeid.sys>; name=execute(attributes:)
62+
// COMPLETE-DAG: Decl[InstanceMethod]/CurrNominal: <name>swift_addAttributes</name>(); typename=<typeid.sys>Void</typeid.sys>; name=swift_addAttributes()
63+
// COMPLETE-DAG: Decl[InstanceMethod]/CurrNominal: <name>swift_addAttributes</name>(<callarg><callarg.label>_</callarg.label> <callarg.param>attrs</callarg.param>: <callarg.type>[<typeid.sys>AnyHashable</typeid.sys> : <keyword>Any</keyword>]!</callarg.type><callarg.default/></callarg>); typename=<typeid.sys>Void</typeid.sys>; name=swift_addAttributes(:)
64+
65+
// COMPLETE-NOT: name=addOptions()
66+
// COMPLETE-NOT: name=addAttributes()
67+
// COMPLETE: End completions
68+
}
69+

0 commit comments

Comments
 (0)