Merge remote-tracking branch 'origin/main' into nulls-last

Author: aljazerzen

prql-compiler/prqlc/src/cli.rs

@@ -11,7 +11,7 @@ use std::str::FromStr;
 use prql_compiler::semantic::{self, reporting::*};
 use prql_compiler::{ast::pl::Lineage, pl_to_prql};
 use prql_compiler::{downcast, Options, Target};
-use prql_compiler::{pl_to_rq_tree, prql_to_pl, prql_to_pl_tree, rq_to_sql, FileTree, Span};
+use prql_compiler::{pl_to_rq_tree, prql_to_pl, prql_to_pl_tree, rq_to_sql, SourceTree, Span};
 
 use crate::watch;
 
@@ -177,7 +177,7 @@ impl Command {
         Ok(())
     }
 
-    fn execute<'a>(&self, sources: &'a FileTree, main_path: &'a str) -> Result<Vec<u8>> {
+    fn execute<'a>(&self, sources: &'a SourceTree, main_path: &'a str) -> Result<Vec<u8>> {
         let main_path = main_path
             .split('.')
             .filter(|x| !x.is_empty())
@@ -193,7 +193,7 @@ impl Command {
                 }
             }
             Command::Format(_) => {
-                let (_, source) = sources.files.clone().into_iter().exactly_one()?;
+                let (_, source) = sources.sources.clone().into_iter().exactly_one()?;
                 let ast = prql_to_pl(&source)?;
 
                 pl_to_prql(ast)?.as_bytes().to_vec()
@@ -206,7 +206,7 @@ impl Command {
                     .map_err(|e| e.composed(sources, true))?;
 
                 let mut out = Vec::new();
-                for (source_id, source) in &sources.files {
+                for (source_id, source) in &sources.sources {
                     let source_id = source_id.to_str().unwrap().to_string();
                     out.extend(label_references(&context, source_id, source.clone()));
                 }
@@ -215,7 +215,7 @@ impl Command {
                 out
             }
             Command::Annotate(_) => {
-                let (_, source) = sources.files.clone().into_iter().exactly_one()?;
+                let (_, source) = sources.sources.clone().into_iter().exactly_one()?;
 
                 // TODO: potentially if there is code performing a role beyond
                 // presentation, it should be a library function; and we could
@@ -287,7 +287,7 @@ impl Command {
         })
     }
 
-    fn read_input(&mut self) -> Result<(FileTree, String)> {
+    fn read_input(&mut self) -> Result<(SourceTree, String)> {
         // Possibly this should be called by the relevant subcommands passing in
         // `input`, rather than matching on them and grabbing `input` from
         // `self`? But possibly if everything moves to `io_args`, then this is
@@ -376,7 +376,7 @@ mod clio_extended {
     use std::path::Path;
 
     use clap::builder::TypedValueParser;
-    use prql_compiler::FileTree;
+    use prql_compiler::SourceTree;
     use walkdir::WalkDir;
 
     #[derive(Debug)]
@@ -427,7 +427,7 @@ mod clio_extended {
             }
         }
 
-        pub fn read_to_tree(&mut self) -> anyhow::Result<FileTree<String>> {
+        pub fn read_to_tree(&mut self) -> anyhow::Result<SourceTree<String>> {
             let mut only_file = String::new();
 
             match self {
@@ -438,7 +438,7 @@ mod clio_extended {
                     let root_path = Path::new(root_path);
 
                     // special case: actually walk the dirs
-                    let mut files = HashMap::new();
+                    let mut sources = HashMap::new();
                     for entry in WalkDir::new(root_path) {
                         let entry = entry?;
                         let path = entry.path();
@@ -447,18 +447,16 @@ mod clio_extended {
                             let file_contents = fs::read_to_string(path)?;
                             let path = path.strip_prefix(root_path)?.to_path_buf();
 
-                            files.insert(path, file_contents);
+                            sources.insert(path, file_contents);
                         }
                     }
 
-                    return Ok(FileTree { files });
+                    return Ok(SourceTree::new(sources));
                 }
             };
 
             let path = Path::new(self.path()).to_path_buf();
-            Ok(FileTree {
-                files: [(path, only_file)].into(),
-            })
+            Ok(SourceTree::single(path, only_file))
         }
     }
 
@@ -646,16 +644,13 @@ group a_column (take 10 | sort b_column | derive {the_number = rank, last = lag
                 hide_signature_comment: true,
                 target: "sql.any".to_string(),
             },
-            &FileTree {
-                files: [
-                    ("_project.prql".into(), "orders.x | select y".to_string()),
-                    (
-                        "orders.prql".into(),
-                        "let x = (from z | select {y, u})".to_string(),
-                    ),
-                ]
-                .into(),
-            },
+            &SourceTree::new([
+                ("_project.prql".into(), "orders.x | select y".to_string()),
+                (
+                    "orders.prql".into(),
+                    "let x = (from z | select {y, u})".to_string(),
+                ),
+            ]),
             "main",
         )
         .unwrap();
@@ -689,7 +684,7 @@ group a_column (take 10 | sort b_column | derive {the_number = rank, last = lag
         .unwrap();
 
         assert_display_snapshot!(String::from_utf8(output).unwrap().trim(), @r###"
-        files:
+        sources:
           '':
           - name: main
             VarDef:
@@ -712,6 +707,8 @@ group a_column (take 10 | sort b_column | derive {the_number = rank, last = lag
                         - y
               ty_expr: null
               kind: Main
+        source_ids:
+          1: ''
         "###);
     }
     #[test]
@@ -816,12 +813,12 @@ group a_column (take 10 | sort b_column | derive {the_number = rank, last = lag
                   args:
                   - kind:
                       ColumnRef: 2
-                    span: span-chars-47-53
+                    span: 1:47-53
                   - kind:
                       Literal:
                         Integer: 0
-                    span: span-chars-56-57
-              span: span-chars-47-57
+                    span: 1:56-57
+              span: 1:47-57
           - Sort:
             - direction: Asc
               column: 2

prql-compiler/src/error.rs

@@ -1,7 +1,6 @@
 pub use anyhow::Result;
 
 use ariadne::{Cache, Config, Label, Report, ReportKind, Source};
-use itertools::Itertools;
 use serde::de::Visitor;
 use serde::{Deserialize, Serialize};
 
@@ -11,12 +10,14 @@ use std::fmt::{self, Debug, Display, Formatter};
 use std::ops::{Add, Range};
 use std::path::PathBuf;
 
-use crate::FileTree;
+use crate::SourceTree;
 
 #[derive(Clone, PartialEq, Eq, Copy)]
 pub struct Span {
     pub start: usize,
     pub end: usize,
+
+    pub source_id: usize,
 }
 
 #[derive(Debug, Clone)]
@@ -251,21 +252,23 @@ impl ErrorMessages {
     }
 
     /// Computes message location and builds the pretty display.
-    pub fn composed(mut self, sources: &FileTree, color: bool) -> Self {
+    pub fn composed(mut self, sources: &SourceTree, color: bool) -> Self {
         let mut cache = FileTreeCache::new(sources);
 
-        // TODO: get id from the error
-        let Ok((id, _)) = sources.files.iter().exactly_one() else {
-            return self;
-        };
-
         for e in &mut self.inner {
-            let Ok(source) = cache.fetch(id) else {
+            let Some(span) = e.span else {
+                continue;
+            };
+            let Some(source_path) = sources.source_ids.get(&span.source_id) else {
+                continue;
+            };
+
+            let Ok(source) = cache.fetch(source_path) else {
                 continue
             };
             e.location = e.compose_location(source);
 
-            e.display = e.compose_display(id.clone(), &mut cache, color);
+            e.display = e.compose_display(source_path.clone(), &mut cache, color);
         }
         self
     }
@@ -274,17 +277,17 @@ impl ErrorMessages {
 impl ErrorMessage {
     fn compose_display(
         &self,
-        source_id: PathBuf,
+        source_path: PathBuf,
         cache: &mut FileTreeCache,
         color: bool,
     ) -> Option<String> {
         let config = Config::default().with_color(color);
 
         let span = Range::from(self.span?);
 
-        let mut report = Report::build(ReportKind::Error, source_id.clone(), span.start)
+        let mut report = Report::build(ReportKind::Error, source_path.clone(), span.start)
             .with_config(config)
-            .with_label(Label::new((source_id, span)).with_message(&self.reason));
+            .with_label(Label::new((source_path, span)).with_message(&self.reason));
 
         if let Some(code) = &self.code {
             report = report.with_code(code);
@@ -312,11 +315,11 @@ impl ErrorMessage {
 }
 
 struct FileTreeCache<'a> {
-    file_tree: &'a FileTree,
+    file_tree: &'a SourceTree,
     cache: HashMap<PathBuf, Source>,
 }
 impl<'a> FileTreeCache<'a> {
-    fn new(file_tree: &'a FileTree) -> Self {
+    fn new(file_tree: &'a SourceTree) -> Self {
         FileTreeCache {
             file_tree,
             cache: HashMap::new(),
@@ -326,7 +329,7 @@ impl<'a> FileTreeCache<'a> {
 
 impl<'a> Cache<PathBuf> for FileTreeCache<'a> {
     fn fetch(&mut self, id: &PathBuf) -> Result<&Source, Box<dyn fmt::Debug + '_>> {
-        let file_contents = match self.file_tree.files.get(id) {
+        let file_contents = match self.file_tree.sources.get(id) {
             Some(v) => v,
             None => return Err(Box::new(format!("Unknown file `{id:?}`"))),
         };
@@ -369,13 +372,14 @@ impl From<Span> for Range<usize> {
     }
 }
 
-impl Add<Span> for Span {
+impl Add<usize> for Span {
     type Output = Span;
 
-    fn add(self, rhs: Span) -> Span {
+    fn add(self, rhs: usize) -> Span {
         Span {
-            start: self.start.min(rhs.start),
-            end: self.end.max(rhs.end),
+            start: self.start + rhs,
+            end: self.end + rhs,
+            source_id: self.source_id,
         }
     }
 }
@@ -420,7 +424,7 @@ impl<T, E: WithErrorInfo> WithErrorInfo for Result<T, E> {
 
 impl Debug for Span {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
-        write!(f, "span-chars-{}-{}", self.start, self.end)
+        write!(f, "{}:{}-{}", self.source_id, self.start, self.end)
     }
 }
 
@@ -445,7 +449,7 @@ impl<'de> Deserialize<'de> for Span {
             type Value = Span;
 
             fn expecting(&self, f: &mut fmt::Formatter) -> fmt::Result {
-                write!(f, "A span string of form `span-chars-x-y`")
+                write!(f, "A span string of form `file_id:x-y`")
             }
 
             fn visit_str<E>(self, v: &str) -> std::result::Result<Self::Value, E>
@@ -454,18 +458,27 @@ impl<'de> Deserialize<'de> for Span {
             {
                 use serde::de;
 
-                if let Some(span) = v.strip_prefix("span-chars-") {
-                    if let Some((start, end)) = span.split_once('-') {
+                if let Some((file_id, char_span)) = v.split_once(':') {
+                    let file_id = file_id
+                        .parse::<usize>()
+                        .map_err(|e| de::Error::custom(e.to_string()))?;
+
+                    if let Some((start, end)) = char_span.split_once('-') {
                         let start = start
                             .parse::<usize>()
                             .map_err(|e| de::Error::custom(e.to_string()))?;
                         let end = end
                             .parse::<usize>()
                             .map_err(|e| de::Error::custom(e.to_string()))?;
 
-                        return Ok(Span { start, end });
+                        return Ok(Span {
+                            start,
+                            end,
+                            source_id: file_id,
+                        });
                     }
                 }
+
                 Err(de::Error::custom("malformed span"))
             }