From 63aeedecbdf902b119a13d19ce600dc411aade53 Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Wed, 28 Feb 2024 20:17:44 -0800 Subject: [PATCH] [SourceKit] Only report textual results inside comments and strings from syntactic rename ranges --- include/swift/IDE/Utils.h | 14 +++++++ .../SyntacticRenameRangeDetails.cpp | 13 +++---- .../find-rename-ranges/foo_arity1.expected | 10 ++--- .../find-rename-ranges/foo_remove.expected | 10 ++--- .../find-rename-ranges/keywordbase.expected | 6 +-- .../Refactoring/find-rename-ranges/z.expected | 3 +- .../Outputs/textual/MyClass.swift.expected | 38 ------------------- .../Outputs/textual/foo.swift.expected | 13 ++++--- .../refactoring/SyntacticRename/textual.swift | 3 +- 9 files changed, 37 insertions(+), 73 deletions(-) delete mode 100644 test/refactoring/SyntacticRename/Outputs/textual/MyClass.swift.expected diff --git a/include/swift/IDE/Utils.h b/include/swift/IDE/Utils.h index 5062814344780..76a8318bc8929 100644 --- a/include/swift/IDE/Utils.h +++ b/include/swift/IDE/Utils.h @@ -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, }; diff --git a/lib/Refactoring/SyntacticRenameRangeDetails.cpp b/lib/Refactoring/SyntacticRenameRangeDetails.cpp index 979afcc102bb4..1e36c85c6c646 100644 --- a/lib/Refactoring/SyntacticRenameRangeDetails.cpp +++ b/lib/Refactoring/SyntacticRenameRangeDetails.cpp @@ -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; diff --git a/test/SourceKit/Refactoring/find-rename-ranges/foo_arity1.expected b/test/SourceKit/Refactoring/find-rename-ranges/foo_arity1.expected index f93896352332e..f79bc81d54484 100644 --- a/test/SourceKit/Refactoring/find-rename-ranges/foo_arity1.expected +++ b/test/SourceKit/Refactoring/find-rename-ranges/foo_arity1.expected @@ -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: diff --git a/test/SourceKit/Refactoring/find-rename-ranges/foo_remove.expected b/test/SourceKit/Refactoring/find-rename-ranges/foo_remove.expected index ba5d75c2f825f..2af504a1e6af3 100644 --- a/test/SourceKit/Refactoring/find-rename-ranges/foo_remove.expected +++ b/test/SourceKit/Refactoring/find-rename-ranges/foo_remove.expected @@ -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: diff --git a/test/SourceKit/Refactoring/find-rename-ranges/keywordbase.expected b/test/SourceKit/Refactoring/find-rename-ranges/keywordbase.expected index 172298be67de0..eb5a3ee5b8661 100644 --- a/test/SourceKit/Refactoring/find-rename-ranges/keywordbase.expected +++ b/test/SourceKit/Refactoring/find-rename-ranges/keywordbase.expected @@ -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: diff --git a/test/SourceKit/Refactoring/find-rename-ranges/z.expected b/test/SourceKit/Refactoring/find-rename-ranges/z.expected index 4aa9594b22f11..c87b581e6bcc0 100644 --- a/test/SourceKit/Refactoring/find-rename-ranges/z.expected +++ b/test/SourceKit/Refactoring/find-rename-ranges/z.expected @@ -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: diff --git a/test/refactoring/SyntacticRename/Outputs/textual/MyClass.swift.expected b/test/refactoring/SyntacticRename/Outputs/textual/MyClass.swift.expected deleted file mode 100644 index d9cd068bc5268..0000000000000 --- a/test/refactoring/SyntacticRename/Outputs/textual/MyClass.swift.expected +++ /dev/null @@ -1,38 +0,0 @@ -/* - /*foo:unknown*/foo() is not /*foo:unknown*/foo(first:) -*/ -/// This describes /*foo:unknown*/foo and /*foo:unknown*/foo -func /*foo:def*/foo() { - let /*foo:def*/foo = "Here is /*foo:unknown*/foo" - // /*foo:unknown*/foo's return - #selector(Struct . /*foo:unknown*/foo(_:aboveSubview:)) - #selector(/*foo:unknown*/foo(_:)) - #selector(#selector(/*foo:unknown*/foo)) - - #if true - /*foo*/foo = 2 - /*foo*/foo() - /*foo:call*/foo() - /*foo:unknown*/foo = 3 - /*foo:unknown*/foo() - #if false - /*foo:unknown*/foo += 2 - /*foo:unknown*/foo() - #endif - #else - /*foo:unknown*/foo = 4 - #endif - - return 1 -} - -#if false -class /*MyClass:unknown*/MyClass {} -_ = /*MyClass:unknown*/Mismatch() -_ = /*MyClass:unknown*/MyClass() -#else -class /*MyClass:unknown*/MyClass {} -_ = /*MyClass:unknown*/Mismatch() -_ = /*MyClass:unknown*/MyClass() -#endif - diff --git a/test/refactoring/SyntacticRename/Outputs/textual/foo.swift.expected b/test/refactoring/SyntacticRename/Outputs/textual/foo.swift.expected index da524e0798f1a..0e4464b6ebf50 100644 --- a/test/refactoring/SyntacticRename/Outputs/textual/foo.swift.expected +++ b/test/refactoring/SyntacticRename/Outputs/textual/foo.swift.expected @@ -7,20 +7,20 @@ func /*foo:def*/foo() { // /*foo:unknown*/foo's return #selector(Struct . /*foo:unknown*/foo(_:aboveSubview:)) #selector(/*foo:unknown*/foo(_:)) - #selector(#selector(/*foo:unknown*/foo)) + #selector(#selector(/*foo:unknown*/foo)) #if true /*foo*/foo = 2 /*foo*/foo() /*foo:call*/foo() - /*foo:unknown*/foo = 3 - /*foo:unknown*/foo() + /*foo:unknown*/foo = 3 + /*foo:unknown*/foo() #if false - /*foo:unknown*/foo += 2 - /*foo:unknown*/foo() + /*foo:unknown*/foo += 2 + /*foo:unknown*/foo() #endif #else - /*foo:unknown*/foo = 4 + /*foo:unknown*/foo = 4 #endif return 1 @@ -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. diff --git a/test/refactoring/SyntacticRename/textual.swift b/test/refactoring/SyntacticRename/textual.swift index 42490104c18af..7a7cd7445756c 100644 --- a/test/refactoring/SyntacticRename/textual.swift +++ b/test/refactoring/SyntacticRename/textual.swift @@ -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 -