Skip to content

[IDE][Refactoring] Handle 'callAsFunction' specially in syntactic rename #30964

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions include/swift/IDE/Refactoring.h
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,17 @@ collectAvailableRefactorings(SourceFile *SF, ResolvedCursorInfo CursorInfo,
std::vector<RefactoringKind> &Scratch,
bool ExcludeRename);

/// Stores information about the reference that rename availability is being
/// queried on.
struct RenameRefInfo {
SourceFile *SF; ///< The source file containing the reference.
SourceLoc Loc; ///< The reference's source location.
bool IsArgLabel; ///< Whether Loc is on an arg label, rather than base name.
};

ArrayRef<RenameAvailabiliyInfo>
collectRenameAvailabilityInfo(const ValueDecl *VD,
Optional<RenameRefInfo> RefInfo,
std::vector<RenameAvailabiliyInfo> &Scratch);

} // namespace ide
Expand Down
15 changes: 13 additions & 2 deletions include/swift/IDE/Utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ namespace swift {
class Type;
class Decl;
class DeclContext;
class CallExpr;
class ClangNode;
class ClangImporter;
class Token;
Expand Down Expand Up @@ -226,6 +227,12 @@ struct ResolvedLoc {
bool IsInSelector;
};

/// Used by NameMatcher to track parent CallExprs when walking a checked AST.
struct CallingParent {
Expr *ApplicableTo;
CallExpr *Call;
};


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

/// The stack of parent CallExprs and the innermost expression they apply to.
std::vector<CallingParent> ParentCalls;

SourceManager &getSourceMgr() const;

SourceLoc nextLoc() const;
Expand All @@ -256,11 +266,11 @@ class NameMatcher: public ASTWalker {
bool shouldSkip(SourceRange Range);
bool shouldSkip(CharSourceRange Range);
bool tryResolve(ASTWalker::ParentTy Node, SourceLoc NameLoc);
bool tryResolve(ASTWalker::ParentTy Node, DeclNameLoc NameLoc, Expr *Arg,
bool checkParentForLabels = false);
bool tryResolve(ASTWalker::ParentTy Node, DeclNameLoc NameLoc, Expr *Arg);
bool tryResolve(ASTWalker::ParentTy Node, SourceLoc NameLoc, LabelRangeType RangeType,
ArrayRef<CharSourceRange> LabelLocs);
bool handleCustomAttrs(Decl *D);
Expr *getApplicableArgFor(Expr* E);

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

enum class RangeKind : int8_t {
Expand Down
94 changes: 77 additions & 17 deletions lib/IDE/Refactoring.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -316,21 +316,35 @@ class Renamer {
// FIXME: handle escaped keyword names `init`
bool IsSubscript = Old.base() == "subscript" && Config.IsFunctionLike;
bool IsInit = Old.base() == "init" && Config.IsFunctionLike;
bool IsKeywordBase = IsInit || IsSubscript;

// FIXME: this should only be treated specially for instance methods.
bool IsCallAsFunction = Old.base() == "callAsFunction" &&
Config.IsFunctionLike;

bool IsSpecialBase = IsInit || IsSubscript || IsCallAsFunction;

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

if (!Config.IsFunctionLike || !IsKeywordBase) {
if (!Config.IsFunctionLike || !IsSpecialBase) {
if (renameBase(Resolved.Range, RefactoringRangeKind::BaseName))
return RegionType::Mismatch;

} else if (IsKeywordBase && Config.Usage == NameUsage::Definition) {
} else if (IsInit || IsCallAsFunction) {
if (renameBase(Resolved.Range, RefactoringRangeKind::KeywordBaseName)) {
// The base name doesn't need to match (but may) for calls, but
// it should for definitions and references.
if (Config.Usage == NameUsage::Definition ||
Config.Usage == NameUsage::Reference) {
return RegionType::Mismatch;
}
}
} else if (IsSubscript && Config.Usage == NameUsage::Definition) {
if (renameBase(Resolved.Range, RefactoringRangeKind::KeywordBaseName))
return RegionType::Mismatch;
}
Expand Down Expand Up @@ -528,9 +542,11 @@ static const ValueDecl *getRelatedSystemDecl(const ValueDecl *VD) {
return nullptr;
}

static Optional<RefactoringKind> getAvailableRenameForDecl(const ValueDecl *VD) {
static Optional<RefactoringKind>
getAvailableRenameForDecl(const ValueDecl *VD,
Optional<RenameRefInfo> RefInfo) {
std::vector<RenameAvailabiliyInfo> Scratch;
for (auto &Info : collectRenameAvailabilityInfo(VD, Scratch)) {
for (auto &Info : collectRenameAvailabilityInfo(VD, RefInfo, Scratch)) {
if (Info.AvailableKind == RenameAvailableKind::Available)
return Info.Kind;
}
Expand Down Expand Up @@ -744,15 +760,21 @@ bool RefactoringActionLocalRename::
isApplicable(ResolvedCursorInfo CursorInfo, DiagnosticEngine &Diag) {
if (CursorInfo.Kind != CursorInfoKind::ValueRef)
return false;
auto RenameOp = getAvailableRenameForDecl(CursorInfo.ValueD);

Optional<RenameRefInfo> RefInfo;
if (CursorInfo.IsRef)
RefInfo = {CursorInfo.SF, CursorInfo.Loc, CursorInfo.IsKeywordArgument};

auto RenameOp = getAvailableRenameForDecl(CursorInfo.ValueD, RefInfo);
return RenameOp.hasValue() &&
RenameOp.getValue() == RefactoringKind::LocalRename;
}

static void analyzeRenameScope(ValueDecl *VD, DiagnosticEngine &Diags,
static void analyzeRenameScope(ValueDecl *VD, Optional<RenameRefInfo> RefInfo,
DiagnosticEngine &Diags,
llvm::SmallVectorImpl<DeclContext *> &Scopes) {
Scopes.clear();
if (!getAvailableRenameForDecl(VD).hasValue()) {
if (!getAvailableRenameForDecl(VD, RefInfo).hasValue()) {
Diags.diagnose(SourceLoc(), diag::value_decl_no_loc, VD->getFullName());
return;
}
Expand Down Expand Up @@ -786,7 +808,12 @@ bool RefactoringActionLocalRename::performChange() {
if (CursorInfo.isValid() && CursorInfo.ValueD) {
ValueDecl *VD = CursorInfo.CtorTyRef ? CursorInfo.CtorTyRef : CursorInfo.ValueD;
llvm::SmallVector<DeclContext *, 8> Scopes;
analyzeRenameScope(VD, DiagEngine, Scopes);

Optional<RenameRefInfo> RefInfo;
if (CursorInfo.IsRef)
RefInfo = {CursorInfo.SF, CursorInfo.Loc, CursorInfo.IsKeywordArgument};

analyzeRenameScope(VD, RefInfo, DiagEngine, Scopes);
if (Scopes.empty())
return true;
RenameRangeCollector rangeCollector(VD, PreferredName);
Expand Down Expand Up @@ -3623,9 +3650,11 @@ accept(SourceManager &SM, RegionType RegionType,
Impl.accept(SM, Range);
}
}

ArrayRef<RenameAvailabiliyInfo>
swift::ide::collectRenameAvailabilityInfo(const ValueDecl *VD,
std::vector<RenameAvailabiliyInfo> &Scratch) {
Optional<RenameRefInfo> RefInfo,
std::vector<RenameAvailabiliyInfo> &Scratch) {
RenameAvailableKind AvailKind = RenameAvailableKind::Available;
if (getRelatedSystemDecl(VD)){
AvailKind = RenameAvailableKind::Unavailable_system_symbol;
Expand All @@ -3650,6 +3679,30 @@ swift::ide::collectRenameAvailabilityInfo(const ValueDecl *VD,
if (auto CD = dyn_cast<ConstructorDecl>(VD)) {
if (!CD->getParameters()->size())
return Scratch;

if (RefInfo && !RefInfo->IsArgLabel) {
NameMatcher Matcher(*(RefInfo->SF));
auto Resolved = Matcher.resolve({RefInfo->Loc, /*ResolveArgs*/true});
if (Resolved.LabelRanges.empty())
return Scratch;
}
}

// Disallow renaming 'callAsFunction' method with no arguments.
if (auto FD = dyn_cast<FuncDecl>(VD)) {
// FIXME: syntactic rename can only decide by checking the spelling, not
// whether it's an instance method, so we do the same here for now.
if (FD->getBaseIdentifier() == FD->getASTContext().Id_callAsFunction) {
if (!FD->getParameters()->size())
return Scratch;

if (RefInfo && !RefInfo->IsArgLabel) {
NameMatcher Matcher(*(RefInfo->SF));
auto Resolved = Matcher.resolve({RefInfo->Loc, /*ResolveArgs*/true});
if (Resolved.LabelRanges.empty())
return Scratch;
}
}
}
}

Expand Down Expand Up @@ -3682,7 +3735,10 @@ collectAvailableRefactorings(SourceFile *SF,
case CursorInfoKind::ExprStart:
break;
case CursorInfoKind::ValueRef: {
auto RenameOp = getAvailableRenameForDecl(CursorInfo.ValueD);
Optional<RenameRefInfo> RefInfo;
if (CursorInfo.IsRef)
RefInfo = {CursorInfo.SF, CursorInfo.Loc, CursorInfo.IsKeywordArgument};
auto RenameOp = getAvailableRenameForDecl(CursorInfo.ValueD, RefInfo);
if (RenameOp.hasValue() &&
RenameOp.getValue() == RefactoringKind::GlobalRename)
AllKinds.push_back(RenameOp.getValue());
Expand Down Expand Up @@ -3919,8 +3975,12 @@ int swift::ide::findLocalRenameRanges(
return true;
}
ValueDecl *VD = CursorInfo.CtorTyRef ? CursorInfo.CtorTyRef : CursorInfo.ValueD;
Optional<RenameRefInfo> RefInfo;
if (CursorInfo.IsRef)
RefInfo = {CursorInfo.SF, CursorInfo.Loc, CursorInfo.IsKeywordArgument};

llvm::SmallVector<DeclContext *, 8> Scopes;
analyzeRenameScope(VD, Diags, Scopes);
analyzeRenameScope(VD, RefInfo, Diags, Scopes);
if (Scopes.empty())
return true;
RenameRangeCollector RangeCollector(VD, StringRef());
Expand Down
5 changes: 5 additions & 0 deletions lib/IDE/SourceEntityWalker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,15 @@ class SemaAnnotator : public ASTWalker {
bool shouldIgnore(Decl *D, bool &ShouldVisitChildren);

ValueDecl *extractDecl(Expr *Fn) const {
Fn = Fn->getSemanticsProvidingExpr();
if (auto *DRE = dyn_cast<DeclRefExpr>(Fn))
return DRE->getDecl();
if (auto ApplyE = dyn_cast<ApplyExpr>(Fn))
return extractDecl(ApplyE->getFn());
if (auto *ACE = dyn_cast<AutoClosureExpr>(Fn)) {
if (auto *Unwrapped = ACE->getUnwrappedCurryThunkExpr())
return extractDecl(Unwrapped);
}
return nullptr;
}
};
Expand Down
Loading