java.project.sourcePaths doesn't refresh diagnostics #1771
Conversation
| IPath path = unit.getPath(); | ||
| IFile file = ResourcesPlugin.getWorkspace().getRoot().getFile(path); | ||
| if (file.exists()) { | ||
| String contents = null; | ||
| try { | ||
| if (unit.hasUnsavedChanges()) { | ||
| contents = unit.getSource(); | ||
| } | ||
| } catch (Exception e) { | ||
| JavaLanguageServerPlugin.logException(e.getMessage(), e); | ||
| } | ||
| unit.discardWorkingCopy(); | ||
| if (unit.equals(CoreASTProvider.getInstance().getActiveJavaElement())) { | ||
| CoreASTProvider.getInstance().disposeAST(); | ||
| } | ||
| unit = JavaCore.createCompilationUnitFrom(file); | ||
| unit.becomeWorkingCopy(null); | ||
| if (contents != null) { | ||
| unit.getBuffer().setContents(contents); | ||
| } | ||
| } |
There was a problem hiding this comment.
I didn't understand the purpose of discardWorkingCopy and becomeWorkingCopy again. For me, it's not necessary.
There was a problem hiding this comment.
I haven't looked too closely yet, but I'd be curious whether commitWorkingCopy(..) or reconcile(..) might achieve the same result. If you're doing this to re-trigger problem detection, I believe reconcile has an option to force that.
There was a problem hiding this comment.
I didn't understand the purpose of discardWorkingCopy and becomeWorkingCopy again. For me, it's not necessary.
We have to discard the old working copy (discardWorkingCopy, the line 79), create a new compilation unit (JavaCore.createCompilationUnitFrom,the line 83) and change it into a working copy (becomeWorkingCopy,the line 84).
If we remove the line 79, we will make a memory leak. If we remove the line 84, problems aren't reported to the problem requestor of the working copy owner.
You can try the following:
- remove the lines 79 and 84 (discardWorkingCopy and becomeWorkingCopy)
- follow the steps from java.project.sourcePaths doesn't refresh diagnostics #1769 (comment)
- try to edit Foo.java
Problems aren't reported properly.
I haven't looked too closely yet, but I'd be curious whether commitWorkingCopy(..) or reconcile(..) might achieve the same result. If you're doing this to re-trigger problem detection, I believe reconcile has an option to force that.
I have tried them, but they don't help.
There was a problem hiding this comment.
After investigating this a bit more, I can see why the code here is needed.
Generally we create a working copy for a compilation unit when the file is opened, and discard it when it's closed. This makes sense since we need a working copy to make modifications.
When the source path is updated with the correct value, we update the project configuration (source, output, classpaths) but it's likely the working copy needs some updating too, or it will remain in the state with an improper source path). The only hint I could find for updating the working copy is in didChange(..). So looking at DocumentLifeCycleHandler#handleChanged(..) I see sharedASTProvider.disposeAST() and cu.makeConsistent(..) (in triggerValidation(..)) . @snjeza, have you tried diposeAST() combined with makeConsistent() ? If that still doesn't work, we can probably look at taking the change since it does seem to fix things.
There was a problem hiding this comment.
@snjeza, have you tried diposeAST() combined with makeConsistent()
Yes, I am. It doesn't work.
There was a problem hiding this comment.
Looks like we do some very similar stuff in https://github.com/eclipse/eclipse.jdt.ls/blob/master/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/BaseDocumentLifeCycleHandler.java and particularly checkPackageDeclaration.
I'll just verify the tests are fine and I think we can merge.
.../src/org/eclipse/jdt/ls/core/internal/managers/InvisibleProjectPreferenceChangeListener.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Snjezana Peco <[email protected]>
When we update a source path, all working copies in this source folder are invalid. They have a new package fragment root.
Closing/opening fixes it in Eclipse. |
Fixes #1769
Signed-off-by: Snjezana Peco [email protected]