You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by "shivam71 (via GitHub)" <gi...@apache.org> on 2025/10/29 11:15:55 UTC
[PR] Propagate refactoring problems to LSP client [netbeans]
shivam71 opened a new pull request, #8968:
URL: https://github.com/apache/netbeans/pull/8968
## Issue
- LSP Server didn't inform LSP Client about refactoring changes that can lead to broken code.
- For example
- Trying to move a non static data member to another class often breaks code.
- Changing method parameters by misspelling type of the parameter like 'string'
- Such problems are found by Refactoring Plugins.
- Prior to the current change such problems (if not deemed fatal) were not propagated to the LSP Client.
- In Netbeans IDE such warnings are shown and confirmation is asked for proceedings with the warnings.
- With the current change such non fatal problems would be shown as warning message to LSP Client.
- Further LSP Server would ask for confirmation to proceed despite the warnings.
Example: Warnings in Netbeans
<img width="1512" height="982" alt="Netbeans_Refactoring_Warnings" src="https://github.com/user-attachments/assets/948c720d-a834-4980-b61f-f4a2899223b7" />
After this change same warnings propagated to LSP Client (VSCode).
<img width="1203" height="894" alt="VSCode_Warnings" src="https://github.com/user-attachments/assets/fd8c3ff5-1a8d-4ac0-9cf7-e14e9ae15250" />
------
<details open>
<summary>Click to collapse/expand PR instructions</summary>
By opening a pull request you confirm that, unless explicitly stated otherwise, the changes -
- are all your own work, and you have the right to contribute them.
- are contributed solely under the terms and conditions of the Apache License 2.0 (see section 5 of the license for more information).
Please make sure (eg. `git log`) that all commits have a valid name and email address for you in the Author field.
If you're a first time contributor, see the Contributing guidelines for more information.
If you're a committer, please label the PR before pressing "Create pull request" so that the right test jobs can run.
### PR approval and merge checklist:
1. [ ] Was this PR [correctly labeled](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=240884239#PRsandYouAreviewerGuide-PRtriggeredCIJobs(conditionalCIpipeline)), did the right tests run? When did they run?
2. [ ] Is this PR [squashed](https://cwiki.apache.org/confluence/display/NETBEANS/git%3A+squash+and+merge)?
3. [ ] Are author name / email address correct? Are [co-authors](https://docs.github.com/en/pull-requests/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors#creating-co-authored-commits-on-the-command-line) correctly listed? Do the commit messages need updates?
3. [ ] Does the PR title and description still fit after the Nth iteration? Is the description sufficient to appear in the release notes?
If this PR targets the delivery branch: [don't merge](https://cwiki.apache.org/confluence/display/NETBEANS/Pull+requests+for+delivery). ([full wiki article](https://cwiki.apache.org/confluence/display/NETBEANS/PRs+and+You+-+A+reviewer+Guide))
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Propagate refactoring problems to LSP client [netbeans]
Posted by "shivam71 (via GitHub)" <gi...@apache.org>.
shivam71 commented on PR #8968:
URL: https://github.com/apache/netbeans/pull/8968#issuecomment-3484036834
Sorry for the MoveRefactoring formatting related diffs , will revert those .
Okay I go over the code and try to refactor to reduce duplication .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Propagate refactoring problems to LSP client [netbeans]
Posted by "shivam71 (via GitHub)" <gi...@apache.org>.
shivam71 commented on PR #8968:
URL: https://github.com/apache/netbeans/pull/8968#issuecomment-3485222409
@lahodaj I have tried to refactor a bit , please review again . Thank you !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Propagate refactoring problems to LSP client [netbeans]
Posted by "lahodaj (via GitHub)" <gi...@apache.org>.
lahodaj merged PR #8968:
URL: https://github.com/apache/netbeans/pull/8968
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Propagate refactoring problems to LSP client [netbeans]
Posted by "lahodaj (via GitHub)" <gi...@apache.org>.
lahodaj commented on PR #8968:
URL: https://github.com/apache/netbeans/pull/8968#issuecomment-3481972691
Overall +1 on reporting the warnings to the user.
But, please:
- there's a lot of code duplication in each refactoring class that handles the warnings, etc. Is there a chance that would be factored out into `CodeRefactoring`? Maybe it would also allow to reduce churn in that class?
- in `java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/refactoring/MoveRefactoring.java`, it seems there's a huge amount of changes, which are probably mostly formatting(?) - I would suggest to undo that, it makes it very hard to review that part of the PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Propagate refactoring problems to LSP client [netbeans]
Posted by "lahodaj (via GitHub)" <gi...@apache.org>.
lahodaj commented on PR #8968:
URL: https://github.com/apache/netbeans/pull/8968#issuecomment-3502663653
Functionally seems OK to me. There are some formatting/cleanup that I would suggest to do, shown here:
https://github.com/shivam71/netbeans/pull/1
(reducing access, removing stuff no longer needed, keeping spaces consistent with the file).
If I was writing this, I would probably inline `CodeRefactoring.warningsMessageParams` and `CodeRefactoring.prepare` into their callers, but I don't think that is strictly necessary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists