From 92c2dbecdc7cb2d04f29b2303d1eea55b3f568e3 Mon Sep 17 00:00:00 2001 From: Toine Hartman Date: Wed, 27 Nov 2024 14:03:32 +0100 Subject: [PATCH 1/3] Check early (loosely) whether entered name is valid. --- .../lang/rascal/lsp/refactor/Rename.rsc | 43 ++++++++----------- .../rascal/lang/rascal/lsp/refactor/Util.rsc | 13 ++++++ .../lang/rascal/tests/rename/ValidNames.rsc | 3 ++ 3 files changed, 33 insertions(+), 26 deletions(-) diff --git a/rascal-lsp/src/main/rascal/lang/rascal/lsp/refactor/Rename.rsc b/rascal-lsp/src/main/rascal/lang/rascal/lsp/refactor/Rename.rsc index 3707ccb5f..1749fd4e6 100644 --- a/rascal-lsp/src/main/rascal/lang/rascal/lsp/refactor/Rename.rsc +++ b/rascal-lsp/src/main/rascal/lang/rascal/lsp/refactor/Rename.rsc @@ -37,6 +37,7 @@ module lang::rascal::lsp::refactor::Rename import Exception; import IO; +import Grammar; import List; import Location; import Map; @@ -76,35 +77,23 @@ void throwAnyErrors(program(_, msgs)) { throwAnyErrors(msgs); } -set[IllegalRenameReason] rascalCheckLegalName(str name, set[IdRole] defRoles) { - set[IllegalRenameReason] tryParseAs(type[&T <: Tree] begin, str idDescription) { - try { - parse(begin, rascalEscapeName(name)); - return {}; - } catch ParseError(_): { - return {invalidName(name, idDescription)}; - } - } +private set[IllegalRenameReason] rascalCheckLegalName(str name, set[IdRole] roles) { + escName = rascalEscapeName(name); + tuple[type[&T <: 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">; + if (size(syntaxRoles & roles) > 0) asType = <#Nonterminal, "non-terminal name">; - bool isSyntaxRole = any(role <- defRoles, role in syntaxRoles); - bool isField = any(role <- defRoles, role is fieldId); - bool isConstructor = any(role <- defRoles, role is constructorId); + if (!tryParseAs(asType.as, escName)) return {invalidName(escName, asType.desc)}; + return {}; +} - set[IllegalRenameReason] reasons = {}; - if (isSyntaxRole) { - reasons += tryParseAs(#Nonterminal, "non-terminal name"); - } - if (isField) { - reasons += tryParseAs(#NonterminalLabel, "constructor field name"); +private void rascalCheckLegalName(str name, Symbol sym) { + g = grammar(#start[Module]); + if (!tryParseAs(type(sym, g.rules), name)) { + throw illegalRename("\'\' is not a valid name at this position", {invalidName(name, "")}); } - if (isConstructor) { - reasons += tryParseAs(#NonterminalLabel, "constructor name"); - } - if (!(isSyntaxRole || isField || isConstructor)) { - reasons += tryParseAs(#Name, "identifier"); - } - - return reasons; } private set[IllegalRenameReason] rascalCheckDefinitionsOutsideWorkspace(WorkspaceInfo ws, set[loc] defs) = @@ -531,6 +520,8 @@ Edits rascalRenameSymbol(Tree cursorT, set[loc] workspaceFolders, str newName, P loc cursorLoc = cursorT.src; str cursorName = ""; + rascalCheckLegalName(newName, typeOf(cursorT)); + step("collecting workspace information", 1); WorkspaceInfo ws = workspaceInfo( // Get path config diff --git a/rascal-lsp/src/main/rascal/lang/rascal/lsp/refactor/Util.rsc b/rascal-lsp/src/main/rascal/lang/rascal/lsp/refactor/Util.rsc index d062a5df2..9d7fdc7e9 100644 --- a/rascal-lsp/src/main/rascal/lang/rascal/lsp/refactor/Util.rsc +++ b/rascal-lsp/src/main/rascal/lang/rascal/lsp/refactor/Util.rsc @@ -30,6 +30,7 @@ import IO; import List; import Location; import Message; +import ParseTree; import String; import util::Maybe; @@ -79,6 +80,18 @@ start[Module] parseModuleWithSpacesCached(loc l) { return parseModuleWithSpacesCached(l, lastModified(l)); } +@synopsis{ + Try to parse string `name` as reified type `begin` and return whether this succeeded. +} +bool tryParseAs(type[&T <: Tree] begin, str name, bool allowAmbiguity = false) { + try { + parse(begin, name, allowAmbiguity = allowAmbiguity); + return true; + } catch ParseError(_): { + return false; + } +} + Maybe[&B] flatMap(nothing(), Maybe[&B](&A) _) = nothing(); Maybe[&B] flatMap(just(&A a), Maybe[&B](&A) f) = f(a); diff --git a/rascal-lsp/src/main/rascal/lang/rascal/tests/rename/ValidNames.rsc b/rascal-lsp/src/main/rascal/lang/rascal/tests/rename/ValidNames.rsc index 76ab56160..545e1bf84 100644 --- a/rascal-lsp/src/main/rascal/lang/rascal/tests/rename/ValidNames.rsc +++ b/rascal-lsp/src/main/rascal/lang/rascal/tests/rename/ValidNames.rsc @@ -57,3 +57,6 @@ test bool newNameHasNumericPrefix() = testRename("int foo = 8;", newName = "8abc @expected{illegalRename} test bool newNameIsEscapedInvalid() = testRename("int foo = 8;", newName = "\\8int"); + +@expected{illegalRename} +test bool qualifiedNameWhereNameExpected() = testRename("int foo = 8;", newName = "Foo::foo"); From c60bd76295bac1534ac0acfaae2c56d2c040c9da Mon Sep 17 00:00:00 2001 From: Toine Hartman Date: Wed, 27 Nov 2024 14:22:14 +0100 Subject: [PATCH 2/3] Change exception type, so the exception shows it just once. --- .../src/main/rascal/lang/rascal/lsp/refactor/Exception.rsc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rascal-lsp/src/main/rascal/lang/rascal/lsp/refactor/Exception.rsc b/rascal-lsp/src/main/rascal/lang/rascal/lsp/refactor/Exception.rsc index 2f6ec3df3..c51d72d4b 100644 --- a/rascal-lsp/src/main/rascal/lang/rascal/lsp/refactor/Exception.rsc +++ b/rascal-lsp/src/main/rascal/lang/rascal/lsp/refactor/Exception.rsc @@ -40,7 +40,7 @@ data IllegalRenameReason | definitionsOutsideWorkspace(set[loc] defs) ; -data RuntimeException +data RenameException = illegalRename(str message, set[IllegalRenameReason] reason) | unsupportedRename(str message, rel[loc location, str message] issues = {}) | unexpectedFailure(str message) From 4fa9c790f2a537892f1b15d6218cecc4e3366777 Mon Sep 17 00:00:00 2001 From: Toine Hartman Date: Wed, 27 Nov 2024 14:46:49 +0100 Subject: [PATCH 3/3] Check for validity of escaped name. --- .../src/main/rascal/lang/rascal/lsp/refactor/Rename.rsc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rascal-lsp/src/main/rascal/lang/rascal/lsp/refactor/Rename.rsc b/rascal-lsp/src/main/rascal/lang/rascal/lsp/refactor/Rename.rsc index 1749fd4e6..22752533a 100644 --- a/rascal-lsp/src/main/rascal/lang/rascal/lsp/refactor/Rename.rsc +++ b/rascal-lsp/src/main/rascal/lang/rascal/lsp/refactor/Rename.rsc @@ -90,9 +90,10 @@ private set[IllegalRenameReason] rascalCheckLegalName(str name, set[IdRole] role } private void rascalCheckLegalName(str name, Symbol sym) { + escName = rascalEscapeName(name); g = grammar(#start[Module]); - if (!tryParseAs(type(sym, g.rules), name)) { - throw illegalRename("\'\' is not a valid name at this position", {invalidName(name, "")}); + if (!tryParseAs(type(sym, g.rules), escName)) { + throw illegalRename("\'\' is not a valid name at this position", {invalidName(escName, "")}); } }