Warn about unknown server property keys at run

docs/src/commands/run.md

@@ -9,7 +9,8 @@ missing or out of date is installed first: the Java runtime, the Minecraft
 binary, the mod loader, the mods (added, updated, and removed to match the
 manifest), and the tunnel agent when a tunnel is configured. The generated
 configuration files and player lists are rewritten from the manifest on
-every start.
+every start. A warning is printed for every key in the manifest's
+`properties` block that is not a known server property.
 
 Only one running server is allowed per instance directory; a second
 `mc run` refuses to start.

docs/src/reference/manifest.md

@@ -194,6 +194,11 @@ Values may be strings, integers, floats, or booleans. Keys containing a dot
 must be quoted, as above; a nested block spells the same key, so
 `rcon { broadcast "yes" }` sets `rcon.broadcast`.
 
+Any key is accepted, since mods read their own settings from the same
+file, but [`mc run`](../commands/run.md) prints a warning for each key in
+this block that is not a known server property, so a typo does not go
+unnoticed.
+
 Keys managed by mc are rejected: an entry for a key that a `server` field
 or the top-level `name` and `description` already drive is an error naming
 the field to use instead. Every managed entry is reported in the same

src/minecraft/server_properties.rs

@@ -212,6 +212,11 @@ pub const MANAGED_PROPERTY_KEYS: [(&str, &str); 18] = [
     ("white-list", "set `allow-list` in `server` instead")
 ];
 
+pub struct PropertyEntries {
+    pub entries: BTreeMap<String, String>,
+    pub unknown_keys: Vec<String>
+}
+
 // The subset of server.properties driven by `mc.kdl` and the environment.
 // Serializing it yields the managed keys.
 #[derive(Serialize)]
@@ -305,17 +310,28 @@ impl ServerProperties {
         &self,
         overrides: &BTreeMap<String, String>,
         managed: &BTreeMap<String, String>
-    ) -> McResult<BTreeMap<String, String>> {
+    ) -> McResult<PropertyEntries> {
         let s = serde_java_properties::to_string(self)
             .context("could not serialize server.properties")?;
 
         let mut entries: BTreeMap<String, String> = serde_java_properties::from_str(&s)
             .context("could not parse the generated server.properties")?;
 
+        // Mods read their own keys from the same file, so an override for a
+        // key that is not a vanilla one is reported rather than rejected.
+        let unknown_keys = overrides
+            .keys()
+            .filter(|key| !entries.contains_key(*key))
+            .cloned()
+            .collect();
+
         entries.extend(overrides.clone());
         entries.extend(managed.clone());
 
-        Ok(entries)
+        Ok(PropertyEntries {
+            entries,
+            unknown_keys
+        })
     }
 
     pub fn entries_to_string(entries: &BTreeMap<String, String>) -> McResult<String> {

src/minecraft/tests.rs

@@ -1,3 +1,5 @@
+use std::collections::BTreeMap;
+
 use chrono::DateTime;
 use chrono::Utc;
 use uuid::Uuid;
@@ -6,6 +8,7 @@ use crate::minecraft::players::BanDetails;
 use crate::minecraft::players::BanEntry;
 use crate::minecraft::players::OpEntry;
 use crate::minecraft::players::offline_uuid;
+use crate::minecraft::server_properties::ServerProperties;
 use crate::utils::errors::McResult;
 
 #[test]
@@ -51,3 +54,30 @@ fn op_entries_use_the_server_field_names() -> McResult<()> {
 
     Ok(())
 }
+
+#[test]
+fn unknown_property_keys_are_reported() -> McResult<()> {
+    let overrides = BTreeMap::from([
+        (String::from("spawn-protection"), String::from("0")),
+        (String::from("query.port"), String::from("25565")),
+        (String::from("spawn-protetion"), String::from("0")),
+        (String::from("fabric.custom"), String::from("x"))
+    ]);
+
+    let result = ServerProperties::default().to_entries(&overrides, &BTreeMap::new())?;
+
+    assert_eq!(
+        result.unknown_keys,
+        vec!["fabric.custom", "spawn-protetion"]
+    );
+    assert_eq!(
+        result.entries.get("spawn-protection").map(String::as_str),
+        Some("0")
+    );
+    assert_eq!(
+        result.entries.get("spawn-protetion").map(String::as_str),
+        Some("0")
+    );
+
+    Ok(())
+}

src/ops/run.rs

@@ -19,6 +19,7 @@ use crate::manifest::Manifest;
 use crate::manifest::ManifestPaths;
 use crate::minecraft::log4j;
 use crate::minecraft::server_properties::ManagedServerProperties;
+use crate::minecraft::server_properties::PropertyEntries;
 use crate::minecraft::server_properties::ServerProperties;
 use crate::ops;
 use crate::ops::backups::BackupOptions;
@@ -215,7 +216,17 @@ pub async fn run(context: &mut McContext, options: &RunOptions) -> McResult<Opti
 
     let property_overrides = manifest.server.property_overrides()?;
 
-    let mut property_entries = properties.to_entries(&property_overrides, &managed_entries)?;
+    let PropertyEntries {
+        entries: mut property_entries,
+        unknown_keys
+    } = properties.to_entries(&property_overrides, &managed_entries)?;
+
+    for key in unknown_keys {
+        _ = context.shell().warn(format!(
+            "`{}` in the `properties` block of mc.kdl is not a known server property",
+            key
+        ));
+    }
 
     // rcon without a password would expose an unauthenticated console, so the
     // switch is derived from the effective password instead of set directly.