Skip to content

Commit 624c07b

Browse files
rintarorintaro
authored
Merge pull request #42200 from rintaro/ide-completion-rdar89051832
[CodeCompletion] Don't mark some undesirable imported default values
2 parents 2019212 + 2b9ee76 commit 624c07b

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
@@ -348,7 +348,7 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
348348

349349
static bool hasInterestingDefaultValue(const ParamDecl *param);
350350

351-
bool addItemWithoutDefaultArgs(const AbstractFunctionDecl *func);
351+
bool shouldAddItemWithoutDefaultArgs(const AbstractFunctionDecl *func);
352352

353353
/// Build argument patterns for calling. Returns \c true if any content was
354354
/// 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
@@ -873,19 +873,88 @@ void CompletionLookup::addVarDeclRef(const VarDecl *VD,
873873
Builder.addFlair(CodeCompletionFlairBit::ExpressionSpecific);
874874
}
875875

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

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

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

897-
bool CompletionLookup::addItemWithoutDefaultArgs(
966+
bool CompletionLookup::shouldAddItemWithoutDefaultArgs(
898967
const AbstractFunctionDecl *func) {
899968
if (!func || !Sink.addCallWithNoDefaultArgs)
900969
return false;
@@ -924,7 +993,8 @@ bool CompletionLookup::addCallArgumentPatterns(
924993
bool hasDefault = false;
925994
if (!declParams.empty()) {
926995
const ParamDecl *PD = declParams[i];
927-
hasDefault = PD->isDefaultArgument();
996+
hasDefault =
997+
PD->isDefaultArgument() && !isNonDesirableImportedDefaultArg(PD);
928998
// Skip default arguments if we're either not including them or they
929999
// aren't interesting
9301000
if (hasDefault &&
@@ -1189,7 +1259,7 @@ void CompletionLookup::addFunctionCallPattern(
11891259
if (isImplicitlyCurriedInstanceMethod) {
11901260
addPattern({AFD->getImplicitSelfDecl()}, /*includeDefaultArgs=*/true);
11911261
} else {
1192-
if (addItemWithoutDefaultArgs(AFD))
1262+
if (shouldAddItemWithoutDefaultArgs(AFD))
11931263
addPattern(AFD->getParameters()->getArray(),
11941264
/*includeDefaultArgs=*/false);
11951265
addPattern(AFD->getParameters()->getArray(),
@@ -1385,7 +1455,7 @@ void CompletionLookup::addMethodCall(const FuncDecl *FD,
13851455
if (trivialTrailingClosure)
13861456
addMethodImpl(/*includeDefaultArgs=*/false,
13871457
/*trivialTrailingClosure=*/true);
1388-
if (addItemWithoutDefaultArgs(FD))
1458+
if (shouldAddItemWithoutDefaultArgs(FD))
13891459
addMethodImpl(/*includeDefaultArgs=*/false);
13901460
addMethodImpl(/*includeDefaultArgs=*/true);
13911461
}
@@ -1475,7 +1545,7 @@ void CompletionLookup::addConstructorCall(const ConstructorDecl *CD,
14751545
}
14761546
};
14771547

1478-
if (ConstructorType && addItemWithoutDefaultArgs(CD))
1548+
if (ConstructorType && shouldAddItemWithoutDefaultArgs(CD))
14791549
addConstructorImpl(/*includeDefaultArgs=*/false);
14801550
addConstructorImpl();
14811551
}

‎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)