feat(config): Reject invalid configuration combinations (#2077)

Fixes #1946

This PR implements validation to reject invalid configuration
combinations in qlty.toml files.

## Changes

- Add validation to `EnabledPlugin` to detect mutually exclusive options
- `package_file` and `extra_packages` cannot be used together
- Provide clear error message when both options are specified
- Add comprehensive unit tests and integration tests
- Validation occurs during configuration parsing/post-processing

## Testing

The implementation includes tests that verify:
- Valid configurations with only `package_file`
- Valid configurations with only `extra_packages`
- Valid configurations with neither option
- Error handling when both options are specified

Generated with [Claude Code](https://claude.ai/code)

---------

Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Co-authored-by: brynary <[email protected]>

Author: brynary

qlty-config/src/config/builder.rs

@@ -252,7 +252,7 @@ impl Builder {
         }
     }
 
-    fn toml_to_config(toml: Value) -> Result<QltyConfig> {
+    pub fn toml_to_config(toml: Value) -> Result<QltyConfig> {
         let mut config: QltyConfig = Self::parse_toml_as_config(toml)?;
         Self::insert_ignores_from_exclude_patterns(&mut config);
         let config = Self::post_process_config(config);
@@ -275,6 +275,9 @@ impl Builder {
         let mut config = config.clone();
 
         for enabled_plugin in &mut config.plugin {
+            enabled_plugin.validate().with_context(|| {
+                format!("Configuration error for plugin '{}'", enabled_plugin.name)
+            })?;
             let plugin_definition =
                 config
                     .plugins
@@ -386,4 +389,120 @@ mod test {
         let result = Builder::extract_sources(Table(input));
         assert!(result.is_err());
     }
+
+    #[test]
+    fn test_validate_plugin_with_mutually_exclusive_options() {
+        let invalid_config = toml! {
+            config_version = "0"
+
+            [[plugin]]
+            name = "rubocop"
+            version = "1.56.3"
+            extra_packages = ["[email protected]"]
+            package_file = "Gemfile"
+
+            [plugins.definitions.rubocop]
+            runtime = "ruby"
+        };
+
+        let result = Builder::toml_to_config(Table(invalid_config));
+        assert!(result.is_err());
+
+        let error = result.unwrap_err();
+        let error_chain = error
+            .chain()
+            .map(|e| e.to_string())
+            .collect::<Vec<_>>()
+            .join(" ");
+
+        assert!(error_chain.contains("rubocop"));
+        assert!(error_chain.contains("package_file"));
+        assert!(error_chain.contains("extra_packages"));
+        assert!(error_chain.contains("mutually exclusive"));
+    }
+
+    #[test]
+    fn test_validate_plugin_with_package_file_only() {
+        let valid_config = toml! {
+            config_version = "0"
+
+            [[plugin]]
+            name = "rubocop"
+            version = "1.56.3"
+            package_file = "Gemfile"
+
+            [plugins.definitions.rubocop]
+            runtime = "ruby"
+        };
+
+        let result = Builder::toml_to_config(Table(valid_config));
+        assert!(result.is_ok());
+    }
+
+    #[test]
+    fn test_validate_plugin_with_extra_packages_only() {
+        let valid_config = toml! {
+            config_version = "0"
+
+            [[plugin]]
+            name = "rubocop"
+            version = "1.56.3"
+            extra_packages = ["[email protected]"]
+
+            [plugins.definitions.rubocop]
+            runtime = "ruby"
+        };
+
+        let result = Builder::toml_to_config(Table(valid_config));
+        assert!(result.is_ok());
+    }
+
+    #[test]
+    fn test_validate_plugin_with_package_filters_but_no_package_file() {
+        let invalid_config = toml! {
+            config_version = "0"
+
+            [[plugin]]
+            name = "rubocop"
+            version = "1.56.3"
+            package_filters = ["some-filter"]
+
+            [plugins.definitions.rubocop]
+            runtime = "ruby"
+        };
+
+        let result = Builder::toml_to_config(Table(invalid_config));
+        assert!(result.is_err());
+
+        let error = result.unwrap_err();
+        let error_chain = error
+            .chain()
+            .map(|e| e.to_string())
+            .collect::<Vec<_>>()
+            .join(" ");
+
+        assert!(error_chain.contains("rubocop"));
+        assert!(error_chain.contains("package_filters"));
+        assert!(error_chain.contains("package_file"));
+        assert!(error_chain.contains("requires"));
+    }
+
+    #[test]
+    fn test_validate_plugin_with_package_filters_and_package_file() {
+        let valid_config = toml! {
+            config_version = "0"
+
+            [[plugin]]
+            name = "rubocop"
+            version = "1.56.3"
+            package_file = "Gemfile"
+            package_filters = ["some-filter"]
+
+            [plugins.definitions.rubocop]
+            runtime = "ruby"
+        };
+
+        let result = Builder::toml_to_config(Table(valid_config));
+        assert!(result.is_ok());
+    }
 }

qlty-config/src/config/plugin.rs

@@ -699,6 +699,26 @@ pub struct PluginFetch {
     pub path: String,
 }
 
+impl EnabledPlugin {
+    pub fn validate(&self) -> Result<()> {
+        if self.package_file.is_some() && !self.extra_packages.is_empty() {
+            return Err(anyhow::anyhow!(
+                "Plugin '{}' has both 'package_file' and 'extra_packages' configured. These options are mutually exclusive.",
+                self.name
+            ));
+        }
+
+        if !self.package_filters.is_empty() && self.package_file.is_none() {
+            return Err(anyhow::anyhow!(
+                "Plugin '{}' has 'package_filters' configured but no 'package_file'. The 'package_filters' option requires 'package_file' to be specified.",
+                self.name
+            ));
+        }
+
+        Ok(())
+    }
+}
+
 impl PluginFetch {
     pub fn download_file_to(&self, directories: &[PathBuf]) -> Result<()> {
         let response = ureq::get(&self.url)
@@ -881,3 +901,124 @@ impl fmt::Display for Platform {
         write!(f, "{}", name)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_enabled_plugin_validate_success_with_package_file() {
+        let plugin = EnabledPlugin {
+            name: "test-plugin".to_string(),
+            package_file: Some("Gemfile".to_string()),
+            extra_packages: vec![],
+            ..Default::default()
+        };
+
+        assert!(plugin.validate().is_ok());
+    }
+
+    #[test]
+    fn test_enabled_plugin_validate_success_with_extra_packages() {
+        let plugin = EnabledPlugin {
+            name: "test-plugin".to_string(),
+            package_file: None,
+            extra_packages: vec![ExtraPackage {
+                name: "some-package".to_string(),
+                version: "1.0.0".to_string(),
+            }],
+            ..Default::default()
+        };
+
+        assert!(plugin.validate().is_ok());
+    }
+
+    #[test]
+    fn test_enabled_plugin_validate_success_with_neither() {
+        let plugin = EnabledPlugin {
+            name: "test-plugin".to_string(),
+            package_file: None,
+            extra_packages: vec![],
+            ..Default::default()
+        };
+
+        assert!(plugin.validate().is_ok());
+    }
+
+    #[test]
+    fn test_enabled_plugin_validate_failure_with_both() {
+        let plugin = EnabledPlugin {
+            name: "test-plugin".to_string(),
+            package_file: Some("Gemfile".to_string()),
+            extra_packages: vec![ExtraPackage {
+                name: "some-package".to_string(),
+                version: "1.0.0".to_string(),
+            }],
+            ..Default::default()
+        };
+
+        let result = plugin.validate();
+        assert!(result.is_err());
+
+        let error_message = result.unwrap_err().to_string();
+        assert!(error_message.contains("test-plugin"));
+        assert!(error_message.contains("package_file"));
+        assert!(error_message.contains("extra_packages"));
+        assert!(error_message.contains("mutually exclusive"));
+    }
+
+    #[test]
+    fn test_enabled_plugin_validate_success_with_package_file_and_filters() {
+        let plugin = EnabledPlugin {
+            name: "test-plugin".to_string(),
+            package_file: Some("Gemfile".to_string()),
+            package_filters: vec!["some-filter".to_string()],
+            ..Default::default()
+        };
+
+        assert!(plugin.validate().is_ok());
+    }
+
+    #[test]
+    fn test_enabled_plugin_validate_success_with_package_file_no_filters() {
+        let plugin = EnabledPlugin {
+            name: "test-plugin".to_string(),
+            package_file: Some("Gemfile".to_string()),
+            package_filters: vec![],
+            ..Default::default()
+        };
+
+        assert!(plugin.validate().is_ok());
+    }
+
+    #[test]
+    fn test_enabled_plugin_validate_success_without_package_file_or_filters() {
+        let plugin = EnabledPlugin {
+            name: "test-plugin".to_string(),
+            package_file: None,
+            package_filters: vec![],
+            ..Default::default()
+        };
+
+        assert!(plugin.validate().is_ok());
+    }
+
+    #[test]
+    fn test_enabled_plugin_validate_failure_with_filters_but_no_package_file() {
+        let plugin = EnabledPlugin {
+            name: "test-plugin".to_string(),
+            package_file: None,
+            package_filters: vec!["some-filter".to_string()],
+            ..Default::default()
+        };
+
+        let result = plugin.validate();
+        assert!(result.is_err());
+
+        let error_message = result.unwrap_err().to_string();
+        assert!(error_message.contains("test-plugin"));
+        assert!(error_message.contains("package_filters"));
+        assert!(error_message.contains("package_file"));
+        assert!(error_message.contains("requires"));
+    }
+}