[#1316| Add end line and end column positions to Issue

[#1316] Add end line and end column positions to Issue. Also initialise lines and columns with 0. 

Previously, end line and end column information was not part of Issue, IssueImpl, IssueLocation, AbstractDiagnostic, and ExceptionDiagnostic. It is helpful to have this information in these classes, because otherwise (i.e. now in LanguageServerImpl#toDiagnostic(Issue)) this information has to be computed by loading the Document again.

closes #1316 

Signed-off-by: mmews <marcus.mews@numberfour.eu>

org.eclipse.xtext.ide/src/org/eclipse/xtext/ide/server/LanguageServerImpl.java

@@ -453,13 +453,7 @@ public class LanguageServerImpl implements LanguageServer, WorkspaceService, Tex
 		initialized.thenAccept((initParams) -> {
 			PublishDiagnosticsParams publishDiagnosticsParams = new PublishDiagnosticsParams();
 			publishDiagnosticsParams.setUri(uriExtensions.toUriString(uri));
-			// this is not a premature optimization but a trick to handle issues of deleted resources
-			if (!issues.iterator().hasNext()) {
-				publishDiagnosticsParams.setDiagnostics(Collections.emptyList());
-			} else {
-				publishDiagnosticsParams.setDiagnostics(
-						workspaceManager.doRead(uri, (document, resource) -> toDiagnostics(issues, document)));
-			}
+			publishDiagnosticsParams.setDiagnostics(toDiagnostics(issues));
 			client.publishDiagnostics(publishDiagnosticsParams);
 		});
 	}
@@ -468,11 +462,11 @@ public class LanguageServerImpl implements LanguageServer, WorkspaceService, Tex
 	 * Convert the given issues to diagnostics. Does not return any issue with severity {@link Severity#IGNORE ignore}
 	 * by default.
 	 */
-	protected List<Diagnostic> toDiagnostics(Iterable<? extends Issue> issues, Document document) {
+	protected List<Diagnostic> toDiagnostics(Iterable<? extends Issue> issues) {
 		List<Diagnostic> result = new ArrayList<>();
 		for (Issue issue : issues) {
 			if (issue.getSeverity() != Severity.IGNORE) {
-				result.add(toDiagnostic(issue, document));
+				result.add(toDiagnostic(issue));
 			}
 		}
 		return result;
@@ -481,19 +475,21 @@ public class LanguageServerImpl implements LanguageServer, WorkspaceService, Tex
 	/**
 	 * Convert the given issue to a diagnostic.
 	 */
-	protected Diagnostic toDiagnostic(Issue issue, Document document) {
+	protected Diagnostic toDiagnostic(Issue issue) {
 		Diagnostic result = new Diagnostic();
 		result.setCode(issue.getCode());
 		result.setMessage(issue.getMessage());
 		result.setSeverity(toDiagnosticSeverity(issue.getSeverity()));
-		Position start = document.getPosition(issue.getOffset());
-		Position end = document.getPosition(issue.getOffset() + issue.getLength());
+
+		// line and column numbers in LSP are 0-based
+		Position start = new Position(issue.getLineNumber() - 1, issue.getColumn() - 1);
+		Position end = new Position(issue.getLineNumberEnd() - 1, issue.getColumnEnd() - 1);
 		result.setRange(new Range(start, end));
 		return result;
 	}
 
 	/**
-	 * Convert the serverity to a lsp {@link DiagnosticSeverity}.
+	 * Convert the severity to a lsp {@link DiagnosticSeverity}.
 	 *
 	 * Defaults to severity {@link DiagnosticSeverity#Hint hint}.
 	 */

org.eclipse.xtext/src/org/eclipse/xtext/diagnostics/AbstractDiagnostic.java

@@ -58,6 +58,25 @@ public abstract class AbstractDiagnostic implements Diagnostic {
 			return node.getStartLine();
 		return -1;
 	}
+	
+
+	@Override
+	public int getLineEnd() {
+		INode node = getNode();
+		if (node != null)
+			return node.getEndLine();
+		return -1;
+	}
+
+	@Override
+	public int getColumnEnd() {
+		INode node = getNode();
+		if (node != null) {
+			LineAndColumn lineAndColumn = NodeModelUtils.getLineAndColumn(node, getOffset() + getLength());
+			return lineAndColumn.getColumn();
+		}
+		return 0;
+	}
 
 	@Override
 	public String getLocation() {
@@ -86,5 +105,4 @@ public abstract class AbstractDiagnostic implements Diagnostic {
 		b.append(getMessage());
 		return b.toString();
 	}
-
 }

org.eclipse.xtext/src/org/eclipse/xtext/diagnostics/Diagnostic.java

@@ -10,7 +10,8 @@ package org.eclipse.xtext.diagnostics;
 /**
  * A specialized {@link org.eclipse.emf.ecore.resource.Resource.Diagnostic} that knows 
  * about the region in the document, e.g. the {@link #getOffset()} and {@link #getLength()}
- * of the issue source.
+ * of the issue source. A region starts in a start line before a start column and ends in an end
+ * line before an end column.
  * 
  * Implementors should inherit from {@link AbstractDiagnostic} instead of implementing this 
  * interface directly.
@@ -54,4 +55,25 @@ public interface Diagnostic extends org.eclipse.emf.ecore.resource.Resource.Diag
 	 */
 	int getLength();
 
+	/**
+	 * Returns the end line location of the issue within the source.
+	 * Line <code>1</code> is the first line of a document.
+	 * 
+	 * @return the end line location of the issue.
+	 * 
+	 * @since 2.21
+	 */
+	int getLineEnd();
+
+	/**
+	 * Returns the end column location of the issue within the source.
+	 * The region does not include the end column character itself.
+	 * Column <code>1</code> is the first column of a line.
+	 * 
+	 * @return the end column location of the issue.
+	 * 
+	 * @since 2.21
+	 */
+	int getColumnEnd();
+
 }

org.eclipse.xtext/src/org/eclipse/xtext/diagnostics/ExceptionDiagnostic.java

@@ -38,6 +38,16 @@ public class ExceptionDiagnostic implements Diagnostic {
 		return 1;
 	}
 
+	@Override
+	public int getColumnEnd() {
+		return 0;
+	}
+
+	@Override
+	public int getLineEnd() {
+		return 1;
+	}
+
 	@Override
 	public String getLocation() {
 		return null;

org.eclipse.xtext/src/org/eclipse/xtext/validation/DiagnosticConverterImpl.java

@@ -38,17 +38,25 @@ public class DiagnosticConverterImpl implements IDiagnosticConverter {
 		/**
 		 * 1-based line number.
 		 */
-		public Integer lineNumber;
+		public Integer lineNumber = 0;
 		/**
 		 * 1-based column.
 		 * @since 2.9
 		 */
-		public Integer column;
+		public Integer column = 0;
+		/**
+		 * 1-based line number end.
+		 */
+		public Integer lineNumberEnd = 0;
+		/**
+		 * 1-based column end.
+		 */
+		public Integer columnEnd = 0;
 		/**
 		 * 0-based offset.
 		 */
-		public Integer offset;
-		public Integer length;
+		public Integer offset = 0;
+		public Integer length = 0;
 	}
 	
 	@Override
@@ -70,6 +78,8 @@ public class DiagnosticConverterImpl implements IDiagnosticConverter {
 			issue.setUriToProblem(castedDiagnostic.getUriToProblem());
 			issue.setCode(castedDiagnostic.getCode());
 			issue.setData(castedDiagnostic.getData());
+			issue.setLineNumberEnd(castedDiagnostic.getLineEnd());
+			issue.setColumnEnd(castedDiagnostic.getColumnEnd());
 		}
 		issue.setType(CheckType.FAST);
 		acceptor.accept(issue);
@@ -90,6 +100,8 @@ public class DiagnosticConverterImpl implements IDiagnosticConverter {
 			issue.setColumn(locationData.column);
 			issue.setOffset(locationData.offset);
 			issue.setLength(locationData.length);
+			issue.setLineNumberEnd(locationData.lineNumberEnd);
+			issue.setColumnEnd(locationData.columnEnd);
 		}
 		final EObject causer = getCauser(diagnostic);
 		if (causer != null)
@@ -191,14 +203,7 @@ public class DiagnosticConverterImpl implements IDiagnosticConverter {
 			if (diagnostic instanceof RangeBasedDiagnostic) {
 				RangeBasedDiagnostic castedDiagnostic = (RangeBasedDiagnostic) diagnostic;
 				INode parserNode = NodeModelUtils.getNode(causer);
-				IssueLocation result = new IssueLocation();
-				if (parserNode != null) {
-					LineAndColumn lineAndColumn = NodeModelUtils.getLineAndColumn(parserNode, castedDiagnostic.getOffset());
-					result.lineNumber = lineAndColumn.getLine();
-					result.column = lineAndColumn.getColumn();
-				}
-				result.offset = castedDiagnostic.getOffset();
-				result.length = castedDiagnostic.getLength();
+				IssueLocation result = getLocationForNode(parserNode, castedDiagnostic.getOffset(), castedDiagnostic.getLength());
 				return result;
 			} else if (diagnostic instanceof FeatureBasedDiagnostic) {
 				 FeatureBasedDiagnostic castedDiagnostic = (FeatureBasedDiagnostic) diagnostic;
@@ -239,21 +244,45 @@ public class DiagnosticConverterImpl implements IDiagnosticConverter {
 					containingFeature.isMany() ? ((EList<?>) container.eGet(containingFeature)).indexOf(obj)
 							: ValidationMessageAcceptor.INSIGNIFICANT_INDEX);
 		}
-		IssueLocation result = new IssueLocation();
-		result.lineNumber = 1;
-		result.column = 1;
-		result.offset = 0;
-		result.length = 0;
-		return result;
+		
+		// place issue at start of document since we lack location information
+		IssueLocation startOfDocumentLocation = new IssueLocation();
+		startOfDocumentLocation.offset = 0;
+		startOfDocumentLocation.length = 0;
+		startOfDocumentLocation.lineNumber = 1;
+		startOfDocumentLocation.column = 1;
+		startOfDocumentLocation.lineNumberEnd = 1;
+		startOfDocumentLocation.columnEnd = 1;
+		return new IssueLocation();
 	}
 
+	/**
+	 * Computes {@link IssueLocation} for the given node.
+	 */
 	protected IssueLocation getLocationForNode(INode node) {
 		ITextRegionWithLineInformation nodeRegion = node.getTextRegionWithLineInformation();
+		int offset = nodeRegion.getOffset();
+		int length = nodeRegion.getLength();
+		return getLocationForNode(node, offset, length);
+	}
+
+	/**
+	 * Computes {@link IssueLocation} for the given offset and length in the given node.
+	 * 
+	 * @since 2.21
+	 */
+	protected IssueLocation getLocationForNode(INode node, int offset, int length) {
 		IssueLocation result = new IssueLocation();
-		result.lineNumber = nodeRegion.getLineNumber();
-		result.offset = nodeRegion.getOffset();
-		result.column = NodeModelUtils.getLineAndColumn(node, result.offset).getColumn();
-		result.length = nodeRegion.getLength();
+		result.offset = offset;
+		result.length = length;
+		
+		LineAndColumn lineAndColumnStart = NodeModelUtils.getLineAndColumn(node, offset);
+		result.lineNumber = lineAndColumnStart.getLine();
+		result.column = lineAndColumnStart.getColumn();
+		
+		LineAndColumn lineAndColumnEnd = NodeModelUtils.getLineAndColumn(node, offset + length);
+		result.lineNumberEnd = lineAndColumnEnd.getLine();
+		result.columnEnd = lineAndColumnEnd.getColumn();
 		return result;
 	}
 

org.eclipse.xtext/src/org/eclipse/xtext/validation/Issue.java

@@ -41,19 +41,41 @@ public interface Issue {
 
 	/**
 	 * Returns the one-based line number of the issue.
+	 * Values smaller than 1 and null values are invalid and indicate the absence of line information on this issue.
 	 */
 	Integer getLineNumber();
+
+	/**
+	 * Returns the one-based line number of the end of the issue.
+	 * Values smaller than 1 and null values are invalid and indicate the absence of line information on this issue.
+	 * 
+	 * @since 2.21
+	 */
+	Integer getLineNumberEnd();
 	
 	/**
 	 * Returns the column in the line of the issue. It's not the virtual column but literally
 	 * the character offset in the column, e.g. tab ('\t') counts as one character.
 	 * The first char in a line has column number 1, the number is one-based.
+	 * Values smaller than 1 and null values are invalid and indicate the absence of column information on this issue.
 	 * 
-	 * If no column information is available, returns -1.
+	 * The region defined by line and column information includes the character at the start column.
 	 * 
 	 * @since 2.9
 	 */
 	Integer getColumn();
+	
+	/**
+	 * Returns the end column in the line of the issue. It's not the virtual end column but literally
+	 * the character end offset in the column, e.g. tab ('\t') counts as one character.
+	 * The first char in a line has column number 1, the number is one-based.
+	 * Values smaller than 1 and null values are invalid and indicate the absence of column information on this issue.
+	 * 
+	 * The region defined by line and column information does not include the character at the end column.
+	 * 
+	 * @since 2.21
+	 */
+	Integer getColumnEnd();
 
 	Integer getOffset();
 
@@ -70,7 +92,9 @@ public interface Issue {
 		
 		private static Logger LOG = Logger.getLogger(IssueImpl.class);
 
-		private Integer length, lineNumber, offset, column;
+		private Integer offset = 0, length = 0;
+		private Integer lineNumber = 0, column = 0;
+		private Integer lineNumberEnd = 0, columnEnd = 0;
 		private String code, message;
 		private boolean isSyntaxError = false;
 		private URI uriToProblem;
@@ -100,6 +124,20 @@ public interface Issue {
 		public void setLineNumber(Integer lineNumber) {
 			this.lineNumber = lineNumber;
 		}
+
+		/**
+		 * @since 2.21
+		 */
+		public Integer getLineNumberEnd() {
+			return lineNumberEnd;
+		}
+
+		/**
+		 * @since 2.21
+		 */
+		public void setLineNumberEnd(Integer lineNumberEnd) {
+			this.lineNumberEnd = lineNumberEnd;
+		}
 		
 		/**
 		 * @since 2.9
@@ -116,6 +154,20 @@ public interface Issue {
 			this.column = column;
 		}
 
+		/**
+		 * @since 2.21
+		 */
+		public Integer getColumnEnd() {
+			return columnEnd;
+		}
+
+		/**
+		 * @since 2.21
+		 */
+		public void setColumnEnd(Integer columnEnd) {
+			this.columnEnd = columnEnd;
+		}
+
 		@Override
 		public Integer getOffset() {
 			return offset;