Merge pull request #71966 from ahoppen/ahoppen/dont-rename-textual-code-results

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

Author: ahoppen

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,
 };
 

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;

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:

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:

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:

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:

test/refactoring/SyntacticRename/Outputs/textual/MyClass.swift.expected

@@ -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
@@ -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.

test/refactoring/SyntacticRename/Outputs/textual/foo.swift.expected

@@ -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 -