Fix NPE in DocumentMonitor

Signed-off-by: Odin Dahlström <[email protected]>
eclipse-jdtls · Jul 4, 2023 · 7a0313e · 7a0313e
1 parent 66669c8
commit 7a0313e
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 3 deletions.
diff --git a/...t.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/BaseDocumentLifeCycleHandler.java b/...t.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/BaseDocumentLifeCycleHandler.java
@@ -757,7 +757,7 @@ public boolean belongsTo(Object family) {
 	public class DocumentMonitor {
 
 		private final String uri;
-		private final int initialVersion;
+		private final Integer initialVersion;
 
 		public DocumentMonitor(String uri) {
 			this.uri = uri;
@@ -769,7 +769,14 @@ public DocumentMonitor(String uri) {
 		 * of this monitor, {@code false} otherwise.
 		 */
 		public boolean hasChanged() {
-			return initialVersion != documentVersions.get(uri);
+			Integer currentVersion = documentVersions.get(uri);
+			// If the initial and current version is null, it would indicate that
+			// the document is not open. In such cases, the LSP spec still says:
+			// "a server's ability to fulfill requests is independent of whether
+			// a text document is open or closed". In order to service such
+			// requests, we have to assume that a closed document has not changed.
+			// See also: https://github.com/eclipse/eclipse.jdt.ls/discussions/2706
+			return !Objects.equals(initialVersion, currentVersion);
 		}
 
 		/**

diff --git a/....ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/DocumentLifeCycleHandlerTest.java b/....ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/DocumentLifeCycleHandlerTest.java
@@ -841,14 +841,37 @@ public void testDocumentMonitor() throws Exception {
 		DocumentMonitor documentMonitor = lifeCycleHandler.new DocumentMonitor(JDTUtils.toURI(cu));
 		documentMonitor.checkChanged();
 		changeDocumentFull(cu, content, 2);
+		assertChanged(documentMonitor);
+		closeDocument(cu);
+	}
+
+	@Test
+	public void testDocumentMonitorClosedDocument() throws Exception {
+		IJavaProject javaProject = newEmptyProject();
+		IPackageFragmentRoot sourceFolder = javaProject.getPackageFragmentRoot(javaProject.getProject().getFolder("src"));
+		IPackageFragment fooPackage = sourceFolder.createPackageFragment("foo", false, null);
+
+		String content = "package foo;\n";
+		ICompilationUnit cu = fooPackage.createCompilationUnit("Foo.java", content, false, null);
+
+		DocumentMonitor documentMonitorBeforeOpen = lifeCycleHandler.new DocumentMonitor(JDTUtils.toURI(cu));
+		openDocument(cu, content, 1);
+		changeDocumentFull(cu, content, 2);
+		assertChanged(documentMonitorBeforeOpen); // Version changed (null -> 2)
+		closeDocument(cu);
+
+		DocumentMonitor documentMonitorAfterClose = lifeCycleHandler.new DocumentMonitor(JDTUtils.toURI(cu));
+		documentMonitorAfterClose.checkChanged(); // Version not changed (null -> null)
+	}
+
+	private void assertChanged(DocumentMonitor documentMonitor) {
 		try {
 			documentMonitor.checkChanged();
 			fail("Should have thrown ResponseErrorException");
 		}
 		catch (ResponseErrorException e) {
 			assertEquals(e.getResponseError().getCode(), -32801); // ContentModified error code
 		}
-		closeDocument(cu);
 	}
 
 	private File createTempFile(File parent, String fileName, String content) throws IOException {