Skip to content
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

Support has in rename refactoring #495

Merged
merged 20 commits into from
Nov 8, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
026331c
Add annotated changes to rename framework.
toinehartman Oct 15, 2024
5bce79e
Semantic & static fixes.
toinehartman Oct 17, 2024
9e67aee
Implement user-confirmed rename of fields in has-expression.
toinehartman Oct 17, 2024
2bef0bc
Add use of `has` to data field tests.
toinehartman Oct 29, 2024
2a1ec50
Add license to new file.
toinehartman Oct 30, 2024
318a614
Fix Rascal->Java translation of change annotations.
toinehartman Oct 30, 2024
40c90a1
Renames with annotations take precendence over forced ones.
toinehartman Oct 30, 2024
e28c7c6
Remove unused parameter.
toinehartman Oct 31, 2024
53753c5
Merge remote-tracking branch 'origin/main' into feature/rename-refact…
toinehartman Oct 31, 2024
f9c659e
Organize imports.
toinehartman Oct 31, 2024
68df61c
Merge remote-tracking branch 'origin/main' into feature/rename-refact…
toinehartman Nov 6, 2024
ebdf1f7
Improved descriptions in refactor preview.
toinehartman Nov 6, 2024
adfd046
Use correctly typed default for rename evaluator call.
toinehartman Nov 6, 2024
e606290
Reword `has` annotation
toinehartman Nov 7, 2024
75dd87c
Fix warnings.
toinehartman Nov 7, 2024
b3aeec7
Fix use of has triggering refactor preview.
toinehartman Nov 8, 2024
8b7ceb1
Simplify TextEdit extension and use relations for annotation tracking.
toinehartman Nov 8, 2024
68a43f1
Merge remote-tracking branch 'origin/main' into feature/rename-refact…
toinehartman Nov 8, 2024
ce74879
Remove name wildcard.
toinehartman Nov 8, 2024
d964b20
Merge remote-tracking branch 'origin/main' into feature/rename-refact…
toinehartman Nov 8, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
import io.usethesource.vallang.ISet;
import io.usethesource.vallang.ISourceLocation;
import io.usethesource.vallang.IString;
import io.usethesource.vallang.ITuple;
import io.usethesource.vallang.IValue;
import io.usethesource.vallang.IValueFactory;
import io.usethesource.vallang.exceptions.FactTypeUseException;
Expand Down Expand Up @@ -204,15 +205,15 @@ public InterruptibleFuture<IList> getDocumentSymbols(IConstructor module) {
}


public InterruptibleFuture<IList> getRename(ITree module, Position cursor, Set<ISourceLocation> workspaceFolders, Function<ISourceLocation, PathConfig> getPathConfig, String newName, ColumnMaps columns) {
public InterruptibleFuture<ITuple> getRename(ITree module, Position cursor, Set<ISourceLocation> workspaceFolders, Function<ISourceLocation, PathConfig> getPathConfig, String newName, ColumnMaps columns) {
var moduleLocation = TreeAdapter.getLocation(module);
Position pos = Locations.toRascalPosition(moduleLocation, cursor, columns);
var cursorTree = TreeAdapter.locateLexical(module, pos.getLine(), pos.getCharacter());

return runEvaluator("Rascal rename", semanticEvaluator, eval -> {
try {
IFunction rascalGetPathConfig = eval.getFunctionValueFactory().function(getPathConfigType, (t, u) -> addResources(getPathConfig.apply((ISourceLocation) t[0])));
return (IList) eval.call("rascalRenameSymbol", cursorTree, VF.set(workspaceFolders.toArray(ISourceLocation[]::new)), VF.string(newName), rascalGetPathConfig);
return (ITuple) eval.call("rascalRenameSymbol", cursorTree, VF.set(workspaceFolders.toArray(ISourceLocation[]::new)), VF.string(newName), rascalGetPathConfig);
} catch (Throw e) {
if (e.getException() instanceof IConstructor) {
var exception = (IConstructor)e.getException();
Expand All @@ -231,7 +232,7 @@ public InterruptibleFuture<IList> getRename(ITree module, Position cursor, Set<I
}
throw e;
}
}, VF.list(), exec, false, client);
}, VF.tuple(), exec, false, client);
toinehartman marked this conversation as resolved.
Show resolved Hide resolved
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
Expand Down Expand Up @@ -83,7 +82,6 @@
import org.eclipse.lsp4j.TextDocumentSyncKind;
import org.eclipse.lsp4j.VersionedTextDocumentIdentifier;
import org.eclipse.lsp4j.WorkspaceEdit;
import org.eclipse.lsp4j.jsonrpc.CompletableFutures;
import org.eclipse.lsp4j.jsonrpc.ResponseErrorException;
import org.eclipse.lsp4j.jsonrpc.messages.Either;
import org.eclipse.lsp4j.jsonrpc.messages.ResponseError;
Expand All @@ -94,7 +92,6 @@
import org.rascalmpl.library.util.PathConfig;
import org.rascalmpl.parser.gtd.exception.ParseError;
import org.rascalmpl.uri.URIResolverRegistry;
import org.rascalmpl.values.IRascalValueFactory;
import org.rascalmpl.values.parsetrees.ITree;
import org.rascalmpl.vscode.lsp.BaseWorkspaceService;
import org.rascalmpl.vscode.lsp.IBaseLanguageClient;
Expand All @@ -107,8 +104,8 @@
import org.rascalmpl.vscode.lsp.util.CodeActions;
import org.rascalmpl.vscode.lsp.util.Diagnostics;
import org.rascalmpl.vscode.lsp.util.DocumentChanges;
import org.rascalmpl.vscode.lsp.util.FoldingRanges;
import org.rascalmpl.vscode.lsp.util.DocumentSymbols;
import org.rascalmpl.vscode.lsp.util.FoldingRanges;
import org.rascalmpl.vscode.lsp.util.SemanticTokenizer;
import org.rascalmpl.vscode.lsp.util.Versioned;
import org.rascalmpl.vscode.lsp.util.locations.ColumnMaps;
Expand All @@ -117,6 +114,7 @@
import org.rascalmpl.vscode.lsp.util.locations.impl.TreeSearch;

import io.usethesource.vallang.IList;
import io.usethesource.vallang.IMap;
import io.usethesource.vallang.ISourceLocation;
import io.usethesource.vallang.IValue;

Expand Down Expand Up @@ -301,8 +299,12 @@ public CompletableFuture<WorkspaceEdit> rename(RenameParams params) {
.thenApply(Versioned::get)
.handle((t, r) -> (t == null ? (file.getMostRecentTree().get()) : t))
.thenCompose(tr -> rascalServices.getRename(tr, params.getPosition(), workspaceFolders, facts::getPathConfig, params.getNewName(), columns).get())
.thenApply(c -> new WorkspaceEdit(DocumentChanges.translateDocumentChanges(this, c)))
;
.thenApply(t -> {
WorkspaceEdit wsEdit = new WorkspaceEdit();
wsEdit.setDocumentChanges(DocumentChanges.translateDocumentChanges(this, (IList) t.get(0)));
wsEdit.setChangeAnnotations(DocumentChanges.translateChangeAnnotations((IMap) t.get(1)));
DavyLandman marked this conversation as resolved.
Show resolved Hide resolved
return wsEdit;
});
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,11 @@

import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

import org.eclipse.lsp4j.AnnotatedTextEdit;
import org.eclipse.lsp4j.ChangeAnnotation;
import org.eclipse.lsp4j.CreateFile;
import org.eclipse.lsp4j.DeleteFile;
import org.eclipse.lsp4j.Range;
Expand All @@ -43,10 +46,13 @@
import org.rascalmpl.vscode.lsp.util.locations.LineColumnOffsetMap;
import org.rascalmpl.vscode.lsp.util.locations.Locations;

import io.usethesource.vallang.IBool;
import io.usethesource.vallang.IConstructor;
import io.usethesource.vallang.IList;
import io.usethesource.vallang.IMap;
import io.usethesource.vallang.ISourceLocation;
import io.usethesource.vallang.IString;
import io.usethesource.vallang.ITuple;
import io.usethesource.vallang.IValue;

/**
Expand Down Expand Up @@ -90,7 +96,9 @@ public static List<Either<TextDocumentEdit, ResourceOperation>> translateDocumen
private static List<TextEdit> translateTextEdits(final IBaseTextDocumentService docService, IList edits) {
return edits.stream()
.map(e -> (IConstructor) e)
.map(c -> new TextEdit(locationToRange(docService, (ISourceLocation) c.get("range")), ((IString) c.get("replacement")).getValue()))
.map(c -> (c.has("annotation")
? new AnnotatedTextEdit(locationToRange(docService, (ISourceLocation) c.get("range")), ((IString) c.get("replacement")).getValue(), ((IString) c.get("annotation")).getValue())
: new TextEdit(locationToRange(docService, (ISourceLocation) c.get("range")), ((IString) c.get("replacement")).getValue())))
.collect(Collectors.toList());
}

Expand All @@ -102,4 +110,19 @@ private static Range locationToRange(final IBaseTextDocumentService docService,
private static String getFileURI(IConstructor edit, String label) {
return ((ISourceLocation) edit.get(label)).getURI().toString();
}

public static Map<String, ChangeAnnotation> translateChangeAnnotations(IMap annos) {
return annos.stream()
.map(entry -> (ITuple) entry)
.map(entry -> {
String annoId = ((IString) entry.get(0)).getValue();
ChangeAnnotation anno = new ChangeAnnotation();
IConstructor c = (IConstructor) entry.get(1);
anno.setLabel(((IString) c.get("label")).getValue());
anno.setDescription(((IString) c.get("description")).getValue());
anno.setNeedsConfirmation(((IBool) c.get("needsConfirmation")).getValue());
return Map.entry(annoId, anno);
})
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
}
}
66 changes: 53 additions & 13 deletions rascal-lsp/src/main/rascal/lang/rascal/lsp/refactor/Rename.rsc
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,15 @@ extend lang::rascal::lsp::refactor::Exception;
import lang::rascal::lsp::refactor::Util;
import lang::rascal::lsp::refactor::WorkspaceInfo;

import analysis::diff::edits::TextEdits;
import lang::rascal::lsp::refactor::TextEdits;

import util::FileSystem;
import util::Maybe;
import util::Monitor;
import util::Reflective;

alias Edits = tuple[list[DocumentEdit], map[ChangeAnnotationId, ChangeAnnotation]];

// Rascal compiler-specific extension
void throwAnyErrors(list[ModuleMessages] mmsgs) {
for (mmsg <- mmsgs) {
Expand Down Expand Up @@ -190,7 +192,7 @@ private set[IllegalRenameReason] rascalCollectIllegalRenames(WorkspaceInfo ws, s
private str rascalEscapeName(str name) = name in getRascalReservedIdentifiers() ? "\\<name>" : name;

// Find the smallest trees of defined non-terminal type with a source location in `useDefs`
private set[loc] rascalFindNamesInUseDefs(start[Module] m, set[loc] useDefs) {
private map[loc, loc] rascalFindNamesInUseDefs(start[Module] m, set[loc] useDefs) {
map[loc, loc] useDefNameAt = ();
useDefsToDo = useDefs;
visit(m.top) {
Expand All @@ -206,7 +208,7 @@ private set[loc] rascalFindNamesInUseDefs(start[Module] m, set[loc] useDefs) {
throw unsupportedRename("Rename unsupported", issues={<l, "Cannot find the name for this definition in <m.src.top>."> | l <- useDefsToDo});
}

return range(useDefNameAt);
return useDefNameAt;
}

Maybe[loc] rascalLocationOfName(Name n) = just(n.src);
Expand All @@ -227,16 +229,25 @@ Maybe[loc] rascalLocationOfName(Nonterminal nt) = just(nt.src);
Maybe[loc] rascalLocationOfName(NonterminalLabel l) = just(l.src);
default Maybe[loc] rascalLocationOfName(Tree t) = nothing();

private tuple[set[IllegalRenameReason] reasons, list[TextEdit] edits] computeTextEdits(WorkspaceInfo ws, start[Module] m, set[loc] defs, set[loc] uses, str name) {
if (reasons := rascalCollectIllegalRenames(ws, m, defs, uses, name), reasons != {}) {
private tuple[set[IllegalRenameReason] reasons, list[TextEdit] edits] computeTextEdits(WorkspaceInfo ws, start[Module] m, set[RenameLocation] defs, set[RenameLocation] uses, str name) {
if (reasons := rascalCollectIllegalRenames(ws, m, toLocs(defs), toLocs(uses), name), reasons != {}) {
return <reasons, []>;
}

replaceName = rascalEscapeName(name);
return <{}, [replace(l, replaceName) | l <- rascalFindNamesInUseDefs(m, defs + uses)]>;

set[RenameLocation] renames = defs + uses;
set[loc] renameLocs = toLocs(renames);
map[loc, loc] namesAt = rascalFindNamesInUseDefs(m, renameLocs);
rel[loc, Maybe[ChangeAnnotationId]] annosAt = {<r.l, r.annotation> | r <- renames};

return <{}, [{just(annotation), *_} := annosAt[l]
? replace(namesAt[l], replaceName, annotation)
: replace(namesAt[l], replaceName)
| l <- renameLocs]>;
}

private tuple[set[IllegalRenameReason] reasons, list[TextEdit] edits] computeTextEdits(WorkspaceInfo ws, loc moduleLoc, set[loc] defs, set[loc] uses, str name) =
private tuple[set[IllegalRenameReason] reasons, list[TextEdit] edits] computeTextEdits(WorkspaceInfo ws, loc moduleLoc, set[RenameLocation] defs, set[RenameLocation] uses, str name) =
computeTextEdits(ws, parseModuleWithSpacesCached(moduleLoc), defs, uses, name);

private bool rascalIsFunctionLocalDefs(WorkspaceInfo ws, set[loc] defs) {
Expand Down Expand Up @@ -508,8 +519,8 @@ private bool rascalContainsName(loc l, str name) {
2. It does not change the semantics of the application.
3. It does not change definitions outside of the current workspace.
}
list[DocumentEdit] rascalRenameSymbol(Tree cursorT, set[loc] workspaceFolders, str newName, PathConfig(loc) getPathConfig)
= job("renaming <cursorT> to <newName>", list[DocumentEdit](void(str, int) step) {
Edits rascalRenameSymbol(Tree cursorT, set[loc] workspaceFolders, str newName, PathConfig(loc) getPathConfig)
= job("renaming <cursorT> to <newName>", Edits(void(str, int) step) {
loc cursorLoc = cursorT.src;
str cursorName = "<cursorT>";

Expand Down Expand Up @@ -568,14 +579,43 @@ list[DocumentEdit] rascalRenameSymbol(Tree cursorT, set[loc] workspaceFolders, s
}

step("collecting uses of \'<cursorName>\'", 1);
<defs, uses, getRenames> = rascalGetDefsUses(ws, cur, rascalMayOverloadSameName);

rel[loc file, loc defines] defsPerFile = {<d.top, d> | d <- defs};
rel[loc file, loc uses] usesPerFile = {<u.top, u> | u <- uses};
map[ChangeAnnotationId, ChangeAnnotation] changeAnnotations = ();
ChangeAnnotationRegister registerChangeAnnotation = ChangeAnnotationId(str label, str description, bool needsConfirmation) {
ChangeAnnotationId makeKey(str label, int suffix) = "<label>_<suffix>";

int suffix = 1;
while (makeKey(label, suffix) in changeAnnotations) {
suffix += 1;
}

ChangeAnnotationId id = makeKey(label, suffix);
changeAnnotations[id] = changeAnnotation(label, description, needsConfirmation);

return id;
};

set[RenameLocation] registerChangeAnnotations(set[RenameLocation] locs, str label, str description = "These changes are required for a correct renaming. They can be previewed here, but it is not advised to disable them.", bool needsConfirmation = false) {
set[RenameLocation] modifiedLocs = {};
for (rl <- locs) {
if (nothing() := rl.annotation) {
modifiedLocs += rl[annotation = just(registerChangeAnnotation(label, description, needsConfirmation))];
} else {
modifiedLocs += rl;
}
}
return modifiedLocs;
}

<defs, uses, getRenames> = rascalGetDefsUses(ws, cur, rascalMayOverloadSameName, registerChangeAnnotation);

rel[loc file, RenameLocation defines] defsPerFile = {<d.l.top, d> | d <- registerChangeAnnotations(defs, "Definitions")};
rel[loc file, RenameLocation uses] usesPerFile = {<u.l.top, u> | u <- registerChangeAnnotations(uses, "References")};

set[loc] \files = defsPerFile.file + usesPerFile.file;

step("checking rename validity", 1);

map[loc, tuple[set[IllegalRenameReason] reasons, list[TextEdit] edits]] moduleResults =
(file: <reasons, edits> | file <- \files, <reasons, edits> := computeTextEdits(ws, file, defsPerFile[file], usesPerFile[file], newName));

Expand All @@ -587,7 +627,7 @@ list[DocumentEdit] rascalRenameSymbol(Tree cursorT, set[loc] workspaceFolders, s
list[DocumentEdit] changes = [changed(file, moduleResults[file].edits) | file <- moduleResults];
list[DocumentEdit] renames = [renamed(from, to) | <from, to> <- getRenames(newName)];

return changes + renames;
return <changes + renames, changeAnnotations>;
}, totalWork = 6);

//// WORKAROUNDS
Expand Down
42 changes: 42 additions & 0 deletions rascal-lsp/src/main/rascal/lang/rascal/lsp/refactor/TextEdits.rsc
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
@license{
Copyright (c) 2018-2023, NWO-I CWI and Swat.engineering
All rights reserved.

Redistribution and use in source and binary forms, with or without
modification, are permitted provided that the following conditions are met:

1. Redistributions of source code must retain the above copyright notice,
this list of conditions and the following disclaimer.

2. Redistributions in binary form must reproduce the above copyright notice,
this list of conditions and the following disclaimer in the documentation
and/or other materials provided with the distribution.

THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
POSSIBILITY OF SUCH DAMAGE.
}
module lang::rascal::lsp::refactor::TextEdits

extend analysis::diff::edits::TextEdits;

alias ChangeAnnotationId = str;
toinehartman marked this conversation as resolved.
Show resolved Hide resolved

data ChangeAnnotation
toinehartman marked this conversation as resolved.
Show resolved Hide resolved
= changeAnnotation(str label, str description, bool needsConfirmation)
toinehartman marked this conversation as resolved.
Show resolved Hide resolved
;

data TextEdit
= replace(loc range, str replacement, ChangeAnnotationId annotation)
;

alias ChangeAnnotationRegister =
ChangeAnnotationId(str label, str description, bool needsConfirmation);
Loading
Loading