refactor: drop interior mutable settings from State

crates/jrsonnet-evaluator/src/lib.rs

@@ -27,7 +27,7 @@ pub mod val;
 
 use std::{
 	any::Any,
-	cell::{Ref, RefCell, RefMut},
+	cell::{RefCell, RefMut},
 	fmt::{self, Debug},
 	path::Path,
 };
@@ -147,24 +147,6 @@ impl_context_initializer! {
 	A @ B C D E F G
 }
 
-/// Dynamically reconfigurable evaluation settings
-#[derive(Trace)]
-pub struct EvaluationSettings {
-	/// Context initializer, which will be used for imports and everything
-	/// [`NoopContextInitializer`] is used by default, most likely you want to have `jrsonnet-stdlib`
-	pub context_initializer: TraceBox<dyn ContextInitializer>,
-	/// Used to resolve file locations/contents
-	pub import_resolver: TraceBox<dyn ImportResolver>,
-}
-impl Default for EvaluationSettings {
-	fn default() -> Self {
-		Self {
-			context_initializer: tb!(()),
-			import_resolver: tb!(DummyImportResolver),
-		}
-	}
-}
-
 #[derive(Trace)]
 struct FileData {
 	string: Option<IStr>,
@@ -207,16 +189,19 @@ impl FileData {
 	}
 }
 
-#[derive(Default, Trace)]
+#[derive(Trace)]
 pub struct EvaluationStateInternals {
 	/// Internal state
 	file_cache: RefCell<GcHashMap<SourcePath, FileData>>,
-	/// Settings, safe to change at runtime
-	settings: RefCell<EvaluationSettings>,
+	/// Context initializer, which will be used for imports and everything
+	/// [`NoopContextInitializer`] is used by default, most likely you want to have `jrsonnet-stdlib`
+	context_initializer: TraceBox<dyn ContextInitializer>,
+	/// Used to resolve file locations/contents
+	import_resolver: TraceBox<dyn ImportResolver>,
 }
 
 /// Maintains stack trace and import resolution
-#[derive(Default, Clone, Trace)]
+#[derive(Clone, Trace)]
 pub struct State(Cc<EvaluationStateInternals>);
 
 impl State {
@@ -228,7 +213,7 @@ impl State {
 		let file = match file {
 			RawEntryMut::Occupied(ref mut d) => d.get_mut(),
 			RawEntryMut::Vacant(v) => {
-				let data = self.settings().import_resolver.load_file_contents(&path)?;
+				let data = self.import_resolver().load_file_contents(&path)?;
 				v.insert(
 					path.clone(),
 					FileData::new_string(
@@ -252,7 +237,7 @@ impl State {
 		let file = match file {
 			RawEntryMut::Occupied(ref mut d) => d.get_mut(),
 			RawEntryMut::Vacant(v) => {
-				let data = self.settings().import_resolver.load_file_contents(&path)?;
+				let data = self.import_resolver().load_file_contents(&path)?;
 				v.insert(path.clone(), FileData::new_bytes(data.as_slice().into()))
 					.1
 			}
@@ -279,7 +264,7 @@ impl State {
 		let file = match file {
 			RawEntryMut::Occupied(ref mut d) => d.get_mut(),
 			RawEntryMut::Vacant(v) => {
-				let data = self.settings().import_resolver.load_file_contents(&path)?;
+				let data = self.import_resolver().load_file_contents(&path)?;
 				v.insert(
 					path.clone(),
 					FileData::new_string(
@@ -350,8 +335,7 @@ impl State {
 
 	/// Creates context with all passed global variables
 	pub fn create_default_context(&self, source: Source) -> Context {
-		let context_initializer = &self.settings().context_initializer;
-		context_initializer.initialize(self.clone(), source)
+		self.context_initializer().initialize(self.clone(), source)
 	}
 
 	/// Creates context with all passed global variables, calling custom modifier
@@ -360,7 +344,7 @@ impl State {
 		source: Source,
 		context_initializer: impl ContextInitializer,
 	) -> Context {
-		let default_initializer = &self.settings().context_initializer;
+		let default_initializer = self.context_initializer();
 		let mut builder = ContextBuilder::with_capacity(
 			self.clone(),
 			default_initializer.reserve_vars() + context_initializer.reserve_vars(),
@@ -409,49 +393,6 @@ impl State {
 	fn file_cache(&self) -> RefMut<'_, GcHashMap<SourcePath, FileData>> {
 		self.0.file_cache.borrow_mut()
 	}
-	pub fn settings(&self) -> Ref<'_, EvaluationSettings> {
-		self.0.settings.borrow()
-	}
-	pub fn settings_mut(&self) -> RefMut<'_, EvaluationSettings> {
-		self.0.settings.borrow_mut()
-	}
-	pub fn add_global(&self, name: IStr, value: Thunk<Val>) {
-		#[derive(Trace)]
-		struct GlobalsCtx {
-			globals: RefCell<GcHashMap<IStr, Thunk<Val>>>,
-			inner: TraceBox<dyn ContextInitializer>,
-		}
-		impl ContextInitializer for GlobalsCtx {
-			fn reserve_vars(&self) -> usize {
-				self.inner.reserve_vars() + self.globals.borrow().len()
-			}
-			fn populate(&self, for_file: Source, builder: &mut ContextBuilder) {
-				self.inner.populate(for_file, builder);
-				for (name, val) in self.globals.borrow().iter() {
-					builder.bind(name.clone(), val.clone());
-				}
-			}
-
-			fn as_any(&self) -> &dyn Any {
-				self
-			}
-		}
-		let mut settings = self.settings_mut();
-		let initializer = &mut settings.context_initializer;
-		if let Some(global) = initializer.as_any().downcast_ref::<GlobalsCtx>() {
-			global.globals.borrow_mut().insert(name, value);
-		} else {
-			let inner = std::mem::replace(&mut settings.context_initializer, tb!(()));
-			settings.context_initializer = tb!(GlobalsCtx {
-				globals: {
-					let mut out = GcHashMap::with_capacity(1);
-					out.insert(name, value);
-					RefCell::new(out)
-				},
-				inner
-			});
-		}
-	}
 }
 
 #[derive(Trace)]
@@ -523,16 +464,51 @@ impl State {
 	pub fn resolve(&self, path: impl AsRef<Path>) -> Result<SourcePath> {
 		self.import_resolver().resolve(path.as_ref())
 	}
-	pub fn import_resolver(&self) -> Ref<'_, dyn ImportResolver> {
-		Ref::map(self.settings(), |s| &*s.import_resolver)
+	pub fn import_resolver(&self) -> &dyn ImportResolver {
+		&*self.0.import_resolver
+	}
+	pub fn context_initializer(&self) -> &dyn ContextInitializer {
+		&*self.0.context_initializer
+	}
+}
+
+impl State {
+	pub fn builder() -> StateBuilder {
+		StateBuilder::default()
+	}
+}
+
+impl Default for State {
+	fn default() -> Self {
+		Self::builder().build()
 	}
-	pub fn set_import_resolver(&self, resolver: impl ImportResolver) {
-		self.settings_mut().import_resolver = tb!(resolver);
+}
+
+#[derive(Default)]
+pub struct StateBuilder {
+	import_resolver: Option<TraceBox<dyn ImportResolver>>,
+	context_initializer: Option<TraceBox<dyn ContextInitializer>>,
+}
+impl StateBuilder {
+	pub fn import_resolver(&mut self, import_resolver: impl ImportResolver) -> &mut Self {
+		let _ = self.import_resolver.insert(tb!(import_resolver));
+		self
 	}
-	pub fn context_initializer(&self) -> Ref<'_, dyn ContextInitializer> {
-		Ref::map(self.settings(), |s| &*s.context_initializer)
+	pub fn context_initializer(
+		&mut self,
+		context_initializer: impl ContextInitializer,
+	) -> &mut Self {
+		let _ = self.context_initializer.insert(tb!(context_initializer));
+		self
 	}
-	pub fn set_context_initializer(&self, initializer: impl ContextInitializer) {
-		self.settings_mut().context_initializer = tb!(initializer);
+	pub fn build(mut self) -> State {
+		State(Cc::new(EvaluationStateInternals {
+			file_cache: RefCell::new(GcHashMap::new()),
+			context_initializer: self.context_initializer.take().unwrap_or_else(|| tb!(())),
+			import_resolver: self
+				.import_resolver
+				.take()
+				.unwrap_or_else(|| tb!(DummyImportResolver)),
+		}))
 	}
 }

tests/tests/as_native.rs

@@ -1,14 +1,16 @@
-use jrsonnet_evaluator::{Result, State};
-use jrsonnet_stdlib::StateExt;
+use jrsonnet_evaluator::{trace::PathResolver, FileImportResolver, Result, State};
+use jrsonnet_stdlib::ContextInitializer;
 
 mod common;
 
 #[test]
 fn as_native() -> Result<()> {
-	let s = State::default();
-	s.with_stdlib();
+	let mut s = State::builder();
+	s.context_initializer(ContextInitializer::new(PathResolver::new_cwd_fallback()))
+		.import_resolver(FileImportResolver::default());
+	let s = s.build();
 
-	let val = s.evaluate_snippet("snip".to_owned(), r#"function(a, b) a + b"#)?;
+	let val = s.evaluate_snippet("snip".to_owned(), r"function(a, b) a + b")?;
 	let func = val.as_func().expect("this is function");
 
 	let native = func.into_native::<((u32, u32), u32)>();

tests/tests/builtin.rs

@@ -2,10 +2,13 @@ mod common;
 
 use jrsonnet_evaluator::{
 	function::{builtin, builtin::Builtin, CallLocation, FuncVal},
+	parser::Source,
+	trace::PathResolver,
 	typed::Typed,
-	ContextBuilder, Result, State, Thunk, Val,
+	ContextBuilder, ContextInitializer, FileImportResolver, Result, State, Thunk, Val,
 };
-use jrsonnet_stdlib::StateExt;
+use jrsonnet_gcmodule::Trace;
+use jrsonnet_stdlib::ContextInitializer as StdContextInitializer;
 
 #[builtin]
 fn a() -> Result<u32> {
@@ -29,15 +32,30 @@ fn basic_function() -> Result<()> {
 fn native_add(a: u32, b: u32) -> Result<u32> {
 	Ok(a + b)
 }
+#[derive(Trace)]
+struct NativeAddContextInitializer;
+impl ContextInitializer for NativeAddContextInitializer {
+	fn populate(&self, _for_file: Source, builder: &mut ContextBuilder) {
+		builder.bind(
+			"nativeAdd",
+			Thunk::evaluated(Val::function(native_add::INST)),
+		);
+	}
+
+	fn as_any(&self) -> &dyn std::any::Any {
+		self
+	}
+}
 
 #[test]
 fn call_from_code() -> Result<()> {
-	let s = State::default();
-	s.with_stdlib();
-	s.add_global(
-		"nativeAdd".into(),
-		Thunk::evaluated(Val::function(native_add::INST)),
-	);
+	let mut s = State::builder();
+	s.context_initializer((
+		StdContextInitializer::new(PathResolver::new_cwd_fallback()),
+		NativeAddContextInitializer,
+	))
+	.import_resolver(FileImportResolver::default());
+	let s = s.build();
 
 	let v = s.evaluate_snippet(
 		"snip".to_owned(),
@@ -62,15 +80,27 @@ fn curried_add(this: &curried_add, b: u32) -> Result<u32> {
 fn curry_add(a: u32) -> Result<FuncVal> {
 	Ok(FuncVal::builtin(curried_add { a }))
 }
+#[derive(Trace)]
+struct CurryAddContextInitializer;
+impl ContextInitializer for CurryAddContextInitializer {
+	fn populate(&self, _for_file: Source, builder: &mut ContextBuilder) {
+		builder.bind("curryAdd", Thunk::evaluated(Val::function(curry_add::INST)));
+	}
+
+	fn as_any(&self) -> &dyn std::any::Any {
+		self
+	}
+}
 
 #[test]
 fn nonstatic_builtin() -> Result<()> {
-	let s = State::default();
-	s.with_stdlib();
-	s.add_global(
-		"curryAdd".into(),
-		Thunk::evaluated(Val::function(curry_add::INST)),
-	);
+	let mut s = State::builder();
+	s.context_initializer((
+		StdContextInitializer::new(PathResolver::new_cwd_fallback()),
+		CurryAddContextInitializer,
+	))
+	.import_resolver(FileImportResolver::default());
+	let s = s.build();
 
 	let v = s.evaluate_snippet(
 		"snip".to_owned(),

tests/tests/common.rs

@@ -1,8 +1,10 @@
 use jrsonnet_evaluator::{
 	bail,
 	function::{builtin, FuncVal},
-	ObjValueBuilder, Result, State, Thunk, Val,
+	parser::Source,
+	ContextBuilder, ContextInitializer as ContextInitializerT, ObjValueBuilder, Result, Thunk, Val,
 };
+use jrsonnet_gcmodule::Trace;
 
 #[macro_export]
 macro_rules! ensure_eq {
@@ -74,11 +76,18 @@ fn param_names(fun: FuncVal) -> Vec<String> {
 	}
 }
 
-#[allow(dead_code)]
-pub fn with_test(s: &State) {
-	let mut bobj = ObjValueBuilder::new();
-	bobj.method("assertThrow", assert_throw::INST);
-	bobj.method("paramNames", param_names::INST);
+#[derive(Trace)]
+pub struct ContextInitializer;
+impl ContextInitializerT for ContextInitializer {
+	fn populate(&self, _for_file: Source, builder: &mut ContextBuilder) {
+		let mut bobj = ObjValueBuilder::new();
+		bobj.method("assertThrow", assert_throw::INST);
+		bobj.method("paramNames", param_names::INST);
 
-	s.add_global("test".into(), Thunk::evaluated(Val::Obj(bobj.build())))
+		builder.bind("test", Thunk::evaluated(Val::Obj(bobj.build())));
+	}
+
+	fn as_any(&self) -> &dyn std::any::Any {
+		self
+	}
 }

tests/tests/golden.rs

@@ -8,14 +8,19 @@ use jrsonnet_evaluator::{
 	trace::{CompactFormat, PathResolver, TraceFormat},
 	FileImportResolver, State,
 };
-use jrsonnet_stdlib::StateExt;
+use jrsonnet_stdlib::ContextInitializer;
 mod common;
+use common::ContextInitializer as TestContextInitializer;
 
 fn run(file: &Path) -> String {
-	let s = State::default();
-	s.with_stdlib();
-	common::with_test(&s);
-	s.set_import_resolver(FileImportResolver::default());
+	let mut s = State::builder();
+	s.context_initializer((
+		ContextInitializer::new(PathResolver::new_cwd_fallback()),
+		TestContextInitializer,
+	))
+	.import_resolver(FileImportResolver::default());
+	let s = s.build();
+
 	let trace_format = CompactFormat {
 		resolver: PathResolver::FileName,
 		max_trace: 20,