Skip to content

Commit db097aa

Browse files
rintarorintaro
committed
[CodeCompletion] Don't mark some undesirable imported default values
For ObjC NS_OPTIONS/NSDictionary parameters with some specific names, ClangImporter implicitly adds default values. However, that behavior is sometimes not really desirable. For example, for: -(void)addAttributes:(NSDictionary *)attrs; 'attrs' is defaulted, but calling 'addAttributes()' doesn't make any sense. In code-completion, consider such paramters non-defaulted. rdar://89051832 (cherry picked from commit 2b9ee76)
1 parent 43d5ddd commit db097aa

File tree

2 files changed

+142
-3
lines changed

2 files changed

+142
-3
lines changed

‎lib/IDE/CompletionLookup.cpp‎

Lines changed: 73 additions & 3 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:
@@ -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 &&

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