Hard-fail on invalid ScopeKnownError config, warn on others

docs/docs/commands/lint.md

@@ -11,3 +11,8 @@ When run `scope lint` will check:
 
 To validate `ScopeReportLocation`'s, inputs are generated and templates are rendered. This allows report templates to be validated before they exposed to others.
 
+`ScopeKnownError` and `ScopeDoctorGroup` aren't checked here because they're already validated
+at config load time, before any command (including `lint`) runs. An invalid `ScopeKnownError`
+will fail the command outright rather than showing up as a lint finding; see
+[ScopeKnownError](../models/ScopeKnownError.mdx#validation).
+

docs/docs/models/ScopeDoctorGroup.mdx

@@ -89,6 +89,12 @@ summary; its dependencies that get trimmed along with it are not.
 A command can either be relative, or use the PATH.
 To target a script relative to the group it must start with `.`, and giving a relative path to the group file.
 
+## Validation
+
+A `ScopeDoctorGroup` that fails to load (for example, a `check` or `fix` command whose template
+fails to render) is logged as a warning naming the file, and skipped. The rest of your
+configuration still loads normally.
+
 ## Schema
 
 <Tabs>

docs/docs/models/ScopeKnownError.mdx

@@ -21,6 +21,13 @@ spec:
   help: The command had an error, try reading the logs around there to find out what happened.
 ```
 
+## Validation
+
+An invalid `ScopeKnownError` (a pattern with a reserved capture group name like `captures` or
+`working_dir`, or a `fix` whose template fails to render) causes `scope` to fail to load
+entirely, rather than silently ignoring that one check. A known error that isn't actually
+registered would otherwise give false confidence that `scope` is watching for it.
+
 ## Schema
 
 <Tabs>

docs/docs/models/ScopeReportLocation.mdx

@@ -65,6 +65,11 @@ spec:
       url: http://localhost:8000
 ```
 
+### Validation
+
+A `ScopeReportLocation` that fails to load is logged as a warning naming the file, and skipped.
+The rest of your configuration still loads normally.
+
 ### Schema
 
 <Tabs>

src/shared/config_load.rs

@@ -70,7 +70,7 @@ impl ConfigOptions {
         };
 
         let config_path = self.find_scope_paths(&working_dir);
-        let found_config = FoundConfig::new(self, working_dir, config_path).await;
+        let found_config = FoundConfig::new(self, working_dir, config_path).await?;
 
         debug!("Loaded config {:?}", found_config);
 
@@ -132,7 +132,7 @@ impl FoundConfig {
         config_options: &ConfigOptions,
         working_dir: PathBuf,
         config_path: Vec<PathBuf>,
-    ) -> Self {
+    ) -> Result<Self> {
         let default_path = std::env::var("PATH").unwrap_or_default();
 
         let mut config_path = config_path.to_vec();
@@ -164,13 +164,29 @@ impl FoundConfig {
             run_id: config_options.get_run_id(),
         };
 
+        let mut known_error_failures: Vec<String> = Vec::new();
         for raw_config in raw_config {
-            if let Ok(value) = raw_config.try_into() {
-                this.add_model(value);
+            let file_path = raw_config.metadata().file_path();
+            let kind = raw_config.kind.clone();
+            match ParsedConfig::try_from(raw_config) {
+                Ok(value) => this.add_model(value),
+                Err(e) => {
+                    let message =
+                        format!("Unable to convert config from {} because {}", file_path, e);
+                    if is_hard_fail_kind(&kind) {
+                        known_error_failures.push(message);
+                    } else {
+                        warn!(target: "user", "{}", message);
+                    }
+                }
             }
         }
 
-        this
+        if !known_error_failures.is_empty() {
+            return Err(anyhow!(known_error_failures.join("\n")));
+        }
+
+        Ok(this)
     }
 
     pub fn write_raw_config_to_disk(&self) -> Result<PathBuf> {
@@ -205,6 +221,13 @@ impl FoundConfig {
     }
 }
 
+/// A `ScopeKnownError` that fails to load is worse than useless: it looks like the tool is
+/// watching for that error when it isn't. Other kinds degrade gracefully (warn and continue),
+/// but known-error failures abort config load entirely.
+fn is_hard_fail_kind(kind: &str) -> bool {
+    kind.eq_ignore_ascii_case("ScopeKnownError")
+}
+
 fn insert_if_absent<T: HelpMetadata>(map: &mut BTreeMap<String, T>, entry: T) {
     let name = entry.name().to_string();
     if map.contains_key(&name) {

tests/scope_root.rs

@@ -70,3 +70,145 @@ spec:
     ));
     test_helper.clean_work_dir();
 }
+
+#[test]
+fn test_known_error_with_reserved_capture_name_fails_load() {
+    let test_helper = ScopeTestHelper::new(
+        "test_known_error_with_reserved_capture_name_fails_load",
+        "empty",
+    );
+    let example_file = "apiVersion: scope.github.com/v1alpha
+kind: ScopeKnownError
+metadata:
+  name: bad-reserved
+  description: uses a reserved capture group name
+spec:
+  pattern: \"boom: (?<captures>.*)\"
+  help: \"should never load\"
+";
+    test_helper
+        .work_dir
+        .child(".scope/bad-reserved.yaml")
+        .write_str(example_file)
+        .unwrap();
+
+    let result = test_helper.run_command(&["list"]);
+
+    result
+        .failure()
+        .stdout(predicate::str::contains("bad-reserved.yaml"))
+        .stdout(predicate::str::contains("reserved"));
+    test_helper.clean_work_dir();
+}
+
+#[test]
+fn test_known_error_with_broken_fix_template_fails_load() {
+    let test_helper = ScopeTestHelper::new(
+        "test_known_error_with_broken_fix_template_fails_load",
+        "empty",
+    );
+    let example_file = "apiVersion: scope.github.com/v1alpha
+kind: ScopeKnownError
+metadata:
+  name: bad-fix-template
+  description: has a broken fix template
+spec:
+  pattern: \"boom: (.*)\"
+  help: \"should never load\"
+  fix:
+    commands:
+      - \"echo {{ unclosed\"
+";
+    test_helper
+        .work_dir
+        .child(".scope/bad-fix-template.yaml")
+        .write_str(example_file)
+        .unwrap();
+
+    let result = test_helper.run_command(&["list"]);
+
+    result
+        .failure()
+        .stdout(predicate::str::contains("bad-fix-template.yaml"));
+    test_helper.clean_work_dir();
+}
+
+#[test]
+fn test_multiple_known_error_failures_are_all_reported() {
+    let test_helper = ScopeTestHelper::new(
+        "test_multiple_known_error_failures_are_all_reported",
+        "empty",
+    );
+    let bad_reserved = "apiVersion: scope.github.com/v1alpha
+kind: ScopeKnownError
+metadata:
+  name: bad-reserved
+  description: uses a reserved capture group name
+spec:
+  pattern: \"boom: (?<captures>.*)\"
+  help: \"should never load\"
+";
+    let bad_fix_template = "apiVersion: scope.github.com/v1alpha
+kind: ScopeKnownError
+metadata:
+  name: bad-fix-template
+  description: has a broken fix template
+spec:
+  pattern: \"boom: (.*)\"
+  help: \"should never load\"
+  fix:
+    commands:
+      - \"echo {{ unclosed\"
+";
+    test_helper
+        .work_dir
+        .child(".scope/bad-reserved.yaml")
+        .write_str(bad_reserved)
+        .unwrap();
+    test_helper
+        .work_dir
+        .child(".scope/bad-fix-template.yaml")
+        .write_str(bad_fix_template)
+        .unwrap();
+
+    let result = test_helper.run_command(&["list"]);
+
+    result
+        .failure()
+        .stdout(predicate::str::contains("bad-reserved.yaml"))
+        .stdout(predicate::str::contains("bad-fix-template.yaml"));
+    test_helper.clean_work_dir();
+}
+
+#[test]
+fn test_doctor_group_broken_template_warns_and_continues() {
+    let test_helper = ScopeTestHelper::new(
+        "test_doctor_group_broken_template_warns_and_continues",
+        "empty",
+    );
+    let example_file = "apiVersion: scope.github.com/v1alpha
+kind: ScopeDoctorGroup
+metadata:
+  name: broken-template-group
+  description: has a broken check command template
+spec:
+  actions:
+    - name: broken-check
+      description: check with a broken template
+      check:
+        commands:
+          - \"echo {{ unclosed\"
+";
+    test_helper
+        .work_dir
+        .child(".scope/broken-template-group.yaml")
+        .write_str(example_file)
+        .unwrap();
+
+    let result = test_helper.run_command(&["list"]);
+
+    result
+        .success()
+        .stdout(predicate::str::contains("broken-template-group.yaml"));
+    test_helper.clean_work_dir();
+}