Skip to content

Commit

Permalink
Improve diagnostic location in nls (#1856)
Browse files Browse the repository at this point in the history
* Improve diagnostic location

* insta review
  • Loading branch information
jneem authored Mar 15, 2024
1 parent 342655c commit e5bdef6
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 12 deletions.
15 changes: 9 additions & 6 deletions lsp/nls/src/diagnostic.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::ops::Range;

use codespan::{FileId, Files};
use codespan_reporting::diagnostic::{self, Diagnostic};
use codespan_reporting::diagnostic::{self, Diagnostic, LabelStyle};
use lsp_types::{DiagnosticRelatedInformation, NumberOrString};
use nickel_lang_core::{error::UNKNOWN_SOURCE_NAME, position::RawSpan};
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -117,12 +117,15 @@ impl DiagnosticCompat for SerializableDiagnostic {

if !diagnostic.message.is_empty() {
// What location should we use for the "overall" diagnostic? `Diagnostic` doesn't
// have an "overall" location, so we arbitrarily take the location of the first label.
if let Some(range) = within_file_labels
// have an "overall" location, so we arbitrarily take the location of the first primary label
// (falling back to the first label if there's no primary one).
let maybe_label = within_file_labels
.clone()
.next()
.map(|label| lsp_types::Range::from_codespan(&label.file_id, &label.range, files))
{
.find(|lab| lab.style == LabelStyle::Primary)
.or_else(|| within_file_labels.clone().next());

if let Some(label) = maybe_label {
let range = lsp_types::Range::from_codespan(&label.file_id, &label.range, files);
let message = if diagnostic.notes.is_empty() {
diagnostic.message.clone()
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ source: lsp/nls/tests/main.rs
expression: output
---
(file:///diagnostics-array.ncl, 2:4-2:7: applied to this expression)
(file:///diagnostics-array.ncl, 3:12-3:18: contract broken by a value)
(file:///diagnostics-array.ncl, 2:4-2:7: contract broken by a value)
(file:///diagnostics-array.ncl, 3:12-3:18: expected array element type)
(file:///diagnostics-array.ncl, 5:12-5:17: applied to this expression)
(file:///diagnostics-array.ncl, 6:20-6:26: contract broken by the value of `num`)
(file:///diagnostics-array.ncl, 5:12-5:17: contract broken by the value of `num`)
(file:///diagnostics-array.ncl, 6:20-6:26: expected type)
(file:///diagnostics-array.ncl, 8:6-8:9: applied to this expression)
(file:///diagnostics-array.ncl, 9:19-9:25: contract broken by a value)
(file:///diagnostics-array.ncl, 8:6-8:9: contract broken by a value)
(file:///diagnostics-array.ncl, 9:19-9:25: expected array element type)
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@
source: lsp/nls/tests/main.rs
expression: output
---
(file:///diagnostics-basic.ncl, 1:8-1:14: contract broken by the value of `num`)
(file:///diagnostics-basic.ncl, 1:8-1:14: expected type)
(file:///diagnostics-basic.ncl, 1:17-1:20: applied to this expression)
(file:///diagnostics-basic.ncl, 1:17-1:20: contract broken by the value of `num`)
(file:///diagnostics-basic.ncl, 2:9-2:12: applied to this expression)
(file:///diagnostics-basic.ncl, 3:13-3:19: contract broken by the value of `num2`)
(file:///diagnostics-basic.ncl, 2:9-2:12: contract broken by the value of `num2`)
(file:///diagnostics-basic.ncl, 3:13-3:19: expected type)

0 comments on commit e5bdef6

Please sign in to comment.