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.

Sign up for GitHub

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

Fix rename refactoring type errors #559

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Fix rename refactoring type errors #559

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
aa0051c
Change type of rename exceptions.
toinehartman Jan 10, 2025
3475af1
Fix `tryParseAs` types.
toinehartman Jan 10, 2025
57ad93e
Add missing imports.
toinehartman Jan 10, 2025
9c91050
Simplify rename error check.
toinehartman Jan 10, 2025
78e510c
Add another missing import.
toinehartman Jan 10, 2025
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
2 changes: 1 addition & 1 deletion rascal-lsp/src/main/java/org/rascalmpl/vscode/lsp/rascal/RascalLanguageServices.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ public InterruptibleFuture<ITuple> getRename(ITree cursorTree, Set<ISourceLocati
} catch (Throw e) {
if (e.getException() instanceof IConstructor) {
var exception = (IConstructor)e.getException();
if (exception.getType().getAbstractDataType().getName().equals("RenameException")) {
if (exception.getName().contains("Rename")) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there anyway that now or in the future user code is called that could throw an exception? If so, is "having Rename" in the name discerning enough? Maybe we can also check the type including the "package" part to make sure it is an "official" Rename exception?

// instead of the generic exception handler, we deal with these ourselfs
// and report an LSP error, such that the IDE shows them in a user friendly way
String message;
Expand Down
4 changes: 2 additions & 2 deletions rascal-lsp/src/main/rascal/lang/rascal/lsp/refactor/Exception.rsc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ data IllegalRenameReason
| definitionsOutsideWorkspace(set[loc] defs)
;

data RenameException
data RuntimeException
= illegalRename(str message, set[IllegalRenameReason] reason)
| unsupportedRename(str message, rel[loc location, str message] issues = {})
| unexpectedFailure(str message)
| unexpectedRenameFailure(str message)
;

str describe(invalidName(name, idDescription)) = "\'<name>\' is not a valid <idDescription>";
Expand Down
15 changes: 8 additions & 7 deletions rascal-lsp/src/main/rascal/lang/rascal/lsp/refactor/Rename.rsc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,9 @@ void throwAnyErrors(program(_, msgs)) {
throwAnyErrors(msgs);
}

private set[IllegalRenameReason] rascalCheckLegalName(str name, set[IdRole] roles) {
private set[IllegalRenameReason] rascalCheckLegalNameByRoles(str name, set[IdRole] roles) {
escName = rascalEscapeName(name);
tuple[type[&T <: Tree] as, str desc] asType = <#Name, "identifier">;
tuple[type[Tree] as, str desc] asType = <#Name, "identifier">;
if ({moduleId(), *_} := roles) asType = <#QualifiedName, "module name">;
if ({constructorId(), *_} := roles) asType = <#NonterminalLabel, "constructor name">;
if ({fieldId(), *_} := roles) asType = <#NonterminalLabel, "constructor field name">;
Expand All @@ -90,10 +90,11 @@ private set[IllegalRenameReason] rascalCheckLegalName(str name, set[IdRole] role
return {};
}

private void rascalCheckLegalName(str name, Symbol sym) {
private void rascalCheckLegalNameByType(str name, Symbol sym) {
escName = rascalEscapeName(name);
g = grammar(#start[Module]);
if (tryParseAs(type(sym, g.rules), escName) is nothing) {
if (type[Tree] t := type(sym, g.rules)
, tryParseAs(t, escName) is nothing) {
throw illegalRename("\'<escName>\' is not a valid name at this position", {invalidName(escName, "<sym>")});
}
}
Expand Down Expand Up @@ -177,7 +178,7 @@ private set[IllegalRenameReason] rascalCollectIllegalRenames(TModel ws, rel[loc
set[loc] editFiles = defsPerFile.file + usesPerFile.file;

set[IllegalRenameReason] reasons = {};
reasons += rascalCheckLegalName(newName, definitionsRel(ws)[defsPerFile.rename.l].idRole);
reasons += rascalCheckLegalNameByRoles(newName, definitionsRel(ws)[defsPerFile.rename.l].idRole);
reasons += rascalCheckDefinitionsOutsideWorkspace(ws, defsPerFile.rename.l);
reasons += rascalCheckCausesDoubleDeclarations(ws, defsPerFile.rename.l, newNameDefs, newName);
for (file <- editFiles) {
Expand Down Expand Up @@ -495,7 +496,7 @@ private set[TModel] rascalTModels(set[loc] fs, PathConfig pcfg) {
set[TModel] tmodels = {};
for (str modName <- ms.moduleLocs) {
<found, tm, ms> = getTModelForModule(modName, ms);
if (!found) throw unexpectedFailure("Cannot read TModel for module \'<modName>\'\n<toString(ms.messages)>");
if (!found) throw unexpectedRenameFailure("Cannot read TModel for module \'<modName>\'\n<toString(ms.messages)>");
tmodels += convertTModel2PhysicalLocs(tm);
}
return tmodels;
Expand Down Expand Up @@ -599,7 +600,7 @@ Edits rascalRenameSymbol(Tree cursorT, set[loc] workspaceFolders, str newName, P
loc cursorLoc = cursorT.src;
str cursorName = "<cursorT>";

rascalCheckLegalName(newName, typeOf(cursorT));
rascalCheckLegalNameByType(newName, typeOf(cursorT));

step("preloading minimal workspace information", 1);
set[TModel] localTmodelsForFiles(ProjectFiles projectFiles) = tmodelsForProjectFiles(projectFiles, rascalTModels, getPathConfig);
Expand Down
1 change: 1 addition & 0 deletions rascal-lsp/src/main/rascal/lang/rascal/tests/rename/Grammars.rsc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ POSSIBILITY OF SUCH DAMAGE.
module lang::rascal::tests::rename::Grammars

import lang::rascal::tests::rename::TestUtils;
import lang::rascal::lsp::refactor::Exception;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra import but no use of the import? The same goes for imports in ProjectOnDisk.rsc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These imports fix errors regarding undefined @expected exceptions in these modules.


test bool productionType() = testRenameOccurrences({0, 1, 2, 3}, "
'Foo func(Foo f) = f.child;
Expand Down
2 changes: 2 additions & 0 deletions rascal-lsp/src/main/rascal/lang/rascal/tests/rename/ProjectOnDisk.rsc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,10 @@ POSSIBILITY OF SUCH DAMAGE.
module lang::rascal::tests::rename::ProjectOnDisk

import lang::rascal::lsp::refactor::Rename;
import lang::rascal::lsp::refactor::Util;
import lang::rascal::tests::rename::TestUtils;
import util::Reflective;
import lang::rascalcore::check::Checker;

Edits testProjectOnDisk(loc projectDir, str file, str oldName, int occurrence = 0, str newName = "<oldName>_new") {
PathConfig pcfg;
Expand Down
Loading