Skip to content

Commit bce68fa

Browse files
author
Nathan Hawes
committed
[IDE][Refactoring] Handle 'callAsFunction' specially in syntactic rename.
This change makes us treat it exactly as we do 'init'. We don't allow renaming the base name, and don't fail if the basename doesn't match for calls. Also: - explicit init calls/references like `MyType.init(42)` are now reported with 'init' as a keywordBase range, rather than nothing. - cursor info no longer reports rename as available on init/callAsFunction calls without arguments, as there's nothing to rename in that case. - Improved detection of when a referenced function is a call (rather than reference) across syntactic rename, cursor-info, and indexing. Resolves rdar://problem/60340429
1 parent 4264b39 commit bce68fa

24 files changed

+441
-62
lines changed

include/swift/IDE/Refactoring.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,17 @@ collectAvailableRefactorings(SourceFile *SF, ResolvedCursorInfo CursorInfo,
141141
std::vector<RefactoringKind> &Scratch,
142142
bool ExcludeRename);
143143

144+
/// Stores information about the reference that rename availability is being
145+
/// queried on.
146+
struct RenameRefInfo {
147+
SourceFile *SF; ///< The source file containing the reference.
148+
SourceLoc Loc; ///< The reference's source location.
149+
bool IsArgLabel; ///< Whether Loc is on an arg label, rather than base name.
150+
};
151+
144152
ArrayRef<RenameAvailabiliyInfo>
145153
collectRenameAvailabilityInfo(const ValueDecl *VD,
154+
Optional<RenameRefInfo> RefInfo,
146155
std::vector<RenameAvailabiliyInfo> &Scratch);
147156

148157
} // namespace ide

include/swift/IDE/Utils.h

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ namespace swift {
4848
class Type;
4949
class Decl;
5050
class DeclContext;
51+
class CallExpr;
5152
class ClangNode;
5253
class ClangImporter;
5354
class Token;
@@ -226,6 +227,12 @@ struct ResolvedLoc {
226227
bool IsInSelector;
227228
};
228229

230+
/// Used by NameMatcher to track parent CallExprs when walking a checked AST.
231+
struct CallingParent {
232+
Expr *ApplicableTo;
233+
CallExpr *Call;
234+
};
235+
229236

230237
/// Finds the parse-only AST nodes and corresponding name and param/argument
231238
/// label ranges for a given list of input name start locations
@@ -244,6 +251,9 @@ class NameMatcher: public ASTWalker {
244251
unsigned InactiveConfigRegionNestings = 0;
245252
unsigned SelectorNestings = 0;
246253

254+
/// The stack of parent CallExprs and the innermost expression they apply to.
255+
std::vector<CallingParent> ParentCalls;
256+
247257
SourceManager &getSourceMgr() const;
248258

249259
SourceLoc nextLoc() const;
@@ -256,11 +266,11 @@ class NameMatcher: public ASTWalker {
256266
bool shouldSkip(SourceRange Range);
257267
bool shouldSkip(CharSourceRange Range);
258268
bool tryResolve(ASTWalker::ParentTy Node, SourceLoc NameLoc);
259-
bool tryResolve(ASTWalker::ParentTy Node, DeclNameLoc NameLoc, Expr *Arg,
260-
bool checkParentForLabels = false);
269+
bool tryResolve(ASTWalker::ParentTy Node, DeclNameLoc NameLoc, Expr *Arg);
261270
bool tryResolve(ASTWalker::ParentTy Node, SourceLoc NameLoc, LabelRangeType RangeType,
262271
ArrayRef<CharSourceRange> LabelLocs);
263272
bool handleCustomAttrs(Decl *D);
273+
Expr *getApplicableArgFor(Expr* E);
264274

265275
std::pair<bool, Expr*> walkToExprPre(Expr *E) override;
266276
Expr* walkToExprPost(Expr *E) override;
@@ -281,6 +291,7 @@ class NameMatcher: public ASTWalker {
281291
public:
282292
explicit NameMatcher(SourceFile &SrcFile) : SrcFile(SrcFile) { }
283293
std::vector<ResolvedLoc> resolve(ArrayRef<UnresolvedLoc> Locs, ArrayRef<Token> Tokens);
294+
ResolvedLoc resolve(UnresolvedLoc Loc);
284295
};
285296

286297
enum class RangeKind : int8_t {

lib/IDE/Refactoring.cpp

Lines changed: 77 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -316,21 +316,35 @@ class Renamer {
316316
// FIXME: handle escaped keyword names `init`
317317
bool IsSubscript = Old.base() == "subscript" && Config.IsFunctionLike;
318318
bool IsInit = Old.base() == "init" && Config.IsFunctionLike;
319-
bool IsKeywordBase = IsInit || IsSubscript;
319+
320+
// FIXME: this should only be treated specially for instance methods.
321+
bool IsCallAsFunction = Old.base() == "callAsFunction" &&
322+
Config.IsFunctionLike;
323+
324+
bool IsSpecialBase = IsInit || IsSubscript || IsCallAsFunction;
320325

321-
// Filter out non-semantic keyword basename locations with no labels.
326+
// Filter out non-semantic special basename locations with no labels.
322327
// We've already filtered out those in active code, so these are
323-
// any appearance of just 'init' or 'subscript' in strings, comments, and
324-
// inactive code.
325-
if (IsKeywordBase && (Config.Usage == NameUsage::Unknown &&
326-
Resolved.LabelType == LabelRangeType::None))
328+
// any appearance of just 'init', 'subscript', or 'callAsFunction' in
329+
// strings, comments, and inactive code.
330+
if (IsSpecialBase && (Config.Usage == NameUsage::Unknown &&
331+
Resolved.LabelType == LabelRangeType::None))
327332
return RegionType::Unmatched;
328333

329-
if (!Config.IsFunctionLike || !IsKeywordBase) {
334+
if (!Config.IsFunctionLike || !IsSpecialBase) {
330335
if (renameBase(Resolved.Range, RefactoringRangeKind::BaseName))
331336
return RegionType::Mismatch;
332337

333-
} else if (IsKeywordBase && Config.Usage == NameUsage::Definition) {
338+
} else if (IsInit || IsCallAsFunction) {
339+
if (renameBase(Resolved.Range, RefactoringRangeKind::KeywordBaseName)) {
340+
// The base name doesn't need to match (but may) for calls, but
341+
// it should for definitions and references.
342+
if (Config.Usage == NameUsage::Definition ||
343+
Config.Usage == NameUsage::Reference) {
344+
return RegionType::Mismatch;
345+
}
346+
}
347+
} else if (IsSubscript && Config.Usage == NameUsage::Definition) {
334348
if (renameBase(Resolved.Range, RefactoringRangeKind::KeywordBaseName))
335349
return RegionType::Mismatch;
336350
}
@@ -528,9 +542,11 @@ static const ValueDecl *getRelatedSystemDecl(const ValueDecl *VD) {
528542
return nullptr;
529543
}
530544

531-
static Optional<RefactoringKind> getAvailableRenameForDecl(const ValueDecl *VD) {
545+
static Optional<RefactoringKind>
546+
getAvailableRenameForDecl(const ValueDecl *VD,
547+
Optional<RenameRefInfo> RefInfo) {
532548
std::vector<RenameAvailabiliyInfo> Scratch;
533-
for (auto &Info : collectRenameAvailabilityInfo(VD, Scratch)) {
549+
for (auto &Info : collectRenameAvailabilityInfo(VD, RefInfo, Scratch)) {
534550
if (Info.AvailableKind == RenameAvailableKind::Available)
535551
return Info.Kind;
536552
}
@@ -744,15 +760,21 @@ bool RefactoringActionLocalRename::
744760
isApplicable(ResolvedCursorInfo CursorInfo, DiagnosticEngine &Diag) {
745761
if (CursorInfo.Kind != CursorInfoKind::ValueRef)
746762
return false;
747-
auto RenameOp = getAvailableRenameForDecl(CursorInfo.ValueD);
763+
764+
Optional<RenameRefInfo> RefInfo;
765+
if (CursorInfo.IsRef)
766+
RefInfo = {CursorInfo.SF, CursorInfo.Loc, CursorInfo.IsKeywordArgument};
767+
768+
auto RenameOp = getAvailableRenameForDecl(CursorInfo.ValueD, RefInfo);
748769
return RenameOp.hasValue() &&
749770
RenameOp.getValue() == RefactoringKind::LocalRename;
750771
}
751772

752-
static void analyzeRenameScope(ValueDecl *VD, DiagnosticEngine &Diags,
773+
static void analyzeRenameScope(ValueDecl *VD, Optional<RenameRefInfo> RefInfo,
774+
DiagnosticEngine &Diags,
753775
llvm::SmallVectorImpl<DeclContext *> &Scopes) {
754776
Scopes.clear();
755-
if (!getAvailableRenameForDecl(VD).hasValue()) {
777+
if (!getAvailableRenameForDecl(VD, RefInfo).hasValue()) {
756778
Diags.diagnose(SourceLoc(), diag::value_decl_no_loc, VD->getFullName());
757779
return;
758780
}
@@ -786,7 +808,12 @@ bool RefactoringActionLocalRename::performChange() {
786808
if (CursorInfo.isValid() && CursorInfo.ValueD) {
787809
ValueDecl *VD = CursorInfo.CtorTyRef ? CursorInfo.CtorTyRef : CursorInfo.ValueD;
788810
llvm::SmallVector<DeclContext *, 8> Scopes;
789-
analyzeRenameScope(VD, DiagEngine, Scopes);
811+
812+
Optional<RenameRefInfo> RefInfo;
813+
if (CursorInfo.IsRef)
814+
RefInfo = {CursorInfo.SF, CursorInfo.Loc, CursorInfo.IsKeywordArgument};
815+
816+
analyzeRenameScope(VD, RefInfo, DiagEngine, Scopes);
790817
if (Scopes.empty())
791818
return true;
792819
RenameRangeCollector rangeCollector(VD, PreferredName);
@@ -3623,9 +3650,11 @@ accept(SourceManager &SM, RegionType RegionType,
36233650
Impl.accept(SM, Range);
36243651
}
36253652
}
3653+
36263654
ArrayRef<RenameAvailabiliyInfo>
36273655
swift::ide::collectRenameAvailabilityInfo(const ValueDecl *VD,
3628-
std::vector<RenameAvailabiliyInfo> &Scratch) {
3656+
Optional<RenameRefInfo> RefInfo,
3657+
std::vector<RenameAvailabiliyInfo> &Scratch) {
36293658
RenameAvailableKind AvailKind = RenameAvailableKind::Available;
36303659
if (getRelatedSystemDecl(VD)){
36313660
AvailKind = RenameAvailableKind::Unavailable_system_symbol;
@@ -3650,6 +3679,30 @@ swift::ide::collectRenameAvailabilityInfo(const ValueDecl *VD,
36503679
if (auto CD = dyn_cast<ConstructorDecl>(VD)) {
36513680
if (!CD->getParameters()->size())
36523681
return Scratch;
3682+
3683+
if (RefInfo && !RefInfo->IsArgLabel) {
3684+
NameMatcher Matcher(*(RefInfo->SF));
3685+
auto Resolved = Matcher.resolve({RefInfo->Loc, /*ResolveArgs*/true});
3686+
if (Resolved.LabelRanges.empty())
3687+
return Scratch;
3688+
}
3689+
}
3690+
3691+
// Disallow renaming 'callAsFunction' method with no arguments.
3692+
if (auto FD = dyn_cast<FuncDecl>(VD)) {
3693+
// FIXME: syntactic rename can only decide by checking the spelling, not
3694+
// whether it's an instance method, so we do the same here for now.
3695+
if (FD->getBaseIdentifier() == FD->getASTContext().Id_callAsFunction) {
3696+
if (!FD->getParameters()->size())
3697+
return Scratch;
3698+
3699+
if (RefInfo && !RefInfo->IsArgLabel) {
3700+
NameMatcher Matcher(*(RefInfo->SF));
3701+
auto Resolved = Matcher.resolve({RefInfo->Loc, /*ResolveArgs*/true});
3702+
if (Resolved.LabelRanges.empty())
3703+
return Scratch;
3704+
}
3705+
}
36533706
}
36543707
}
36553708

@@ -3682,7 +3735,10 @@ collectAvailableRefactorings(SourceFile *SF,
36823735
case CursorInfoKind::ExprStart:
36833736
break;
36843737
case CursorInfoKind::ValueRef: {
3685-
auto RenameOp = getAvailableRenameForDecl(CursorInfo.ValueD);
3738+
Optional<RenameRefInfo> RefInfo;
3739+
if (CursorInfo.IsRef)
3740+
RefInfo = {CursorInfo.SF, CursorInfo.Loc, CursorInfo.IsKeywordArgument};
3741+
auto RenameOp = getAvailableRenameForDecl(CursorInfo.ValueD, RefInfo);
36863742
if (RenameOp.hasValue() &&
36873743
RenameOp.getValue() == RefactoringKind::GlobalRename)
36883744
AllKinds.push_back(RenameOp.getValue());
@@ -3919,8 +3975,12 @@ int swift::ide::findLocalRenameRanges(
39193975
return true;
39203976
}
39213977
ValueDecl *VD = CursorInfo.CtorTyRef ? CursorInfo.CtorTyRef : CursorInfo.ValueD;
3978+
Optional<RenameRefInfo> RefInfo;
3979+
if (CursorInfo.IsRef)
3980+
RefInfo = {CursorInfo.SF, CursorInfo.Loc, CursorInfo.IsKeywordArgument};
3981+
39223982
llvm::SmallVector<DeclContext *, 8> Scopes;
3923-
analyzeRenameScope(VD, Diags, Scopes);
3983+
analyzeRenameScope(VD, RefInfo, Diags, Scopes);
39243984
if (Scopes.empty())
39253985
return true;
39263986
RenameRangeCollector RangeCollector(VD, StringRef());

lib/IDE/SourceEntityWalker.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,10 +86,15 @@ class SemaAnnotator : public ASTWalker {
8686
bool shouldIgnore(Decl *D, bool &ShouldVisitChildren);
8787

8888
ValueDecl *extractDecl(Expr *Fn) const {
89+
Fn = Fn->getSemanticsProvidingExpr();
8990
if (auto *DRE = dyn_cast<DeclRefExpr>(Fn))
9091
return DRE->getDecl();
9192
if (auto ApplyE = dyn_cast<ApplyExpr>(Fn))
9293
return extractDecl(ApplyE->getFn());
94+
if (auto *ACE = dyn_cast<AutoClosureExpr>(Fn)) {
95+
if (auto *Unwrapped = ACE->getUnwrappedCurryThunkExpr())
96+
return extractDecl(Unwrapped);
97+
}
9398
return nullptr;
9499
}
95100
};

0 commit comments

Comments
 (0)