Added and improved user input validation for LSPTemplateUI #7893
Conversation
shivam71
commented
Oct 22, 2024
shivam71
commented
- Project,Package and Object names checked to be valid identifiers . Giving LSP client error messages for funky names like "123Funky$ Name"
- Project and Object placement directories checked for write permission.
Achal1607
added
Java
|
@lahodaj Please review my changes . |
|
|
||
| @Override | ||
| public CompletionStage<String> apply(String packageName) { | ||
| if (!SourceVersion.isName(packageName)) { |
There was a problem hiding this comment.
Here, we are specifically looking for a package name, so the check makes sense to me.
| @@ -215,12 +238,14 @@ public CompletionStage<Pair<DataFolder,String>> apply(String path) { | |||
| } | |||
| targetPath.getParentFile().mkdirs(); | |||
| FileObject fo = FileUtil.toFileObject(targetPath.getParentFile()); | |||
| if (fo == null || !fo.isFolder()) { | |||
| if (fo == null || !fo.isFolder() || !targetPath.getParentFile().canWrite() || !SourceVersion.isName(targetPath.getName())) { | |||
There was a problem hiding this comment.
I am not quite sure I read the patch correctly, but this is the folder where the project will be created. And the name of this folder does not necessarily need to follow the Java name rules(?). I mean, I can have a Java project in a directory called my-current-java-project, and there's no problem in that? Or do I miss something here?
There was a problem hiding this comment.
Right it will be the name of the folder where the project will be created . We need the java name rules because that is also used for the name of the main class . So if the path is /Users/Projects/my-current-java-project it will try to create a project with the main class name as my-current-java-project.
There was a problem hiding this comment.
this still checks java constraints for non-java projects.
|
|
||
| @Override | ||
| public CompletionStage<String> apply(String name) { | ||
| if (!SourceVersion.isName(name)) { |
There was a problem hiding this comment.
This may be OK for Java templates, but probably not for others (like plain files, XML/HTML files, etc.) And this will be used for all file types, I think. I don't know offhand how to do validation only for Java, it might need another attribute on the template(?). @sdedic? Do I miss something?
There was a problem hiding this comment.
I don't know offhand how to do validation only for Java, it might need another attribute on the template(?). @sdedic? Do I miss something?
I think a validator could specified by a template's file attribute, or by template's MIME type (for singe-file templates).
There was a problem hiding this comment.
@lahodaj Thanks for pointing that out ! I looked up a bit more on this , it seems that all java templates have names ending with ".java" suffix , Can you confirm that ? If thats the case then I can condition on the template name whether to apply the validation check or not .
There was a problem hiding this comment.
Updated the PR with conditional validation using MIME type .
- Validations for package name, object name and folder paths
shivam71
force-pushed
the
LSPTemplateUI_UserInputValidation
branch
from
1dd8d80 to
8e94890
Compare
