refactor(hydroflow_lang): improve diagnostics by re-spanning #root (#…

…1280) Inspired by fixing the spans in `demux_enum` in #1271 * re-span `#root` to `op_span` for better diagnostics * use better span `func.inputs` in `demux` and `demux_enum` * clippy fixups in `source_json`, `source_stdin` * fix #1201 (for the most part)
hydro-project · Jun 5, 2024 · 70a3f9e · 70a3f9e
1 parent 6ba72c5
commit 70a3f9e
Show file tree

Hide file tree

Showing 17 changed files with 99 additions and 171 deletions.
diff --git a/hydroflow/tests/compile-fail/surface_demux_badclosure.stderr b/hydroflow/tests/compile-fail/surface_demux_badclosure.stderr
@@ -1,15 +1,8 @@
 error: Closure provided to `demux(..)` must have two arguments: the first argument is the item, and the second argument lists ports. E.g. the second argument could be `var_args!(port_a, port_b, ..)`.
-  --> tests/compile-fail/surface_demux_badclosure.rs:5:48
-   |
-5  |           my_demux = source_iter(0..10) -> demux(|var_args!(a, b, c)| {
-   |  ________________________________________________^
-6  | |             match item % 3 {
-7  | |                 0 => a.give(item),
-8  | |                 1 => b.give(item),
-9  | |                 2 => c.give(item),
-10 | |             }
-11 | |         });
-   | |_________^
+ --> tests/compile-fail/surface_demux_badclosure.rs:5:49
+  |
+5 |         my_demux = source_iter(0..10) -> demux(|var_args!(a, b, c)| {
+  |                                                 ^^^^^^^^^^^^^^^^^^
 
 warning: unused import: `var_args`
  --> tests/compile-fail/surface_demux_badclosure.rs:1:35

diff --git a/hydroflow/tests/compile-fail/surface_demuxenum_notenum.stderr b/hydroflow/tests/compile-fail/surface_demuxenum_notenum.stderr
@@ -6,18 +6,10 @@ error[E0277]: the trait bound `Shape: DemuxEnumBase` is not satisfied
    |
    = note: Use `#[derive(hydroflow::DemuxEnum)]`
 note: required by a bound in `check_impl_demux_enum`
-  --> tests/compile-fail/surface_demuxenum_notenum.rs:8:18
-   |
-8  |       let mut df = hydroflow_syntax! {
-   |  __________________^
-9  | |         my_demux = source_iter([
-10 | |             Shape { area: 10.0 },
-11 | |             Shape { area: 9.0 },
-...  |
-16 | |         my_demux[Ellipse] -> for_each(std::mem::drop);
-17 | |     };
-   | |_____^ required by this bound in `check_impl_demux_enum`
-   = note: this error originates in the macro `hydroflow_syntax` (in Nightly builds, run with -Z macro-backtrace for more info)
+  --> tests/compile-fail/surface_demuxenum_notenum.rs:12:28
+   |
+12 |         ]) -> demux_enum::<Shape>();
+   |                            ^^^^^ required by this bound in `check_impl_demux_enum`
 
 error[E0223]: ambiguous associated type
   --> tests/compile-fail/surface_demuxenum_notenum.rs:14:18
@@ -66,17 +58,8 @@ help: if there were a trait named `Example` with associated type `Square` implem
 error[E0277]: the trait bound `Shape: DemuxEnum<_>` is not satisfied
   --> tests/compile-fail/surface_demuxenum_notenum.rs:12:15
    |
-8  |       let mut df = hydroflow_syntax! {
-   |  __________________-
-9  | |         my_demux = source_iter([
-10 | |             Shape { area: 10.0 },
-11 | |             Shape { area: 9.0 },
-12 | |         ]) -> demux_enum::<Shape>();
-   | |               ^^^^^^^^^^^^^^^^^^^^^ the trait `DemuxEnum<_>` is not implemented for `Shape`
-...  |
-16 | |         my_demux[Ellipse] -> for_each(std::mem::drop);
-17 | |     };
-   | |_____- required by a bound introduced by this call
+12 |         ]) -> demux_enum::<Shape>();
+   |               ^^^^^^^^^^^^^^^^^^^^^ the trait `DemuxEnum<_>` is not implemented for `Shape`
    |
    = note: Ensure there is exactly one output for each enum variant.
    = note: Ensure that the type for each output is a tuple of the field for the variant: `()`, `(a,)`, or `(a, b, ...)`.
diff --git a/hydroflow/tests/compile-fail/surface_demuxenum_port_extra.stderr b/hydroflow/tests/compile-fail/surface_demuxenum_port_extra.stderr
@@ -10,24 +10,19 @@ error[E0599]: no variant named `Ellipse` found for enum `Shape`
 error[E0308]: mismatched types
   --> tests/compile-fail/surface_demuxenum_port_extra.rs:17:15
    |
-12 |       let mut df = hydroflow_syntax! {
-   |  __________________-
-13 | |         my_demux = source_iter([
-14 | |             Shape::Rectangle { w: 10.0, h: 8.0 },
-15 | |             Shape::Square(9.0),
-16 | |             Shape::Circle { r: 5.0 },
-17 | |         ]) -> demux_enum::<Shape>();
-   | |               ^^^^^^^^^^^^^^^^^^^^^ expected a tuple with 3 elements, found one with 4 elements
-18 | |         my_demux[Rectangle] -> for_each(std::mem::drop);
-   | |                                ------------------------ one of the found opaque types
-19 | |         my_demux[Circle] -> for_each(std::mem::drop);
-   | |                             ------------------------ one of the found opaque types
-20 | |         my_demux[Square] -> for_each(std::mem::drop);
-   | |                             ------------------------ one of the found opaque types
-21 | |         my_demux[Ellipse] -> for_each(std::mem::drop);
-   | |                              ------------------------ one of the found opaque types
-22 | |     };
-   | |_____- arguments to this function are incorrect
+17 |         ]) -> demux_enum::<Shape>();
+   |               ^^^^^^^^^^^^^^^^^^^^^
+   |               |
+   |               expected a tuple with 3 elements, found one with 4 elements
+   |               arguments to this function are incorrect
+18 |         my_demux[Rectangle] -> for_each(std::mem::drop);
+   |                                ------------------------ one of the found opaque types
+19 |         my_demux[Circle] -> for_each(std::mem::drop);
+   |                             ------------------------ one of the found opaque types
+20 |         my_demux[Square] -> for_each(std::mem::drop);
+   |                             ------------------------ one of the found opaque types
+21 |         my_demux[Ellipse] -> for_each(std::mem::drop);
+   |                              ------------------------ one of the found opaque types
    |
    = note: expected mutable reference `&mut (_, _, _)`
               found mutable reference `&mut (impl Pusherator<Item = _>, impl Pusherator<Item = _>, impl Pusherator<Item = _>, impl Pusherator<Item = _>)`

diff --git a/hydroflow/tests/compile-fail/surface_demuxenum_port_missing.stderr b/hydroflow/tests/compile-fail/surface_demuxenum_port_missing.stderr
@@ -1,20 +1,15 @@
 error[E0308]: mismatched types
   --> tests/compile-fail/surface_demuxenum_port_missing.rs:17:15
    |
-12 |       let mut df = hydroflow_syntax! {
-   |  __________________-
-13 | |         my_demux = source_iter([
-14 | |             Shape::Rectangle { w: 10.0, h: 8.0 },
-15 | |             Shape::Square(9.0),
-16 | |             Shape::Circle { r: 5.0 },
-17 | |         ]) -> demux_enum::<Shape>();
-   | |               ^^^^^^^^^^^^^^^^^^^^^ expected a tuple with 3 elements, found one with 2 elements
-18 | |         my_demux[Rectangle] -> for_each(std::mem::drop);
-   | |                                ------------------------ one of the found opaque types
-19 | |         my_demux[Circle] -> for_each(std::mem::drop);
-   | |                             ------------------------ one of the found opaque types
-20 | |     };
-   | |_____- arguments to this function are incorrect
+17 |         ]) -> demux_enum::<Shape>();
+   |               ^^^^^^^^^^^^^^^^^^^^^
+   |               |
+   |               expected a tuple with 3 elements, found one with 2 elements
+   |               arguments to this function are incorrect
+18 |         my_demux[Rectangle] -> for_each(std::mem::drop);
+   |                                ------------------------ one of the found opaque types
+19 |         my_demux[Circle] -> for_each(std::mem::drop);
+   |                             ------------------------ one of the found opaque types
    |
    = note: expected mutable reference `&mut (_, _, _)`
               found mutable reference `&mut (impl Pusherator<Item = _>, impl Pusherator<Item = _>)`

diff --git a/hydroflow/tests/compile-fail/surface_demuxenum_wrongenum.stderr b/hydroflow/tests/compile-fail/surface_demuxenum_wrongenum.stderr
@@ -7,18 +7,10 @@ error[E0277]: the trait bound `std::option::Option<()>: DemuxEnumBase` is not sa
    = note: Use `#[derive(hydroflow::DemuxEnum)]`
    = help: the trait `DemuxEnumBase` is implemented for `Shape`
 note: required by a bound in `check_impl_demux_enum`
-  --> tests/compile-fail/surface_demuxenum_wrongenum.rs:12:18
+  --> tests/compile-fail/surface_demuxenum_wrongenum.rs:17:28
    |
-12 |       let mut df = hydroflow_syntax! {
-   |  __________________^
-13 | |         my_demux = source_iter([
-14 | |             Shape::Rectangle { w: 10.0, h: 8.0 },
-15 | |             Shape::Square(9.0),
-...  |
-20 | |         my_demux[Square] -> for_each(std::mem::drop);
-21 | |     };
-   | |_____^ required by this bound in `check_impl_demux_enum`
-   = note: this error originates in the macro `hydroflow_syntax` (in Nightly builds, run with -Z macro-backtrace for more info)
+17 |         ]) -> demux_enum::<Option<()>>();
+   |                            ^^^^^^^^^^ required by this bound in `check_impl_demux_enum`
 
 error[E0599]: no variant named `Circle` found for enum `std::option::Option<()>`
   --> tests/compile-fail/surface_demuxenum_wrongenum.rs:19:18
@@ -41,18 +33,8 @@ error[E0599]: no variant named `Square` found for enum `std::option::Option<()>`
 error[E0277]: the trait bound `std::option::Option<()>: DemuxEnum<_>` is not satisfied
   --> tests/compile-fail/surface_demuxenum_wrongenum.rs:17:15
    |
-12 |       let mut df = hydroflow_syntax! {
-   |  __________________-
-13 | |         my_demux = source_iter([
-14 | |             Shape::Rectangle { w: 10.0, h: 8.0 },
-15 | |             Shape::Square(9.0),
-16 | |             Shape::Circle { r: 5.0 },
-17 | |         ]) -> demux_enum::<Option<()>>();
-   | |               ^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `DemuxEnum<_>` is not implemented for `std::option::Option<()>`
-...  |
-20 | |         my_demux[Square] -> for_each(std::mem::drop);
-21 | |     };
-   | |_____- required by a bound introduced by this call
+17 |         ]) -> demux_enum::<Option<()>>();
+   |               ^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `DemuxEnum<_>` is not implemented for `std::option::Option<()>`
    |
    = note: Ensure there is exactly one output for each enum variant.
    = note: Ensure that the type for each output is a tuple of the field for the variant: `()`, `(a,)`, or `(a, b, ...)`.

diff --git a/hydroflow/tests/compile-fail/surface_demuxenum_wrongfields_1.stderr b/hydroflow/tests/compile-fail/surface_demuxenum_wrongfields_1.stderr
@@ -1,18 +1,8 @@
 error[E0271]: type mismatch resolving `<impl Pusherator<Item = u32> as Pusherator>::Item == (f64,)`
   --> tests/compile-fail/surface_demuxenum_wrongfields_1.rs:17:15
    |
-12 |       let mut df = hydroflow_syntax! {
-   |  __________________-
-13 | |         my_demux = source_iter([
-14 | |             Shape::Rectangle { w: 10.0, h: 8.0 },
-15 | |             Shape::Square(9.0),
-16 | |             Shape::Circle { r: 5.0 },
-17 | |         ]) -> demux_enum::<Shape>();
-   | |               ^^^^^^^^^^^^^^^^^^^^^ expected `(f64,)`, found `u32`
-...  |
-20 | |         my_demux[Square] -> for_each(|side: u32| ());
-21 | |     };
-   | |_____- required by a bound introduced by this call
+17 |         ]) -> demux_enum::<Shape>();
+   |               ^^^^^^^^^^^^^^^^^^^^^ expected `(f64,)`, found `u32`
    |
    = note: expected tuple `(f64,)`
                found type `u32`

diff --git a/hydroflow/tests/compile-fail/surface_demuxenum_wrongfields_2.stderr b/hydroflow/tests/compile-fail/surface_demuxenum_wrongfields_2.stderr
@@ -1,18 +1,8 @@
 error[E0271]: type mismatch resolving `<impl Pusherator<Item = (u32,)> as Pusherator>::Item == (f64,)`
   --> tests/compile-fail/surface_demuxenum_wrongfields_2.rs:17:15
    |
-12 |       let mut df = hydroflow_syntax! {
-   |  __________________-
-13 | |         my_demux = source_iter([
-14 | |             Shape::Rectangle { w: 10.0, h: 8.0 },
-15 | |             Shape::Square(9.0),
-16 | |             Shape::Circle { r: 5.0 },
-17 | |         ]) -> demux_enum::<Shape>();
-   | |               ^^^^^^^^^^^^^^^^^^^^^ expected `(f64,)`, found `(u32,)`
-...  |
-20 | |         my_demux[Square] -> for_each(|side: (u32,)| ());
-21 | |     };
-   | |_____- required by a bound introduced by this call
+17 |         ]) -> demux_enum::<Shape>();
+   |               ^^^^^^^^^^^^^^^^^^^^^ expected `(f64,)`, found `(u32,)`
    |
    = note: expected tuple `(f64,)`
               found tuple `(u32,)`

diff --git a/hydroflow/tests/compile-fail/surface_fold_keyed_generics_bad.rs.ignore b/hydroflow/tests/compile-fail/surface_fold_keyed_generics_bad.rs.ignore
@@ -1,5 +1,3 @@
-use hydroflow::hydroflow_syntax;
-
 fn main() {
     let mut df = hydroflow::hydroflow_syntax! {
         source_iter(["hello", "world"])

diff --git a/hydroflow/tests/compile-fail/surface_partition_badclosure.stderr b/hydroflow/tests/compile-fail/surface_partition_badclosure.stderr
@@ -1,12 +1,5 @@
 error: Closure provided to `partition(..)` must have two arguments: the first argument is the item, and for named ports the second argument must contain a Rust 'slice pattern' to determine the port names and order. For example, the second argument could be `[foo, bar, baz]` for ports `foo`, `bar`, and `baz`.
-  --> tests/compile-fail/surface_partition_badclosure.rs:5:56
-   |
-5  |           my_partition = source_iter(0..10) -> partition(|[a, b, c]| {
-   |  ________________________________________________________^
-6  | |             match item % 3 {
-7  | |                 0 => a,
-8  | |                 1 => b,
-9  | |                 2 => c,
-10 | |             }
-11 | |         });
-   | |_________^
+ --> tests/compile-fail/surface_partition_badclosure.rs:5:57
+  |
+5 |         my_partition = source_iter(0..10) -> partition(|[a, b, c]| {
+  |                                                         ^^^^^^^^^
diff --git a/hydroflow/tests/compile-fail/surface_source_interval_badarg.stderr b/hydroflow/tests/compile-fail/surface_source_interval_badarg.stderr
@@ -1,13 +1,11 @@
 error[E0308]: mismatched types
  --> tests/compile-fail/surface_source_interval_badarg.rs:7:25
   |
-5 |       let mut df = hydroflow_syntax! {
-  |  __________________-
-6 | |         // Should be a `Duration`.
-7 | |         source_interval(5) -> for_each(std::mem::drop);
-  | |                         ^ expected `Duration`, found integer
-8 | |     };
-  | |_____- arguments to this function are incorrect
+7 |         source_interval(5) -> for_each(std::mem::drop);
+  |         ----------------^-
+  |         |               |
+  |         |               expected `Duration`, found integer
+  |         arguments to this function are incorrect
   |
 note: function defined here
  --> $CARGO/tokio-1.35.1/src/time/interval.rs

diff --git a/hydroflow_lang/src/graph/hydroflow_graph.rs b/hydroflow_lang/src/graph/hydroflow_graph.rs
@@ -12,15 +12,16 @@ use slotmap::{Key, SecondaryMap, SlotMap, SparseSecondaryMap};
 use syn::spanned::Spanned;
 
 use super::graph_write::{Dot, GraphWrite, Mermaid};
-use super::ops::{find_op_op_constraints, OperatorWriteOutput, WriteContextArgs, OPERATORS};
+use super::ops::{
+    find_op_op_constraints, null_write_iterator_fn, DelayType, OperatorWriteOutput,
+    WriteContextArgs, OPERATORS,
+};
 use super::{
-    get_operator_generics, Color, DiMulGraph, FlowProps, GraphEdgeId, GraphNode, GraphNodeId,
-    GraphSubgraphId, OperatorInstance, PortIndexValue, Varname, CONTEXT, HANDOFF_NODE_STR,
-    HYDROFLOW,
+    change_spans, get_operator_generics, Color, DiMulGraph, FlowProps, GraphEdgeId, GraphNode,
+    GraphNodeId, GraphSubgraphId, OperatorInstance, PortIndexValue, Varname, CONTEXT,
+    HANDOFF_NODE_STR, HYDROFLOW, MODULE_BOUNDARY_NODE_STR,
 };
 use crate::diagnostic::{Diagnostic, Level};
-use crate::graph::ops::{null_write_iterator_fn, DelayType};
-use crate::graph::MODULE_BOUNDARY_NODE_STR;
 use crate::pretty_span::{PrettyRowCol, PrettySpan};
 use crate::process_singletons;
 
@@ -886,6 +887,8 @@ impl HydroflowGraph {
 
                         let op_span = node.span();
                         let op_name = op_inst.op_constraints.name;
+                        // Use op's span for root. #root is expected to be correct, any errors should span back to the op gen.
+                        let root = change_spans(root.clone(), op_span);
                         // TODO(mingwei): Just use `op_inst.op_constraints`?
                         let op_constraints = OPERATORS
                             .iter()
@@ -959,7 +962,7 @@ impl HydroflowGraph {
                                 );
 
                             let context_args = WriteContextArgs {
-                                root,
+                                root: &root,
                                 // There's a bit of dark magic hidden in `Span`s... you'd think it's just a `file:line:column`,
                                 // but it has one extra bit of info for _name resolution_, used for `Ident`s. `Span::call_site()`
                                 // has the (unhygienic) resolution we want, an ident is just solely determined by its string name,

diff --git a/hydroflow_lang/src/graph/mod.rs b/hydroflow_lang/src/graph/mod.rs
@@ -411,3 +411,32 @@ pub fn build_hfcode(
     }
     (None, diagnostics)
 }
+
+/// Changes all of token's spans to `span`, recursing into groups.
+fn change_spans(tokens: TokenStream, span: Span) -> TokenStream {
+    use proc_macro2::{Group, TokenTree};
+    tokens
+        .into_iter()
+        .map(|token| match token {
+            TokenTree::Group(mut group) => {
+                group.set_span(span);
+                TokenTree::Group(Group::new(
+                    group.delimiter(),
+                    change_spans(group.stream(), span),
+                ))
+            }
+            TokenTree::Ident(mut ident) => {
+                ident.set_span(span);
+                TokenTree::Ident(ident)
+            }
+            TokenTree::Punct(mut punct) => {
+                punct.set_span(span);
+                TokenTree::Punct(punct)
+            }
+            TokenTree::Literal(mut literal) => {
+                literal.set_span(span);
+                TokenTree::Literal(literal)
+            }
+        })
+        .collect()
+}
diff --git a/hydroflow_lang/src/graph/ops/demux.rs b/hydroflow_lang/src/graph/ops/demux.rs
@@ -81,7 +81,7 @@ pub const DEMUX: OperatorConstraints = OperatorConstraints {
         };
         if 2 != func.inputs.len() {
             diagnostics.push(Diagnostic::spanned(
-                func.span(),
+                func.inputs.span(),
                 Level::Error,
                 &*format!(
                     "Closure provided to `{}(..)` must have two arguments: \