improve error locations for unexpected properties (#664)

crates/taplo-cli/src/printing.rs

@@ -123,7 +123,7 @@ impl<E: Environment> Taplo<E> {
         let mut out_diag = Vec::<u8>::new();
         for err in errors {
             let msg = err.error.to_string();
-            for text_range in err.node.text_ranges() {
+            for text_range in err.text_ranges() {
                 let diag = Diagnostic::error()
                     .with_message(err.error.to_string())
                     .with_labels(Vec::from([

crates/taplo-common/src/schema/mod.rs

@@ -10,7 +10,10 @@ use parking_lot::Mutex;
 use regex::Regex;
 use serde_json::Value;
 use std::{borrow::Cow, num::NonZeroUsize, sync::Arc};
-use taplo::dom::{self, node::Key, KeyOrIndex, Keys};
+use taplo::{
+    dom::{self, node::Key, KeyOrIndex, Keys},
+    rowan::TextRange,
+};
 use thiserror::Error;
 use tokio::sync::Semaphore;
 use url::Url;
@@ -679,6 +682,13 @@ impl NodeValidationError {
         let mut keys = Keys::empty();
         let mut node = root.clone();
 
+        match &error.kind {
+            ValidationErrorKind::AdditionalProperties { unexpected } => {
+                keys = keys.extend(unexpected.iter().map(Key::from).map(KeyOrIndex::Key));
+            }
+            _ => {}
+        }
+
         'outer: for path in &error.instance_path {
             match path {
                 jsonschema::paths::PathChunk::Property(p) => match node {
@@ -705,6 +715,25 @@ impl NodeValidationError {
 
         Ok(Self { keys, node, error })
     }
+
+    pub fn text_ranges(&self) -> Box<dyn Iterator<Item = TextRange> + '_> {
+        let include_children = match self.error.kind {
+            ValidationErrorKind::AdditionalProperties { .. } => false,
+            _ => true,
+        };
+
+        if self.keys.is_empty() {
+            return Box::new(self.node.text_ranges(include_children).into_iter());
+        }
+
+        Box::new(
+            self.keys
+                .clone()
+                .into_iter()
+                .map(move |key| self.node.get(key).text_ranges(include_children))
+                .flatten(),
+        )
+    }
 }
 
 mod formats {

crates/taplo-lsp/src/diagnostics.rs

@@ -1,11 +1,10 @@
 use crate::world::{DocumentState, WorkspaceState, World};
-use either::Either;
 use lsp_async_stub::{util::LspExt, Context, RequestWriter};
 use lsp_types::{
     notification, Diagnostic, DiagnosticRelatedInformation, DiagnosticSeverity, Location,
     PublishDiagnosticsParams, Url,
 };
-use taplo::dom::{KeyOrIndex, Node};
+use taplo::dom::Node;
 use taplo_common::environment::Environment;
 
 #[tracing::instrument(skip_all)]
@@ -298,34 +297,28 @@ async fn collect_schema_errors<E: Environment>(
             "using schema"
         );
 
-        match ws.schemas.validate_root(&schema_association.url, dom).await {
-            Ok(errors) => diags.extend(errors.into_iter().flat_map(|err| {
-                let ranges = if let Some(KeyOrIndex::Key(k)) = err.keys.into_iter().last() {
-                    Either::Left(k.text_ranges())
-                } else {
-                    Either::Right(err.node.text_ranges())
-                };
-
-                let error = err.error;
-
-                ranges.map(move |range| {
-                    let range = doc.mapper.range(range).unwrap_or_default().into_lsp();
-                    Diagnostic {
-                        range,
-                        severity: Some(DiagnosticSeverity::ERROR),
-                        code: None,
-                        code_description: None,
-                        source: Some("Even Better TOML".into()),
-                        message: error.to_string(),
-                        related_information: None,
-                        tags: None,
-                        data: None,
-                    }
-                })
-            })),
+        let errors = match ws.schemas.validate_root(&schema_association.url, dom).await {
+            Ok(errors) => errors,
             Err(error) => {
                 tracing::error!(?error, "schema validation failed");
+                return;
             }
+        };
+
+        for error in errors {
+            let range = doc
+                .mapper
+                .range(error.text_ranges().next().unwrap())
+                .unwrap()
+                .into_lsp();
+
+            diags.push(Diagnostic {
+                range,
+                severity: Some(DiagnosticSeverity::ERROR),
+                source: Some("Even Better TOML".into()),
+                message: error.error.to_string(),
+                ..Default::default()
+            });
         }
     }
 }

crates/taplo-lsp/src/handlers/document_symbols.rs

@@ -58,11 +58,11 @@ fn symbols_for_value(
     mapper: &Mapper,
     symbols: &mut Vec<DocumentSymbol>,
 ) {
-    let own_range = mapper.range(join_ranges(node.text_ranges())).unwrap();
+    let own_range = mapper.range(join_ranges(node.text_ranges(true))).unwrap();
 
     let range = if let Some(key_r) = key_range {
         mapper
-            .range(key_r.cover(join_ranges(node.text_ranges())))
+            .range(key_r.cover(join_ranges(node.text_ranges(true))))
             .unwrap()
     } else {
         own_range

crates/taplo-lsp/src/query.rs

@@ -469,8 +469,8 @@ fn full_range(keys: &Keys, node: &Node) -> TextRange {
         .last()
         .map(Key::text_ranges)
     else {
-        return join_ranges(node.text_ranges());
+        return join_ranges(node.text_ranges(true));
     };
 
-    join_ranges(last_key.chain(node.text_ranges()))
+    join_ranges(last_key.chain(node.text_ranges(true)))
 }

crates/taplo/src/dom/node.rs

@@ -244,16 +244,18 @@ impl Node {
         Ok(all.into_iter())
     }
 
-    pub fn text_ranges(&self) -> impl ExactSizeIterator<Item = TextRange> {
+    pub fn text_ranges(&self, include_children: bool) -> impl ExactSizeIterator<Item = TextRange> {
         let mut ranges = Vec::with_capacity(1);
 
         match self {
             Node::Table(v) => {
-                let entries = v.entries().read();
+                if include_children {
+                    let entries = v.entries().read();
 
-                for (k, entry) in entries.iter() {
-                    ranges.extend(k.text_ranges());
-                    ranges.extend(entry.text_ranges());
+                    for (k, entry) in entries.iter() {
+                        ranges.extend(k.text_ranges());
+                        ranges.extend(entry.text_ranges(true));
+                    }
                 }
 
                 if let Some(mut r) = v.syntax().map(|s| s.text_range()) {
@@ -265,9 +267,11 @@ impl Node {
                 }
             }
             Node::Array(v) => {
-                let items = v.items().read();
-                for item in items.iter() {
-                    ranges.extend(item.text_ranges());
+                if include_children {
+                    let items = v.items().read();
+                    for item in items.iter() {
+                        ranges.extend(item.text_ranges(true));
+                    }
                 }
 
                 if let Some(mut r) = v.syntax().map(|s| s.text_range()) {

crates/taplo/src/formatter/mod.rs

@@ -318,7 +318,7 @@ where
         let matched = dom.find_all_matches(keys, false)?;
 
         for (_, node) in matched {
-            s.extend(node.text_ranges().map(|r| (r, opts.clone())));
+            s.extend(node.text_ranges(false).map(|r| (r, opts.clone())));
         }
     }