From 6eb3612345c34e67acdc71662ea94f0952a48fd9 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 6 Dec 2020 17:47:32 +0000 Subject: An import location is not independent from the import mode --- dhall/src/semantics/parse.rs | 22 +-- dhall/src/semantics/resolve/resolve.rs | 202 ++++++++++----------- dhall/tests/import/failure/cycle.txt | 2 +- .../import/success/unit/MixImportModesB.dhall | 2 +- dhall/tests/spec.rs | 2 - 5 files changed, 109 insertions(+), 121 deletions(-) diff --git a/dhall/src/semantics/parse.rs b/dhall/src/semantics/parse.rs index fa7ba18..5870d47 100644 --- a/dhall/src/semantics/parse.rs +++ b/dhall/src/semantics/parse.rs @@ -2,47 +2,39 @@ use std::path::Path; use url::Url; use crate::error::Error; -use crate::semantics::resolve::{ - download_http_text, ImportLocation, ImportLocationKind, -}; -use crate::syntax::binary; -use crate::syntax::parse_expr; +use crate::semantics::resolve::{download_http_text, ImportLocation}; +use crate::syntax::{binary, parse_expr}; use crate::Parsed; pub fn parse_file(f: &Path) -> Result { let text = std::fs::read_to_string(f)?; let expr = parse_expr(&text)?; - let root_kind = ImportLocationKind::Local(f.to_owned()); - let root = ImportLocation { kind: root_kind }; + let root = ImportLocation::local_dhall_code(f.to_owned()); Ok(Parsed(expr, root)) } pub fn parse_remote(url: Url) -> Result { let body = download_http_text(url.clone())?; let expr = parse_expr(&body)?; - let root_kind = ImportLocationKind::Remote(url); - let root = ImportLocation { kind: root_kind }; + let root = ImportLocation::remote_dhall_code(url); Ok(Parsed(expr, root)) } pub fn parse_str(s: &str) -> Result { let expr = parse_expr(s)?; - let root_kind = ImportLocationKind::Missing; - let root = ImportLocation { kind: root_kind }; + let root = ImportLocation::dhall_code_of_unknown_origin(); Ok(Parsed(expr, root)) } pub fn parse_binary(data: &[u8]) -> Result { let expr = binary::decode(data)?; - let root_kind = ImportLocationKind::Missing; - let root = ImportLocation { kind: root_kind }; + let root = ImportLocation::dhall_code_of_unknown_origin(); Ok(Parsed(expr, root)) } pub fn parse_binary_file(f: &Path) -> Result { let data = crate::utils::read_binary_file(f)?; let expr = binary::decode(&data)?; - let root_kind = ImportLocationKind::Local(f.to_owned()); - let root = ImportLocation { kind: root_kind }; + let root = ImportLocation::local_dhall_code(f.to_owned()); Ok(Parsed(expr, root)) } diff --git a/dhall/src/semantics/resolve/resolve.rs b/dhall/src/semantics/resolve/resolve.rs index 7838eb9..16987de 100644 --- a/dhall/src/semantics/resolve/resolve.rs +++ b/dhall/src/semantics/resolve/resolve.rs @@ -20,8 +20,9 @@ use crate::{Parsed, Resolved, Typed}; // TODO: evaluate import headers pub type Import = syntax::Import<()>; +/// The location of some data, usually some dhall code. #[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub enum ImportLocationKind { +enum ImportLocationKind { /// Local file Local(PathBuf), /// Remote file @@ -32,53 +33,14 @@ pub enum ImportLocationKind { Missing, } -/// The location of some data, usually some dhall code. +/// The location of some data. #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct ImportLocation { - pub kind: ImportLocationKind, + kind: ImportLocationKind, + mode: ImportMode, } impl ImportLocationKind { - /// Given an import pointing to `target` found in the current location, compute the next - /// location, or error if not allowed. - /// `sanity_check` indicates whether to check if that location is allowed to be referenced, - /// for example to prevent a remote file from reading an environment variable. - fn chain( - &self, - target: &ImportTarget<()>, - sanity_check: bool, - ) -> Result { - Ok(match target { - ImportTarget::Local(prefix, path) => { - self.chain_local(*prefix, path)? - } - ImportTarget::Remote(remote) => { - if sanity_check { - if let ImportLocationKind::Remote(..) = self { - // TODO: allow if CORS check passes - return Err(ImportError::SanityCheck.into()); - } - } - let mut url = Url::parse(&format!( - "{}://{}", - remote.scheme, remote.authority - ))?; - url.set_path(&remote.path.file_path.iter().join("/")); - url.set_query(remote.query.as_ref().map(String::as_ref)); - ImportLocationKind::Remote(url) - } - ImportTarget::Env(var_name) => { - if sanity_check { - if let ImportLocationKind::Remote(..) = self { - return Err(ImportError::SanityCheck.into()); - } - } - ImportLocationKind::Env(var_name.clone()) - } - ImportTarget::Missing => ImportLocationKind::Missing, - }) - } - fn chain_local( &self, prefix: FilePrefix, @@ -142,10 +104,12 @@ impl ImportLocationKind { }) } - fn fetch_dhall(self) -> Result { + fn fetch_dhall(&self) -> Result { Ok(match self { - ImportLocationKind::Local(path) => Parsed::parse_file(&path)?, - ImportLocationKind::Remote(url) => Parsed::parse_remote(url)?, + ImportLocationKind::Local(path) => Parsed::parse_file(path)?, + ImportLocationKind::Remote(url) => { + Parsed::parse_remote(url.clone())? + } ImportLocationKind::Env(var_name) => { let val = match env::var(var_name) { Ok(val) => val, @@ -159,10 +123,10 @@ impl ImportLocationKind { }) } - fn fetch_text(self) -> Result { + fn fetch_text(&self) -> Result { Ok(match self { - ImportLocationKind::Local(path) => std::fs::read_to_string(&path)?, - ImportLocationKind::Remote(url) => download_http_text(url)?, + ImportLocationKind::Local(path) => std::fs::read_to_string(path)?, + ImportLocationKind::Remote(url) => download_http_text(url.clone())?, ImportLocationKind::Env(var_name) => match env::var(var_name) { Ok(val) => val, Err(_) => return Err(ImportError::MissingEnvVar.into()), @@ -173,15 +137,17 @@ impl ImportLocationKind { }) } - fn into_location(self) -> Expr { + fn to_location(&self) -> Expr { let (field_name, arg) = match self { ImportLocationKind::Local(path) => { ("Local", Some(path.to_string_lossy().into_owned())) } ImportLocationKind::Remote(url) => { - ("Remote", Some(url.into_string())) + ("Remote", Some(url.to_string())) + } + ImportLocationKind::Env(name) => { + ("Environment", Some(name.clone())) } - ImportLocationKind::Env(name) => ("Environment", Some(name)), ImportLocationKind::Missing => ("Missing", None), }; @@ -199,29 +165,92 @@ impl ImportLocationKind { } impl ImportLocation { + pub fn dhall_code_of_unknown_origin() -> Self { + ImportLocation { + kind: ImportLocationKind::Missing, + mode: ImportMode::Code, + } + } + pub fn local_dhall_code(path: PathBuf) -> Self { + ImportLocation { + kind: ImportLocationKind::Local(path), + mode: ImportMode::Code, + } + } + pub fn remote_dhall_code(url: Url) -> Self { + ImportLocation { + kind: ImportLocationKind::Remote(url), + mode: ImportMode::Code, + } + } + /// Given an import pointing to `target` found in the current location, compute the next /// location, or error if not allowed. /// `sanity_check` indicates whether to check if that location is allowed to be referenced, /// for example to prevent a remote file from reading an environment variable. - fn chain( - &self, - target: &ImportTarget<()>, - sanity_check: bool, - ) -> Result { - let kind = self.kind.chain(target, sanity_check)?; - Ok(ImportLocation { kind }) - } - - fn fetch_dhall(self) -> Result { - self.kind.fetch_dhall() - } - - fn fetch_text(self) -> Result { - self.kind.fetch_text() + fn chain(&self, import: &Import) -> Result { + // Makes no sense to chain an import if the current file is not a dhall file. + assert!(matches!(self.mode, ImportMode::Code)); + let kind = match &import.location { + ImportTarget::Local(prefix, path) => { + self.kind.chain_local(*prefix, path)? + } + ImportTarget::Remote(remote) => { + if matches!(self.kind, ImportLocationKind::Remote(..)) + && !matches!(import.mode, ImportMode::Location) + { + // TODO: allow if CORS check passes + return Err(ImportError::SanityCheck.into()); + } + let mut url = Url::parse(&format!( + "{}://{}", + remote.scheme, remote.authority + ))?; + url.set_path(&remote.path.file_path.iter().join("/")); + url.set_query(remote.query.as_ref().map(String::as_ref)); + ImportLocationKind::Remote(url) + } + ImportTarget::Env(var_name) => { + if matches!(self.kind, ImportLocationKind::Remote(..)) + && !matches!(import.mode, ImportMode::Location) + { + return Err(ImportError::SanityCheck.into()); + } + ImportLocationKind::Env(var_name.clone()) + } + ImportTarget::Missing => ImportLocationKind::Missing, + }; + Ok(ImportLocation { + kind, + mode: import.mode, + }) } - fn into_location(self) -> Expr { - self.kind.into_location() + /// Fetches the expression corresponding to this location. + fn fetch(&self, env: &mut ImportEnv, span: Span) -> Result { + let (hir, ty) = match self.mode { + ImportMode::Code => { + let parsed = self.kind.fetch_dhall()?; + let typed = resolve_with_env(env, parsed)?.typecheck()?; + let hir = typed.normalize().to_hir(); + (hir, typed.ty) + } + ImportMode::RawText => { + let text = self.kind.fetch_text()?; + let hir = Hir::new( + HirKind::Expr(ExprKind::TextLit(text.into())), + span, + ); + (hir, Type::from_builtin(Builtin::Text)) + } + ImportMode::Location => { + let expr = self.kind.to_location(); + let hir = skip_resolve_expr(&expr)?; + let ty = hir.typecheck_noenv()?.ty().clone(); + (hir, ty) + } + }; + Ok(Typed { hir, ty }) } } @@ -274,35 +303,6 @@ fn check_hash(import: &Import, typed: &Typed, span: Span) -> Result<(), Error> { Ok(()) } -fn resolve_one_import( - env: &mut ImportEnv, - import: &Import, - location: ImportLocation, - span: Span, -) -> Result { - let (hir, ty) = match import.mode { - ImportMode::Code => { - let parsed = location.fetch_dhall()?; - let typed = resolve_with_env(env, parsed)?.typecheck()?; - let hir = typed.normalize().to_hir(); - (hir, typed.ty) - } - ImportMode::RawText => { - let text = location.fetch_text()?; - let hir = - Hir::new(HirKind::Expr(ExprKind::TextLit(text.into())), span); - (hir, Type::from_builtin(Builtin::Text)) - } - ImportMode::Location => { - let expr = location.into_location(); - let hir = skip_resolve_expr(&expr)?; - let ty = hir.typecheck_noenv()?.ty().clone(); - (hir, ty) - } - }; - Ok(Typed { hir, ty }) -} - /// Desugar the first level of the expression. fn desugar(expr: &Expr) -> Cow<'_, Expr> { match expr.kind() { @@ -395,9 +395,7 @@ fn resolve_with_env( &mut NameEnv::new(), &expr, &mut |import, span| { - let do_sanity_check = import.mode != ImportMode::Location; - let location = - base_location.chain(&import.location, do_sanity_check)?; + let location = base_location.chain(&import)?; // If the import is in the in-memory cache, or the hash is in the on-disk cache, return // the cached contents. @@ -420,7 +418,7 @@ fn resolve_with_env( // Resolve this import, making sure that recursive imports don't cycle back to the // current one. let res = env.with_cycle_detection(location.clone(), |env| { - resolve_one_import(env, &import, location.clone(), span.clone()) + location.fetch(env, span.clone()) }); let typed = match res { Ok(typed) => typed, diff --git a/dhall/tests/import/failure/cycle.txt b/dhall/tests/import/failure/cycle.txt index a103312..641667f 100644 --- a/dhall/tests/import/failure/cycle.txt +++ b/dhall/tests/import/failure/cycle.txt @@ -10,7 +10,7 @@ Type error: error: error --> :1:1 | 1 | ../data/cycle.dhall - | ^^^^^^^^^^^^^^^^^^^ ImportCycle([ImportLocation { kind: Local("./dhall-lang/tests/import/data/cycle.dhall") }, ImportLocation { kind: Local("./dhall-lang/tests/import/failure/cycle.dhall") }], ImportLocation { kind: Local("./dhall-lang/tests/import/data/cycle.dhall") }) + | ^^^^^^^^^^^^^^^^^^^ ImportCycle([ImportLocation { kind: Local("./dhall-lang/tests/import/data/cycle.dhall"), mode: Code }, ImportLocation { kind: Local("./dhall-lang/tests/import/failure/cycle.dhall"), mode: Code }], ImportLocation { kind: Local("./dhall-lang/tests/import/data/cycle.dhall"), mode: Code }) | | | diff --git a/dhall/tests/import/success/unit/MixImportModesB.dhall b/dhall/tests/import/success/unit/MixImportModesB.dhall index a4e02f5..430e0bc 100644 --- a/dhall/tests/import/success/unit/MixImportModesB.dhall +++ b/dhall/tests/import/success/unit/MixImportModesB.dhall @@ -1 +1 @@ -{ loc = < Environment: Text | Local: Text | Missing | Remote: Text >.Local "./dhall/tests/import/data/simple.dhall", n = < Environment: Text | Local: Text | Missing | Remote: Text >.Local "./dhall/tests/import/data/simple.dhall", txt = < Environment: Text | Local: Text | Missing | Remote: Text >.Local "./dhall/tests/import/data/simple.dhall" } +{ loc = < Environment: Text | Local: Text | Missing | Remote: Text >.Local "./dhall/tests/import/data/simple.dhall", n = 3, txt = "3\n" } diff --git a/dhall/tests/spec.rs b/dhall/tests/spec.rs index d3db321..6b156f6 100644 --- a/dhall/tests/spec.rs +++ b/dhall/tests/spec.rs @@ -535,8 +535,6 @@ fn ignore_test(variant: SpecTestKind, path: &str) -> bool { // Failing for now, we should fix that. let is_failing_for_now = false - // TODO: fix that one - // || path == "import/success/unit/MixImportModes" // TODO: fails because of caching issues. || path == "type-inference/success/prelude" // TODO: do not recover from cyclic imports -- cgit v1.2.3