Rewatch: warnings for unsupported/unknown rescript.json fields (#8031)

* Rewatch: warnings for unsupported/unknown rescript.json fields

* CHANGELOG

* Format

* rewatch tests

* Fixed: gentypeconfig is a supported field

Author: cknitt

CHANGELOG.md

@@ -20,6 +20,8 @@
 
 #### :bug: Bug fix
 
+- Rewatch: warnings for unsupported/unknown rescript.json fields. https://github.com/rescript-lang/rescript/pull/8031
+
 #### :memo: Documentation
 
 #### :nail_care: Polish

rewatch/Cargo.lock

@@ -24,6 +24,7 @@ num_cpus = "1.17.0"
 regex = "1.7.1"
 serde = { version = "1.0.152", features = ["derive"] }
 serde_json = { version = "1.0.93" }
+serde_ignored = "0.1.11"
 sysinfo = "0.29.10"
 tempfile = "3.10.1"
 

rewatch/Cargo.toml

@@ -18,7 +18,7 @@ This document contains a list of all bsconfig parameters with remarks, and wheth
 | generators            | array of Rule-Generator |        |     [_]      |
 | cut-generators        | boolean                 |        |     [_]      |
 | jsx                   | JSX                     |        |     [x]      |
-| gentypeconfig         | Gentype                 |        |     [_]      |
+| gentypeconfig         | Gentype                 |        |     [x]      |
 | compiler-flags        | array of string         |        |     [x]      |
 | warnings              | Warnings                |        |     [x]      |
 | ppx-flags             | array of string         |        |     [x]      |

rewatch/CompilerConfigurationSpec.md

@@ -370,7 +370,7 @@ pub fn incremental_build(
             println!("{}", &compile_warnings);
         }
         if initial_build {
-            log_deprecations(build_state);
+            log_config_warnings(build_state);
         }
         if helpers::contains_ascii_characters(&compile_errors) {
             println!("{}", &compile_errors);
@@ -399,7 +399,7 @@ pub fn incremental_build(
             println!("{}", &compile_warnings);
         }
         if initial_build {
-            log_deprecations(build_state);
+            log_config_warnings(build_state);
         }
 
         // Write per-package compiler metadata to `lib/bs/compiler-info.json` (idempotent)
@@ -409,7 +409,7 @@ pub fn incremental_build(
     }
 }
 
-fn log_deprecations(build_state: &BuildCommandState) {
+fn log_config_warnings(build_state: &BuildCommandState) {
     build_state.packages.iter().for_each(|(_, package)| {
         // Only warn for local dependencies, not external packages
         if package.is_local_dep {
@@ -426,6 +426,18 @@ fn log_deprecations(build_state: &BuildCommandState) {
                     }
                 },
             );
+
+            package
+                .config
+                .get_unsupported_fields()
+                .iter()
+                .for_each(|field| log_unsupported_config_field(&package.name, field));
+
+            package
+                .config
+                .get_unknown_fields()
+                .iter()
+                .for_each(|field| log_unknown_config_field(&package.name, field));
         }
     });
 }
@@ -438,6 +450,20 @@ fn log_deprecated_config_field(package_name: &str, field_name: &str, new_field_n
     println!("\n{}", style(warning).yellow());
 }
 
+fn log_unsupported_config_field(package_name: &str, field_name: &str) {
+    let warning = format!(
+        "The field '{field_name}' found in the package config of '{package_name}' is not supported by ReScript 12's new build system."
+    );
+    println!("\n{}", style(warning).yellow());
+}
+
+fn log_unknown_config_field(package_name: &str, field_name: &str) {
+    let warning = format!(
+        "Unknown field '{field_name}' found in the package config of '{package_name}'. This option will be ignored."
+    );
+    println!("\n{}", style(warning).yellow());
+}
+
 // write build.ninja files in the packages after a non-incremental build
 // this is necessary to bust the editor tooling cache. The editor tooling
 // is watching this file.

rewatch/src/build.rs

@@ -308,6 +308,10 @@ pub struct Config {
     #[serde(skip)]
     deprecation_warnings: Vec<DeprecationWarning>,
 
+    // Holds unknown fields we encountered while parsing
+    #[serde(skip, default)]
+    unknown_fields: Vec<String>,
+
     #[serde(default = "default_path")]
     pub path: PathBuf,
 }
@@ -435,9 +439,14 @@ impl Config {
 
     /// Try to convert a bsconfig from a string to a bsconfig struct
     pub fn new_from_json_string(config_str: &str) -> Result<Self> {
-        let mut config = serde_json::from_str::<Config>(config_str)?;
+        let mut deserializer = serde_json::Deserializer::from_str(config_str);
+        let mut unknown_fields = Vec::new();
+        let mut config: Config = serde_ignored::deserialize(&mut deserializer, |path| {
+            unknown_fields.push(path.to_string());
+        })?;
 
         config.handle_deprecations()?;
+        config.unknown_fields = unknown_fields;
 
         Ok(config)
     }
@@ -662,6 +671,42 @@ impl Config {
         &self.deprecation_warnings
     }
 
+    pub fn get_unknown_fields(&self) -> Vec<String> {
+        self.unknown_fields
+            .iter()
+            .filter(|field| !self.is_unsupported_field(field))
+            .cloned()
+            .collect()
+    }
+
+    pub fn get_unsupported_fields(&self) -> Vec<String> {
+        self.unknown_fields
+            .iter()
+            .filter(|field| self.is_unsupported_field(field))
+            .cloned()
+            .collect::<Vec<_>>()
+    }
+
+    fn is_unsupported_field(&self, field: &str) -> bool {
+        const UNSUPPORTED_TOP_LEVEL_FIELDS: &[&str] = &[
+            "version",
+            "ignored-dirs",
+            "generators",
+            "cut-generators",
+            "pp-flags",
+            "js-post-build",
+            "entries",
+            "use-stdlib",
+            "external-stdlib",
+            "bs-external-includes",
+            "reanalyze",
+        ];
+
+        let top_level = field.split(|c| ['.', '['].contains(&c)).next().unwrap_or(field);
+
+        UNSUPPORTED_TOP_LEVEL_FIELDS.contains(&top_level)
+    }
+
     fn handle_deprecations(&mut self) -> Result<()> {
         if self.dependencies.is_some() && self.bs_dependencies.is_some() {
             bail!("dependencies and bs-dependencies are mutually exclusive. Please use 'dependencies'.");
@@ -729,6 +774,7 @@ pub mod tests {
             deprecation_warnings: vec![],
             experimental_features: None,
             allowed_dependents: args.allowed_dependents,
+            unknown_fields: vec![],
             path: args.path,
         }
     }
@@ -1023,6 +1069,42 @@ pub mod tests {
         assert!(config.get_deprecations().is_empty());
     }
 
+    #[test]
+    fn test_unknown_fields_are_collected() {
+        let json = r#"
+        {
+            "name": "testrepo",
+            "sources": {
+                "dir": "src",
+                "subdirs": true
+            },
+            "some-new-field": true
+        }
+        "#;
+
+        let config = Config::new_from_json_string(json).expect("a valid json string");
+        assert_eq!(config.get_unknown_fields(), vec!["some-new-field".to_string()]);
+        assert!(config.get_unsupported_fields().is_empty());
+    }
+
+    #[test]
+    fn test_unsupported_fields_are_collected() {
+        let json = r#"
+        {
+            "name": "testrepo",
+            "sources": {
+                "dir": "src",
+                "subdirs": true
+            },
+            "ignored-dirs": ["scripts"]
+        }
+        "#;
+
+        let config = Config::new_from_json_string(json).expect("a valid json string");
+        assert_eq!(config.get_unsupported_fields(), vec!["ignored-dirs".to_string()]);
+        assert!(config.get_unknown_fields().is_empty());
+    }
+
     #[test]
     fn test_compiler_flags() {
         let json = r#"

rewatch/src/config.rs

@@ -10,6 +10,8 @@
     "in-source": true
   },
   "suffix": ".mjs",
+  "ignored-dirs": ["scripts"],
+  "some-new-field": true,
   "bs-dependencies": [],
   "bs-dev-dependencies": [],
   "bsc-flags": []

rewatch/testrepo/packages/deprecated-config/rescript.json

@@ -11,6 +11,10 @@ Use 'dev-dependencies' instead.
 The field 'bsc-flags' found in the package config of '@testrepo/deprecated-config' is deprecated and will be removed in a future version.
 Use 'compiler-flags' instead.
 
+The field 'ignored-dirs' found in the package config of '@testrepo/deprecated-config' is not supported by ReScript 12's new build system.
+
+Unknown field 'some-new-field' found in the package config of '@testrepo/deprecated-config'. This option will be ignored.
+
   We've found a bug for you!
   /packages/with-dev-deps/src/FileToTest.res:2:6-11
 

rewatch/tests/snapshots/bs-dev-dependency-used-by-non-dev-source.txt

@@ -10,3 +10,7 @@ Use 'dev-dependencies' instead.
 
 The field 'bsc-flags' found in the package config of '@testrepo/deprecated-config' is deprecated and will be removed in a future version.
 Use 'compiler-flags' instead.
+
+The field 'ignored-dirs' found in the package config of '@testrepo/deprecated-config' is not supported by ReScript's new build system (rewatch).
+
+Unknown field 'some-new-field' found in the package config of '@testrepo/deprecated-config'. This option will be ignored.

rewatch/tests/snapshots/clean-rebuild.txt

@@ -11,6 +11,10 @@ Use 'dev-dependencies' instead.
 The field 'bsc-flags' found in the package config of '@testrepo/deprecated-config' is deprecated and will be removed in a future version.
 Use 'compiler-flags' instead.
 
+The field 'ignored-dirs' found in the package config of '@testrepo/deprecated-config' is not supported by ReScript 12's new build system.
+
+Unknown field 'some-new-field' found in the package config of '@testrepo/deprecated-config'. This option will be ignored.
+
 Can't continue... Found a circular dependency in your code:
 Dep01 (packages/dep01/src/Dep01.res)
  → Dep02 (packages/dep02/src/Dep02.res)