Browse Source

Add Rename functionality (#1514)

This feature depends on having the right span information for
identifiers in the parse tree. Currently, some changes are needed to
either add the missing span information in some cases, or improve the
already present spans (look for one-off errors).

The rest of the "goto-*" functions also depend on the availability of
proper spans. But in their case, as no modification is made to the
source code, lack of accurate spans is less of an issue. But `rename`
does modify the source code and hence having incorrect spans can result
in unintentionally overwriting pieces of source code, which is highly
undesirable.

The changes made as part of this PR provide the necessary code for
`rename` to work. Correcting the parse tree is expected to be done as
part of future PRs

A non-exhaustive list of changes to be made to parse tree:
(https://github.com/hyperledger/solang/pull/1411#issuecomment-1648031602)

1. No loc for contract variables type - array_type_dynamic_push.sol L6
2. No loc for constructor base call - abstract_contract_inheritance.sol
L14
3. No loc for function modifier arguments -
function_modifier_arguments.sol L8
4. No loc for function override base contracts -
virtual_functions_override.sol L2
5. Struct literal struct loc and Constructor contract loc not available
- inline_assembly.sol L12
6. Internal Function Location wrong when preceded by a contract name. -
abi_encode_call.sol L3
7. array with size length function (stored as NumLiteral) -
array_type_fixed_length.sol L7
8. struct literals fields point to the struct and not fields, function
calls with struct literals => wrong loc - function_arguments.sol L12
9. function calls with contract names : contract defintions absent - -
base_contract_function_call.sol L25
10. enum variants vs enum name - struct_type.sol L29
11. using - using.sol L10

---------

Signed-off-by: Govardhan G D <chioni1620@gmail.com>
Govardhan G D 2 years ago
parent
commit
c53d8d998e

+ 66 - 7
src/bin/languageserver/mod.rs

@@ -30,10 +30,11 @@ use tower_lsp::{
         ExecuteCommandOptions, ExecuteCommandParams, GotoDefinitionParams, GotoDefinitionResponse,
         ExecuteCommandOptions, ExecuteCommandParams, GotoDefinitionParams, GotoDefinitionResponse,
         Hover, HoverContents, HoverParams, HoverProviderCapability,
         Hover, HoverContents, HoverParams, HoverProviderCapability,
         ImplementationProviderCapability, InitializeParams, InitializeResult, InitializedParams,
         ImplementationProviderCapability, InitializeParams, InitializeResult, InitializedParams,
-        Location, MarkedString, MessageType, OneOf, Position, Range, ReferenceParams,
+        Location, MarkedString, MessageType, OneOf, Position, Range, ReferenceParams, RenameParams,
         ServerCapabilities, SignatureHelpOptions, TextDocumentContentChangeEvent,
         ServerCapabilities, SignatureHelpOptions, TextDocumentContentChangeEvent,
-        TextDocumentSyncCapability, TextDocumentSyncKind, TypeDefinitionProviderCapability, Url,
-        WorkspaceFoldersServerCapabilities, WorkspaceServerCapabilities,
+        TextDocumentSyncCapability, TextDocumentSyncKind, TextEdit,
+        TypeDefinitionProviderCapability, Url, WorkspaceEdit, WorkspaceFoldersServerCapabilities,
+        WorkspaceServerCapabilities,
     },
     },
     Client, LanguageServer, LspService, Server,
     Client, LanguageServer, LspService, Server,
 };
 };
@@ -1651,7 +1652,10 @@ impl<'a> Builder<'a> {
                 ReferenceEntry {
                 ReferenceEntry {
                     start: file
                     start: file
                         .get_offset(range.start.line as usize, range.start.character as usize),
                         .get_offset(range.start.line as usize, range.start.character as usize),
-                    stop: file.get_offset(range.end.line as usize, range.end.character as usize),
+                    // 1 is added to account for the fact that `Lapper` expects half open ranges of the type:  [`start`, `stop`)
+                    // i.e, `start` included but `stop` excluded.
+                    stop: file.get_offset(range.end.line as usize, range.end.character as usize)
+                        + 1,
                     val: di.clone(),
                     val: di.clone(),
                 },
                 },
             ));
             ));
@@ -1776,6 +1780,7 @@ impl LanguageServer for SolangServer {
                 type_definition_provider: Some(TypeDefinitionProviderCapability::Simple(true)),
                 type_definition_provider: Some(TypeDefinitionProviderCapability::Simple(true)),
                 implementation_provider: Some(ImplementationProviderCapability::Simple(true)),
                 implementation_provider: Some(ImplementationProviderCapability::Simple(true)),
                 references_provider: Some(OneOf::Left(true)),
                 references_provider: Some(OneOf::Left(true)),
+                rename_provider: Some(OneOf::Left(true)),
                 ..ServerCapabilities::default()
                 ..ServerCapabilities::default()
             },
             },
         })
         })
@@ -1912,8 +1917,7 @@ impl LanguageServer for SolangServer {
                     .find(offset, offset + 1)
                     .find(offset, offset + 1)
                     .min_by(|a, b| (a.stop - a.start).cmp(&(b.stop - b.start)))
                     .min_by(|a, b| (a.stop - a.start).cmp(&(b.stop - b.start)))
                 {
                 {
-                    let loc = pt::Loc::File(0, hover.start, hover.stop);
-                    let range = loc_to_range(&loc, &cache.file);
+                    let range = get_range_exclusive(hover.start, hover.stop, &cache.file);
 
 
                     return Ok(Some(Hover {
                     return Ok(Some(Hover {
                         contents: HoverContents::Scalar(MarkedString::from_markdown(
                         contents: HoverContents::Scalar(MarkedString::from_markdown(
@@ -2109,7 +2113,7 @@ impl LanguageServer for SolangServer {
                     .filter(|r| r.val == reference)
                     .filter(|r| r.val == reference)
                     .map(move |r| Location {
                     .map(move |r| Location {
                         uri: uri.clone(),
                         uri: uri.clone(),
-                        range: get_range(r.start, r.stop, &cache.file),
+                        range: get_range_exclusive(r.start, r.stop, &cache.file),
                     })
                     })
             })
             })
             .collect();
             .collect();
@@ -2133,6 +2137,55 @@ impl LanguageServer for SolangServer {
 
 
         Ok(locations)
         Ok(locations)
     }
     }
+
+    /// Called when "Rename Symbol" is called by the user on the client side.
+    ///
+    /// Expected to return a list of changes to be made in user code so that every occurrence of the code object is renamed.
+    ///
+    /// ### Arguments
+    /// * `RenameParams`
+    ///     * provides the source code location (filename, line number, column number) of the code object for which the request was made.
+    ///     * the new symbol that the code object should go by.
+    ///
+    /// ### Edge cases
+    /// * Returns `Err` when an invalid file path is received.
+    /// * Returns `Ok(None)` when the definition of code object is not found in user code.
+    async fn rename(&self, params: RenameParams) -> Result<Option<WorkspaceEdit>> {
+        // fetch the `DefinitionIndex` of the code object in question
+        let def_params: GotoDefinitionParams = GotoDefinitionParams {
+            text_document_position_params: params.text_document_position,
+            work_done_progress_params: params.work_done_progress_params,
+            partial_result_params: Default::default(),
+        };
+        let Some(reference) = self.get_reference_from_params(def_params).await? else {
+            return Ok(None);
+        };
+
+        // the new name of the code object
+        let new_text = params.new_name;
+
+        // create `TextEdit` instances that represent the changes to be made for every occurrence of the old symbol
+        // these `TextEdit` objects are then grouped into separate list per source file to which they bolong
+        let caches = &self.files.lock().await.caches;
+        let ws = caches
+            .iter()
+            .map(|(p, cache)| {
+                let uri = Url::from_file_path(p).unwrap();
+                let text_edits: Vec<_> = cache
+                    .references
+                    .iter()
+                    .filter(|r| r.val == reference)
+                    .map(|r| TextEdit {
+                        range: get_range_exclusive(r.start, r.stop, &cache.file),
+                        new_text: new_text.clone(),
+                    })
+                    .collect();
+                (uri, text_edits)
+            })
+            .collect::<HashMap<_, _>>();
+
+        Ok(Some(WorkspaceEdit::new(ws)))
+    }
 }
 }
 
 
 /// Calculate the line and column from the Loc offset received from the parser
 /// Calculate the line and column from the Loc offset received from the parser
@@ -2149,6 +2202,12 @@ fn get_range(start: usize, end: usize, file: &ast::File) -> Range {
     Range::new(start, end)
     Range::new(start, end)
 }
 }
 
 
+// Get `Range` when the parameters passed represent a half open range of type [start, stop)
+// Used when `Range` is to be extracted from `Interval` from the `rust_lapper` crate.
+fn get_range_exclusive(start: usize, end: usize, file: &ast::File) -> Range {
+    get_range(start, end - 1, file)
+}
+
 fn get_type_definition(ty: &Type) -> Option<DefinitionType> {
 fn get_type_definition(ty: &Type) -> Option<DefinitionType> {
     match ty {
     match ty {
         Type::Enum(id) => Some(DefinitionType::Enum(*id)),
         Type::Enum(id) => Some(DefinitionType::Enum(*id)),

+ 57 - 9
vscode/src/test/suite/extension.test.ts

@@ -97,6 +97,12 @@ suite('Extension Test Suite', function () {
     await testrefs(refsdoc1);
     await testrefs(refsdoc1);
   });
   });
 
 
+  // Tests for rename
+  this.timeout(20000);
+  const renamedoc1 = getDocUri('rename.sol');
+  test('Testing for Rename', async () => {
+    await testrename(renamedoc1);
+  });
 });
 });
 
 
 function toRange(lineno1: number, charno1: number, lineno2: number, charno2: number) {
 function toRange(lineno1: number, charno1: number, lineno2: number, charno2: number) {
@@ -290,25 +296,25 @@ async function testrefs(docUri: vscode.Uri) {
   assert.strictEqual(loc01.range.start.line, 30);
   assert.strictEqual(loc01.range.start.line, 30);
   assert.strictEqual(loc01.range.start.character, 16);
   assert.strictEqual(loc01.range.start.character, 16);
   assert.strictEqual(loc01.range.end.line, 30);
   assert.strictEqual(loc01.range.end.line, 30);
-  assert.strictEqual(loc01.range.end.character, 22);
+  assert.strictEqual(loc01.range.end.character, 21);
   assert.strictEqual(loc01.uri.path, docUri.path);
   assert.strictEqual(loc01.uri.path, docUri.path);
   const loc02 = actualref0[2] as vscode.Location;
   const loc02 = actualref0[2] as vscode.Location;
   assert.strictEqual(loc02.range.start.line, 33);
   assert.strictEqual(loc02.range.start.line, 33);
   assert.strictEqual(loc02.range.start.character, 16);
   assert.strictEqual(loc02.range.start.character, 16);
   assert.strictEqual(loc02.range.end.line, 33);
   assert.strictEqual(loc02.range.end.line, 33);
-  assert.strictEqual(loc02.range.end.character, 22);
+  assert.strictEqual(loc02.range.end.character, 21);
   assert.strictEqual(loc02.uri.path, docUri.path);
   assert.strictEqual(loc02.uri.path, docUri.path);
   const loc03 = actualref0[3] as vscode.Location;
   const loc03 = actualref0[3] as vscode.Location;
   assert.strictEqual(loc03.range.start.line, 36);
   assert.strictEqual(loc03.range.start.line, 36);
   assert.strictEqual(loc03.range.start.character, 16);
   assert.strictEqual(loc03.range.start.character, 16);
   assert.strictEqual(loc03.range.end.line, 36);
   assert.strictEqual(loc03.range.end.line, 36);
-  assert.strictEqual(loc03.range.end.character, 22);
+  assert.strictEqual(loc03.range.end.character, 21);
   assert.strictEqual(loc03.uri.path, docUri.path);
   assert.strictEqual(loc03.uri.path, docUri.path);
   const loc04 = actualref0[4] as vscode.Location;
   const loc04 = actualref0[4] as vscode.Location;
   assert.strictEqual(loc04.range.start.line, 39);
   assert.strictEqual(loc04.range.start.line, 39);
   assert.strictEqual(loc04.range.start.character, 16);
   assert.strictEqual(loc04.range.start.character, 16);
   assert.strictEqual(loc04.range.end.line, 39);
   assert.strictEqual(loc04.range.end.line, 39);
-  assert.strictEqual(loc04.range.end.character, 22);
+  assert.strictEqual(loc04.range.end.character, 21);
   assert.strictEqual(loc04.uri.path, docUri.path);
   assert.strictEqual(loc04.uri.path, docUri.path);
 
 
   const pos1 = new vscode.Position(28, 12);
   const pos1 = new vscode.Position(28, 12);
@@ -328,34 +334,76 @@ async function testrefs(docUri: vscode.Uri) {
   assert.strictEqual(loc11.range.start.line, 28);
   assert.strictEqual(loc11.range.start.line, 28);
   assert.strictEqual(loc11.range.start.character, 12);
   assert.strictEqual(loc11.range.start.character, 12);
   assert.strictEqual(loc11.range.end.line, 28);
   assert.strictEqual(loc11.range.end.line, 28);
-  assert.strictEqual(loc11.range.end.character, 14);
+  assert.strictEqual(loc11.range.end.character, 13);
   assert.strictEqual(loc11.uri.path, docUri.path);
   assert.strictEqual(loc11.uri.path, docUri.path);
   const loc12 = actualref1[2] as vscode.Location;
   const loc12 = actualref1[2] as vscode.Location;
   assert.strictEqual(loc12.range.start.line, 29);
   assert.strictEqual(loc12.range.start.line, 29);
   assert.strictEqual(loc12.range.start.character, 16);
   assert.strictEqual(loc12.range.start.character, 16);
   assert.strictEqual(loc12.range.end.line, 29);
   assert.strictEqual(loc12.range.end.line, 29);
-  assert.strictEqual(loc12.range.end.character, 18);
+  assert.strictEqual(loc12.range.end.character, 17);
   assert.strictEqual(loc12.uri.path, docUri.path);
   assert.strictEqual(loc12.uri.path, docUri.path);
   const loc13 = actualref1[3] as vscode.Location;
   const loc13 = actualref1[3] as vscode.Location;
   assert.strictEqual(loc13.range.start.line, 32);
   assert.strictEqual(loc13.range.start.line, 32);
   assert.strictEqual(loc13.range.start.character, 16);
   assert.strictEqual(loc13.range.start.character, 16);
   assert.strictEqual(loc13.range.end.line, 32);
   assert.strictEqual(loc13.range.end.line, 32);
-  assert.strictEqual(loc13.range.end.character, 18);
+  assert.strictEqual(loc13.range.end.character, 17);
   assert.strictEqual(loc13.uri.path, docUri.path);
   assert.strictEqual(loc13.uri.path, docUri.path);
   const loc14 = actualref1[4] as vscode.Location;
   const loc14 = actualref1[4] as vscode.Location;
   assert.strictEqual(loc14.range.start.line, 35);
   assert.strictEqual(loc14.range.start.line, 35);
   assert.strictEqual(loc14.range.start.character, 16);
   assert.strictEqual(loc14.range.start.character, 16);
   assert.strictEqual(loc14.range.end.line, 35);
   assert.strictEqual(loc14.range.end.line, 35);
-  assert.strictEqual(loc14.range.end.character, 18);
+  assert.strictEqual(loc14.range.end.character, 17);
   assert.strictEqual(loc14.uri.path, docUri.path);
   assert.strictEqual(loc14.uri.path, docUri.path);
   const loc15 = actualref1[5] as vscode.Location;
   const loc15 = actualref1[5] as vscode.Location;
   assert.strictEqual(loc15.range.start.line, 38);
   assert.strictEqual(loc15.range.start.line, 38);
   assert.strictEqual(loc15.range.start.character, 16);
   assert.strictEqual(loc15.range.start.character, 16);
   assert.strictEqual(loc15.range.end.line, 38);
   assert.strictEqual(loc15.range.end.line, 38);
-  assert.strictEqual(loc15.range.end.character, 18);
+  assert.strictEqual(loc15.range.end.character, 17);
   assert.strictEqual(loc15.uri.path, docUri.path);
   assert.strictEqual(loc15.uri.path, docUri.path);
 }
 }
 
 
+async function testrename(docUri: vscode.Uri) {
+  await activate(docUri);
+
+  const pos0 = new vscode.Position(9, 8);
+  const newname0 = "changed";
+  const rename0 = (await vscode.commands.executeCommand(
+    'vscode.executeDocumentRenameProvider',
+    docUri,
+    pos0,
+    newname0,
+  )) as vscode.WorkspaceEdit;
+
+  assert(rename0.has(docUri));
+
+  const loc0 = rename0.get(docUri);
+
+  const loc00 = loc0[0] as vscode.TextEdit;
+  assert.strictEqual(loc00.range.start.line, 0);
+  assert.strictEqual(loc00.range.start.character, 41);
+  assert.strictEqual(loc00.range.end.line, 0);
+  assert.strictEqual(loc00.range.end.character, 42);
+  assert.strictEqual(loc00.newText, newname0);
+  const loc01 = loc0[1] as vscode.TextEdit;
+  assert.strictEqual(loc01.range.start.line, 1);
+  assert.strictEqual(loc01.range.start.character, 4);
+  assert.strictEqual(loc01.range.end.line, 1);
+  assert.strictEqual(loc01.range.end.character, 5);
+  assert.strictEqual(loc01.newText, newname0);
+  const loc02 = loc0[2] as vscode.TextEdit;
+  assert.strictEqual(loc02.range.start.line, 9);
+  assert.strictEqual(loc02.range.start.character, 8);
+  assert.strictEqual(loc02.range.end.line, 9);
+  assert.strictEqual(loc02.range.end.character, 9);
+  assert.strictEqual(loc02.newText, newname0);
+  const loc03 = loc0[3] as vscode.TextEdit;
+  assert.strictEqual(loc03.range.start.line, 9);
+  assert.strictEqual(loc03.range.start.character, 12);
+  assert.strictEqual(loc03.range.end.line, 9);
+  assert.strictEqual(loc03.range.end.character, 13);
+  assert.strictEqual(loc03.newText, newname0);
+}
+
 async function testhover(docUri: vscode.Uri) {
 async function testhover(docUri: vscode.Uri) {
   await activate(docUri);
   await activate(docUri);
 
 

+ 12 - 0
vscode/src/testFixture/rename.sol

@@ -0,0 +1,12 @@
+function foo(uint256 n) returns (uint256 d) {
+    d = 2;
+    for (;;) {
+        if (n == 0) {
+            break;
+        }
+
+        n = n - 1;
+
+        d = d + 2;
+    }
+}