diagnostic: update configuration options to use clap's ArgAction

src/diagnostic.rs

@@ -607,6 +607,11 @@ impl AnalyzerModule for StyleAnalyzer {
     fn analyze(&self, source: &str, config: &DiagnosticConfig) -> Vec<Diagnostic> {
         let mut diagnostics = Vec::new();
 
+        // Only perform style analysis if enabled
+        if !config.enable_style_warnings {
+            return diagnostics;
+        }
+
         // Check for missing shebang
         if !source.starts_with("#!") {
             let range = Range::new(Position::new(0, 0), Position::new(0, 0));
@@ -802,14 +807,29 @@ impl StyleAnalyzer {
 pub struct SemanticAnalyzer;
 
 impl AnalyzerModule for SemanticAnalyzer {
-    fn analyze(&self, source: &str, _config: &DiagnosticConfig) -> Vec<Diagnostic> {
+    fn analyze(&self, source: &str, config: &DiagnosticConfig) -> Vec<Diagnostic> {
         let mut diagnostics = Vec::new();
 
-        // Parse and validate nftables-specific constructs
+        // Always run semantic validation (syntax/semantic errors)
         diagnostics.extend(self.validate_table_declarations(source));
-        diagnostics.extend(self.validate_chain_declarations(source));
+        diagnostics.extend(self.validate_chain_declarations_semantic(source));
         diagnostics.extend(self.validate_cidr_notation(source));
-        diagnostics.extend(self.check_for_redundant_rules(source));
+
+        // Best practices checks (only if enabled)
+        if config.enable_best_practices {
+            diagnostics.extend(self.validate_chain_best_practices(source));
+            diagnostics.extend(self.check_for_redundant_rules(source));
+        }
+
+        // Performance hints (only if enabled)
+        if config.enable_performance_hints {
+            diagnostics.extend(self.check_performance_hints(source));
+        }
+
+        // Security warnings (only if enabled)
+        if config.enable_security_warnings {
+            diagnostics.extend(self.check_security_warnings(source));
+        }
 
         diagnostics
     }
@@ -880,7 +900,7 @@ impl SemanticAnalyzer {
         diagnostics
     }
 
-    fn validate_chain_declarations(&self, source: &str) -> Vec<Diagnostic> {
+    fn validate_chain_declarations_semantic(&self, source: &str) -> Vec<Diagnostic> {
         let mut diagnostics = Vec::new();
 
         for (line_idx, line) in source.lines().enumerate() {
@@ -888,7 +908,7 @@ impl SemanticAnalyzer {
             let trimmed = line.trim();
 
             if trimmed.starts_with("type ") && trimmed.contains("hook") {
-                // Validate chain type and hook
+                // Validate chain type and hook (semantic validation)
                 if let Some(hook_pos) = trimmed.find("hook") {
                     let hook_part = &trimmed[hook_pos..];
                     let hook_words: Vec<&str> = hook_part.split_whitespace().collect();
@@ -916,22 +936,109 @@ impl SemanticAnalyzer {
                         }
                     }
                 }
+            }
+        }
 
-                // Check for missing policy in filter chains
+        diagnostics
-                if trimmed.contains("type filter") && !trimmed.contains("policy") {
+    }
+
+    fn validate_chain_best_practices(&self, source: &str) -> Vec<Diagnostic> {
+        let mut diagnostics = Vec::new();
+
+        for (line_idx, line) in source.lines().enumerate() {
+            let line_num = line_idx as u32;
+            let trimmed = line.trim();
+
+            // Check for missing policy in filter chains (best practice)
+            if trimmed.contains("type filter") && !trimmed.contains("policy") {
+                let range = Range::new(
+                    Position::new(line_num, 0),
+                    Position::new(line_num, line.len() as u32),
+                );
+                let diagnostic = Diagnostic::new(
+                    range,
+                    DiagnosticSeverity::Warning,
+                    DiagnosticCode::ChainWithoutPolicy,
+                    "Filter chain should have an explicit policy".to_string(),
+                );
+                diagnostics.push(diagnostic);
+            }
+        }
+
+        diagnostics
+    }
+
+    fn check_performance_hints(&self, source: &str) -> Vec<Diagnostic> {
+        let mut diagnostics = Vec::new();
+
+        for (line_idx, line) in source.lines().enumerate() {
+            let line_num = line_idx as u32;
+            let trimmed = line.trim();
+
+            // Check for large sets without timeout (performance hint)
+            if trimmed.contains("set ") && trimmed.contains("{") && !trimmed.contains("timeout") {
+                // Simple heuristic: if set definition is long, suggest timeout
+                if trimmed.len() > 100 {
                     let range = Range::new(
                         Position::new(line_num, 0),
                         Position::new(line_num, line.len() as u32),
                     );
                     let diagnostic = Diagnostic::new(
                         range,
-                        DiagnosticSeverity::Warning,
+                        DiagnosticSeverity::Hint,
-                        DiagnosticCode::ChainWithoutPolicy,
+                        DiagnosticCode::LargeSetWithoutTimeout,
-                        "Filter chain should have an explicit policy".to_string(),
+                        "Consider adding a timeout to large sets for better performance"
+                            .to_string(),
                     );
                     diagnostics.push(diagnostic);
                 }
             }
+
+            // Check for missing counters (performance hint)
+            if (trimmed.contains(" accept") || trimmed.contains(" drop"))
+                && !trimmed.contains("counter")
+            {
+                let range = Range::new(
+                    Position::new(line_num, 0),
+                    Position::new(line_num, line.len() as u32),
+                );
+                let diagnostic = Diagnostic::new(
+                    range,
+                    DiagnosticSeverity::Hint,
+                    DiagnosticCode::MissingCounters,
+                    "Consider adding counters to rules for monitoring and debugging".to_string(),
+                );
+                diagnostics.push(diagnostic);
+            }
+        }
+
+        diagnostics
+    }
+
+    fn check_security_warnings(&self, source: &str) -> Vec<Diagnostic> {
+        let mut diagnostics = Vec::new();
+
+        for (line_idx, line) in source.lines().enumerate() {
+            let line_num = line_idx as u32;
+            let trimmed = line.trim();
+
+            // Check for overly permissive rules (security warning)
+            if trimmed.contains(" accept")
+                && (trimmed.contains("0.0.0.0/0") || trimmed.contains("::/0"))
+            {
+                let range = Range::new(
+                    Position::new(line_num, 0),
+                    Position::new(line_num, line.len() as u32),
+                );
+                let diagnostic = Diagnostic::new(
+                    range,
+                    DiagnosticSeverity::Warning,
+                    DiagnosticCode::SecurityRisk,
+                    "Rule accepts traffic from anywhere - consider restricting source addresses"
+                        .to_string(),
+                );
+                diagnostics.push(diagnostic);
+            }
         }
 
         diagnostics

src/main.rs

@@ -93,19 +93,19 @@ enum Commands {
         json: bool,
 
         /// Include style warnings in diagnostics
-        #[arg(long, default_value = "true")]
+        #[arg(long, action = clap::ArgAction::Set, default_value = "true")]
         style_warnings: bool,
 
         /// Include best practice recommendations in diagnostics
-        #[arg(long, default_value = "true")]
+        #[arg(long, action = clap::ArgAction::Set, default_value = "true")]
         best_practices: bool,
 
         /// Include performance hints in diagnostics
-        #[arg(long, default_value = "true")]
+        #[arg(long, action = clap::ArgAction::Set, default_value = "true")]
         performance_hints: bool,
 
         /// Include security warnings in diagnostics
-        #[arg(long, default_value = "true")]
+        #[arg(long, action = clap::ArgAction::Set, default_value = "true")]
         security_warnings: bool,
 
         /// Diagnostic modules to run (comma-separated: lexical,syntax,style,semantic)