From 772afa0049b230c2f8ed423a31621d24218e317f Mon Sep 17 00:00:00 2001 From: Yaroslav Bolyukin Date: Sat, 07 Feb 2026 23:18:00 +0000 Subject: [PATCH] refactor: saner imports from TLA/std.extVars --- --- a/bindings/jsonnet/src/import.rs +++ b/bindings/jsonnet/src/import.rs @@ -14,7 +14,7 @@ use jrsonnet_evaluator::{ bail, error::{ErrorKind::*, Result}, - ImportResolver, + AsPathLike, ImportResolver, ResolvePath, }; use jrsonnet_gcmodule::Acyclic; use jrsonnet_parser::{SourceDirectory, SourceFile, SourcePath}; @@ -38,7 +38,7 @@ out: RefCell>>, } impl ImportResolver for CallbackImportResolver { - fn resolve_from(&self, from: &SourcePath, path: &str) -> Result { + fn resolve_from(&self, from: &SourcePath, path: &dyn AsPathLike) -> Result { let base = if let Some(p) = from.downcast_ref::() { let mut o = p.path().to_owned(); o.pop(); @@ -51,7 +51,11 @@ unreachable!("can't resolve this path"); }; let base = unsafe { crate::unparse_path(&base) }; - let rel = CString::new(path).unwrap(); + let rel = path.as_path(); + let rel = match rel { + ResolvePath::Str(s) => CString::new(s.as_bytes()).unwrap(), + ResolvePath::Path(p) => unsafe { crate::unparse_path(p) }, + }; let found_here: *mut c_char = null_mut(); let mut buf = null_mut(); --- a/bindings/jsonnet/src/lib.rs +++ b/bindings/jsonnet/src/lib.rs @@ -28,7 +28,7 @@ rustc_hash::FxHashMap, stack::set_stack_depth_limit, trace::{CompactFormat, PathResolver, TraceFormat}, - FileImportResolver, IStr, ImportResolver, Result, State, Val, + AsPathLike, FileImportResolver, IStr, ImportResolver, Result, State, Val, }; use jrsonnet_gcmodule::Acyclic; use jrsonnet_parser::SourcePath; @@ -62,18 +62,18 @@ } } -unsafe fn unparse_path(input: &Path) -> Cow<'_, CStr> { +unsafe fn unparse_path(input: &Path) -> CString { #[cfg(target_family = "unix")] { use std::os::unix::ffi::OsStrExt; let str = CString::new(input.as_os_str().as_bytes()).expect("input has zero byte in it"); - Cow::Owned(str) + str } #[cfg(not(target_family = "unix"))] { let str = input.as_os_str().to_str().expect("bad utf-8"); let cstr = CString::new(str).expect("input has NUL inside"); - Cow::Owned(cstr) + cstr } } @@ -93,16 +93,12 @@ self.inner.borrow().load_file_contents(resolved) } - fn resolve_from(&self, from: &SourcePath, path: &str) -> Result { + fn resolve_from(&self, from: &SourcePath, path: &dyn AsPathLike) -> Result { self.inner.borrow().resolve_from(from, path) } - fn resolve_from_default(&self, path: &str) -> Result { + fn resolve_from_default(&self, path: &dyn AsPathLike) -> Result { self.inner.borrow().resolve_from_default(path) - } - - fn resolve(&self, path: &Path) -> Result { - self.inner.borrow().resolve(path) } } --- a/bindings/jsonnet/src/vars_tlas.rs +++ b/bindings/jsonnet/src/vars_tlas.rs @@ -3,7 +3,6 @@ use std::{ffi::CStr, os::raw::c_char}; use jrsonnet_evaluator::{function::TlaArg, IStr}; -use jrsonnet_parser::{ParserSettings, Source}; use crate::VM; @@ -84,14 +83,7 @@ let code = unsafe { CStr::from_ptr(code) }; let name: IStr = name.to_str().expect("name is not utf-8").into(); - let code: IStr = code.to_str().expect("code is not utf-8").into(); - let code = jrsonnet_parser::parse( - &code, - &ParserSettings { - source: Source::new_virtual(format!("").into(), code.clone()), - }, - ) - .expect("can't parse TLA code"); + let code: String = code.to_str().expect("code is not utf-8").to_owned(); - vm.tla_args.insert(name, TlaArg::Code(code)); + vm.tla_args.insert(name, TlaArg::InlineCode(code)); } --- a/cmds/jrsonnet/src/main.rs +++ b/cmds/jrsonnet/src/main.rs @@ -182,7 +182,7 @@ let input_str = std::str::from_utf8(&input)?; s.evaluate_snippet("".to_owned(), input_str)? } else { - s.import(&input)? + s.import(input.as_str())? }; let tla = opts.tla.tla_opts()?; --- a/crates/jrsonnet-cli/src/stdlib.rs +++ b/crates/jrsonnet-cli/src/stdlib.rs @@ -1,7 +1,7 @@ -use std::{fs::read_to_string, str::FromStr}; +use std::str::FromStr; use clap::Parser; -use jrsonnet_evaluator::{trace::PathResolver, Result}; +use jrsonnet_evaluator::{function::TlaArg, trace::PathResolver, Result}; use jrsonnet_stdlib::ContextInitializer; #[derive(Clone)] @@ -54,25 +54,20 @@ #[derive(Clone)] pub struct ExtFile { pub name: String, - pub value: String, + pub path: String, } impl FromStr for ExtFile { type Err = String; fn from_str(s: &str) -> std::result::Result { - let out: Vec<&str> = s.split('=').collect(); - if out.len() != 2 { + let Some((name, path)) = s.split_once('=') else { return Err("bad ext-file syntax".to_owned()); - } - let file = read_to_string(out[1]); - match file { - Ok(content) => Ok(Self { - name: out[0].into(), - value: content, - }), - Err(e) => Err(format!("{e}")), - } + }; + Ok(Self { + name: name.into(), + path: path.into(), + }) } } @@ -110,16 +105,27 @@ } let ctx = ContextInitializer::new(PathResolver::new_cwd_fallback()); for ext in &self.ext_str { - ctx.add_ext_str((&ext.name as &str).into(), (&ext.value as &str).into()); + ctx.settings_mut().ext_vars.insert( + ext.name.as_str().into(), + TlaArg::String(ext.value.as_str().into()), + ); } for ext in &self.ext_str_file { - ctx.add_ext_str((&ext.name as &str).into(), (&ext.value as &str).into()); + ctx.settings_mut().ext_vars.insert( + ext.name.as_str().into(), + TlaArg::ImportStr(ext.path.clone()), + ); } for ext in &self.ext_code { - ctx.add_ext_code(&ext.name as &str, &ext.value as &str)?; + ctx.settings_mut().ext_vars.insert( + ext.name.as_str().into(), + TlaArg::InlineCode(ext.value.clone()), + ); } for ext in &self.ext_code_file { - ctx.add_ext_code(&ext.name as &str, &ext.value as &str)?; + ctx.settings_mut() + .ext_vars + .insert(ext.name.as_str().into(), TlaArg::Import(ext.path.clone())); } Ok(Some(ctx)) } --- a/crates/jrsonnet-cli/src/tla.rs +++ b/crates/jrsonnet-cli/src/tla.rs @@ -1,12 +1,5 @@ use clap::Parser; -use jrsonnet_evaluator::{ - error::{ErrorKind, Result}, - function::TlaArg, - gc::WithCapacityExt as _, - rustc_hash::FxHashMap, - IStr, -}; -use jrsonnet_parser::{ParserSettings, Source}; +use jrsonnet_evaluator::{IStr, error::Result, function::TlaArg, gc::WithCapacityExt as _, rustc_hash::FxHashMap}; use crate::{ExtFile, ExtStr}; @@ -35,37 +28,27 @@ impl TlaOpts { pub fn tla_opts(&self) -> Result> { let mut out = FxHashMap::new(); - for (name, value) in self - .tla_str - .iter() - .map(|c| (&c.name, &c.value)) - .chain(self.tla_str_file.iter().map(|c| (&c.name, &c.value))) - { - out.insert(name.into(), TlaArg::String(value.into())); + for ext in &self.tla_str { + out.insert( + ext.name.as_str().into(), + TlaArg::String(ext.value.as_str().into()), + ); } - for (name, code) in self - .tla_code - .iter() - .map(|c| (&c.name, &c.value)) - .chain(self.tla_code_file.iter().map(|c| (&c.name, &c.value))) - { - let source = Source::new_virtual(format!("").into(), code.into()); + for ext in &self.tla_str_file { out.insert( - (name as &str).into(), - TlaArg::Code( - jrsonnet_parser::parse( - code, - &ParserSettings { - source: source.clone(), - }, - ) - .map_err(|e| ErrorKind::ImportSyntaxError { - path: source, - error: Box::new(e), - })?, - ), + ext.name.as_str().into(), + TlaArg::ImportStr(ext.name.as_str().into()), + ); + } + for ext in &self.tla_code { + out.insert( + ext.name.as_str().into(), + TlaArg::InlineCode(ext.value.clone()), ); } + for ext in &self.tla_code_file { + out.insert(ext.name.as_str().into(), TlaArg::Import(ext.path.clone())); + } Ok(out) } } --- a/crates/jrsonnet-evaluator/src/async_import.rs +++ b/crates/jrsonnet-evaluator/src/async_import.rs @@ -1,7 +1,6 @@ -use std::{any::Any, cell::RefCell, future::Future, path::Path}; +use std::{any::Any, cell::RefCell, future::Future}; use jrsonnet_gcmodule::Acyclic; -use jrsonnet_interner::IStr; use jrsonnet_parser::{ ArgsDesc, AssertStmt, BindSpec, CompSpec, Destruct, Expr, FieldMember, FieldName, ForSpecData, IfSpecData, LocExpr, Member, ObjBody, Param, ParamsDesc, ParserSettings, SliceDesc, Source, @@ -9,10 +8,10 @@ }; use rustc_hash::FxHashMap; -use crate::{bail, FileData, ImportResolver, State}; +use crate::{AsPathLike, FileData, ImportResolver, ResolvePathOwned, State}; pub struct Import { - path: IStr, + path: ResolvePathOwned, expression: bool, } @@ -137,7 +136,7 @@ Expr::Import(v) | Expr::ImportStr(v) | Expr::ImportBin(v) => { if let Expr::Str(s) = &*v.expr() { out.0.push(Import { - path: s.clone(), + path: ResolvePathOwned::Str(s.to_string()), expression: matches!(&*expr.expr(), Expr::Import(_)), }); } @@ -229,16 +228,14 @@ fn resolve_from( &self, from: &SourcePath, - path: &str, + path: &dyn AsPathLike, ) -> impl Future>; fn resolve_from_default( &self, - path: &str, + path: &dyn AsPathLike, ) -> impl Future> { async { self.resolve_from(&SourcePath::default(), path).await } } - /// Resolves absolute path, doesn't supports jpath and other fancy things - fn resolve(&self, path: &Path) -> impl Future>; /// Load resolved file /// This should only be called with value returned @@ -253,31 +250,25 @@ #[derive(Acyclic)] struct ResolvedImportResolver { - resolved: RefCell>, + resolved: RefCell>, } impl ImportResolver for ResolvedImportResolver { fn load_file_contents(&self, _resolved: &SourcePath) -> crate::Result> { unreachable!("all files should be loaded at this point"); } - fn resolve_from(&self, from: &SourcePath, path: &str) -> crate::Result { + fn resolve_from(&self, from: &SourcePath, path: &dyn AsPathLike) -> crate::Result { Ok(self .resolved .borrow() - .get(&(from.clone(), path.into())) + .get(&(from.clone(), path.as_path().to_owned())) .expect("all imports should be resolved at this point") .0 .clone()) } - fn resolve_from_default(&self, path: &str) -> crate::Result { + fn resolve_from_default(&self, path: &dyn AsPathLike) -> crate::Result { self.resolve_from(&SourcePath::default(), path) - } - - fn resolve(&self, path: &Path) -> crate::Result { - bail!(crate::error::ErrorKind::AbsoluteImportNotSupported( - path.to_owned() - )) } } @@ -288,7 +279,7 @@ } #[allow(clippy::future_not_send)] -pub async fn async_import(s: State, handler: H, path: impl AsRef) -> Result<(), H::Error> +pub async fn async_import(s: State, handler: H, path: &dyn AsPathLike) -> Result<(), H::Error> where H: AsyncImportResolver, { @@ -299,7 +290,7 @@ let mut resolved_map = resolved.resolved.borrow_mut(); let mut queue = vec![Job::LoadFile { - path: handler.resolve(path.as_ref()).await?, + path: handler.resolve_from_default(path).await?, parse: true, }]; while let Some(job) = queue.pop() { --- a/crates/jrsonnet-evaluator/src/error.rs +++ b/crates/jrsonnet-evaluator/src/error.rs @@ -2,7 +2,6 @@ cmp::Ordering, convert::Infallible, fmt::{Debug, Display}, - path::PathBuf, }; use jrsonnet_gcmodule::Trace; @@ -16,7 +15,7 @@ stdlib::format::FormatError, typed::TypeLocError, val::ConvertNumValueError, - ObjValue, + ObjValue, ResolvePathOwned, }; pub(crate) fn format_found(list: &[IStr], what: &str) -> String { @@ -180,9 +179,7 @@ StandaloneSuper, #[error("can't resolve {1} from {0}")] - ImportFileNotFound(SourcePath, String), - #[error("can't resolve absolute {0}")] - AbsoluteImportFileNotFound(PathBuf), + ImportFileNotFound(SourcePath, ResolvePathOwned), #[error("resolved file not found: {:?}", .0)] ResolvedFileNotFound(SourcePath), #[error("can't import {0}: is a directory")] @@ -192,9 +189,7 @@ #[error("import io error: {0}")] ImportIo(String), #[error("tried to import {1} from {0}, but imports are not supported")] - ImportNotSupported(SourcePath, String), - #[error("tried to import {0}, but absolute imports are not supported")] - AbsoluteImportNotSupported(PathBuf), + ImportNotSupported(SourcePath, ResolvePathOwned), #[error("can't import from virtual file")] CantImportFromVirtualFile, #[error( --- a/crates/jrsonnet-evaluator/src/evaluate/mod.rs +++ b/crates/jrsonnet-evaluator/src/evaluate/mod.rs @@ -682,7 +682,7 @@ }; let tmp = loc.clone().0; let s = ctx.state(); - let resolved_path = s.resolve_from(tmp.source_path(), path as &str)?; + let resolved_path = s.resolve_from(tmp.source_path(), path)?; match i { Import(_) => in_frame( CallLocation::new(&loc), --- a/crates/jrsonnet-evaluator/src/function/arglike.rs +++ b/crates/jrsonnet-evaluator/src/function/arglike.rs @@ -2,7 +2,7 @@ use jrsonnet_gcmodule::Trace; use jrsonnet_interner::IStr; -use jrsonnet_parser::{ArgsDesc, LocExpr}; +use jrsonnet_parser::{ArgsDesc, LocExpr, SourceFifo, SourcePath}; use crate::{evaluate, typed::Typed, Context, Result, Thunk, Val}; @@ -41,22 +41,34 @@ #[derive(Clone, Trace)] pub enum TlaArg { String(IStr), - Code(LocExpr), Val(Val), Lazy(Thunk), + Import(String), + ImportStr(String), + InlineCode(String), } impl ArgLike for TlaArg { - fn evaluate_arg(&self, ctx: Context, tailstrict: bool) -> Result> { + fn evaluate_arg(&self, ctx: Context, _tailstrict: bool) -> Result> { match self { Self::String(s) => Ok(Thunk::evaluated(Val::string(s.clone()))), - Self::Code(code) => Ok(if tailstrict { - Thunk::evaluated(evaluate(ctx, code)?) - } else { - let code = code.clone(); - Thunk!(move || evaluate(ctx, &code)) - }), Self::Val(val) => Ok(Thunk::evaluated(val.clone())), Self::Lazy(lazy) => Ok(lazy.clone()), + Self::Import(p) => { + let resolved = ctx.state().resolve_from_default(&p.as_str())?; + Ok(Thunk!(move || ctx.state().import_resolved(resolved))) + } + Self::ImportStr(p) => { + let resolved = ctx.state().resolve_from_default(&p.as_str())?; + Ok(Thunk!(move || ctx + .state() + .import_resolved_str(resolved) + .map(Val::string))) + } + Self::InlineCode(p) => { + let resolved = + SourcePath::new(SourceFifo("".to_owned(), p.as_bytes().into())); + Ok(Thunk!(move || ctx.state().import_resolved(resolved))) + } } } } --- a/crates/jrsonnet-evaluator/src/import.rs +++ b/crates/jrsonnet-evaluator/src/import.rs @@ -1,7 +1,8 @@ use std::{ any::Any, + borrow::Cow, env::current_dir, - fs, + fmt, fs, io::{ErrorKind, Read}, path::{Path, PathBuf}, }; @@ -9,12 +10,85 @@ use fs::File; use jrsonnet_gcmodule::Acyclic; use jrsonnet_interner::IBytes; -use jrsonnet_parser::{SourceDirectory, SourceFifo, SourceFile, SourcePath}; +use jrsonnet_parser::{IStr, SourceDirectory, SourceFifo, SourceFile, SourcePath}; use crate::{ bail, error::{ErrorKind::*, Result}, }; +#[derive(Clone, Debug, Acyclic, Eq, Hash, PartialEq)] +pub enum ResolvePathOwned { + Str(String), + Path(PathBuf), +} +impl fmt::Display for ResolvePathOwned { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + ResolvePathOwned::Str(s) => write!(f, "{s}"), + ResolvePathOwned::Path(p) => write!(f, "{}", p.display()), + } + } +} +#[derive(Clone, Copy)] +pub enum ResolvePath<'s> { + Str(&'s str), + Path(&'s Path), +} +impl ResolvePath<'_> { + pub fn to_owned(self) -> ResolvePathOwned { + match self { + ResolvePath::Str(s) => ResolvePathOwned::Str(s.to_owned()), + ResolvePath::Path(p) => ResolvePathOwned::Path(p.to_owned()), + } + } +} +impl AsRef for ResolvePath<'_> { + fn as_ref(&self) -> &Path { + match self { + ResolvePath::Str(s) => s.as_ref(), + ResolvePath::Path(p) => p, + } + } +} +pub trait AsPathLike { + fn as_path(&self) -> ResolvePath<'_>; +} +impl AsPathLike for &T +where + T: AsPathLike + ?Sized, +{ + fn as_path(&self) -> ResolvePath<'_> { + (*self).as_path() + } +} +impl AsPathLike for str { + fn as_path(&self) -> ResolvePath<'_> { + ResolvePath::Str(self) + } +} +impl AsPathLike for IStr { + fn as_path(&self) -> ResolvePath<'_> { + ResolvePath::Str(self) + } +} +impl AsPathLike for Cow<'_, Path> { + fn as_path(&self) -> ResolvePath<'_> { + ResolvePath::Path(self.as_ref()) + } +} +impl AsPathLike for Path { + fn as_path(&self) -> ResolvePath<'_> { + ResolvePath::Path(self) + } +} +impl AsPathLike for ResolvePathOwned { + fn as_path(&self) -> ResolvePath<'_> { + match self { + ResolvePathOwned::Str(s) => ResolvePath::Str(s), + ResolvePathOwned::Path(path_buf) => ResolvePath::Path(path_buf), + } + } +} /// Implements file resolution logic for `import` and `importStr` pub trait ImportResolver: Acyclic + Any { @@ -24,15 +98,11 @@ /// /// `from` should only be returned from [`ImportResolver::resolve`], or from other defined file, any other value /// may result in panic - fn resolve_from(&self, from: &SourcePath, path: &str) -> Result { - bail!(ImportNotSupported(from.clone(), path.into())) + fn resolve_from(&self, from: &SourcePath, path: &dyn AsPathLike) -> Result { + bail!(ImportNotSupported(from.clone(), path.as_path().to_owned())) } - fn resolve_from_default(&self, path: &str) -> Result { + fn resolve_from_default(&self, path: &dyn AsPathLike) -> Result { self.resolve_from(&SourcePath::default(), path) - } - /// Resolves absolute path, doesn't supports jpath and other fancy things - fn resolve(&self, path: &Path) -> Result { - bail!(AbsoluteImportNotSupported(path.to_owned())) } /// Load resolved file @@ -105,7 +175,8 @@ } impl ImportResolver for FileImportResolver { - fn resolve_from(&self, from: &SourcePath, path: &str) -> Result { + fn resolve_from(&self, from: &SourcePath, path: &dyn AsPathLike) -> Result { + let path = path.as_path(); let mut direct = if let Some(f) = from.downcast_ref::() { let mut o = f.path().to_owned(); o.pop(); @@ -130,12 +201,6 @@ } } bail!(ImportFileNotFound(from.clone(), path.to_owned())) - } - fn resolve(&self, path: &Path) -> Result { - let Some(source) = check_path(path)? else { - bail!(AbsoluteImportFileNotFound(path.to_owned())) - }; - Ok(source) } fn load_file_contents(&self, id: &SourcePath) -> Result> { @@ -155,7 +220,7 @@ Ok(out) } - fn resolve_from_default(&self, path: &str) -> Result { + fn resolve_from_default(&self, path: &dyn AsPathLike) -> Result { self.resolve_from(&SourcePath::default(), path) } } --- a/crates/jrsonnet-evaluator/src/lib.rs +++ b/crates/jrsonnet-evaluator/src/lib.rs @@ -29,7 +29,6 @@ cell::{RefCell, RefMut}, collections::hash_map::Entry, fmt::{self, Debug}, - path::Path, rc::Rc, }; @@ -349,12 +348,12 @@ } /// Has same semantics as `import 'path'` called from `from` file - pub fn import_from(&self, from: &SourcePath, path: &str) -> Result { - let resolved = self.resolve_from(from, path)?; + pub fn import_from(&self, from: &SourcePath, path: impl AsPathLike) -> Result { + let resolved = self.resolve_from(from, &path)?; self.import_resolved(resolved) } - pub fn import(&self, path: impl AsRef) -> Result { - let resolved = self.resolve(path)?; + pub fn import(&self, path: impl AsPathLike) -> Result { + let resolved = self.resolve_from_default(&path)?; self.import_resolved(resolved) } @@ -468,14 +467,12 @@ impl State { // Only panics in case of [`ImportResolver`] contract violation #[allow(clippy::missing_panics_doc)] - pub fn resolve_from(&self, from: &SourcePath, path: &str) -> Result { - self.import_resolver().resolve_from(from, path.as_ref()) + pub fn resolve_from(&self, from: &SourcePath, path: &dyn AsPathLike) -> Result { + self.import_resolver().resolve_from(from, path) } - - // Only panics in case of [`ImportResolver`] contract violation #[allow(clippy::missing_panics_doc)] - pub fn resolve(&self, path: impl AsRef) -> Result { - self.import_resolver().resolve(path.as_ref()) + pub fn resolve_from_default(&self, path: &dyn AsPathLike) -> Result { + self.import_resolver().resolve_from_default(path) } pub fn import_resolver(&self) -> &dyn ImportResolver { &*self.0.import_resolver --- a/crates/jrsonnet-stdlib/src/lib.rs +++ b/crates/jrsonnet-stdlib/src/lib.rs @@ -12,7 +12,7 @@ pub use encoding::*; pub use hash::*; use jrsonnet_evaluator::{ - error::{ErrorKind::*, Result}, + error::Result, function::{CallLocation, FuncVal, TlaArg}, trace::PathResolver, val::NumValue, @@ -377,23 +377,11 @@ .ext_vars .insert(name, TlaArg::String(value)); } - pub fn add_ext_code(&self, name: &str, code: impl Into) -> Result<()> { - let code = code.into(); - let source = extvar_source(name, code.clone()); - let parsed = jrsonnet_parser::parse( - &code, - &jrsonnet_parser::ParserSettings { - source: source.clone(), - }, - ) - .map_err(|e| ImportSyntaxError { - path: source, - error: Box::new(e), - })?; + pub fn add_ext_code(&self, name: &str, code: impl AsRef) -> Result<()> { // self.data_mut().volatile_files.insert(source_name, code); self.settings_mut() .ext_vars - .insert(name.into(), TlaArg::Code(parsed)); + .insert(name.into(), TlaArg::InlineCode(code.as_ref().to_owned())); Ok(()) } pub fn add_native(&self, name: impl Into, cb: impl Into) { -- gitstuff