Skip to content

[SourceKit] Only report textual results inside comments and strings from syntactic rename ranges #71966

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
14 changes: 14 additions & 0 deletions include/swift/IDE/Utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -437,12 +437,26 @@ class DeclNameViewer {
};

enum class RegionType {
/// We could not match the rename location to a symbol to be renamed and the
/// symbol was originally a text match result (has `RenameLocUsage::Unknown`).
Unmatched,
/// We could not match the rename location to a symbol to be renamed and the
/// symbol came from the index (does not have `RenameLocUsage::Unknown`).
Mismatch,
/// We were able to match the result to a location in source code that's
/// active with respect to the current compiler arguments.
ActiveCode,
/// We were able to match the result to a location in source code that's
/// inactive with respect to the current compiler arguments.
///
/// Currently, we don't evaluate #if so all occurrences inside #if blocks
/// are considered inactive.
InactiveCode,
/// The location is inside a string literal.
String,
/// The location is inside a `#selector`.
Selector,
/// The location is inside a comment.
Comment,
};

Expand Down
13 changes: 6 additions & 7 deletions lib/Refactoring/SyntacticRenameRangeDetails.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -406,19 +406,18 @@ RegionType RenameRangeDetailCollector::addSyntacticRenameRanges(
// Unknown name usage occurs if we don't have an entry in the index that
// tells us whether the location is a call, reference or a definition. The
// most common reasons why this happens is if the editor is adding syntactic
// results (eg. from comments or string literals).
// results to cover comments or string literals.
//
// Determine whether we should include them.
if (regionKind == RegionType::ActiveCode) {
// If the reference is in active code, we should have had a name usage
// from the index. Since we don't, they are likely unrelated symbols that
// happen to have the same name. Don't return them as matching ranges.
// We only want to include these textual matches inside comments and string
// literals. All other matches inside are likely bogus results.
if (regionKind != RegionType::Comment && regionKind != RegionType::String) {
return RegionType::Unmatched;
}

if (specialBaseName != SpecialBaseName::None &&
resolved.labelType == LabelRangeType::None) {
// Filter out non-semantic special basename locations with no labels.
// We've already filtered out those in active code, so these are
// We've already filtered out those in code, so these are
// any appearance of just 'init', 'subscript', or 'callAsFunction' in
// strings, comments, and inactive code.
return RegionType::Unmatched;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,9 @@ source.edit.kind.active:
18:17-18:20 source.refactoring.range.kind.basename
source.edit.kind.string:
22:12-22:15 source.refactoring.range.kind.basename
source.edit.kind.selector:
23:19-23:22 source.refactoring.range.kind.basename
23:23-23:24 source.refactoring.range.kind.selector-argument-label arg-index=0
source.edit.kind.selector:
24:19-24:22 source.refactoring.range.kind.basename
source.edit.kind.selector:
25:19-25:22 source.refactoring.range.kind.basename
source.edit.kind.unknown:
source.edit.kind.unknown:
source.edit.kind.unknown:
source.edit.kind.string:
26:17-26:20 source.refactoring.range.kind.basename
source.edit.kind.active:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,9 @@ source.edit.kind.active:
18:17-18:20 source.refactoring.range.kind.basename
source.edit.kind.string:
22:12-22:15 source.refactoring.range.kind.basename
source.edit.kind.selector:
23:19-23:22 source.refactoring.range.kind.basename
23:23-23:24 source.refactoring.range.kind.selector-argument-label arg-index=0
source.edit.kind.selector:
24:19-24:22 source.refactoring.range.kind.basename
source.edit.kind.selector:
25:19-25:22 source.refactoring.range.kind.basename
source.edit.kind.unknown:
source.edit.kind.unknown:
source.edit.kind.unknown:
source.edit.kind.string:
26:17-26:20 source.refactoring.range.kind.basename
source.edit.kind.active:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,5 @@ source.edit.kind.unknown:
source.edit.kind.unknown:
source.edit.kind.unknown:
source.edit.kind.unknown:
source.edit.kind.inactive:
103:17-103:21 source.refactoring.range.kind.keyword-basename
103:22-103:23 source.refactoring.range.kind.call-argument-label arg-index=0
103:23-103:25 source.refactoring.range.kind.call-argument-colon arg-index=0
source.edit.kind.unknown:
104:17-104:21 source.refactoring.range.kind.keyword-basename
source.edit.kind.unknown:
3 changes: 1 addition & 2 deletions test/SourceKit/Refactoring/find-rename-ranges/z.expected
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,4 @@ source.edit.kind.active:
8:9-8:10 source.refactoring.range.kind.basename
source.edit.kind.inactive:
10:16-10:17 source.refactoring.range.kind.basename
source.edit.kind.inactive:
12:16-12:17 source.refactoring.range.kind.basename
source.edit.kind.unknown:

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,20 @@ func /*foo:def*/<base>foo</base>() {
// /*foo:unknown*/<base>foo</base>'s return
#selector(Struct . /*foo:unknown*/foo(_:aboveSubview:))
#selector(/*foo:unknown*/foo(_:))
#selector(#selector(/*foo:unknown*/<base>foo</base>))
#selector(#selector(/*foo:unknown*/foo))

#if true
/*foo*/<base>foo</base> = 2
/*foo*/<base>foo</base>()
/*foo:call*/<base>foo</base>()
/*foo:unknown*/<base>foo</base> = 3
/*foo:unknown*/<base>foo</base>()
/*foo:unknown*/foo = 3
/*foo:unknown*/foo()
#if false
/*foo:unknown*/<base>foo</base> += 2
/*foo:unknown*/<base>foo</base>()
/*foo:unknown*/foo += 2
/*foo:unknown*/foo()
#endif
#else
/*foo:unknown*/<base>foo</base> = 4
/*foo:unknown*/foo = 4
#endif

return 1
Expand All @@ -36,3 +36,4 @@ _ = /*MyClass:unknown*/Mismatch()
_ = /*MyClass:unknown*/MyClass()
#endif

// All occcurrences of MyClass are outside of comments and string literals, so there's nothing to rename. swift-refactor indicates this by outputing empty results.
3 changes: 2 additions & 1 deletion test/refactoring/SyntacticRename/textual.swift
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,5 @@ _ = /*MyClass:unknown*/MyClass()
// RUN: %refactor -find-rename-ranges -source-filename %s -pos="foo" -is-function-like -old-name "foo()" >> %t.ranges/textual_foo.swift
// RUN: diff -u %S/Outputs/textual/foo.swift.expected %t.ranges/textual_foo.swift
// RUN: %refactor -find-rename-ranges -source-filename %s -pos="MyClass" -is-non-protocol-type -old-name "MyClass" -new-name "YourClass" >> %t.ranges/textual_MyClass.swift
// RUN: diff -u %S/Outputs/textual/MyClass.swift.expected %t.ranges/textual_MyClass.swift
// All occcurrences of MyClass are outside of comments and string literals, so there's nothing to rename. swift-refactor indicates this by outputing empty results.
// RUN: diff -u %t.ranges/textual_MyClass.swift -