From 2a45d0b983ccb649ccd3ddee48c0541bfbb941c0 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Mon, 2 Jun 2025 05:14:21 +0300 Subject: [PATCH 01/26] initial diagnostics implementation --- Cargo.lock | 85 ++++ Cargo.toml | 3 + src/diagnostic.rs | 1141 +++++++++++++++++++++++++++++++++++++++++++++ src/main.rs | 106 ++++- 4 files changed, 1326 insertions(+), 9 deletions(-) create mode 100644 src/diagnostic.rs diff --git a/Cargo.lock b/Cargo.lock index 411176d..898d315 100755 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,6 +2,15 @@ # It is not intended for manual editing. version = 4 +[[package]] +name = "aho-corasick" +version = "1.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e60d3430d3a69478ad0993f19238d2df97c507009a52b3c10addcd7f6bcb916" +dependencies = [ + "memchr", +] + [[package]] name = "anstream" version = "0.6.13" @@ -189,6 +198,12 @@ dependencies = [ "hashbrown", ] +[[package]] +name = "itoa" +version = "1.0.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4a5f13b858c8d314ee3e8f639011f7ccefe71f97f96e50151fb991f267928e2c" + [[package]] name = "lazy_static" version = "1.5.0" @@ -245,6 +260,12 @@ dependencies = [ "logos-codegen", ] +[[package]] +name = "memchr" +version = "2.7.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "78ca9ab1a0babb1e7d5695e3530886289c18cf2f87ec19a575a0abdce112e3a3" + [[package]] name = "nff" version = "0.1.0" @@ -253,6 +274,9 @@ dependencies = [ "clap", "cstree", "logos", + "regex", + "serde", + "serde_json", "text-size", "thiserror", ] @@ -307,6 +331,29 @@ dependencies = [ "bitflags", ] +[[package]] +name = "regex" +version = "1.11.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b544ef1b4eac5dc2db33ea63606ae9ffcfac26c1416a2806ae0bf5f56b201191" +dependencies = [ + "aho-corasick", + "memchr", + "regex-automata", + "regex-syntax", +] + +[[package]] +name = "regex-automata" +version = "0.4.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "809e8dc61f6de73b46c85f4c96486310fe304c434cfa43669d7b40f711150908" +dependencies = [ + "aho-corasick", + "memchr", + "regex-syntax", +] + [[package]] name = "regex-syntax" version = "0.8.5" @@ -322,6 +369,12 @@ dependencies = [ "semver", ] +[[package]] +name = "ryu" +version = "1.0.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "28d3b2b1366ec20994f1fd18c3c594f05c5dd4bc44d8bb0c1c632c8d6829481f" + [[package]] name = "scopeguard" version = "1.2.0" @@ -334,6 +387,38 @@ version = "1.0.26" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "56e6fa9c48d24d85fb3de5ad847117517440f6beceb7798af16b4a87d616b8d0" +[[package]] +name = "serde" +version = "1.0.219" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5f0e2c6ed6606019b4e29e69dbaba95b11854410e5347d525002456dbbb786b6" +dependencies = [ + "serde_derive", +] + +[[package]] +name = "serde_derive" +version = "1.0.219" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5b0276cf7f2c73365f7157c8123c21cd9a50fbbd844757af28ca1f5925fc2a00" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "serde_json" +version = "1.0.140" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "20068b6e96dc6c9bd23e01df8827e6c7e1f2fddd43c21810382803c136b99373" +dependencies = [ + "itoa", + "memchr", + "ryu", + "serde", +] + [[package]] name = "smallvec" version = "1.15.0" diff --git a/Cargo.toml b/Cargo.toml index c13c4eb..23e80a3 100755 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,3 +12,6 @@ thiserror = "2.0" logos = "0.15" cstree = "0.12" text-size = "1.1" +serde = { version = "1.0", features = ["derive"] } +serde_json = "1.0" +regex = "1.11.1" diff --git a/src/diagnostic.rs b/src/diagnostic.rs new file mode 100644 index 0000000..a1a6014 --- /dev/null +++ b/src/diagnostic.rs @@ -0,0 +1,1141 @@ +//! Diagnostic system for nftables configuration files +//! +//! This module provides comprehensive diagnostic capabilities including: +//! - Syntax errors with precise location information +//! - Semantic validation warnings +//! - Style and best practice recommendations +//! - Language Server Protocol (LSP) compatible output +//! - JSON output for tooling integration + +use crate::lexer::LexError; +use crate::parser::{ParseError, Parser}; +use serde::{Deserialize, Serialize}; +use serde_json; +use std::collections::{HashMap, HashSet}; +use std::fmt; +use text_size::TextSize; + +/// Diagnostic severity levels following LSP specification +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub enum DiagnosticSeverity { + /// Reports an error that prevents successful processing + Error = 1, + /// Reports a warning that should be addressed + Warning = 2, + /// Reports information that might be useful + Information = 3, + /// Reports a hint for potential improvements + Hint = 4, +} + +impl fmt::Display for DiagnosticSeverity { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + DiagnosticSeverity::Error => write!(f, "error"), + DiagnosticSeverity::Warning => write!(f, "warning"), + DiagnosticSeverity::Information => write!(f, "info"), + DiagnosticSeverity::Hint => write!(f, "hint"), + } + } +} + +/// Diagnostic codes for categorizing issues +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub enum DiagnosticCode { + // Syntax errors + SyntaxError, + UnexpectedToken, + MissingToken, + UnterminatedString, + InvalidNumber, + InvalidToken, + + // Semantic errors + UnknownTableFamily, + UnknownChainType, + UnknownHook, + InvalidPriority, + InvalidPolicy, + DuplicateTableName, + DuplicateChainName, + UndefinedVariable, + InvalidCidrNotation, + InvalidPortRange, + InvalidProtocol, + + // Style warnings + MissingShebang, + InconsistentIndentation, + TrailingWhitespace, + TooManyEmptyLines, + LongLine, + PreferredAlternative, + + // Best practices + ChainWithoutPolicy, + RuleWithoutAction, + OverlyPermissiveRule, + DuplicateRule, + ConflictingRules, + UnusedVariable, + UnusedSet, + DeprecatedSyntax, + MissingDocumentation, + SecurityRisk, + + // Performance + IneffientRuleOrder, + LargeSetWithoutTimeout, + MissingCounters, + + // Indentation and formatting + MixedIndentation, + IncorrectIndentationLevel, + MissingSpaceAfterComma, + ExtraWhitespace, + + // nftables specific + ChainMissingHook, + InvalidTableFamily, + InvalidChainPriority, + MissingChainType, + RedundantRule, + UnnecessaryJump, +} + +impl fmt::Display for DiagnosticCode { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let code = match self { + DiagnosticCode::SyntaxError => "NFT001", + DiagnosticCode::UnexpectedToken => "NFT002", + DiagnosticCode::MissingToken => "NFT003", + DiagnosticCode::UnterminatedString => "NFT004", + DiagnosticCode::InvalidNumber => "NFT005", + DiagnosticCode::InvalidToken => "NFT006", + + DiagnosticCode::UnknownTableFamily => "NFT101", + DiagnosticCode::UnknownChainType => "NFT102", + DiagnosticCode::UnknownHook => "NFT103", + DiagnosticCode::InvalidPriority => "NFT104", + DiagnosticCode::InvalidPolicy => "NFT105", + DiagnosticCode::DuplicateTableName => "NFT106", + DiagnosticCode::DuplicateChainName => "NFT107", + DiagnosticCode::UndefinedVariable => "NFT108", + DiagnosticCode::InvalidCidrNotation => "NFT109", + DiagnosticCode::InvalidPortRange => "NFT110", + DiagnosticCode::InvalidProtocol => "NFT111", + + DiagnosticCode::MissingShebang => "NFT201", + DiagnosticCode::InconsistentIndentation => "NFT202", + DiagnosticCode::TrailingWhitespace => "NFT203", + DiagnosticCode::TooManyEmptyLines => "NFT204", + DiagnosticCode::LongLine => "NFT205", + DiagnosticCode::PreferredAlternative => "NFT206", + + DiagnosticCode::ChainWithoutPolicy => "NFT301", + DiagnosticCode::RuleWithoutAction => "NFT302", + DiagnosticCode::OverlyPermissiveRule => "NFT303", + DiagnosticCode::DuplicateRule => "NFT304", + DiagnosticCode::ConflictingRules => "NFT305", + DiagnosticCode::UnusedVariable => "NFT306", + DiagnosticCode::UnusedSet => "NFT307", + DiagnosticCode::DeprecatedSyntax => "NFT308", + DiagnosticCode::MissingDocumentation => "NFT309", + DiagnosticCode::SecurityRisk => "NFT310", + + DiagnosticCode::IneffientRuleOrder => "NFT401", + DiagnosticCode::LargeSetWithoutTimeout => "NFT402", + DiagnosticCode::MissingCounters => "NFT403", + + DiagnosticCode::MixedIndentation => "NFT501", + DiagnosticCode::IncorrectIndentationLevel => "NFT502", + DiagnosticCode::MissingSpaceAfterComma => "NFT503", + DiagnosticCode::ExtraWhitespace => "NFT504", + + DiagnosticCode::ChainMissingHook => "NFT601", + DiagnosticCode::InvalidTableFamily => "NFT602", + DiagnosticCode::InvalidChainPriority => "NFT603", + DiagnosticCode::MissingChainType => "NFT604", + DiagnosticCode::RedundantRule => "NFT605", + DiagnosticCode::UnnecessaryJump => "NFT606", + }; + write!(f, "{}", code) + } +} + +/// Position information for diagnostics +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct Position { + pub line: u32, + pub character: u32, +} + +impl Position { + pub fn new(line: u32, character: u32) -> Self { + Self { line, character } + } + + pub fn from_text_size(text_size: TextSize, source: &str) -> Self { + let mut line = 0; + let mut character = 0; + let offset = text_size.into(); + + for (i, ch) in source.char_indices() { + if i >= offset { + break; + } + if ch == '\n' { + line += 1; + character = 0; + } else { + character += 1; + } + } + + Self { line, character } + } +} + +/// Range information for diagnostics +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct Range { + pub start: Position, + pub end: Position, +} + +impl Range { + pub fn new(start: Position, end: Position) -> Self { + Self { start, end } + } + + pub fn single_position(position: Position) -> Self { + Self { + start: position.clone(), + end: position, + } + } +} + +/// Related information for diagnostics +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct DiagnosticRelatedInformation { + pub location: Range, + pub message: String, +} + +/// Code action that can fix a diagnostic +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct CodeAction { + pub title: String, + pub kind: String, + pub edit: Option, +} + +/// Text edit for code actions +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct TextEdit { + pub range: Range, + pub new_text: String, +} + +/// Workspace edit containing text changes +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct WorkspaceEdit { + pub changes: HashMap>, +} + +/// A single diagnostic issue +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct Diagnostic { + /// The range at which the message applies + pub range: Range, + /// The diagnostic's severity + pub severity: DiagnosticSeverity, + /// The diagnostic's code + pub code: DiagnosticCode, + /// A human-readable string describing the source of this diagnostic + pub source: String, + /// The diagnostic's message + pub message: String, + /// Additional metadata about the diagnostic + pub related_information: Vec, + /// Code actions that can address this diagnostic + pub code_actions: Vec, + /// Tags providing additional metadata + pub tags: Vec, +} + +impl Diagnostic { + pub fn new( + range: Range, + severity: DiagnosticSeverity, + code: DiagnosticCode, + message: String, + ) -> Self { + Self { + range, + severity, + code, + source: "nff".to_string(), + message, + related_information: Vec::new(), + code_actions: Vec::new(), + tags: Vec::new(), + } + } +} + +/// Collection of diagnostics for a file +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +pub struct DiagnosticCollection { + pub diagnostics: Vec, + pub file_path: String, + pub source_text: String, +} + +impl DiagnosticCollection { + pub fn new(file_path: String, source_text: String) -> Self { + Self { + diagnostics: Vec::new(), + file_path, + source_text, + } + } + + pub fn extend(&mut self, diagnostics: Vec) { + self.diagnostics.extend(diagnostics); + } + + pub fn errors(&self) -> impl Iterator { + self.diagnostics + .iter() + .filter(|d| d.severity == DiagnosticSeverity::Error) + } + + pub fn has_errors(&self) -> bool { + self.errors().count() > 0 + } + + /// Convert to JSON for LSP or tooling integration + pub fn to_json(&self) -> serde_json::Result { + serde_json::to_string_pretty(self) + } + + /// Convert to a human-readable format + pub fn to_human_readable(&self) -> String { + let mut output = String::new(); + + if self.diagnostics.is_empty() { + output.push_str("No issues found.\n"); + return output; + } + + output.push_str(&format!( + "Found {} issues in {}:\n\n", + self.diagnostics.len(), + self.file_path + )); + + for diagnostic in &self.diagnostics { + output.push_str(&format!( + "{}:{}:{}: {}: {} [{}]\n", + self.file_path, + diagnostic.range.start.line + 1, + diagnostic.range.start.character + 1, + diagnostic.severity, + diagnostic.message, + diagnostic.code + )); + + // Add code snippet context + if let Some(context) = self.get_context_lines(&diagnostic.range, 2) { + for line in context { + output.push_str(&format!(" {}\n", line)); + } + output.push('\n'); + } + } + + output + } + + fn get_context_lines(&self, range: &Range, context_lines: usize) -> Option> { + let lines: Vec<&str> = self.source_text.lines().collect(); + let start_line = range.start.line as usize; + let end_line = range.end.line as usize; + + if start_line >= lines.len() { + return None; + } + + let context_start = start_line.saturating_sub(context_lines); + let context_end = std::cmp::min(end_line + context_lines + 1, lines.len()); + + let mut result = Vec::new(); + for (i, line) in lines[context_start..context_end].iter().enumerate() { + let line_num = context_start + i + 1; + if i + context_start == start_line { + result.push(format!("→ {:4}: {}", line_num, line)); + } else { + result.push(format!(" {:4}: {}", line_num, line)); + } + } + + Some(result) + } +} + +/// Configuration for diagnostic analysis +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct DiagnosticConfig { + /// Enable style warnings + pub enable_style_warnings: bool, + /// Enable best practice checks + pub enable_best_practices: bool, + /// Enable performance hints + pub enable_performance_hints: bool, + /// Enable security warnings + pub enable_security_warnings: bool, + /// Maximum line length for style checks + pub max_line_length: usize, + /// Maximum consecutive empty lines + pub max_empty_lines: usize, + /// Preferred indentation style + pub preferred_indent: Option, +} + +impl Default for DiagnosticConfig { + fn default() -> Self { + Self { + enable_style_warnings: true, + enable_best_practices: true, + enable_performance_hints: true, + enable_security_warnings: true, + max_line_length: 120, + max_empty_lines: 2, + preferred_indent: Some("tabs".to_string()), + } + } +} + +/// Trait for specialized diagnostic analyzers +pub trait AnalyzerModule { + fn analyze(&self, source: &str, config: &DiagnosticConfig) -> Vec; + fn name(&self) -> &'static str; +} + +/// Lexical analysis module +pub struct LexicalAnalyzer; + +impl AnalyzerModule for LexicalAnalyzer { + fn analyze(&self, source: &str, _config: &DiagnosticConfig) -> Vec { + use crate::lexer::NftablesLexer; + + let mut diagnostics = Vec::new(); + let mut lexer = NftablesLexer::new(source); + + match lexer.tokenize() { + Ok(_) => { + // No lexical errors + } + Err(lex_error) => { + let diagnostic = Self::lex_error_to_diagnostic(&lex_error, source); + diagnostics.push(diagnostic); + } + } + + diagnostics + } + + fn name(&self) -> &'static str { + "lexical" + } +} + +impl LexicalAnalyzer { + fn lex_error_to_diagnostic(error: &LexError, source: &str) -> Diagnostic { + match error { + LexError::InvalidToken { position, text } => { + let pos = Position::from_text_size(TextSize::from(*position as u32), source); + let range = Range::new( + pos.clone(), + Position::new(pos.line, pos.character + text.len() as u32), + ); + Diagnostic::new( + range, + DiagnosticSeverity::Error, + DiagnosticCode::InvalidToken, + format!("Invalid token: '{}'", text), + ) + } + LexError::UnterminatedString { position } => { + let pos = Position::from_text_size(TextSize::from(*position as u32), source); + let range = Range::single_position(pos); + Diagnostic::new( + range, + DiagnosticSeverity::Error, + DiagnosticCode::UnterminatedString, + "Unterminated string literal".to_string(), + ) + } + LexError::InvalidNumber { text } => { + if let Some(pos) = source.find(text) { + let start_pos = Position::from_text_size(TextSize::from(pos as u32), source); + let end_pos = + Position::new(start_pos.line, start_pos.character + text.len() as u32); + let range = Range::new(start_pos, end_pos); + Diagnostic::new( + range, + DiagnosticSeverity::Error, + DiagnosticCode::InvalidNumber, + format!("Invalid number: '{}'", text), + ) + } else { + let range = Range::single_position(Position::new(0, 0)); + Diagnostic::new( + range, + DiagnosticSeverity::Error, + DiagnosticCode::InvalidNumber, + format!("Invalid number: '{}'", text), + ) + } + } + } + } +} + +/// Syntax analysis module +pub struct SyntaxAnalyzer; + +impl AnalyzerModule for SyntaxAnalyzer { + fn analyze(&self, source: &str, _config: &DiagnosticConfig) -> Vec { + use crate::lexer::NftablesLexer; + + let mut diagnostics = Vec::new(); + let mut lexer = NftablesLexer::new(source); + + match lexer.tokenize() { + Ok(tokens) => { + let mut parser = Parser::new(tokens); + match parser.parse() { + Ok(_) => { + // No parse errors + } + Err(parse_error) => { + let diagnostic = Self::parse_error_to_diagnostic(&parse_error, source); + diagnostics.push(diagnostic); + } + } + } + Err(_) => { + // Already handled in lexical analysis + } + } + + diagnostics + } + + fn name(&self) -> &'static str { + "syntax" + } +} + +impl SyntaxAnalyzer { + fn parse_error_to_diagnostic(error: &ParseError, _source: &str) -> Diagnostic { + match error { + ParseError::UnexpectedToken { + line, + column, + expected, + found, + } => { + let pos = Position::new(*line as u32, *column as u32); + let range = Range::single_position(pos); + Diagnostic::new( + range, + DiagnosticSeverity::Error, + DiagnosticCode::UnexpectedToken, + format!("Expected {}, found '{}'", expected, found), + ) + } + ParseError::MissingToken { expected } => { + let range = Range::single_position(Position::new(0, 0)); + Diagnostic::new( + range, + DiagnosticSeverity::Error, + DiagnosticCode::MissingToken, + format!("Missing token: expected {}", expected), + ) + } + ParseError::InvalidExpression { message } => { + let range = Range::single_position(Position::new(0, 0)); + Diagnostic::new( + range, + DiagnosticSeverity::Error, + DiagnosticCode::SyntaxError, + format!("Invalid expression: {}", message), + ) + } + ParseError::InvalidStatement { message } => { + let range = Range::single_position(Position::new(0, 0)); + Diagnostic::new( + range, + DiagnosticSeverity::Error, + DiagnosticCode::SyntaxError, + format!("Invalid statement: {}", message), + ) + } + ParseError::SemanticError { message } => { + let range = Range::single_position(Position::new(0, 0)); + Diagnostic::new( + range, + DiagnosticSeverity::Error, + DiagnosticCode::SyntaxError, + format!("Semantic error: {}", message), + ) + } + ParseError::LexError(lex_error) => { + LexicalAnalyzer::lex_error_to_diagnostic(lex_error, _source) + } + ParseError::AnyhowError(anyhow_error) => { + let range = Range::single_position(Position::new(0, 0)); + Diagnostic::new( + range, + DiagnosticSeverity::Error, + DiagnosticCode::SyntaxError, + format!("Parse error: {}", anyhow_error), + ) + } + } + } +} + +/// Style and formatting analysis module +pub struct StyleAnalyzer; + +impl AnalyzerModule for StyleAnalyzer { + fn analyze(&self, source: &str, config: &DiagnosticConfig) -> Vec { + let mut diagnostics = Vec::new(); + + // Check for missing shebang + if !source.starts_with("#!") { + let range = Range::new(Position::new(0, 0), Position::new(0, 0)); + let diagnostic = Diagnostic::new( + range, + DiagnosticSeverity::Warning, + DiagnosticCode::MissingShebang, + "Consider adding a shebang line (e.g., #!/usr/sbin/nft -f)".to_string(), + ); + diagnostics.push(diagnostic); + } + + diagnostics.extend(self.analyze_line_issues(source, config)); + diagnostics.extend(self.analyze_whitespace_issues(source, config)); + diagnostics.extend(self.analyze_indentation(source, config)); + + diagnostics + } + + fn name(&self) -> &'static str { + "style" + } +} + +impl StyleAnalyzer { + fn analyze_line_issues(&self, source: &str, config: &DiagnosticConfig) -> Vec { + let mut diagnostics = Vec::new(); + + for (line_idx, line) in source.lines().enumerate() { + let line_num = line_idx as u32; + + // Long lines + if line.len() > config.max_line_length { + let start = Position::new(line_num, config.max_line_length as u32); + let end = Position::new(line_num, line.len() as u32); + let range = Range::new(start, end); + let diagnostic = Diagnostic::new( + range, + DiagnosticSeverity::Warning, + DiagnosticCode::LongLine, + format!( + "Line too long ({} > {} characters)", + line.len(), + config.max_line_length + ), + ); + diagnostics.push(diagnostic); + } + + // Trailing whitespace + if line.ends_with(' ') || line.ends_with('\t') { + let trimmed_len = line.trim_end().len(); + let start = Position::new(line_num, trimmed_len as u32); + let end = Position::new(line_num, line.len() as u32); + let range = Range::new(start, end); + let diagnostic = Diagnostic::new( + range, + DiagnosticSeverity::Warning, + DiagnosticCode::TrailingWhitespace, + "Trailing whitespace".to_string(), + ); + diagnostics.push(diagnostic); + } + } + + diagnostics + } + + fn analyze_whitespace_issues( + &self, + source: &str, + config: &DiagnosticConfig, + ) -> Vec { + let mut diagnostics = Vec::new(); + let lines: Vec<&str> = source.lines().collect(); + let mut empty_count = 0; + let mut empty_start = 0; + + for (line_idx, line) in lines.iter().enumerate() { + if line.trim().is_empty() { + if empty_count == 0 { + empty_start = line_idx; + } + empty_count += 1; + } else { + if empty_count > config.max_empty_lines { + let start = Position::new(empty_start as u32, 0); + let end = Position::new((empty_start + empty_count - 1) as u32, 0); + let range = Range::new(start, end); + let diagnostic = Diagnostic::new( + range, + DiagnosticSeverity::Warning, + DiagnosticCode::TooManyEmptyLines, + format!( + "Too many consecutive empty lines ({} > {})", + empty_count, config.max_empty_lines + ), + ); + diagnostics.push(diagnostic); + } + empty_count = 0; + } + } + + diagnostics + } + + fn analyze_indentation(&self, source: &str, config: &DiagnosticConfig) -> Vec { + let mut diagnostics = Vec::new(); + let mut has_tabs = false; + let mut has_spaces = false; + + for (line_idx, line) in source.lines().enumerate() { + let line_num = line_idx as u32; + + if line.trim().is_empty() { + continue; + } + + let leading_whitespace: String = line + .chars() + .take_while(|&c| c == ' ' || c == '\t') + .collect(); + + if leading_whitespace.contains('\t') { + has_tabs = true; + } + if leading_whitespace.contains(' ') { + has_spaces = true; + } + + // Check for mixed indentation in a single line + if leading_whitespace.contains('\t') && leading_whitespace.contains(' ') { + let range = Range::new( + Position::new(line_num, 0), + Position::new(line_num, leading_whitespace.len() as u32), + ); + let diagnostic = Diagnostic::new( + range, + DiagnosticSeverity::Warning, + DiagnosticCode::MixedIndentation, + "Mixed tabs and spaces in indentation".to_string(), + ); + diagnostics.push(diagnostic); + } + } + + // Check for mixed indentation across the file + if has_tabs && has_spaces { + if let Some(preferred) = &config.preferred_indent { + let range = Range::single_position(Position::new(0, 0)); + let diagnostic = Diagnostic::new( + range, + DiagnosticSeverity::Information, + DiagnosticCode::InconsistentIndentation, + format!("File uses mixed indentation; prefer {}", preferred), + ); + diagnostics.push(diagnostic); + } + } + + diagnostics + } +} + +/// Semantic analysis module for nftables-specific validation +pub struct SemanticAnalyzer; + +impl AnalyzerModule for SemanticAnalyzer { + fn analyze(&self, source: &str, _config: &DiagnosticConfig) -> Vec { + let mut diagnostics = Vec::new(); + + // Parse and validate nftables-specific constructs + diagnostics.extend(self.validate_table_declarations(source)); + diagnostics.extend(self.validate_chain_declarations(source)); + diagnostics.extend(self.validate_cidr_notation(source)); + diagnostics.extend(self.check_for_redundant_rules(source)); + + diagnostics + } + + fn name(&self) -> &'static str { + "semantic" + } +} + +impl SemanticAnalyzer { + fn validate_table_declarations(&self, source: &str) -> Vec { + let mut diagnostics = Vec::new(); + let mut seen_tables = HashSet::new(); + + for (line_idx, line) in source.lines().enumerate() { + let line_num = line_idx as u32; + let trimmed = line.trim(); + + if trimmed.starts_with("table ") { + let parts: Vec<&str> = trimmed.split_whitespace().collect(); + if parts.len() >= 3 { + let family = parts[1]; + let name = parts[2]; + + // Validate family + match family { + "ip" | "ip6" | "inet" | "arp" | "bridge" | "netdev" => { + // Valid family + } + _ => { + let start_col = line.find(family).unwrap_or(0); + let range = Range::new( + Position::new(line_num, start_col as u32), + Position::new(line_num, (start_col + family.len()) as u32), + ); + let diagnostic = Diagnostic::new( + range, + DiagnosticSeverity::Error, + DiagnosticCode::InvalidTableFamily, + format!("Unknown table family: '{}'", family), + ); + diagnostics.push(diagnostic); + } + } + + // Check for duplicate table names + let table_key = format!("{}:{}", family, name); + if seen_tables.contains(&table_key) { + let start_col = line.find(name).unwrap_or(0); + let range = Range::new( + Position::new(line_num, start_col as u32), + Position::new(line_num, (start_col + name.len()) as u32), + ); + let diagnostic = Diagnostic::new( + range, + DiagnosticSeverity::Error, + DiagnosticCode::DuplicateTableName, + format!("Duplicate table name: '{}'", name), + ); + diagnostics.push(diagnostic); + } else { + seen_tables.insert(table_key); + } + } + } + } + + diagnostics + } + + fn validate_chain_declarations(&self, source: &str) -> Vec { + let mut diagnostics = Vec::new(); + + for (line_idx, line) in source.lines().enumerate() { + let line_num = line_idx as u32; + let trimmed = line.trim(); + + if trimmed.starts_with("type ") && trimmed.contains("hook") { + // Validate chain type and hook + if let Some(hook_pos) = trimmed.find("hook") { + let hook_part = &trimmed[hook_pos..]; + let hook_words: Vec<&str> = hook_part.split_whitespace().collect(); + + if hook_words.len() >= 2 { + let hook = hook_words[1]; + match hook { + "input" | "output" | "forward" | "prerouting" | "postrouting" => { + // Valid hook + } + _ => { + let start_col = line.find(hook).unwrap_or(0); + let range = Range::new( + Position::new(line_num, start_col as u32), + Position::new(line_num, (start_col + hook.len()) as u32), + ); + let diagnostic = Diagnostic::new( + range, + DiagnosticSeverity::Error, + DiagnosticCode::UnknownHook, + format!("Unknown hook: '{}'", hook), + ); + diagnostics.push(diagnostic); + } + } + } + } + + // Check for missing policy in filter chains + 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 validate_cidr_notation(&self, source: &str) -> Vec { + let mut diagnostics = Vec::new(); + + for (line_idx, line) in source.lines().enumerate() { + let line_num = line_idx as u32; + + // Simple CIDR validation without regex dependency + let words: Vec<&str> = line.split_whitespace().collect(); + for word in words { + if word.contains('/') && word.chars().any(|c| c.is_ascii_digit()) { + if let Some(slash_pos) = word.find('/') { + let (ip_part, prefix_part) = word.split_at(slash_pos); + let prefix_part = &prefix_part[1..]; // Remove the '/' + + // Basic IPv4 validation + let ip_parts: Vec<&str> = ip_part.split('.').collect(); + if ip_parts.len() == 4 { + let mut valid_ip = true; + for part in ip_parts { + if part.parse::().is_err() { + valid_ip = false; + break; + } + } + + // Validate prefix length + if valid_ip { + if let Ok(prefix) = prefix_part.parse::() { + if prefix > 32 { + if let Some(start_col) = line.find(word) { + let range = Range::new( + Position::new(line_num, start_col as u32), + Position::new( + line_num, + (start_col + word.len()) as u32, + ), + ); + let diagnostic = Diagnostic::new( + range, + DiagnosticSeverity::Error, + DiagnosticCode::InvalidCidrNotation, + format!( + "Invalid CIDR prefix length: '{}' (max 32 for IPv4)", + prefix + ), + ); + diagnostics.push(diagnostic); + } + } + } else if !prefix_part.is_empty() { + if let Some(start_col) = line.find(word) { + let range = Range::new( + Position::new(line_num, start_col as u32), + Position::new( + line_num, + (start_col + word.len()) as u32, + ), + ); + let diagnostic = Diagnostic::new( + range, + DiagnosticSeverity::Error, + DiagnosticCode::InvalidCidrNotation, + format!("Invalid CIDR notation: '{}'", word), + ); + diagnostics.push(diagnostic); + } + } + } + } + } + } + } + } + + diagnostics + } + + fn check_for_redundant_rules(&self, source: &str) -> Vec { + let mut diagnostics = Vec::new(); + let mut rules = Vec::new(); + + for (line_idx, line) in source.lines().enumerate() { + let line_num = line_idx as u32; + let trimmed = line.trim(); + + // Simple rule detection (lines that contain actions) + if trimmed.contains(" accept") + || trimmed.contains(" drop") + || trimmed.contains(" reject") + { + for (existing_idx, existing_rule) in rules.iter().enumerate() { + if existing_rule == &trimmed { + 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::RedundantRule, + format!( + "Duplicate rule found (first occurrence at line {})", + existing_idx + 1 + ), + ); + diagnostics.push(diagnostic); + break; + } + } + rules.push(trimmed.to_string()); + } + } + + diagnostics + } +} + +/// Main diagnostic analyzer +pub struct DiagnosticAnalyzer { + config: DiagnosticConfig, +} + +impl DiagnosticAnalyzer { + pub fn new(config: DiagnosticConfig) -> Self { + Self { config } + } + + /// Analyze source code with all available modules + pub fn analyze(&self, source: &str, file_path: &str) -> DiagnosticCollection { + self.analyze_with_modules( + source, + file_path, + &["lexical", "syntax", "style", "semantic"], + ) + } + + /// Analyze source code with specific modules + pub fn analyze_with_modules( + &self, + source: &str, + file_path: &str, + module_names: &[&str], + ) -> DiagnosticCollection { + let mut collection = DiagnosticCollection::new(file_path.to_string(), source.to_string()); + + let modules: Vec> = vec![ + Box::new(LexicalAnalyzer), + Box::new(SyntaxAnalyzer), + Box::new(StyleAnalyzer), + Box::new(SemanticAnalyzer), + ]; + + for module in modules { + if module_names.contains(&module.name()) { + let diagnostics = module.analyze(source, &self.config); + collection.extend(diagnostics); + } + } + + collection + } +} + +impl Default for DiagnosticAnalyzer { + fn default() -> Self { + Self::new(DiagnosticConfig::default()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_diagnostic_creation() { + let range = Range::new(Position::new(0, 0), Position::new(0, 10)); + let diagnostic = Diagnostic::new( + range, + DiagnosticSeverity::Error, + DiagnosticCode::SyntaxError, + "Test error".to_string(), + ); + + assert_eq!(diagnostic.severity, DiagnosticSeverity::Error); + assert_eq!(diagnostic.code, DiagnosticCode::SyntaxError); + assert_eq!(diagnostic.message, "Test error"); + } + + #[test] + fn test_position_from_text_size() { + let source = "line 1\nline 2\nline 3"; + let pos = Position::from_text_size(TextSize::from(8), source); + assert_eq!(pos.line, 1); + assert_eq!(pos.character, 1); + } + + #[test] + fn test_style_analysis() { + let analyzer = DiagnosticAnalyzer::default(); + let source = "table inet filter {\n chain input \n chain output\n}"; + let diagnostics = analyzer.analyze(source, "test.nft"); + + // Should find missing shebang and trailing whitespace + assert!(!diagnostics.diagnostics.is_empty()); + assert!( + diagnostics + .diagnostics + .iter() + .any(|d| d.code == DiagnosticCode::MissingShebang) + ); + assert!( + diagnostics + .diagnostics + .iter() + .any(|d| d.code == DiagnosticCode::TrailingWhitespace) + ); + } +} diff --git a/src/main.rs b/src/main.rs index 5cb40c9..7cb3a77 100755 --- a/src/main.rs +++ b/src/main.rs @@ -1,5 +1,6 @@ mod ast; mod cst; +mod diagnostic; mod lexer; mod parser; mod syntax; @@ -12,6 +13,7 @@ use std::path::Path; use thiserror::Error; use crate::cst::CstBuilder; +use crate::diagnostic::{DiagnosticAnalyzer, DiagnosticConfig}; use crate::lexer::NftablesLexer; use crate::parser::Parser as NftablesParser; use crate::syntax::{FormatConfig, IndentStyle, NftablesFormatter}; @@ -28,7 +30,7 @@ enum FormatterError { Io(#[from] io::Error), } -#[derive(Parser, Debug)] +#[derive(Parser, Debug, Clone)] #[command( name = "nff", version = "0.1.0", @@ -63,6 +65,34 @@ struct Args { /// Check syntax only, don't format #[arg(long)] check: bool, + + /// Run diagnostics and show issues (syntax, style, best practices) + #[arg(long)] + diagnostics: bool, + + /// Output diagnostics in JSON format (useful for tooling integration) + #[arg(long)] + json: bool, + + /// Include style warnings in diagnostics + #[arg(long, default_value = "true")] + style_warnings: bool, + + /// Include best practice recommendations in diagnostics + #[arg(long, default_value = "true")] + best_practices: bool, + + /// Include performance hints in diagnostics + #[arg(long, default_value = "true")] + performance_hints: bool, + + /// Include security warnings in diagnostics + #[arg(long, default_value = "true")] + security_warnings: bool, + + /// Diagnostic modules to run (comma-separated: lexical,syntax,style,semantic) + #[arg(long, value_delimiter = ',')] + modules: Option>, } fn process_nftables_config(args: Args) -> Result<()> { @@ -116,7 +146,59 @@ fn process_nftables_config(args: Args) -> Result<()> { eprintln!(); } - // Parse + // Run diagnostics if requested (do this early to catch parse errors) + if args.diagnostics || args.json { + let diagnostic_config = DiagnosticConfig { + enable_style_warnings: args.style_warnings, + enable_best_practices: args.best_practices, + enable_performance_hints: args.performance_hints, + enable_security_warnings: args.security_warnings, + max_line_length: 120, + max_empty_lines: if args.optimize { 1 } else { 2 }, + preferred_indent: Some(match args.indent { + IndentStyle::Tabs => "tabs".to_string(), + IndentStyle::Spaces => "spaces".to_string(), + }), + }; + + let analyzer = DiagnosticAnalyzer::new(diagnostic_config); + + let diagnostics = if let Some(modules) = &args.modules { + let module_names: Vec<&str> = modules.iter().map(|s| s.as_str()).collect(); + analyzer.analyze_with_modules(&source, &args.file, &module_names) + } else { + analyzer.analyze(&source, &args.file) + }; + + if args.json { + // Output JSON format for tooling integration + match diagnostics.to_json() { + Ok(json) => println!("{}", json), + Err(e) => { + if args.json { + // Even JSON serialization errors should be in JSON format when --json is used + let error_json = + format!(r#"{{"error": "JSON serialization failed: {}"}}"#, e); + println!("{}", error_json); + } else { + eprintln!("Error serializing diagnostics to JSON: {}", e); + } + } + } + } else { + // Output human-readable format + println!("{}", diagnostics.to_human_readable()); + } + + // Exit with non-zero code if there are errors + if diagnostics.has_errors() { + std::process::exit(1); + } + + return Ok(()); + } + + // Parse (only if not doing diagnostics) let ruleset = if args.debug { // Use error-recovery parsing for debug mode let (parsed_ruleset, errors) = NftablesParser::parse_with_errors(&source); @@ -177,14 +259,20 @@ fn process_nftables_config(args: Args) -> Result<()> { fn main() -> Result<()> { let args = Args::parse(); - if let Err(e) = process_nftables_config(args) { - eprintln!("Error: {}", e); + if let Err(e) = process_nftables_config(args.clone()) { + if args.json { + // Output error in JSON format when --json flag is used + let error_json = format!(r#"{{"error": "{}"}}"#, e); + println!("{}", error_json); + } else { + eprintln!("Error: {}", e); - // Print the error chain - let mut current = e.source(); - while let Some(cause) = current { - eprintln!(" Caused by: {}", cause); - current = cause.source(); + // Print the error chain + let mut current = e.source(); + while let Some(cause) = current { + eprintln!(" Caused by: {}", cause); + current = cause.source(); + } } std::process::exit(1); -- 2.43.0 From d36120c87bf8fc6b671b92ede2ca06a4bbafec50 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Mon, 2 Jun 2025 05:23:46 +0300 Subject: [PATCH 02/26] diagnostis: fix typo --- src/diagnostic.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/diagnostic.rs b/src/diagnostic.rs index a1a6014..dec4a83 100644 --- a/src/diagnostic.rs +++ b/src/diagnostic.rs @@ -84,7 +84,7 @@ pub enum DiagnosticCode { SecurityRisk, // Performance - IneffientRuleOrder, + InefficientRuleOrder, LargeSetWithoutTimeout, MissingCounters, @@ -143,7 +143,7 @@ impl fmt::Display for DiagnosticCode { DiagnosticCode::MissingDocumentation => "NFT309", DiagnosticCode::SecurityRisk => "NFT310", - DiagnosticCode::IneffientRuleOrder => "NFT401", + DiagnosticCode::InefficientRuleOrder => "NFT401", DiagnosticCode::LargeSetWithoutTimeout => "NFT402", DiagnosticCode::MissingCounters => "NFT403", -- 2.43.0 From 97912966340f5e1b04dcde6da92646fe14d3c790 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Mon, 2 Jun 2025 05:30:26 +0300 Subject: [PATCH 03/26] diagnostics: reduce complexity of redundant rule checks --- src/diagnostic.rs | 37 +++++++++++++++++-------------------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/src/diagnostic.rs b/src/diagnostic.rs index dec4a83..abcc06b 100644 --- a/src/diagnostic.rs +++ b/src/diagnostic.rs @@ -1000,7 +1000,7 @@ impl SemanticAnalyzer { fn check_for_redundant_rules(&self, source: &str) -> Vec { let mut diagnostics = Vec::new(); - let mut rules = Vec::new(); + let mut seen_rules = HashSet::new(); for (line_idx, line) in source.lines().enumerate() { let line_num = line_idx as u32; @@ -1011,26 +1011,23 @@ impl SemanticAnalyzer { || trimmed.contains(" drop") || trimmed.contains(" reject") { - for (existing_idx, existing_rule) in rules.iter().enumerate() { - if existing_rule == &trimmed { - 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::RedundantRule, - format!( - "Duplicate rule found (first occurrence at line {})", - existing_idx + 1 - ), - ); - diagnostics.push(diagnostic); - break; - } + // Check if rule already exists in the HashSet + if seen_rules.contains(trimmed) { + 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::RedundantRule, + "Duplicate rule found".to_string(), + ); + diagnostics.push(diagnostic); + } else { + // Add the rule to the HashSet if it's not a duplicate + seen_rules.insert(trimmed.to_string()); } - rules.push(trimmed.to_string()); } } -- 2.43.0 From 6362ade5bd23db76a9926a96d7a9a0ee8e2930d6 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Mon, 2 Jun 2025 07:56:12 +0300 Subject: [PATCH 04/26] nff: separate format and lint commands --- Cargo.lock | 7 + Cargo.toml | 1 + src/main.rs | 503 ++++++++++++++++++++++++++++++++++++++-------------- 3 files changed, 376 insertions(+), 135 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 898d315..1b11d7f 100755 --- a/Cargo.lock +++ b/Cargo.lock @@ -176,6 +176,12 @@ dependencies = [ "byteorder", ] +[[package]] +name = "glob" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a8d1add55171497b4705a648c6b583acafb01d58050a51727785f0b2c8e0a2b2" + [[package]] name = "hashbrown" version = "0.15.3" @@ -273,6 +279,7 @@ dependencies = [ "anyhow", "clap", "cstree", + "glob", "logos", "regex", "serde", diff --git a/Cargo.toml b/Cargo.toml index 23e80a3..4bb2743 100755 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,3 +15,4 @@ text-size = "1.1" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" regex = "1.11.1" +glob = "0.3" diff --git a/src/main.rs b/src/main.rs index 7cb3a77..91639f7 100755 --- a/src/main.rs +++ b/src/main.rs @@ -6,7 +6,8 @@ mod parser; mod syntax; use anyhow::{Context, Result}; -use clap::Parser; +use clap::{Parser, Subcommand}; +use glob::glob; use std::fs; use std::io::{self, Write}; use std::path::Path; @@ -34,71 +35,176 @@ enum FormatterError { #[command( name = "nff", version = "0.1.0", - about = "A high-quality nftables formatter and beautifier", - long_about = "nff (nftables formatter) is a tool for formatting and beautifying nftables configuration files with proper indentation and structure." + about = "A high-quality nftables formatter and linter", + long_about = "nff (nftables formatter) is a tool for formatting and linting nftables configuration files with proper indentation and structure." )] struct Args { - /// nftables config file (e.g: /etc/nftables.conf) - #[arg(short, long, value_name = "FILE")] - file: String, - - /// Type of indentation - #[arg(short, long, default_value = "tabs", value_parser = clap::value_parser!(IndentStyle))] - indent: IndentStyle, - - /// Output file (writes to stdout if not specified) - #[arg(short, long, value_name = "FILE")] - output: Option, - - /// Optimize output by removing excessive empty lines - #[arg(long)] - optimize: bool, - - /// Number of spaces per indentation level (only used with --indent=spaces) - #[arg(long, default_value = "2", value_name = "N")] - spaces: usize, + #[command(subcommand)] + command: Commands, /// Show debug information (tokens, AST, etc.) - #[arg(long)] + #[arg(long, global = true)] debug: bool, - - /// Check syntax only, don't format - #[arg(long)] - check: bool, - - /// Run diagnostics and show issues (syntax, style, best practices) - #[arg(long)] - diagnostics: bool, - - /// Output diagnostics in JSON format (useful for tooling integration) - #[arg(long)] - json: bool, - - /// Include style warnings in diagnostics - #[arg(long, default_value = "true")] - style_warnings: bool, - - /// Include best practice recommendations in diagnostics - #[arg(long, default_value = "true")] - best_practices: bool, - - /// Include performance hints in diagnostics - #[arg(long, default_value = "true")] - performance_hints: bool, - - /// Include security warnings in diagnostics - #[arg(long, default_value = "true")] - security_warnings: bool, - - /// Diagnostic modules to run (comma-separated: lexical,syntax,style,semantic) - #[arg(long, value_delimiter = ',')] - modules: Option>, } -fn process_nftables_config(args: Args) -> Result<()> { - let path = Path::new(&args.file); +#[derive(Subcommand, Debug, Clone)] +enum Commands { + /// Format nftables configuration files + Format { + /// nftables config file (e.g: /etc/nftables.conf). If not provided, formats all .nft files in current directory + #[arg(value_name = "FILE")] + file: Option, + + /// Type of indentation + #[arg(short, long, default_value = "tabs", value_parser = clap::value_parser!(IndentStyle))] + indent: IndentStyle, + + /// Print formatted output to stdout instead of modifying files in place + #[arg(long)] + stdout: bool, + + /// Optimize output by removing excessive empty lines + #[arg(long)] + optimize: bool, + + /// Number of spaces per indentation level (only used with --indent=spaces) + #[arg(long, default_value = "2", value_name = "N")] + spaces: usize, + + /// Check syntax only, don't format + #[arg(long)] + check: bool, + }, + /// Lint nftables configuration files and show diagnostics + Lint { + /// nftables config file (e.g: /etc/nftables.conf). If not provided, lints all .nft files in current directory + #[arg(value_name = "FILE")] + file: Option, + + /// Output diagnostics in JSON format (useful for tooling integration) + #[arg(long)] + json: bool, + + /// Include style warnings in diagnostics + #[arg(long, default_value = "true")] + style_warnings: bool, + + /// Include best practice recommendations in diagnostics + #[arg(long, default_value = "true")] + best_practices: bool, + + /// Include performance hints in diagnostics + #[arg(long, default_value = "true")] + performance_hints: bool, + + /// Include security warnings in diagnostics + #[arg(long, default_value = "true")] + security_warnings: bool, + + /// Diagnostic modules to run (comma-separated: lexical,syntax,style,semantic) + #[arg(long, value_delimiter = ',')] + modules: Option>, + }, +} + +fn discover_nftables_files() -> Result> { + let mut files = Vec::new(); + + // Common nftables file patterns + let patterns = [ + "*.nft", + "*.nftables", + "/etc/nftables.conf", + "/etc/nftables/*.nft", + ]; + + for pattern in &patterns { + match glob(pattern) { + Ok(paths) => { + for entry in paths { + match entry { + Ok(path) => { + if path.is_file() { + if let Some(path_str) = path.to_str() { + files.push(path_str.to_string()); + } + } + } + Err(e) => eprintln!("Warning: Error reading path: {}", e), + } + } + } + Err(e) => { + // Only warn for non-current directory patterns + if !pattern.starts_with("*.") { + eprintln!("Warning: Failed to search pattern {}: {}", pattern, e); + } + } + } + } + + if files.is_empty() { + return Err(anyhow::anyhow!( + "No nftables files found. Please specify a file explicitly or ensure .nft/.nftables files exist in the current directory." + )); + } + + // Remove duplicates and sort + files.sort(); + files.dedup(); + + Ok(files) +} + +fn process_format_command( + file: Option, + indent: IndentStyle, + stdout: bool, + optimize: bool, + spaces: usize, + check: bool, + debug: bool, +) -> Result<()> { + let files = match file { + Some(f) => vec![f], + None => discover_nftables_files()?, + }; + + let is_multiple_files = files.len() > 1; + for file_path in files { + if let Err(e) = process_single_file_format( + &file_path, + indent, + stdout, + optimize, + spaces, + check, + debug, + is_multiple_files, + ) { + eprintln!("Error processing {}: {}", file_path, e); + if !is_multiple_files { + return Err(e); + } + } + } + + Ok(()) +} + +fn process_single_file_format( + file: &str, + indent: IndentStyle, + stdout: bool, + optimize: bool, + spaces: usize, + check: bool, + debug: bool, + is_multiple_files: bool, +) -> Result<()> { + let path = Path::new(&file); if !path.exists() { - return Err(FormatterError::FileNotFound(args.file).into()); + return Err(FormatterError::FileNotFound(file.to_string()).into()); } if !path.is_file() { @@ -106,12 +212,12 @@ fn process_nftables_config(args: Args) -> Result<()> { } // Read file contents - let source = fs::read_to_string(&args.file) - .with_context(|| format!("Failed to read file: {}", args.file))?; + let source = + fs::read_to_string(&file).with_context(|| format!("Failed to read file: {}", file))?; // Tokenize let mut lexer = NftablesLexer::new(&source); - let tokens = if args.debug { + let tokens = if debug { // Use error-recovery tokenization for debug mode lexer.tokenize_with_errors() } else { @@ -120,7 +226,7 @@ fn process_nftables_config(args: Args) -> Result<()> { .map_err(|e| FormatterError::ParseError(e.to_string()))? }; - if args.debug { + if debug { eprintln!("=== TOKENS ==="); for (i, token) in tokens.iter().enumerate() { eprintln!( @@ -146,60 +252,8 @@ fn process_nftables_config(args: Args) -> Result<()> { eprintln!(); } - // Run diagnostics if requested (do this early to catch parse errors) - if args.diagnostics || args.json { - let diagnostic_config = DiagnosticConfig { - enable_style_warnings: args.style_warnings, - enable_best_practices: args.best_practices, - enable_performance_hints: args.performance_hints, - enable_security_warnings: args.security_warnings, - max_line_length: 120, - max_empty_lines: if args.optimize { 1 } else { 2 }, - preferred_indent: Some(match args.indent { - IndentStyle::Tabs => "tabs".to_string(), - IndentStyle::Spaces => "spaces".to_string(), - }), - }; - - let analyzer = DiagnosticAnalyzer::new(diagnostic_config); - - let diagnostics = if let Some(modules) = &args.modules { - let module_names: Vec<&str> = modules.iter().map(|s| s.as_str()).collect(); - analyzer.analyze_with_modules(&source, &args.file, &module_names) - } else { - analyzer.analyze(&source, &args.file) - }; - - if args.json { - // Output JSON format for tooling integration - match diagnostics.to_json() { - Ok(json) => println!("{}", json), - Err(e) => { - if args.json { - // Even JSON serialization errors should be in JSON format when --json is used - let error_json = - format!(r#"{{"error": "JSON serialization failed: {}"}}"#, e); - println!("{}", error_json); - } else { - eprintln!("Error serializing diagnostics to JSON: {}", e); - } - } - } - } else { - // Output human-readable format - println!("{}", diagnostics.to_human_readable()); - } - - // Exit with non-zero code if there are errors - if diagnostics.has_errors() { - std::process::exit(1); - } - - return Ok(()); - } - - // Parse (only if not doing diagnostics) - let ruleset = if args.debug { + // Parse + let ruleset = if debug { // Use error-recovery parsing for debug mode let (parsed_ruleset, errors) = NftablesParser::parse_with_errors(&source); if !errors.is_empty() { @@ -217,51 +271,230 @@ fn process_nftables_config(args: Args) -> Result<()> { .map_err(|e| FormatterError::ParseError(e.to_string()))? }; - if args.debug { + if debug { eprintln!("=== AST ==="); eprintln!("{:#?}", ruleset); eprintln!(); } - if args.check { - println!("Syntax check passed for: {}", args.file); + if check { + println!("Syntax check passed for: {}", file); return Ok(()); } // Format let config = FormatConfig { - indent_style: args.indent, - spaces_per_level: args.spaces, - optimize: args.optimize, - max_empty_lines: if args.optimize { 1 } else { 2 }, + indent_style: indent, + spaces_per_level: spaces, + optimize, + max_empty_lines: if optimize { 1 } else { 2 }, }; let formatter = NftablesFormatter::new(config); let formatted_output = formatter.format_ruleset(&ruleset); // Write output - match &args.output { - Some(output_file) => { - fs::write(output_file, &formatted_output) - .with_context(|| format!("Failed to write to output file: {}", output_file))?; - println!("Formatted output written to: {}", output_file); + if stdout { + // Output to stdout + if is_multiple_files { + println!("=== {} ===", file); } - None => { - io::stdout() - .write_all(formatted_output.as_bytes()) - .with_context(|| "Failed to write to stdout")?; + io::stdout() + .write_all(formatted_output.as_bytes()) + .with_context(|| "Failed to write to stdout")?; + } else { + // Format in place + fs::write(file, &formatted_output) + .with_context(|| format!("Failed to write formatted content back to: {}", file))?; + if is_multiple_files || debug { + println!("Formatted: {}", file); } } Ok(()) } +fn process_lint_command( + file: Option, + json: bool, + style_warnings: bool, + best_practices: bool, + performance_hints: bool, + security_warnings: bool, + modules: Option>, + debug: bool, +) -> Result<()> { + let files = match file { + Some(f) => vec![f], + None => discover_nftables_files()?, + }; + + let is_multiple_files = files.len() > 1; + for file_path in files { + if let Err(e) = process_single_file_lint( + &file_path, + json, + style_warnings, + best_practices, + performance_hints, + security_warnings, + modules.as_ref(), + debug, + is_multiple_files, + ) { + eprintln!("Error processing {}: {}", file_path, e); + if !is_multiple_files { + return Err(e); + } + } + } + + Ok(()) +} + +fn process_single_file_lint( + file: &str, + json: bool, + style_warnings: bool, + best_practices: bool, + performance_hints: bool, + security_warnings: bool, + modules: Option<&Vec>, + debug: bool, + is_multiple_files: bool, +) -> Result<()> { + let path = Path::new(&file); + if !path.exists() { + return Err(FormatterError::FileNotFound(file.to_string()).into()); + } + + if !path.is_file() { + return Err(FormatterError::InvalidFile("Not a regular file".to_string()).into()); + } + + // Read file contents + let source = + fs::read_to_string(&file).with_context(|| format!("Failed to read file: {}", file))?; + + if debug { + // Tokenize for debug output + let mut lexer = NftablesLexer::new(&source); + let tokens = lexer.tokenize_with_errors(); + + eprintln!("=== TOKENS ==="); + for (i, token) in tokens.iter().enumerate() { + eprintln!( + "{:3}: {:?} @ {:?} = '{}'", + i, token.kind, token.range, token.text + ); + } + eprintln!(); + + // Build and validate CST + eprintln!("=== CST ==="); + let cst_tree = CstBuilder::build_tree(&tokens); + match CstBuilder::validate_tree(&cst_tree) { + Ok(()) => eprintln!("CST validation passed"), + Err(e) => eprintln!("CST validation error: {}", e), + } + eprintln!(); + } + + // Run diagnostics + let diagnostic_config = DiagnosticConfig { + enable_style_warnings: style_warnings, + enable_best_practices: best_practices, + enable_performance_hints: performance_hints, + enable_security_warnings: security_warnings, + max_line_length: 120, + max_empty_lines: 2, + preferred_indent: None, // Don't enforce indent style in lint mode + }; + + let analyzer = DiagnosticAnalyzer::new(diagnostic_config); + + let diagnostics = if let Some(modules) = &modules { + let module_names: Vec<&str> = modules.iter().map(|s| s.as_str()).collect(); + analyzer.analyze_with_modules(&source, &file, &module_names) + } else { + analyzer.analyze(&source, &file) + }; + + if json { + // Output JSON format for tooling integration + match diagnostics.to_json() { + Ok(json) => println!("{}", json), + Err(e) => { + // Even JSON serialization errors should be in JSON format when --json is used + let error_json = format!(r#"{{"error": "JSON serialization failed: {}"}}"#, e); + println!("{}", error_json); + } + } + } else { + // Output human-readable format + if is_multiple_files { + println!("=== {} ===", file); + } + println!("{}", diagnostics.to_human_readable()); + } + + // Exit with non-zero code if there are errors + if diagnostics.has_errors() { + std::process::exit(1); + } + + Ok(()) +} + fn main() -> Result<()> { let args = Args::parse(); - if let Err(e) = process_nftables_config(args.clone()) { - if args.json { - // Output error in JSON format when --json flag is used + let result = match &args.command { + Commands::Format { + file, + indent, + stdout, + optimize, + spaces, + check, + } => process_format_command( + file.clone(), + *indent, + *stdout, + *optimize, + *spaces, + *check, + args.debug, + ), + Commands::Lint { + file, + json, + style_warnings, + best_practices, + performance_hints, + security_warnings, + modules, + } => process_lint_command( + file.clone(), + *json, + *style_warnings, + *best_practices, + *performance_hints, + *security_warnings, + modules.clone(), + args.debug, + ), + }; + + if let Err(e) = result { + // Check if we're in lint mode with JSON output for error formatting + let use_json = match &args.command { + Commands::Lint { json, .. } => *json, + _ => false, + }; + + if use_json { + // Output error in JSON format when --json flag is used in lint mode let error_json = format!(r#"{{"error": "{}"}}"#, e); println!("{}", error_json); } else { -- 2.43.0 From c4a71f2e85250166f456ffbd6da8f990325920e8 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Mon, 2 Jun 2025 08:59:45 +0300 Subject: [PATCH 05/26] nff: add vmap expression support and improve diagnostics --- src/ast.rs | 6 + src/cst.rs | 23 ++++ src/lexer.rs | 22 ++++ src/main.rs | 296 +++++++++++++++++++++++++++++++++++++++++++++++++- src/parser.rs | 82 +++++++++++++- src/syntax.rs | 14 +++ 6 files changed, 440 insertions(+), 3 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index e1a3623..7eeece2 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -125,6 +125,12 @@ pub enum Expression { // Set expressions Set(Vec), + // Vmap expressions (value maps) + Vmap { + expr: Box, + map: Vec<(Expression, Expression)>, + }, + // Range expressions Range { start: Box, diff --git a/src/cst.rs b/src/cst.rs index 9dcd962..4b5f7e4 100644 --- a/src/cst.rs +++ b/src/cst.rs @@ -114,6 +114,18 @@ pub enum SyntaxKind { NewKw, InvalidKw, + // Additional protocol keywords + VmapKw, + NdRouterAdvertKw, + NdNeighborSolicitKw, + NdNeighborAdvertKw, + EchoRequestKw, + DestUnreachableKw, + RouterAdvertisementKw, + TimeExceededKw, + ParameterProblemKw, + PacketTooBigKw, + // Operators EqOp, NeOp, @@ -215,6 +227,17 @@ impl From for SyntaxKind { TokenKind::New => SyntaxKind::NewKw, TokenKind::Invalid => SyntaxKind::InvalidKw, + TokenKind::Vmap => SyntaxKind::VmapKw, + TokenKind::NdRouterAdvert => SyntaxKind::NdRouterAdvertKw, + TokenKind::NdNeighborSolicit => SyntaxKind::NdNeighborSolicitKw, + TokenKind::NdNeighborAdvert => SyntaxKind::NdNeighborAdvertKw, + TokenKind::EchoRequest => SyntaxKind::EchoRequestKw, + TokenKind::DestUnreachable => SyntaxKind::DestUnreachableKw, + TokenKind::RouterAdvertisement => SyntaxKind::RouterAdvertisementKw, + TokenKind::TimeExceeded => SyntaxKind::TimeExceededKw, + TokenKind::ParameterProblem => SyntaxKind::ParameterProblemKw, + TokenKind::PacketTooBig => SyntaxKind::PacketTooBigKw, + TokenKind::Eq => SyntaxKind::EqOp, TokenKind::Ne => SyntaxKind::NeOp, TokenKind::Le => SyntaxKind::LeOp, diff --git a/src/lexer.rs b/src/lexer.rs index e28cdae..4cd0505 100644 --- a/src/lexer.rs +++ b/src/lexer.rs @@ -129,6 +129,28 @@ pub enum TokenKind { #[token("new")] New, + // Additional protocol keywords + #[token("vmap")] + Vmap, + #[token("nd-router-advert")] + NdRouterAdvert, + #[token("nd-neighbor-solicit")] + NdNeighborSolicit, + #[token("nd-neighbor-advert")] + NdNeighborAdvert, + #[token("echo-request")] + EchoRequest, + #[token("destination-unreachable")] + DestUnreachable, + #[token("router-advertisement")] + RouterAdvertisement, + #[token("time-exceeded")] + TimeExceeded, + #[token("parameter-problem")] + ParameterProblem, + #[token("packet-too-big")] + PacketTooBig, + // Actions #[token("accept")] Accept, diff --git a/src/main.rs b/src/main.rs index 91639f7..fd2bca4 100755 --- a/src/main.rs +++ b/src/main.rs @@ -15,7 +15,7 @@ use thiserror::Error; use crate::cst::CstBuilder; use crate::diagnostic::{DiagnosticAnalyzer, DiagnosticConfig}; -use crate::lexer::NftablesLexer; +use crate::lexer::{NftablesLexer, Token, TokenKind}; use crate::parser::Parser as NftablesParser; use crate::syntax::{FormatConfig, IndentStyle, NftablesFormatter}; @@ -27,6 +27,27 @@ enum FormatterError { InvalidFile(String), #[error("Parse error: {0}")] ParseError(String), + #[error("Syntax error at line {line}, column {column}: {message}")] + SyntaxError { + line: usize, + column: usize, + message: String, + suggestion: Option, + }, + #[error("Unsupported nftables syntax at line {line}, column {column}: {feature}")] + UnsupportedSyntax { + line: usize, + column: usize, + feature: String, + suggestion: Option, + }, + #[error("Invalid nftables syntax at line {line}, column {column}: {message}")] + InvalidSyntax { + line: usize, + column: usize, + message: String, + suggestion: Option, + }, #[error("IO error: {0}")] Io(#[from] io::Error), } @@ -268,7 +289,7 @@ fn process_single_file_format( let mut parser = NftablesParser::new(tokens.clone()); parser .parse() - .map_err(|e| FormatterError::ParseError(e.to_string()))? + .map_err(|e| analyze_parse_error(&source, &tokens, &e.to_string()))? }; if debug { @@ -446,6 +467,277 @@ fn process_single_file_lint( Ok(()) } +/// Intelligent error analysis to categorize parse errors and provide location information +fn analyze_parse_error(source: &str, tokens: &[Token], error: &str) -> FormatterError { + // Convert line/column position from token ranges + let lines: Vec<&str> = source.lines().collect(); + + // Look for common error patterns and provide specific messages + if error.contains("unexpected token") || error.contains("expected") { + // Try to find the problematic token + if let Some(error_token) = find_error_token(tokens) { + let (line, column) = position_from_range(&error_token.range, source); + + // Analyze the specific token to categorize the error + match categorize_syntax_error(&error_token, source, &lines) { + ErrorCategory::UnsupportedSyntax { + feature, + suggestion, + } => FormatterError::UnsupportedSyntax { + line, + column, + feature, + suggestion, + }, + ErrorCategory::InvalidSyntax { + message, + suggestion, + } => FormatterError::InvalidSyntax { + line, + column, + message, + suggestion, + }, + ErrorCategory::SyntaxError { + message, + suggestion, + } => FormatterError::SyntaxError { + line, + column, + message, + suggestion, + }, + } + } else { + // Fallback to generic parse error + FormatterError::ParseError(error.to_string()) + } + } else { + FormatterError::ParseError(error.to_string()) + } +} + +#[derive(Debug)] +enum ErrorCategory { + UnsupportedSyntax { + feature: String, + suggestion: Option, + }, + InvalidSyntax { + message: String, + suggestion: Option, + }, + SyntaxError { + message: String, + suggestion: Option, + }, +} + +/// Find the first error token in the token stream +fn find_error_token(tokens: &[Token]) -> Option<&Token> { + tokens + .iter() + .find(|token| matches!(token.kind, TokenKind::Error)) +} + +/// Convert TextRange to line/column position +fn position_from_range(range: &text_size::TextRange, source: &str) -> (usize, usize) { + let start_offset: usize = range.start().into(); + let lines: Vec<&str> = source.lines().collect(); + let mut current_offset = 0; + + for (line_idx, line) in lines.iter().enumerate() { + let line_end = current_offset + line.len(); + if start_offset <= line_end { + let column = start_offset - current_offset; + return (line_idx + 1, column + 1); // 1-based indexing + } + current_offset = line_end + 1; // +1 for newline + } + + (1, 1) // fallback +} + +/// Categorize syntax errors based on token content and context +fn categorize_syntax_error(token: &Token, source: &str, lines: &[&str]) -> ErrorCategory { + let token_text = &token.text; + let (line_num, _) = position_from_range(&token.range, source); + let line_content = lines.get(line_num.saturating_sub(1)).unwrap_or(&""); + + // Check for unsupported nftables features + if is_unsupported_feature(token_text, line_content) { + let (feature, suggestion) = classify_unsupported_feature(token_text, line_content); + return ErrorCategory::UnsupportedSyntax { + feature, + suggestion, + }; + } + + // Check for invalid but supported syntax + if is_invalid_syntax(token_text, line_content) { + let (message, suggestion) = classify_invalid_syntax(token_text, line_content); + return ErrorCategory::InvalidSyntax { + message, + suggestion, + }; + } + + // Default to syntax error + ErrorCategory::SyntaxError { + message: format!("Unexpected token '{}'", token_text), + suggestion: suggest_correction(token_text, line_content), + } +} + +/// Check if the token represents an unsupported nftables feature +fn is_unsupported_feature(token_text: &str, line_content: &str) -> bool { + // List of advanced nftables features that might not be fully supported yet + let unsupported_keywords = [ + "quota", "limit", "counter", "meter", "socket", "fib", "rt", "ipsec", "tunnel", "comp", + "dccp", "sctp", "gre", "esp", "ah", "vlan", "arp", "rateest", "osf", "netdev", "meta", + "exthdr", "payload", "lookup", "dynset", "flow", "hash", "jhash", "symhash", "crc32", + ]; + + unsupported_keywords + .iter() + .any(|&keyword| token_text.contains(keyword) || line_content.contains(keyword)) +} + +/// Check if the syntax is invalid (malformed but within supported features) +fn is_invalid_syntax(token_text: &str, line_content: &str) -> bool { + // Check for common syntax mistakes + if token_text.contains("..") || token_text.contains("::") { + return true; // Double operators usually indicate mistakes + } + + // Check for malformed addresses or ranges + if token_text.contains("/") && !is_valid_cidr(token_text) { + return true; + } + + // Check for malformed brackets/braces + let open_braces = line_content.matches('{').count(); + let close_braces = line_content.matches('}').count(); + if open_braces != close_braces { + return true; + } + + false +} + +/// Classify unsupported feature and provide suggestion +fn classify_unsupported_feature(token_text: &str, line_content: &str) -> (String, Option) { + let feature = if token_text.contains("quota") { + ( + "quota management".to_string(), + Some("Use explicit rule counting instead".to_string()), + ) + } else if token_text.contains("limit") { + ( + "rate limiting".to_string(), + Some("Consider using simpler rule-based rate limiting".to_string()), + ) + } else if token_text.contains("counter") { + ( + "packet counters".to_string(), + Some("Use rule-level statistics instead".to_string()), + ) + } else if line_content.contains("meta") { + ( + "meta expressions".to_string(), + Some("Use explicit protocol matching instead".to_string()), + ) + } else { + (format!("advanced feature '{}'", token_text), None) + }; + + feature +} + +/// Classify invalid syntax and provide suggestion +fn classify_invalid_syntax(token_text: &str, line_content: &str) -> (String, Option) { + if token_text.contains("/") && !is_valid_cidr(token_text) { + return ( + "Invalid CIDR notation".to_string(), + Some("Use format like '192.168.1.0/24' or '::1/128'".to_string()), + ); + } + + if token_text.contains("..") { + return ( + "Invalid range operator".to_string(), + Some("Use '-' for ranges like '1000-2000'".to_string()), + ); + } + + if line_content.contains('{') && !line_content.contains('}') { + return ( + "Unmatched opening brace".to_string(), + Some("Ensure all '{' have matching '}'".to_string()), + ); + } + + ( + format!("Malformed token '{}'", token_text), + Some("Check nftables syntax documentation".to_string()), + ) +} + +/// Suggest correction for common typos +fn suggest_correction(token_text: &str, line_content: &str) -> Option { + // Common typos and their corrections + let corrections = [ + ("tabel", "table"), + ("cahin", "chain"), + ("accpet", "accept"), + ("rejct", "reject"), + ("prtocol", "protocol"), + ("addres", "address"), + ("pririty", "priority"), + ("poicy", "policy"), + ]; + + for (typo, correction) in &corrections { + if token_text.contains(typo) { + return Some(format!("Did you mean '{}'?", correction)); + } + } + + // Context-based suggestions + if line_content.contains("type") && line_content.contains("hook") { + if !line_content.contains("filter") + && !line_content.contains("nat") + && !line_content.contains("route") + { + return Some("Chain type should be 'filter', 'nat', or 'route'".to_string()); + } + } + + None +} + +/// Validate CIDR notation +fn is_valid_cidr(text: &str) -> bool { + if let Some(slash_pos) = text.find('/') { + let (addr, prefix) = text.split_at(slash_pos); + let prefix = &prefix[1..]; // Remove the '/' + + // Check if prefix is a valid number + if let Ok(prefix_len) = prefix.parse::() { + // Basic validation - IPv4 should be <= 32, IPv6 <= 128 + if addr.contains(':') { + prefix_len <= 128 // IPv6 + } else { + prefix_len <= 32 // IPv4 + } + } else { + false + } + } else { + false + } +} + fn main() -> Result<()> { let args = Args::parse(); diff --git a/src/parser.rs b/src/parser.rs index 8db1593..d250d4d 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -465,7 +465,9 @@ impl Parser { fn parse_comparison_expression(&mut self) -> Result { let mut expr = self.parse_range_expression()?; + // Check for operators while let Some(token) = self.peek() { + // Check for comparison operators let operator = match &token.kind { TokenKind::Eq => BinaryOperator::Eq, TokenKind::Ne => BinaryOperator::Ne, @@ -473,7 +475,48 @@ impl Parser { TokenKind::Le => BinaryOperator::Le, TokenKind::Gt => BinaryOperator::Gt, TokenKind::Ge => BinaryOperator::Ge, - _ => break, + _ => { + // Check for vmap after an expression + if matches!(&token.kind, TokenKind::Vmap) { + self.advance(); // consume 'vmap' + + // Parse the map contents + self.consume(TokenKind::LeftBrace, "Expected '{' after vmap")?; + + let mut map = Vec::new(); + + while !self.current_token_is(&TokenKind::RightBrace) && !self.is_at_end() { + // Skip commas and newlines + if self.current_token_is(&TokenKind::Comma) + || self.current_token_is(&TokenKind::Newline) + { + self.advance(); + continue; + } + + // Parse key + let key = self.parse_expression()?; + + // Parse colon separator + self.consume(TokenKind::Colon, "Expected ':' in vmap key-value pair")?; + + // Parse value + let value = self.parse_expression()?; + + // Add the key-value pair to the map + map.push((key, value)); + } + + self.consume(TokenKind::RightBrace, "Expected '}' to close vmap")?; + + // Return a vmap expression with the previous expression as the mapping target + return Ok(Expression::Vmap { + expr: Box::new(expr), + map, + }); + } + break; + } }; self.advance(); // consume operator @@ -753,6 +796,43 @@ impl Parser { let addr = self.advance().unwrap().text.clone(); Ok(Expression::MacAddress(addr)) } + Some(TokenKind::Vmap) => { + self.advance(); // consume 'vmap' + + // Parse the map contents + self.consume(TokenKind::LeftBrace, "Expected '{' after vmap")?; + + let mut map = Vec::new(); + + while !self.current_token_is(&TokenKind::RightBrace) && !self.is_at_end() { + // Skip commas and newlines + if self.current_token_is(&TokenKind::Comma) + || self.current_token_is(&TokenKind::Newline) + { + self.advance(); + continue; + } + + // Parse key + let key = self.parse_expression()?; + + // Parse colon separator + self.consume(TokenKind::Colon, "Expected ':' in vmap key-value pair")?; + + // Parse value + let value = self.parse_expression()?; + + // Add the key-value pair to the map + map.push((key, value)); + } + + self.consume(TokenKind::RightBrace, "Expected '}' to close vmap")?; + + // The expression that came before "vmap" is the expr being mapped + let expr = Box::new(Expression::Identifier("dummy".to_string())); // This will be replaced in post-processing + + Ok(Expression::Vmap { expr, map }) + } Some(TokenKind::LeftBrace) => { self.advance(); // consume '{' let mut elements = Vec::new(); diff --git a/src/syntax.rs b/src/syntax.rs index 2c498d0..37bcb65 100644 --- a/src/syntax.rs +++ b/src/syntax.rs @@ -292,6 +292,20 @@ impl NftablesFormatter { output.push('-'); self.format_expression(output, end); } + + Expression::Vmap { expr, map } => { + self.format_expression(output, expr); + output.push_str(" vmap { "); + for (i, (key, value)) in map.iter().enumerate() { + if i > 0 { + output.push_str(", "); + } + self.format_expression(output, key); + output.push_str(" : "); + self.format_expression(output, value); + } + output.push_str(" }"); + } } } } -- 2.43.0 From e0f93d03072e15345c9eb068e90d0b1dad8e54be Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Mon, 2 Jun 2025 09:31:44 +0300 Subject: [PATCH 06/26] nff: improve diagnostics and vmap expression handling in parser and formatter --- src/ast.rs | 2 +- src/parser.rs | 26 +++++++++++++++++++++----- src/syntax.rs | 12 ++++++++---- 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index 7eeece2..9bc53ae 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -127,7 +127,7 @@ pub enum Expression { // Vmap expressions (value maps) Vmap { - expr: Box, + expr: Option>, map: Vec<(Expression, Expression)>, }, diff --git a/src/parser.rs b/src/parser.rs index d250d4d..d28fffc 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -311,9 +311,8 @@ impl Parser { self.advance(); // consume 'policy' let policy = self.parse_policy()?; chain = chain.with_policy(policy); + self.consume(TokenKind::Semicolon, "Expected ';' after policy")?; } - - self.consume(TokenKind::Semicolon, "Expected ';' after policy")?; } Some(TokenKind::CommentLine(_)) => { self.advance(); @@ -511,7 +510,7 @@ impl Parser { // Return a vmap expression with the previous expression as the mapping target return Ok(Expression::Vmap { - expr: Box::new(expr), + expr: Some(Box::new(expr)), map, }); } @@ -828,8 +827,8 @@ impl Parser { self.consume(TokenKind::RightBrace, "Expected '}' to close vmap")?; - // The expression that came before "vmap" is the expr being mapped - let expr = Box::new(Expression::Identifier("dummy".to_string())); // This will be replaced in post-processing + // No expression available at parse time, will be filled by post-processing if needed + let expr = None; Ok(Expression::Vmap { expr, map }) } @@ -854,6 +853,23 @@ impl Parser { self.consume(TokenKind::RightBrace, "Expected '}' to close set")?; Ok(Expression::Set(elements)) } + Some(TokenKind::Accept) => { + self.advance(); + Ok(Expression::Identifier("accept".to_string())) + } + Some(TokenKind::Drop) => { + self.advance(); + Ok(Expression::Identifier("drop".to_string())) + } + Some(TokenKind::Reject) => { + self.advance(); + Ok(Expression::Identifier("reject".to_string())) + } + Some(TokenKind::Protocol) => { + self.advance(); // consume 'protocol' + let protocol = self.parse_identifier_or_keyword()?; + Ok(Expression::Protocol(protocol)) + } _ => Err(ParseError::InvalidExpression { message: format!( "Unexpected token in expression: {}", diff --git a/src/syntax.rs b/src/syntax.rs index 37bcb65..1b57ab3 100644 --- a/src/syntax.rs +++ b/src/syntax.rs @@ -175,10 +175,11 @@ impl NftablesFormatter { // Add policy on the same line if present if let Some(policy) = &chain.policy { write!(output, " policy {}", policy).unwrap(); + output.push_str(";\n"); + } else { + output.push_str("\n"); } - output.push_str(";\n"); - if !chain.rules.is_empty() && !self.config.optimize { output.push('\n'); } @@ -294,8 +295,11 @@ impl NftablesFormatter { } Expression::Vmap { expr, map } => { - self.format_expression(output, expr); - output.push_str(" vmap { "); + if let Some(expr) = expr { + self.format_expression(output, expr); + output.push(' '); + } + output.push_str("vmap { "); for (i, (key, value)) in map.iter().enumerate() { if i > 0 { output.push_str(", "); -- 2.43.0 From 6f08d27a59dc5441779f6b47c09ffeb57c4660b0 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Mon, 2 Jun 2025 09:40:03 +0300 Subject: [PATCH 07/26] cst: assign explicit values to SyntaxKind variants --- src/cst.rs | 236 ++++++++++++++++++++++++++--------------------------- 1 file changed, 118 insertions(+), 118 deletions(-) diff --git a/src/cst.rs b/src/cst.rs index 4b5f7e4..4d76340 100644 --- a/src/cst.rs +++ b/src/cst.rs @@ -13,150 +13,150 @@ use thiserror::Error; pub enum SyntaxKind { // Root and containers Root = 0, - Table, - Chain, - Rule, - Set, - Map, - Element, + Table = 1, + Chain = 2, + Rule = 3, + Set = 4, + Map = 5, + Element = 6, // Expressions - Expression, - BinaryExpr, - UnaryExpr, - CallExpr, - SetExpr, - RangeExpr, + Expression = 7, + BinaryExpr = 8, + UnaryExpr = 9, + CallExpr = 10, + SetExpr = 11, + RangeExpr = 12, // Statements - Statement, - IncludeStmt, - DefineStmt, - FlushStmt, - AddStmt, - DeleteStmt, + Statement = 13, + IncludeStmt = 14, + DefineStmt = 15, + FlushStmt = 16, + AddStmt = 17, + DeleteStmt = 18, // Literals and identifiers - Identifier, - StringLiteral, - NumberLiteral, - IpAddress, - Ipv6Address, - MacAddress, + Identifier = 19, + StringLiteral = 20, + NumberLiteral = 21, + IpAddress = 22, + Ipv6Address = 23, + MacAddress = 24, // Keywords - TableKw, - ChainKw, - RuleKw, - SetKw, - MapKw, - ElementKw, - IncludeKw, - DefineKw, - FlushKw, - AddKw, - DeleteKw, - InsertKw, - ReplaceKw, + TableKw = 25, + ChainKw = 26, + RuleKw = 27, + SetKw = 28, + MapKw = 29, + ElementKw = 30, + IncludeKw = 31, + DefineKw = 32, + FlushKw = 33, + AddKw = 34, + DeleteKw = 35, + InsertKw = 36, + ReplaceKw = 37, // Chain types and hooks - FilterKw, - NatKw, - RouteKw, - InputKw, - OutputKw, - ForwardKw, - PreroutingKw, - PostroutingKw, + FilterKw = 38, + NatKw = 39, + RouteKw = 40, + InputKw = 41, + OutputKw = 42, + ForwardKw = 43, + PreroutingKw = 44, + PostroutingKw = 45, // Protocols and families - IpKw, - Ip6Kw, - InetKw, - ArpKw, - BridgeKw, - NetdevKw, - TcpKw, - UdpKw, - IcmpKw, - Icmpv6Kw, + IpKw = 46, + Ip6Kw = 47, + InetKw = 48, + ArpKw = 49, + BridgeKw = 50, + NetdevKw = 51, + TcpKw = 52, + UdpKw = 53, + IcmpKw = 54, + Icmpv6Kw = 55, // Match keywords - SportKw, - DportKw, - SaddrKw, - DaddrKw, - ProtocolKw, - NexthdrKw, - TypeKw, - HookKw, - PriorityKw, - PolicyKw, - IifnameKw, - OifnameKw, - CtKw, - StateKw, + SportKw = 56, + DportKw = 57, + SaddrKw = 58, + DaddrKw = 59, + ProtocolKw = 60, + NexthdrKw = 61, + TypeKw = 62, + HookKw = 63, + PriorityKw = 64, + PolicyKw = 65, + IifnameKw = 66, + OifnameKw = 67, + CtKw = 68, + StateKw = 69, // Actions - AcceptKw, - DropKw, - RejectKw, - ReturnKw, - JumpKw, - GotoKw, - ContinueKw, - LogKw, - CommentKw, + AcceptKw = 70, + DropKw = 71, + RejectKw = 72, + ReturnKw = 73, + JumpKw = 74, + GotoKw = 75, + ContinueKw = 76, + LogKw = 77, + CommentKw = 78, // States - EstablishedKw, - RelatedKw, - NewKw, - InvalidKw, - - // Additional protocol keywords - VmapKw, - NdRouterAdvertKw, - NdNeighborSolicitKw, - NdNeighborAdvertKw, - EchoRequestKw, - DestUnreachableKw, - RouterAdvertisementKw, - TimeExceededKw, - ParameterProblemKw, - PacketTooBigKw, + EstablishedKw = 79, + RelatedKw = 80, + NewKw = 81, + InvalidKw = 82, // Operators - EqOp, - NeOp, - LeOp, - GeOp, - LtOp, - GtOp, + EqOp = 83, + NeOp = 84, + LeOp = 85, + GeOp = 86, + LtOp = 87, + GtOp = 88, // Punctuation - LeftBrace, - RightBrace, - LeftParen, - RightParen, - LeftBracket, - RightBracket, - Comma, - Semicolon, - Colon, - Assign, - Dash, - Slash, - Dot, + LeftBrace = 89, + RightBrace = 90, + LeftParen = 91, + RightParen = 92, + LeftBracket = 93, + RightBracket = 94, + Comma = 95, + Semicolon = 96, + Colon = 97, + Assign = 98, + Dash = 99, + Slash = 100, + Dot = 101, // Trivia - Whitespace, - Newline, - Comment, - Shebang, + Whitespace = 102, + Newline = 103, + Comment = 104, + Shebang = 105, // Error recovery - Error, + Error = 106, + + // Additional protocol keywords + VmapKw = 107, + NdRouterAdvertKw = 108, + NdNeighborSolicitKw = 109, + NdNeighborAdvertKw = 110, + EchoRequestKw = 111, + DestUnreachableKw = 112, + RouterAdvertisementKw = 113, + TimeExceededKw = 114, + ParameterProblemKw = 115, + PacketTooBigKw = 116, } impl From for SyntaxKind { -- 2.43.0 From 16586e99278a41a7ef0a6b3840bc1ce8c6e8b28e Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Mon, 2 Jun 2025 09:41:08 +0300 Subject: [PATCH 08/26] parser: improve handling of vmap exprs to allow further comparisons --- src/parser.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/parser.rs b/src/parser.rs index d28fffc..b10ae3d 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -509,10 +509,11 @@ impl Parser { self.consume(TokenKind::RightBrace, "Expected '}' to close vmap")?; // Return a vmap expression with the previous expression as the mapping target - return Ok(Expression::Vmap { + expr = Expression::Vmap { expr: Some(Box::new(expr)), map, - }); + }; + continue; // allow the outer `while` to detect ==, != … afterwards } break; } -- 2.43.0 From 0ed74c6a51618363629638d38fab49d10ee78b32 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Mon, 2 Jun 2025 09:48:23 +0300 Subject: [PATCH 09/26] nff: handle errors and exit appropriately in linter --- src/main.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/main.rs b/src/main.rs index fd2bca4..88f6116 100755 --- a/src/main.rs +++ b/src/main.rs @@ -351,6 +351,8 @@ fn process_lint_command( }; let is_multiple_files = files.len() > 1; + let mut has_errors = false; + for file_path in files { if let Err(e) = process_single_file_lint( &file_path, @@ -364,12 +366,18 @@ fn process_lint_command( is_multiple_files, ) { eprintln!("Error processing {}: {}", file_path, e); + has_errors = true; if !is_multiple_files { return Err(e); } } } + // Exit with non-zero code if any file had errors + if has_errors { + std::process::exit(1); + } + Ok(()) } @@ -459,9 +467,9 @@ fn process_single_file_lint( println!("{}", diagnostics.to_human_readable()); } - // Exit with non-zero code if there are errors + // Return error if there are diagnostics errors if diagnostics.has_errors() { - std::process::exit(1); + return Err(anyhow::anyhow!("Diagnostics found errors in file")); } Ok(()) -- 2.43.0 From fc8e42c096337591173f46996ae651d55c2174d4 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Mon, 2 Jun 2025 09:53:08 +0300 Subject: [PATCH 10/26] diagnostic: handle mixed indentation diagnostics with severity levels --- src/diagnostic.rs | 25 +++++++++++++++++-------- src/main.rs | 2 +- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/diagnostic.rs b/src/diagnostic.rs index abcc06b..0a2e74d 100644 --- a/src/diagnostic.rs +++ b/src/diagnostic.rs @@ -766,16 +766,25 @@ impl StyleAnalyzer { // Check for mixed indentation across the file if has_tabs && has_spaces { - if let Some(preferred) = &config.preferred_indent { - let range = Range::single_position(Position::new(0, 0)); - let diagnostic = Diagnostic::new( - range, + let range = Range::single_position(Position::new(0, 0)); + let (severity, message) = if let Some(preferred) = &config.preferred_indent { + ( DiagnosticSeverity::Information, - DiagnosticCode::InconsistentIndentation, format!("File uses mixed indentation; prefer {}", preferred), - ); - diagnostics.push(diagnostic); - } + ) + } else { + ( + DiagnosticSeverity::Warning, + "File uses mixed indentation (tabs and spaces)".to_string(), + ) + }; + let diagnostic = Diagnostic::new( + range, + severity, + DiagnosticCode::InconsistentIndentation, + message, + ); + diagnostics.push(diagnostic); } diagnostics diff --git a/src/main.rs b/src/main.rs index 88f6116..268c3d8 100755 --- a/src/main.rs +++ b/src/main.rs @@ -352,7 +352,7 @@ fn process_lint_command( let is_multiple_files = files.len() > 1; let mut has_errors = false; - + for file_path in files { if let Err(e) = process_single_file_lint( &file_path, -- 2.43.0 From 363f9cac8645ad7936c1551b7dcc93fcad70156d Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Mon, 2 Jun 2025 09:58:17 +0300 Subject: [PATCH 11/26] diagnostic: add check for excessive trailing empty lines in files --- src/diagnostic.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/diagnostic.rs b/src/diagnostic.rs index 0a2e74d..d989cff 100644 --- a/src/diagnostic.rs +++ b/src/diagnostic.rs @@ -721,6 +721,23 @@ impl StyleAnalyzer { } } + // Check for trailing empty lines at the end of the file + if empty_count > config.max_empty_lines { + let start = Position::new(empty_start as u32, 0); + let end = Position::new((empty_start + empty_count - 1) as u32, 0); + let range = Range::new(start, end); + let diagnostic = Diagnostic::new( + range, + DiagnosticSeverity::Warning, + DiagnosticCode::TooManyEmptyLines, + format!( + "Too many consecutive empty lines at end of file ({} > {})", + empty_count, config.max_empty_lines + ), + ); + diagnostics.push(diagnostic); + } + diagnostics } -- 2.43.0 From 728483d84fc8921a2b60e3426ab830461d3c51df Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Mon, 2 Jun 2025 11:24:41 +0300 Subject: [PATCH 12/26] nix: switch to cargo-nextest --- nix/shell.nix | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/nix/shell.nix b/nix/shell.nix index 965a0eb..f809a12 100644 --- a/nix/shell.nix +++ b/nix/shell.nix @@ -4,6 +4,8 @@ rustfmt, clippy, cargo, + cargo-machete, + cargo-nextest, rustPlatform, }: mkShell { @@ -13,6 +15,8 @@ mkShell { rustfmt clippy cargo + cargo-machete + cargo-nextest ]; RUST_SRC_PATH = "${rustPlatform.rustLibSrc}"; -- 2.43.0 From c0002f58069b00ec47303de26e800960b02b2ab2 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Mon, 2 Jun 2025 11:26:32 +0300 Subject: [PATCH 13/26] nff: LexError should include position for InvalidNumber; update diagnostics --- src/diagnostic.rs | 34 ++++++++++++---------------------- src/lexer.rs | 39 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 49 insertions(+), 24 deletions(-) diff --git a/src/diagnostic.rs b/src/diagnostic.rs index d989cff..c12b927 100644 --- a/src/diagnostic.rs +++ b/src/diagnostic.rs @@ -453,7 +453,7 @@ impl AnalyzerModule for LexicalAnalyzer { } impl LexicalAnalyzer { - fn lex_error_to_diagnostic(error: &LexError, source: &str) -> Diagnostic { + pub fn lex_error_to_diagnostic(error: &LexError, source: &str) -> Diagnostic { match error { LexError::InvalidToken { position, text } => { let pos = Position::from_text_size(TextSize::from(*position as u32), source); @@ -478,27 +478,17 @@ impl LexicalAnalyzer { "Unterminated string literal".to_string(), ) } - LexError::InvalidNumber { text } => { - if let Some(pos) = source.find(text) { - let start_pos = Position::from_text_size(TextSize::from(pos as u32), source); - let end_pos = - Position::new(start_pos.line, start_pos.character + text.len() as u32); - let range = Range::new(start_pos, end_pos); - Diagnostic::new( - range, - DiagnosticSeverity::Error, - DiagnosticCode::InvalidNumber, - format!("Invalid number: '{}'", text), - ) - } else { - let range = Range::single_position(Position::new(0, 0)); - Diagnostic::new( - range, - DiagnosticSeverity::Error, - DiagnosticCode::InvalidNumber, - format!("Invalid number: '{}'", text), - ) - } + LexError::InvalidNumber { position, text } => { + let start_pos = Position::from_text_size(TextSize::from(*position as u32), source); + let end_pos = + Position::new(start_pos.line, start_pos.character + text.len() as u32); + let range = Range::new(start_pos, end_pos); + Diagnostic::new( + range, + DiagnosticSeverity::Error, + DiagnosticCode::InvalidNumber, + format!("Invalid number: '{}'", text), + ) } } } diff --git a/src/lexer.rs b/src/lexer.rs index 4cd0505..bb342dd 100644 --- a/src/lexer.rs +++ b/src/lexer.rs @@ -10,8 +10,8 @@ pub enum LexError { InvalidToken { position: usize, text: String }, #[error("Unterminated string literal starting at position {position}")] UnterminatedString { position: usize }, - #[error("Invalid numeric literal: {text}")] - InvalidNumber { text: String }, + #[error("Invalid numeric literal at position {position}: {text}")] + InvalidNumber { position: usize, text: String }, } /// Result type for lexical analysis @@ -356,6 +356,7 @@ impl<'a> NftablesLexer<'a> { .any(|c| !c.is_ascii_digit() && c != '.' && c != 'x' && c != 'X') { return Err(LexError::InvalidNumber { + position: span.start, text: text.to_owned(), }); } else { @@ -448,4 +449,38 @@ mod tests { panic!("Expected InvalidToken error"); } } + + #[test] + fn test_invalid_number_with_position() { + // Test that we can create a proper diagnostic with position information + use crate::diagnostic::LexicalAnalyzer; + + // Create a source with the same invalid pattern at different positions + let source = "123abc normal 123abc end"; + + // Since normal tokenization splits "123abc" into "123" + "abc", + // let's test the diagnostic creation directly with a mock error + let error1 = LexError::InvalidNumber { + position: 0, + text: "123abc".to_string(), + }; + let error2 = LexError::InvalidNumber { + position: 14, + text: "123abc".to_string(), + }; + + // Test that diagnostics are created with correct positions + let diagnostic1 = LexicalAnalyzer::lex_error_to_diagnostic(&error1, source); + let diagnostic2 = LexicalAnalyzer::lex_error_to_diagnostic(&error2, source); + + // First occurrence should be at position 0 + assert_eq!(diagnostic1.range.start.line, 0); + assert_eq!(diagnostic1.range.start.character, 0); + assert_eq!(diagnostic1.message, "Invalid number: '123abc'"); + + // Second occurrence should be at position 14 (not 0) + assert_eq!(diagnostic2.range.start.line, 0); + assert_eq!(diagnostic2.range.start.character, 14); + assert_eq!(diagnostic2.message, "Invalid number: '123abc'"); + } } -- 2.43.0 From ef10784998a02c7d640d0af4f5bc79284a3d2496 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Mon, 2 Jun 2025 11:30:53 +0300 Subject: [PATCH 14/26] cst: improve `from_raw` method to handle invalid `SyntaxKind` values --- src/cst.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/cst.rs b/src/cst.rs index 4d76340..6feaa60 100644 --- a/src/cst.rs +++ b/src/cst.rs @@ -324,7 +324,13 @@ impl SyntaxKind { } pub fn from_raw(raw: RawSyntaxKind) -> Self { - unsafe { std::mem::transmute(raw.0 as u16) } + match raw.0 { + 0 => SyntaxKind::Root, + 1 => SyntaxKind::Table, + // ... other variants ... + 116 => SyntaxKind::PacketTooBigKw, + _ => SyntaxKind::Error, // Fallback to Error for invalid values + } } } -- 2.43.0 From 3e1003ba9471649bd65f6a26e2208273c1fed99f Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Mon, 2 Jun 2025 11:31:16 +0300 Subject: [PATCH 15/26] parser: skip newline tokens during expression parsing --- src/parser.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/parser.rs b/src/parser.rs index b10ae3d..3e733aa 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -466,6 +466,10 @@ impl Parser { // Check for operators while let Some(token) = self.peek() { + if matches!(token.kind, TokenKind::Newline) { + self.advance(); + continue; + } // Check for comparison operators let operator = match &token.kind { TokenKind::Eq => BinaryOperator::Eq, -- 2.43.0 From 92735f3eeef9061b03504598f23d6c1347a8a4f0 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Mon, 2 Jun 2025 12:00:15 +0300 Subject: [PATCH 16/26] docs: update README with new features --- README.md | 374 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 366 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 4242213..7368374 100644 --- a/README.md +++ b/README.md @@ -30,26 +30,68 @@ supported, I cannot guarantee that _everything_ is supported just yet. - **Validation** - Syntax checking with precise error locations - **Optimization** - Configurable empty line reduction and whitespace control +### Diagnostics & Analysis + +- **Comprehensive diagnostics** - Syntax, semantic, style, and best practice + analysis +- **Modular analysis** - Run specific diagnostic modules (`lexical`, `syntax`, + `style`, `semantic`) +- **LSP-compatible output** - JSON format for editor integration +- **Human-readable reports** - Detailed error messages with context and location + information +- **Configurable severity** - Control which diagnostic categories to + enable/disable + ## Usage +### Formatting + ```bash -# Basic formatting -nff -f /etc/nftables.conf +# Format a specific file (in place) +nff format /etc/nftables.conf + +# Format all .nft files in current directory (in place) +nff format # Custom indentation (4 spaces) -nff -f config.nft --indent spaces --spaces 4 +nff format config.nft --indent spaces --spaces 4 # Optimize formatting (reduce empty lines) -nff -f config.nft --optimize +nff format config.nft --optimize -# Output to file -nff -f config.nft -o formatted.nft +# Output to stdout instead of modifying files +nff format config.nft --stdout # Syntax validation only -nff -f config.nft --check +nff format config.nft --check # Debug output for development (or debugging) -nff -f config.nft --debug +nff format config.nft --debug +``` + +### Linting and Diagnostics + +```bash +# Run comprehensive diagnostics on a file +nff lint /etc/nftables.conf + +# Lint all .nft files in current directory +nff lint + +# JSON output for editor integration +nff lint config.nft --json + +# Run specific diagnostic modules +nff lint config.nft --modules syntax,style + +# Available modules: lexical, syntax, style, semantic +nff lint config.nft --modules semantic + +# Configure diagnostic settings (note: flags are enabled by default) +nff lint config.nft --style-warnings=false --best-practices=false + +# Debug output with diagnostics +nff lint config.nft --debug ``` ## Architecture @@ -69,12 +111,235 @@ graph TD AST --> Formatter Formatter --> Output CST --> Formatter + + Input --> Diagnostics[Diagnostic System] + Diagnostics --> LexAnalyzer[Lexical Analyzer] + Diagnostics --> SyntaxAnalyzer[Syntax Analyzer] + Diagnostics --> StyleAnalyzer[Style Analyzer] + Diagnostics --> SemanticAnalyzer[Semantic Analyzer] + + LexAnalyzer --> DiagOutput[JSON/Human Output] + SyntaxAnalyzer --> DiagOutput + StyleAnalyzer --> DiagOutput + SemanticAnalyzer --> DiagOutput ``` ## Installation Recommended way of installing nff is to use Nix. +### Editor Integration + +#### Neovim Setup + +nff can be integrated into Neovim as a diagnostics source for nftables files. +Here are several setup approaches: + +##### Option 2: Using none-ls + +```lua +local null_ls = require("null-ls") + +null_ls.setup({ + sources = { + -- nftables diagnostics + null_ls.builtins.diagnostics.nff.with({ + command = "nff", + args = { "lint", "$FILENAME", "--json" }, + format = "json", + check_exit_code = false, + filetypes = { "nftables" }, + }), + + -- nftables formatting + null_ls.builtins.formatting.nff.with({ + command = "nff", + args = { "format", "$FILENAME", "--stdout" }, + filetypes = { "nftables" }, + }), + }, +}) +``` + +##### Option 2: Using nvim-lint (recommended) + +```lua +-- ~/.config/nvim/lua/config/lint.lua +require('lint').linters.nff = { + cmd = 'nff', + stdin = false, + args = { 'lint', '%s', '--json' }, + stream = 'stdout', + ignore_exitcode = true, + parser = function(output) + local diagnostics = {} + local ok, decoded = pcall(vim.fn.json_decode, output) + + if not ok or not decoded.diagnostics then + return diagnostics + end + + for _, diagnostic in ipairs(decoded.diagnostics) do + table.insert(diagnostics, { + lnum = diagnostic.range.start.line, + col = diagnostic.range.start.character, + severity = diagnostic.severity == "Error" and vim.diagnostic.severity.ERROR or vim.diagnostic.severity.WARN, + message = diagnostic.message, + source = "nff", + code = diagnostic.code, + }) + end + + return diagnostics + end, +} + +-- Setup linting for nftables files +vim.api.nvim_create_autocmd({ "BufEnter", "BufWritePost" }, { + pattern = "*.nft", + callback = function() + require("lint").try_lint("nff") + end, +}) +``` + +##### Option 3: Custom Lua Function + +For a simple custom solution: + +```lua +-- ~/.config/nvim/lua/nff.lua +local M = {} + +function M.lint_nftables() + local filename = vim.fn.expand('%:p') + if vim.bo.filetype ~= 'nftables' then + return + end + + local cmd = { 'nff', 'lint', filename, '--json' } + + vim.fn.jobstart(cmd, { + stdout_buffered = true, + on_stdout = function(_, data) + if data then + local output = table.concat(data, '\n') + local ok, result = pcall(vim.fn.json_decode, output) + + if ok and result.diagnostics then + local diagnostics = {} + for _, diag in ipairs(result.diagnostics) do + table.insert(diagnostics, { + lnum = diag.range.start.line, + col = diag.range.start.character, + severity = diag.severity == "Error" and vim.diagnostic.severity.ERROR or vim.diagnostic.severity.WARN, + message = diag.message, + source = "nff", + }) + end + + vim.diagnostic.set(vim.api.nvim_create_namespace('nff'), 0, diagnostics) + end + end + end, + }) +end + +-- Auto-run on save +vim.api.nvim_create_autocmd("BufWritePost", { + pattern = "*.nft", + callback = M.lint_nftables, +}) + +return M +``` + +## Diagnostic Categories + +nff provides comprehensive analysis across multiple categories: + +### Syntax Errors + +- Parse errors with precise location information +- Missing tokens (semicolons, braces, etc.) +- Unexpected tokens +- Unterminated strings +- Invalid numbers + +### Semantic Validation + +- Unknown table families (`inet`, `ip`, `ip6`, etc.) +- Invalid chain types and hooks +- Incorrect priority values +- Missing chain policies +- Duplicate table/chain names +- Invalid CIDR notation +- Invalid port ranges + +### Style Warnings + +- Missing shebang line +- Inconsistent indentation (mixed tabs/spaces) +- Trailing whitespace +- Lines exceeding maximum length (configurable) +- Excessive empty lines +- Preferred syntax alternatives + +### Best Practices + +- Chains without explicit policies +- Rules without actions +- Overly permissive rules +- Duplicate or conflicting rules +- Unused variables or sets +- Deprecated syntax usage +- Missing documentation +- Security risks + +### Performance Hints + +- Inefficient rule ordering +- Large sets without timeouts +- Missing counters where beneficial + +## JSON Output Format + +When using `--json`, nff outputs LSP-compatible diagnostics: + +```json +{ + "diagnostics": [ + { + "range": { + "start": { "line": 5, "character": 10 }, + "end": { "line": 5, "character": 20 } + }, + "severity": "Error", + "code": "NFT001", + "source": "nff", + "message": "Expected ';' after policy", + "related_information": [], + "code_actions": [], + "tags": [] + } + ], + "file_path": "config.nft", + "source_text": "..." +} +``` + +### Diagnostic Codes + +nff uses structured diagnostic codes for categorization: + +- **NFT001-NFT099**: Syntax errors +- **NFT101-NFT199**: Semantic errors +- **NFT201-NFT299**: Style warnings +- **NFT301-NFT399**: Best practice recommendations +- **NFT401-NFT499**: Performance hints +- **NFT501-NFT599**: Formatting issues +- **NFT601-NFT699**: nftables-specific validations + ## Development ### Testing @@ -196,6 +461,88 @@ table inet protection { } ``` +## Diagnostics Examples + +### Error Detection + +Input file with issues: + +```nftables +table inet firewall { + chain input { + type filter hook input priority 100 + tcp dport 22 accept + } +} +``` + +Human-readable output: + +``` +Found 2 issues in config.nft: +config.nft:3:37: error: Expected ';' after policy [NFT001] + 1: table inet firewall { + 2: chain input { +→ 3: type filter hook input priority 100 + 4: tcp dport 22 accept + 5: } + +config.nft:3:1: warning: Filter chain should have an explicit policy [NFT301] + 1: table inet firewall { + 2: chain input { +→ 3: type filter hook input priority 100 + 4: tcp dport 22 accept + 5: } +``` + +JSON output: + +```json +{ + "diagnostics": [ + { + "range": { + "start": { "line": 2, "character": 37 }, + "end": { "line": 2, "character": 37 } + }, + "severity": "Error", + "code": "NFT001", + "source": "nff", + "message": "Expected ';' after policy" + }, + { + "range": { + "start": { "line": 2, "character": 0 }, + "end": { "line": 2, "character": 37 } + }, + "severity": "Warning", + "code": "NFT301", + "source": "nff", + "message": "Filter chain should have an explicit policy" + } + ], + "file_path": "config.nft", + "source_text": "..." +} +``` + +### Style Analysis + +Input with style issues: + +```nftables +table inet test{chain input{type filter hook input priority 0;policy drop;tcp dport 22 accept;}} +``` + +Style warnings: + +``` +Found 3 issues in style.nft: +style.nft:1:1: warning: Consider adding a shebang line [NFT201] +style.nft:1:121: warning: Line too long (98 > 80 characters) [NFT205] +style.nft:1:16: warning: Missing space after '{' [NFT503] +``` + ## Contributing ### Code Style @@ -237,6 +584,17 @@ Below are the design goals of nff's architechture. - **Memory efficiency**: Streaming token processing where possible - **Grammar completeness**: Covers full nftables syntax specification +### Diagnostic Architecture + +The diagnostic system uses a modular architecture with specialized analyzers: + +- **Modular design**: Each analyzer focuses on specific concerns (lexical, + syntax, style, semantic) +- **Configurable analysis**: Enable/disable specific diagnostic categories +- **LSP compatibility**: JSON output follows Language Server Protocol standards +- **Performance optimized**: Concurrent analysis when possible +- **Extensible**: Easy to add new diagnostic rules and categories + ## License nff is licensed under [MPL v2.0](LICENSE). See license file for more details on -- 2.43.0 From f7f77a7e19b73905b27e19b4e5efb5406cf91e7d Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Mon, 2 Jun 2025 12:32:33 +0300 Subject: [PATCH 17/26] cst: expand SyntaxKind enum to include additional nftables syntax variants --- src/cst.rs | 115 +++++++++++++- src/main.rs | 448 ++++++++++++++++++++++------------------------------ 2 files changed, 304 insertions(+), 259 deletions(-) diff --git a/src/cst.rs b/src/cst.rs index 6feaa60..412b379 100644 --- a/src/cst.rs +++ b/src/cst.rs @@ -327,7 +327,120 @@ impl SyntaxKind { match raw.0 { 0 => SyntaxKind::Root, 1 => SyntaxKind::Table, - // ... other variants ... + 2 => SyntaxKind::Chain, + 3 => SyntaxKind::Rule, + 4 => SyntaxKind::Set, + 5 => SyntaxKind::Map, + 6 => SyntaxKind::Element, + 7 => SyntaxKind::Expression, + 8 => SyntaxKind::BinaryExpr, + 9 => SyntaxKind::UnaryExpr, + 10 => SyntaxKind::CallExpr, + 11 => SyntaxKind::SetExpr, + 12 => SyntaxKind::RangeExpr, + 13 => SyntaxKind::Statement, + 14 => SyntaxKind::IncludeStmt, + 15 => SyntaxKind::DefineStmt, + 16 => SyntaxKind::FlushStmt, + 17 => SyntaxKind::AddStmt, + 18 => SyntaxKind::DeleteStmt, + 19 => SyntaxKind::Identifier, + 20 => SyntaxKind::StringLiteral, + 21 => SyntaxKind::NumberLiteral, + 22 => SyntaxKind::IpAddress, + 23 => SyntaxKind::Ipv6Address, + 24 => SyntaxKind::MacAddress, + 25 => SyntaxKind::TableKw, + 26 => SyntaxKind::ChainKw, + 27 => SyntaxKind::RuleKw, + 28 => SyntaxKind::SetKw, + 29 => SyntaxKind::MapKw, + 30 => SyntaxKind::ElementKw, + 31 => SyntaxKind::IncludeKw, + 32 => SyntaxKind::DefineKw, + 33 => SyntaxKind::FlushKw, + 34 => SyntaxKind::AddKw, + 35 => SyntaxKind::DeleteKw, + 36 => SyntaxKind::InsertKw, + 37 => SyntaxKind::ReplaceKw, + 38 => SyntaxKind::FilterKw, + 39 => SyntaxKind::NatKw, + 40 => SyntaxKind::RouteKw, + 41 => SyntaxKind::InputKw, + 42 => SyntaxKind::OutputKw, + 43 => SyntaxKind::ForwardKw, + 44 => SyntaxKind::PreroutingKw, + 45 => SyntaxKind::PostroutingKw, + 46 => SyntaxKind::IpKw, + 47 => SyntaxKind::Ip6Kw, + 48 => SyntaxKind::InetKw, + 49 => SyntaxKind::ArpKw, + 50 => SyntaxKind::BridgeKw, + 51 => SyntaxKind::NetdevKw, + 52 => SyntaxKind::TcpKw, + 53 => SyntaxKind::UdpKw, + 54 => SyntaxKind::IcmpKw, + 55 => SyntaxKind::Icmpv6Kw, + 56 => SyntaxKind::SportKw, + 57 => SyntaxKind::DportKw, + 58 => SyntaxKind::SaddrKw, + 59 => SyntaxKind::DaddrKw, + 60 => SyntaxKind::ProtocolKw, + 61 => SyntaxKind::NexthdrKw, + 62 => SyntaxKind::TypeKw, + 63 => SyntaxKind::HookKw, + 64 => SyntaxKind::PriorityKw, + 65 => SyntaxKind::PolicyKw, + 66 => SyntaxKind::IifnameKw, + 67 => SyntaxKind::OifnameKw, + 68 => SyntaxKind::CtKw, + 69 => SyntaxKind::StateKw, + 70 => SyntaxKind::AcceptKw, + 71 => SyntaxKind::DropKw, + 72 => SyntaxKind::RejectKw, + 73 => SyntaxKind::ReturnKw, + 74 => SyntaxKind::JumpKw, + 75 => SyntaxKind::GotoKw, + 76 => SyntaxKind::ContinueKw, + 77 => SyntaxKind::LogKw, + 78 => SyntaxKind::CommentKw, + 79 => SyntaxKind::EstablishedKw, + 80 => SyntaxKind::RelatedKw, + 81 => SyntaxKind::NewKw, + 82 => SyntaxKind::InvalidKw, + 83 => SyntaxKind::EqOp, + 84 => SyntaxKind::NeOp, + 85 => SyntaxKind::LeOp, + 86 => SyntaxKind::GeOp, + 87 => SyntaxKind::LtOp, + 88 => SyntaxKind::GtOp, + 89 => SyntaxKind::LeftBrace, + 90 => SyntaxKind::RightBrace, + 91 => SyntaxKind::LeftParen, + 92 => SyntaxKind::RightParen, + 93 => SyntaxKind::LeftBracket, + 94 => SyntaxKind::RightBracket, + 95 => SyntaxKind::Comma, + 96 => SyntaxKind::Semicolon, + 97 => SyntaxKind::Colon, + 98 => SyntaxKind::Assign, + 99 => SyntaxKind::Dash, + 100 => SyntaxKind::Slash, + 101 => SyntaxKind::Dot, + 102 => SyntaxKind::Whitespace, + 103 => SyntaxKind::Newline, + 104 => SyntaxKind::Comment, + 105 => SyntaxKind::Shebang, + 106 => SyntaxKind::Error, + 107 => SyntaxKind::VmapKw, + 108 => SyntaxKind::NdRouterAdvertKw, + 109 => SyntaxKind::NdNeighborSolicitKw, + 110 => SyntaxKind::NdNeighborAdvertKw, + 111 => SyntaxKind::EchoRequestKw, + 112 => SyntaxKind::DestUnreachableKw, + 113 => SyntaxKind::RouterAdvertisementKw, + 114 => SyntaxKind::TimeExceededKw, + 115 => SyntaxKind::ParameterProblemKw, 116 => SyntaxKind::PacketTooBigKw, _ => SyntaxKind::Error, // Fallback to Error for invalid values } diff --git a/src/main.rs b/src/main.rs index 268c3d8..87a35f8 100755 --- a/src/main.rs +++ b/src/main.rs @@ -34,20 +34,6 @@ enum FormatterError { message: String, suggestion: Option, }, - #[error("Unsupported nftables syntax at line {line}, column {column}: {feature}")] - UnsupportedSyntax { - line: usize, - column: usize, - feature: String, - suggestion: Option, - }, - #[error("Invalid nftables syntax at line {line}, column {column}: {message}")] - InvalidSyntax { - line: usize, - column: usize, - message: String, - suggestion: Option, - }, #[error("IO error: {0}")] Io(#[from] io::Error), } @@ -289,7 +275,7 @@ fn process_single_file_format( let mut parser = NftablesParser::new(tokens.clone()); parser .parse() - .map_err(|e| analyze_parse_error(&source, &tokens, &e.to_string()))? + .map_err(|e| convert_parse_error_to_formatter_error(&e, &source, &tokens))? }; if debug { @@ -475,77 +461,203 @@ fn process_single_file_lint( Ok(()) } -/// Intelligent error analysis to categorize parse errors and provide location information -fn analyze_parse_error(source: &str, tokens: &[Token], error: &str) -> FormatterError { - // Convert line/column position from token ranges - let lines: Vec<&str> = source.lines().collect(); +/// Convert parser errors to formatter errors with proper location information +fn convert_parse_error_to_formatter_error( + error: &crate::parser::ParseError, + source: &str, + tokens: &[Token], +) -> FormatterError { + use crate::parser::ParseError; - // Look for common error patterns and provide specific messages - if error.contains("unexpected token") || error.contains("expected") { - // Try to find the problematic token - if let Some(error_token) = find_error_token(tokens) { - let (line, column) = position_from_range(&error_token.range, source); - - // Analyze the specific token to categorize the error - match categorize_syntax_error(&error_token, source, &lines) { - ErrorCategory::UnsupportedSyntax { - feature, - suggestion, - } => FormatterError::UnsupportedSyntax { - line, - column, - feature, - suggestion, - }, - ErrorCategory::InvalidSyntax { - message, - suggestion, - } => FormatterError::InvalidSyntax { - line, - column, - message, - suggestion, - }, - ErrorCategory::SyntaxError { - message, - suggestion, - } => FormatterError::SyntaxError { - line, - column, - message, - suggestion, - }, + match error { + ParseError::UnexpectedToken { + line, + column, + expected, + found, + } => FormatterError::SyntaxError { + line: *line, + column: *column, + message: format!("Expected {}, found '{}'", expected, found), + suggestion: None, + }, + ParseError::MissingToken { expected } => { + // Try to find current position from last token + let (line, column) = if let Some(last_token) = tokens.last() { + position_from_range(&last_token.range, source) + } else { + (1, 1) + }; + FormatterError::SyntaxError { + line, + column, + message: format!("Missing token: expected {}", expected), + suggestion: None, + } + } + ParseError::InvalidExpression { message } => { + // Try to find the current token position + let (line, column) = find_current_parse_position(tokens, source); + FormatterError::SyntaxError { + line, + column, + message: format!("Invalid expression: {}", message), + suggestion: None, + } + } + ParseError::InvalidStatement { message } => { + let (line, column) = find_current_parse_position(tokens, source); + FormatterError::SyntaxError { + line, + column, + message: format!("Invalid statement: {}", message), + suggestion: None, + } + } + ParseError::SemanticError { message } => { + let (line, column) = find_current_parse_position(tokens, source); + FormatterError::SyntaxError { + line, + column, + message: format!("Semantic error: {}", message), + suggestion: None, + } + } + ParseError::LexError(lex_error) => { + // Convert lexical errors to formatter errors with location + convert_lex_error_to_formatter_error(lex_error, source) + } + ParseError::AnyhowError(anyhow_error) => { + // For anyhow errors, try to extract location from error message and context + let error_msg = anyhow_error.to_string(); + let (line, column) = find_error_location_from_context(&error_msg, tokens, source); + let suggestion = generate_suggestion_for_error(&error_msg); + + FormatterError::SyntaxError { + line, + column, + message: error_msg, + suggestion, } - } else { - // Fallback to generic parse error - FormatterError::ParseError(error.to_string()) } - } else { - FormatterError::ParseError(error.to_string()) } } -#[derive(Debug)] -enum ErrorCategory { - UnsupportedSyntax { - feature: String, - suggestion: Option, - }, - InvalidSyntax { - message: String, - suggestion: Option, - }, - SyntaxError { - message: String, - suggestion: Option, - }, +/// Find the current parsing position from tokens +fn find_current_parse_position(tokens: &[Token], source: &str) -> (usize, usize) { + // Look for the last non-whitespace, non-comment token + for token in tokens.iter().rev() { + match token.kind { + TokenKind::Newline | TokenKind::CommentLine(_) => continue, + _ => return position_from_range(&token.range, source), + } + } + (1, 1) // fallback } -/// Find the first error token in the token stream -fn find_error_token(tokens: &[Token]) -> Option<&Token> { - tokens - .iter() - .find(|token| matches!(token.kind, TokenKind::Error)) +/// Convert lexical errors to formatter errors +fn convert_lex_error_to_formatter_error( + lex_error: &crate::lexer::LexError, + source: &str, +) -> FormatterError { + use crate::lexer::LexError; + + match lex_error { + LexError::InvalidToken { position, text } => { + let (line, column) = offset_to_line_column(*position, source); + FormatterError::SyntaxError { + line, + column, + message: format!("Invalid token: '{}'", text), + suggestion: None, + } + } + LexError::UnterminatedString { position } => { + let (line, column) = offset_to_line_column(*position, source); + FormatterError::SyntaxError { + line, + column, + message: "Unterminated string literal".to_string(), + suggestion: Some("Add closing quote".to_string()), + } + } + LexError::InvalidNumber { position, text } => { + let (line, column) = offset_to_line_column(*position, source); + FormatterError::SyntaxError { + line, + column, + message: format!("Invalid number: '{}'", text), + suggestion: Some("Check number format".to_string()), + } + } + } +} + +/// Convert byte offset to line/column position +fn offset_to_line_column(offset: usize, source: &str) -> (usize, usize) { + let mut line = 1; + let mut column = 1; + + for (i, ch) in source.char_indices() { + if i >= offset { + break; + } + if ch == '\n' { + line += 1; + column = 1; + } else { + column += 1; + } + } + + (line, column) +} + +/// Find error location from context clues in the error message +fn find_error_location_from_context( + error_msg: &str, + tokens: &[Token], + source: &str, +) -> (usize, usize) { + // Look for context clues in the error message + if error_msg.contains("Expected string or identifier, got:") { + // Find the problematic token mentioned in the error + if let Some(bad_token_text) = extract_token_from_error_message(error_msg) { + // Find this token in the token stream + for token in tokens { + if token.text == bad_token_text { + return position_from_range(&token.range, source); + } + } + } + } + + // Fallback to finding last meaningful token + find_current_parse_position(tokens, source) +} + +/// Extract the problematic token from error message +fn extract_token_from_error_message(error_msg: &str) -> Option { + // Parse messages like "Expected string or identifier, got: {" + if let Some(got_part) = error_msg.split("got: ").nth(1) { + Some(got_part.trim().to_string()) + } else { + None + } +} + +/// Generate helpful suggestions based on error message +fn generate_suggestion_for_error(error_msg: &str) -> Option { + if error_msg.contains("Expected string or identifier") { + Some( + "Check if you're missing quotes around a string value or have an unexpected character" + .to_string(), + ) + } else if error_msg.contains("Expected") && error_msg.contains("got:") { + Some("Check syntax and ensure proper nftables structure".to_string()) + } else { + None + } } /// Convert TextRange to line/column position @@ -566,186 +678,6 @@ fn position_from_range(range: &text_size::TextRange, source: &str) -> (usize, us (1, 1) // fallback } -/// Categorize syntax errors based on token content and context -fn categorize_syntax_error(token: &Token, source: &str, lines: &[&str]) -> ErrorCategory { - let token_text = &token.text; - let (line_num, _) = position_from_range(&token.range, source); - let line_content = lines.get(line_num.saturating_sub(1)).unwrap_or(&""); - - // Check for unsupported nftables features - if is_unsupported_feature(token_text, line_content) { - let (feature, suggestion) = classify_unsupported_feature(token_text, line_content); - return ErrorCategory::UnsupportedSyntax { - feature, - suggestion, - }; - } - - // Check for invalid but supported syntax - if is_invalid_syntax(token_text, line_content) { - let (message, suggestion) = classify_invalid_syntax(token_text, line_content); - return ErrorCategory::InvalidSyntax { - message, - suggestion, - }; - } - - // Default to syntax error - ErrorCategory::SyntaxError { - message: format!("Unexpected token '{}'", token_text), - suggestion: suggest_correction(token_text, line_content), - } -} - -/// Check if the token represents an unsupported nftables feature -fn is_unsupported_feature(token_text: &str, line_content: &str) -> bool { - // List of advanced nftables features that might not be fully supported yet - let unsupported_keywords = [ - "quota", "limit", "counter", "meter", "socket", "fib", "rt", "ipsec", "tunnel", "comp", - "dccp", "sctp", "gre", "esp", "ah", "vlan", "arp", "rateest", "osf", "netdev", "meta", - "exthdr", "payload", "lookup", "dynset", "flow", "hash", "jhash", "symhash", "crc32", - ]; - - unsupported_keywords - .iter() - .any(|&keyword| token_text.contains(keyword) || line_content.contains(keyword)) -} - -/// Check if the syntax is invalid (malformed but within supported features) -fn is_invalid_syntax(token_text: &str, line_content: &str) -> bool { - // Check for common syntax mistakes - if token_text.contains("..") || token_text.contains("::") { - return true; // Double operators usually indicate mistakes - } - - // Check for malformed addresses or ranges - if token_text.contains("/") && !is_valid_cidr(token_text) { - return true; - } - - // Check for malformed brackets/braces - let open_braces = line_content.matches('{').count(); - let close_braces = line_content.matches('}').count(); - if open_braces != close_braces { - return true; - } - - false -} - -/// Classify unsupported feature and provide suggestion -fn classify_unsupported_feature(token_text: &str, line_content: &str) -> (String, Option) { - let feature = if token_text.contains("quota") { - ( - "quota management".to_string(), - Some("Use explicit rule counting instead".to_string()), - ) - } else if token_text.contains("limit") { - ( - "rate limiting".to_string(), - Some("Consider using simpler rule-based rate limiting".to_string()), - ) - } else if token_text.contains("counter") { - ( - "packet counters".to_string(), - Some("Use rule-level statistics instead".to_string()), - ) - } else if line_content.contains("meta") { - ( - "meta expressions".to_string(), - Some("Use explicit protocol matching instead".to_string()), - ) - } else { - (format!("advanced feature '{}'", token_text), None) - }; - - feature -} - -/// Classify invalid syntax and provide suggestion -fn classify_invalid_syntax(token_text: &str, line_content: &str) -> (String, Option) { - if token_text.contains("/") && !is_valid_cidr(token_text) { - return ( - "Invalid CIDR notation".to_string(), - Some("Use format like '192.168.1.0/24' or '::1/128'".to_string()), - ); - } - - if token_text.contains("..") { - return ( - "Invalid range operator".to_string(), - Some("Use '-' for ranges like '1000-2000'".to_string()), - ); - } - - if line_content.contains('{') && !line_content.contains('}') { - return ( - "Unmatched opening brace".to_string(), - Some("Ensure all '{' have matching '}'".to_string()), - ); - } - - ( - format!("Malformed token '{}'", token_text), - Some("Check nftables syntax documentation".to_string()), - ) -} - -/// Suggest correction for common typos -fn suggest_correction(token_text: &str, line_content: &str) -> Option { - // Common typos and their corrections - let corrections = [ - ("tabel", "table"), - ("cahin", "chain"), - ("accpet", "accept"), - ("rejct", "reject"), - ("prtocol", "protocol"), - ("addres", "address"), - ("pririty", "priority"), - ("poicy", "policy"), - ]; - - for (typo, correction) in &corrections { - if token_text.contains(typo) { - return Some(format!("Did you mean '{}'?", correction)); - } - } - - // Context-based suggestions - if line_content.contains("type") && line_content.contains("hook") { - if !line_content.contains("filter") - && !line_content.contains("nat") - && !line_content.contains("route") - { - return Some("Chain type should be 'filter', 'nat', or 'route'".to_string()); - } - } - - None -} - -/// Validate CIDR notation -fn is_valid_cidr(text: &str) -> bool { - if let Some(slash_pos) = text.find('/') { - let (addr, prefix) = text.split_at(slash_pos); - let prefix = &prefix[1..]; // Remove the '/' - - // Check if prefix is a valid number - if let Ok(prefix_len) = prefix.parse::() { - // Basic validation - IPv4 should be <= 32, IPv6 <= 128 - if addr.contains(':') { - prefix_len <= 128 // IPv6 - } else { - prefix_len <= 32 // IPv4 - } - } else { - false - } - } else { - false - } -} - fn main() -> Result<()> { let args = Args::parse(); -- 2.43.0 From 0eaa21a3ff6e2a8e045acdf92f198168dd35cfd3 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Mon, 2 Jun 2025 13:46:57 +0300 Subject: [PATCH 18/26] diagnostic: update configuration options to use clap's `ArgAction` --- src/diagnostic.rs | 129 ++++++++++++++++++++++++++++++++++++++++++---- src/main.rs | 8 +-- 2 files changed, 122 insertions(+), 15 deletions(-) diff --git a/src/diagnostic.rs b/src/diagnostic.rs index c12b927..a3a46b0 100644 --- a/src/diagnostic.rs +++ b/src/diagnostic.rs @@ -607,6 +607,11 @@ impl AnalyzerModule for StyleAnalyzer { fn analyze(&self, source: &str, config: &DiagnosticConfig) -> Vec { 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 { + fn analyze(&self, source: &str, config: &DiagnosticConfig) -> Vec { 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 { + fn validate_chain_declarations_semantic(&self, source: &str) -> Vec { 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 - if trimmed.contains("type filter") && !trimmed.contains("policy") { + diagnostics + } + + fn validate_chain_best_practices(&self, source: &str) -> Vec { + 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 { + 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, - DiagnosticCode::ChainWithoutPolicy, - "Filter chain should have an explicit policy".to_string(), + DiagnosticSeverity::Hint, + DiagnosticCode::LargeSetWithoutTimeout, + "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 { + 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 diff --git a/src/main.rs b/src/main.rs index 87a35f8..e7fa847 100755 --- a/src/main.rs +++ b/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) -- 2.43.0 From 8d7fcd6ef446966025db1b857351f78d7a1381ff Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Mon, 2 Jun 2025 14:00:48 +0300 Subject: [PATCH 19/26] lint: introduce `LintError`; count diagnostic errors --- src/main.rs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/main.rs b/src/main.rs index e7fa847..4d32351 100755 --- a/src/main.rs +++ b/src/main.rs @@ -38,6 +38,12 @@ enum FormatterError { Io(#[from] io::Error), } +#[derive(Error, Debug)] +enum LintError { + #[error("Lint errors found in {file_count} file(s)")] + DiagnosticErrors { file_count: usize }, +} + #[derive(Parser, Debug, Clone)] #[command( name = "nff", @@ -337,7 +343,7 @@ fn process_lint_command( }; let is_multiple_files = files.len() > 1; - let mut has_errors = false; + let mut error_file_count = 0; for file_path in files { if let Err(e) = process_single_file_lint( @@ -352,16 +358,18 @@ fn process_lint_command( is_multiple_files, ) { eprintln!("Error processing {}: {}", file_path, e); - has_errors = true; + error_file_count += 1; if !is_multiple_files { return Err(e); } } } - // Exit with non-zero code if any file had errors - if has_errors { - std::process::exit(1); + if error_file_count > 0 { + return Err(LintError::DiagnosticErrors { + file_count: error_file_count, + } + .into()); } Ok(()) -- 2.43.0 From d1cad19fead02a13cae3666d89a2be8c931a7d89 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Mon, 2 Jun 2025 14:11:23 +0300 Subject: [PATCH 20/26] lint: better error handling; count "faultly" files --- src/main.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main.rs b/src/main.rs index 4d32351..3a34967 100755 --- a/src/main.rs +++ b/src/main.rs @@ -42,6 +42,8 @@ enum FormatterError { enum LintError { #[error("Lint errors found in {file_count} file(s)")] DiagnosticErrors { file_count: usize }, + #[error("File discovery error: {0}")] + FileDiscovery(#[from] anyhow::Error), } #[derive(Parser, Debug, Clone)] -- 2.43.0 From b9d8cb6d5d5469db5a221b98e713232010e87cc4 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Mon, 2 Jun 2025 14:32:36 +0300 Subject: [PATCH 21/26] cst: eliminate manual synchronization between enum variants and numeric values --- Cargo.lock | 57 ++++++++ Cargo.toml | 3 +- src/cst.rs | 421 +++++++++++++++++++++++++---------------------------- 3 files changed, 260 insertions(+), 221 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1b11d7f..cd5a4c2 100755 --- a/Cargo.lock +++ b/Cargo.lock @@ -281,6 +281,7 @@ dependencies = [ "cstree", "glob", "logos", + "num_enum", "regex", "serde", "serde_json", @@ -288,6 +289,27 @@ dependencies = [ "thiserror", ] +[[package]] +name = "num_enum" +version = "0.7.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4e613fc340b2220f734a8595782c551f1250e969d87d3be1ae0579e8d4065179" +dependencies = [ + "num_enum_derive", +] + +[[package]] +name = "num_enum_derive" +version = "0.7.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "af1844ef2428cc3e1cb900be36181049ef3d3193c63e43026cfe202983b27a56" +dependencies = [ + "proc-macro-crate", + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "parking_lot" version = "0.12.3" @@ -311,6 +333,15 @@ dependencies = [ "windows-targets", ] +[[package]] +name = "proc-macro-crate" +version = "3.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "edce586971a4dfaa28950c6f18ed55e0406c1ab88bbce2c6f6293a7aaba73d35" +dependencies = [ + "toml_edit", +] + [[package]] name = "proc-macro2" version = "1.0.95" @@ -487,6 +518,23 @@ dependencies = [ "syn", ] +[[package]] +name = "toml_datetime" +version = "0.6.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3da5db5a963e24bc68be8b17b6fa82814bb22ee8660f192bb182771d498f09a3" + +[[package]] +name = "toml_edit" +version = "0.22.26" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "310068873db2c5b3e7659d2cc35d21855dbafa50d1ce336397c666e3cb08137e" +dependencies = [ + "indexmap", + "toml_datetime", + "winnow", +] + [[package]] name = "triomphe" version = "0.1.14" @@ -573,3 +621,12 @@ name = "windows_x86_64_msvc" version = "0.52.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32b752e52a2da0ddfbdbcc6fceadfeede4c939ed16d13e648833a61dfb611ed8" + +[[package]] +name = "winnow" +version = "0.7.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c06928c8748d81b05c9be96aad92e1b6ff01833332f281e8cfca3be4b35fc9ec" +dependencies = [ + "memchr", +] diff --git a/Cargo.toml b/Cargo.toml index 4bb2743..afd902e 100755 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,5 +14,6 @@ cstree = "0.12" text-size = "1.1" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" -regex = "1.11.1" +regex = "1.11" glob = "0.3" +num_enum = "0.7" diff --git a/src/cst.rs b/src/cst.rs index 412b379..b36cad0 100644 --- a/src/cst.rs +++ b/src/cst.rs @@ -4,11 +4,15 @@ use crate::lexer::{Token, TokenKind}; use cstree::{RawSyntaxKind, green::GreenNode, util::NodeOrToken}; +use num_enum::{IntoPrimitive, TryFromPrimitive}; use std::fmt; use thiserror::Error; /// nftables syntax node types -#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] +/// Uses `TryFromPrimitive` for safe conversion from raw values with fallback to `Error`. +#[derive( + Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, TryFromPrimitive, IntoPrimitive, +)] #[repr(u16)] pub enum SyntaxKind { // Root and containers @@ -161,116 +165,128 @@ pub enum SyntaxKind { impl From for SyntaxKind { fn from(kind: TokenKind) -> Self { + use TokenKind::*; match kind { - TokenKind::Table => SyntaxKind::TableKw, - TokenKind::Chain => SyntaxKind::ChainKw, - TokenKind::Rule => SyntaxKind::RuleKw, - TokenKind::Set => SyntaxKind::SetKw, - TokenKind::Map => SyntaxKind::MapKw, - TokenKind::Element => SyntaxKind::ElementKw, - TokenKind::Include => SyntaxKind::IncludeKw, - TokenKind::Define => SyntaxKind::DefineKw, - TokenKind::Flush => SyntaxKind::FlushKw, - TokenKind::Add => SyntaxKind::AddKw, - TokenKind::Delete => SyntaxKind::DeleteKw, - TokenKind::Insert => SyntaxKind::InsertKw, - TokenKind::Replace => SyntaxKind::ReplaceKw, + // Keywords -> Kw variants + Table => SyntaxKind::TableKw, + Chain => SyntaxKind::ChainKw, + Rule => SyntaxKind::RuleKw, + Set => SyntaxKind::SetKw, + Map => SyntaxKind::MapKw, + Element => SyntaxKind::ElementKw, + Include => SyntaxKind::IncludeKw, + Define => SyntaxKind::DefineKw, + Flush => SyntaxKind::FlushKw, + Add => SyntaxKind::AddKw, + Delete => SyntaxKind::DeleteKw, + Insert => SyntaxKind::InsertKw, + Replace => SyntaxKind::ReplaceKw, - TokenKind::Filter => SyntaxKind::FilterKw, - TokenKind::Nat => SyntaxKind::NatKw, - TokenKind::Route => SyntaxKind::RouteKw, + // Chain types and hooks + Filter => SyntaxKind::FilterKw, + Nat => SyntaxKind::NatKw, + Route => SyntaxKind::RouteKw, + Input => SyntaxKind::InputKw, + Output => SyntaxKind::OutputKw, + Forward => SyntaxKind::ForwardKw, + Prerouting => SyntaxKind::PreroutingKw, + Postrouting => SyntaxKind::PostroutingKw, - TokenKind::Input => SyntaxKind::InputKw, - TokenKind::Output => SyntaxKind::OutputKw, - TokenKind::Forward => SyntaxKind::ForwardKw, - TokenKind::Prerouting => SyntaxKind::PreroutingKw, - TokenKind::Postrouting => SyntaxKind::PostroutingKw, + // Protocols and families + Ip => SyntaxKind::IpKw, + Ip6 => SyntaxKind::Ip6Kw, + Inet => SyntaxKind::InetKw, + Arp => SyntaxKind::ArpKw, + Bridge => SyntaxKind::BridgeKw, + Netdev => SyntaxKind::NetdevKw, + Tcp => SyntaxKind::TcpKw, + Udp => SyntaxKind::UdpKw, + Icmp => SyntaxKind::IcmpKw, + Icmpv6 => SyntaxKind::Icmpv6Kw, - TokenKind::Ip => SyntaxKind::IpKw, - TokenKind::Ip6 => SyntaxKind::Ip6Kw, - TokenKind::Inet => SyntaxKind::InetKw, - TokenKind::Arp => SyntaxKind::ArpKw, - TokenKind::Bridge => SyntaxKind::BridgeKw, - TokenKind::Netdev => SyntaxKind::NetdevKw, - TokenKind::Tcp => SyntaxKind::TcpKw, - TokenKind::Udp => SyntaxKind::UdpKw, - TokenKind::Icmp => SyntaxKind::IcmpKw, - TokenKind::Icmpv6 => SyntaxKind::Icmpv6Kw, + // Match keywords + Sport => SyntaxKind::SportKw, + Dport => SyntaxKind::DportKw, + Saddr => SyntaxKind::SaddrKw, + Daddr => SyntaxKind::DaddrKw, + Protocol => SyntaxKind::ProtocolKw, + Nexthdr => SyntaxKind::NexthdrKw, + Type => SyntaxKind::TypeKw, + Hook => SyntaxKind::HookKw, + Priority => SyntaxKind::PriorityKw, + Policy => SyntaxKind::PolicyKw, + Iifname => SyntaxKind::IifnameKw, + Oifname => SyntaxKind::OifnameKw, + Ct => SyntaxKind::CtKw, + State => SyntaxKind::StateKw, - TokenKind::Sport => SyntaxKind::SportKw, - TokenKind::Dport => SyntaxKind::DportKw, - TokenKind::Saddr => SyntaxKind::SaddrKw, - TokenKind::Daddr => SyntaxKind::DaddrKw, - TokenKind::Protocol => SyntaxKind::ProtocolKw, - TokenKind::Nexthdr => SyntaxKind::NexthdrKw, - TokenKind::Type => SyntaxKind::TypeKw, - TokenKind::Hook => SyntaxKind::HookKw, - TokenKind::Priority => SyntaxKind::PriorityKw, - TokenKind::Policy => SyntaxKind::PolicyKw, - TokenKind::Iifname => SyntaxKind::IifnameKw, - TokenKind::Oifname => SyntaxKind::OifnameKw, - TokenKind::Ct => SyntaxKind::CtKw, - TokenKind::State => SyntaxKind::StateKw, + // Actions + Accept => SyntaxKind::AcceptKw, + Drop => SyntaxKind::DropKw, + Reject => SyntaxKind::RejectKw, + Return => SyntaxKind::ReturnKw, + Jump => SyntaxKind::JumpKw, + Goto => SyntaxKind::GotoKw, + Continue => SyntaxKind::ContinueKw, + Log => SyntaxKind::LogKw, + Comment => SyntaxKind::CommentKw, - TokenKind::Accept => SyntaxKind::AcceptKw, - TokenKind::Drop => SyntaxKind::DropKw, - TokenKind::Reject => SyntaxKind::RejectKw, - TokenKind::Return => SyntaxKind::ReturnKw, - TokenKind::Jump => SyntaxKind::JumpKw, - TokenKind::Goto => SyntaxKind::GotoKw, - TokenKind::Continue => SyntaxKind::ContinueKw, - TokenKind::Log => SyntaxKind::LogKw, - TokenKind::Comment => SyntaxKind::CommentKw, + // States + Established => SyntaxKind::EstablishedKw, + Related => SyntaxKind::RelatedKw, + New => SyntaxKind::NewKw, + Invalid => SyntaxKind::InvalidKw, - TokenKind::Established => SyntaxKind::EstablishedKw, - TokenKind::Related => SyntaxKind::RelatedKw, - TokenKind::New => SyntaxKind::NewKw, - TokenKind::Invalid => SyntaxKind::InvalidKw, + // Additional protocol keywords + Vmap => SyntaxKind::VmapKw, + NdRouterAdvert => SyntaxKind::NdRouterAdvertKw, + NdNeighborSolicit => SyntaxKind::NdNeighborSolicitKw, + NdNeighborAdvert => SyntaxKind::NdNeighborAdvertKw, + EchoRequest => SyntaxKind::EchoRequestKw, + DestUnreachable => SyntaxKind::DestUnreachableKw, + RouterAdvertisement => SyntaxKind::RouterAdvertisementKw, + TimeExceeded => SyntaxKind::TimeExceededKw, + ParameterProblem => SyntaxKind::ParameterProblemKw, + PacketTooBig => SyntaxKind::PacketTooBigKw, - TokenKind::Vmap => SyntaxKind::VmapKw, - TokenKind::NdRouterAdvert => SyntaxKind::NdRouterAdvertKw, - TokenKind::NdNeighborSolicit => SyntaxKind::NdNeighborSolicitKw, - TokenKind::NdNeighborAdvert => SyntaxKind::NdNeighborAdvertKw, - TokenKind::EchoRequest => SyntaxKind::EchoRequestKw, - TokenKind::DestUnreachable => SyntaxKind::DestUnreachableKw, - TokenKind::RouterAdvertisement => SyntaxKind::RouterAdvertisementKw, - TokenKind::TimeExceeded => SyntaxKind::TimeExceededKw, - TokenKind::ParameterProblem => SyntaxKind::ParameterProblemKw, - TokenKind::PacketTooBig => SyntaxKind::PacketTooBigKw, + // Operators - direct mapping + Eq => SyntaxKind::EqOp, + Ne => SyntaxKind::NeOp, + Le => SyntaxKind::LeOp, + Ge => SyntaxKind::GeOp, + Lt => SyntaxKind::LtOp, + Gt => SyntaxKind::GtOp, - TokenKind::Eq => SyntaxKind::EqOp, - TokenKind::Ne => SyntaxKind::NeOp, - TokenKind::Le => SyntaxKind::LeOp, - TokenKind::Ge => SyntaxKind::GeOp, - TokenKind::Lt => SyntaxKind::LtOp, - TokenKind::Gt => SyntaxKind::GtOp, + // Punctuation - direct mapping + LeftBrace => SyntaxKind::LeftBrace, + RightBrace => SyntaxKind::RightBrace, + LeftParen => SyntaxKind::LeftParen, + RightParen => SyntaxKind::RightParen, + LeftBracket => SyntaxKind::LeftBracket, + RightBracket => SyntaxKind::RightBracket, + Comma => SyntaxKind::Comma, + Semicolon => SyntaxKind::Semicolon, + Colon => SyntaxKind::Colon, + Assign => SyntaxKind::Assign, + Dash => SyntaxKind::Dash, + Slash => SyntaxKind::Slash, + Dot => SyntaxKind::Dot, - TokenKind::LeftBrace => SyntaxKind::LeftBrace, - TokenKind::RightBrace => SyntaxKind::RightBrace, - TokenKind::LeftParen => SyntaxKind::LeftParen, - TokenKind::RightParen => SyntaxKind::RightParen, - TokenKind::LeftBracket => SyntaxKind::LeftBracket, - TokenKind::RightBracket => SyntaxKind::RightBracket, - TokenKind::Comma => SyntaxKind::Comma, - TokenKind::Semicolon => SyntaxKind::Semicolon, - TokenKind::Colon => SyntaxKind::Colon, - TokenKind::Assign => SyntaxKind::Assign, - TokenKind::Dash => SyntaxKind::Dash, - TokenKind::Slash => SyntaxKind::Slash, - TokenKind::Dot => SyntaxKind::Dot, + // Literals - map data-carrying variants to their types + StringLiteral(_) => SyntaxKind::StringLiteral, + NumberLiteral(_) => SyntaxKind::NumberLiteral, + IpAddress(_) => SyntaxKind::IpAddress, + Ipv6Address(_) => SyntaxKind::Ipv6Address, + MacAddress(_) => SyntaxKind::MacAddress, + Identifier(_) => SyntaxKind::Identifier, - TokenKind::StringLiteral(_) => SyntaxKind::StringLiteral, - TokenKind::NumberLiteral(_) => SyntaxKind::NumberLiteral, - TokenKind::IpAddress(_) => SyntaxKind::IpAddress, - TokenKind::Ipv6Address(_) => SyntaxKind::Ipv6Address, - TokenKind::MacAddress(_) => SyntaxKind::MacAddress, - TokenKind::Identifier(_) => SyntaxKind::Identifier, + // Special tokens + Newline => SyntaxKind::Newline, + CommentLine(_) => SyntaxKind::Comment, + Shebang(_) => SyntaxKind::Shebang, - TokenKind::Newline => SyntaxKind::Newline, - TokenKind::CommentLine(_) => SyntaxKind::Comment, - TokenKind::Shebang(_) => SyntaxKind::Shebang, - - TokenKind::Error => SyntaxKind::Error, + // Error fallback + Error => SyntaxKind::Error, } } } @@ -324,126 +340,7 @@ impl SyntaxKind { } pub fn from_raw(raw: RawSyntaxKind) -> Self { - match raw.0 { - 0 => SyntaxKind::Root, - 1 => SyntaxKind::Table, - 2 => SyntaxKind::Chain, - 3 => SyntaxKind::Rule, - 4 => SyntaxKind::Set, - 5 => SyntaxKind::Map, - 6 => SyntaxKind::Element, - 7 => SyntaxKind::Expression, - 8 => SyntaxKind::BinaryExpr, - 9 => SyntaxKind::UnaryExpr, - 10 => SyntaxKind::CallExpr, - 11 => SyntaxKind::SetExpr, - 12 => SyntaxKind::RangeExpr, - 13 => SyntaxKind::Statement, - 14 => SyntaxKind::IncludeStmt, - 15 => SyntaxKind::DefineStmt, - 16 => SyntaxKind::FlushStmt, - 17 => SyntaxKind::AddStmt, - 18 => SyntaxKind::DeleteStmt, - 19 => SyntaxKind::Identifier, - 20 => SyntaxKind::StringLiteral, - 21 => SyntaxKind::NumberLiteral, - 22 => SyntaxKind::IpAddress, - 23 => SyntaxKind::Ipv6Address, - 24 => SyntaxKind::MacAddress, - 25 => SyntaxKind::TableKw, - 26 => SyntaxKind::ChainKw, - 27 => SyntaxKind::RuleKw, - 28 => SyntaxKind::SetKw, - 29 => SyntaxKind::MapKw, - 30 => SyntaxKind::ElementKw, - 31 => SyntaxKind::IncludeKw, - 32 => SyntaxKind::DefineKw, - 33 => SyntaxKind::FlushKw, - 34 => SyntaxKind::AddKw, - 35 => SyntaxKind::DeleteKw, - 36 => SyntaxKind::InsertKw, - 37 => SyntaxKind::ReplaceKw, - 38 => SyntaxKind::FilterKw, - 39 => SyntaxKind::NatKw, - 40 => SyntaxKind::RouteKw, - 41 => SyntaxKind::InputKw, - 42 => SyntaxKind::OutputKw, - 43 => SyntaxKind::ForwardKw, - 44 => SyntaxKind::PreroutingKw, - 45 => SyntaxKind::PostroutingKw, - 46 => SyntaxKind::IpKw, - 47 => SyntaxKind::Ip6Kw, - 48 => SyntaxKind::InetKw, - 49 => SyntaxKind::ArpKw, - 50 => SyntaxKind::BridgeKw, - 51 => SyntaxKind::NetdevKw, - 52 => SyntaxKind::TcpKw, - 53 => SyntaxKind::UdpKw, - 54 => SyntaxKind::IcmpKw, - 55 => SyntaxKind::Icmpv6Kw, - 56 => SyntaxKind::SportKw, - 57 => SyntaxKind::DportKw, - 58 => SyntaxKind::SaddrKw, - 59 => SyntaxKind::DaddrKw, - 60 => SyntaxKind::ProtocolKw, - 61 => SyntaxKind::NexthdrKw, - 62 => SyntaxKind::TypeKw, - 63 => SyntaxKind::HookKw, - 64 => SyntaxKind::PriorityKw, - 65 => SyntaxKind::PolicyKw, - 66 => SyntaxKind::IifnameKw, - 67 => SyntaxKind::OifnameKw, - 68 => SyntaxKind::CtKw, - 69 => SyntaxKind::StateKw, - 70 => SyntaxKind::AcceptKw, - 71 => SyntaxKind::DropKw, - 72 => SyntaxKind::RejectKw, - 73 => SyntaxKind::ReturnKw, - 74 => SyntaxKind::JumpKw, - 75 => SyntaxKind::GotoKw, - 76 => SyntaxKind::ContinueKw, - 77 => SyntaxKind::LogKw, - 78 => SyntaxKind::CommentKw, - 79 => SyntaxKind::EstablishedKw, - 80 => SyntaxKind::RelatedKw, - 81 => SyntaxKind::NewKw, - 82 => SyntaxKind::InvalidKw, - 83 => SyntaxKind::EqOp, - 84 => SyntaxKind::NeOp, - 85 => SyntaxKind::LeOp, - 86 => SyntaxKind::GeOp, - 87 => SyntaxKind::LtOp, - 88 => SyntaxKind::GtOp, - 89 => SyntaxKind::LeftBrace, - 90 => SyntaxKind::RightBrace, - 91 => SyntaxKind::LeftParen, - 92 => SyntaxKind::RightParen, - 93 => SyntaxKind::LeftBracket, - 94 => SyntaxKind::RightBracket, - 95 => SyntaxKind::Comma, - 96 => SyntaxKind::Semicolon, - 97 => SyntaxKind::Colon, - 98 => SyntaxKind::Assign, - 99 => SyntaxKind::Dash, - 100 => SyntaxKind::Slash, - 101 => SyntaxKind::Dot, - 102 => SyntaxKind::Whitespace, - 103 => SyntaxKind::Newline, - 104 => SyntaxKind::Comment, - 105 => SyntaxKind::Shebang, - 106 => SyntaxKind::Error, - 107 => SyntaxKind::VmapKw, - 108 => SyntaxKind::NdRouterAdvertKw, - 109 => SyntaxKind::NdNeighborSolicitKw, - 110 => SyntaxKind::NdNeighborAdvertKw, - 111 => SyntaxKind::EchoRequestKw, - 112 => SyntaxKind::DestUnreachableKw, - 113 => SyntaxKind::RouterAdvertisementKw, - 114 => SyntaxKind::TimeExceededKw, - 115 => SyntaxKind::ParameterProblemKw, - 116 => SyntaxKind::PacketTooBigKw, - _ => SyntaxKind::Error, // Fallback to Error for invalid values - } + Self::try_from(raw.0 as u16).unwrap_or(SyntaxKind::Error) } } @@ -1350,7 +1247,7 @@ mod tests { let mut lexer = NftablesLexer::new(source); let tokens = lexer.tokenize().expect("Tokenization should succeed"); - // CST is now implemented - test that it works + // Test CST construction with basic table syntax let green_tree = CstBuilder::build_tree(&tokens); // Verify the tree was created successfully @@ -1367,4 +1264,88 @@ mod tests { let cst_result = CstBuilder::parse_to_cst(&tokens); assert!(cst_result.is_ok()); } + + #[test] + fn test_num_enum_improvements() { + // Test that from_raw uses num_enum for conversion + // Invalid values fall back to Error variant + + // Test valid conversions + assert_eq!(SyntaxKind::from_raw(RawSyntaxKind(0)), SyntaxKind::Root); + assert_eq!(SyntaxKind::from_raw(RawSyntaxKind(1)), SyntaxKind::Table); + assert_eq!(SyntaxKind::from_raw(RawSyntaxKind(25)), SyntaxKind::TableKw); + assert_eq!(SyntaxKind::from_raw(RawSyntaxKind(106)), SyntaxKind::Error); + assert_eq!( + SyntaxKind::from_raw(RawSyntaxKind(116)), + SyntaxKind::PacketTooBigKw + ); + + // Test invalid values automatically fall back to Error + assert_eq!(SyntaxKind::from_raw(RawSyntaxKind(999)), SyntaxKind::Error); + assert_eq!(SyntaxKind::from_raw(RawSyntaxKind(1000)), SyntaxKind::Error); + + // Test bidirectional conversion + for variant in [ + SyntaxKind::Root, + SyntaxKind::Table, + SyntaxKind::TableKw, + SyntaxKind::Error, + SyntaxKind::PacketTooBigKw, + ] { + let raw = variant.to_raw(); + let converted_back = SyntaxKind::from_raw(raw); + assert_eq!(variant, converted_back); + } + } + + #[test] + fn test_token_kind_conversion_improvements() { + // Test that From conversion is complete and correct + use crate::lexer::TokenKind; + + // Test keyword mappings + assert_eq!(SyntaxKind::from(TokenKind::Table), SyntaxKind::TableKw); + assert_eq!(SyntaxKind::from(TokenKind::Chain), SyntaxKind::ChainKw); + assert_eq!(SyntaxKind::from(TokenKind::Accept), SyntaxKind::AcceptKw); + + // Test operators + assert_eq!(SyntaxKind::from(TokenKind::Eq), SyntaxKind::EqOp); + assert_eq!(SyntaxKind::from(TokenKind::Lt), SyntaxKind::LtOp); + + // Test punctuation + assert_eq!( + SyntaxKind::from(TokenKind::LeftBrace), + SyntaxKind::LeftBrace + ); + assert_eq!( + SyntaxKind::from(TokenKind::Semicolon), + SyntaxKind::Semicolon + ); + + // Test literals (with data) + assert_eq!( + SyntaxKind::from(TokenKind::StringLiteral("test".to_string())), + SyntaxKind::StringLiteral + ); + assert_eq!( + SyntaxKind::from(TokenKind::NumberLiteral(42)), + SyntaxKind::NumberLiteral + ); + assert_eq!( + SyntaxKind::from(TokenKind::IpAddress("192.168.1.1".to_string())), + SyntaxKind::IpAddress + ); + assert_eq!( + SyntaxKind::from(TokenKind::Identifier("test".to_string())), + SyntaxKind::Identifier + ); + + // Test special tokens + assert_eq!(SyntaxKind::from(TokenKind::Newline), SyntaxKind::Newline); + assert_eq!( + SyntaxKind::from(TokenKind::CommentLine("# comment".to_string())), + SyntaxKind::Comment + ); + assert_eq!(SyntaxKind::from(TokenKind::Error), SyntaxKind::Error); + } } -- 2.43.0 From 13245c08fe4959cafa5920854911e404eb3e8481 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Mon, 2 Jun 2025 15:24:18 +0300 Subject: [PATCH 22/26] nff: add parsing and CST inspection commands --- README.md | 19 ++++++++++++++ src/cst.rs | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++ src/main.rs | 74 +++++++++++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 161 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 7368374..66f23f0 100644 --- a/README.md +++ b/README.md @@ -94,6 +94,25 @@ nff lint config.nft --style-warnings=false --best-practices=false nff lint config.nft --debug ``` +### Parsing and CST Inspection + +```bash +# Parse and display CST structure for debugging +nff parse /etc/nftables.conf + +# Show tree structure with indentation +nff parse config.nft --tree + +# Show detailed node information +nff parse config.nft --verbose + +# Combined tree and verbose output +nff parse config.nft --tree --verbose + +# Debug output with tokens and CST validation +nff parse config.nft --debug +``` + ## Architecture ### Processing Pipeline diff --git a/src/cst.rs b/src/cst.rs index b36cad0..41ac715 100644 --- a/src/cst.rs +++ b/src/cst.rs @@ -381,6 +381,79 @@ impl CstBuilder { Self::validate_tree(&tree)?; Ok(tree) } + + /// Display CST in a human-readable format for debugging + pub fn display_tree(node: &GreenNode, tree_format: bool, verbose: bool) -> String { + let mut output = String::new(); + Self::display_node_recursive(node, 0, tree_format, verbose, &mut output); + output + } + + fn display_node_recursive( + node: &GreenNode, + indent_level: usize, + tree_format: bool, + verbose: bool, + output: &mut String, + ) { + let kind = SyntaxKind::from_raw(node.kind()); + let indent = if tree_format { + " ".repeat(indent_level) + } else { + String::new() + }; + + if tree_format { + output.push_str(&format!("{}├─ {}", indent, kind)); + } else { + output.push_str(&format!("{}{}", indent, kind)); + } + + if verbose { + output.push_str(&format!( + " (kind: {:?}, width: {:?})", + node.kind(), + node.text_len() + )); + } + + output.push('\n'); + + // Display children + for child in node.children() { + match child { + NodeOrToken::Node(child_node) => { + Self::display_node_recursive( + child_node, + indent_level + 1, + tree_format, + verbose, + output, + ); + } + NodeOrToken::Token(token) => { + let token_indent = if tree_format { + " ".repeat(indent_level + 1) + } else { + String::new() + }; + + let token_kind = SyntaxKind::from_raw(token.kind()); + if tree_format { + output.push_str(&format!("{}├─ {}", token_indent, token_kind)); + } else { + output.push_str(&format!("{}{}", token_indent, token_kind)); + } + + if verbose { + output.push_str(&format!(" (width: {:?})", token.text_len())); + } + + output.push('\n'); + } + } + } + } } /// Internal tree builder that constructs CST according to nftables grammar diff --git a/src/main.rs b/src/main.rs index 3a34967..ea6be4f 100755 --- a/src/main.rs +++ b/src/main.rs @@ -120,6 +120,20 @@ enum Commands { #[arg(long, value_delimiter = ',')] modules: Option>, }, + /// Parse and display file in CST format for debugging + Parse { + /// nftables config file to parse + #[arg(value_name = "FILE")] + file: String, + + /// Show tree structure with indentation + #[arg(long)] + tree: bool, + + /// Show detailed node information + #[arg(long)] + verbose: bool, + }, } fn discover_nftables_files() -> Result> { @@ -228,7 +242,7 @@ fn process_single_file_format( // Read file contents let source = - fs::read_to_string(&file).with_context(|| format!("Failed to read file: {}", file))?; + fs::read_to_string(file).with_context(|| format!("Failed to read file: {}", file))?; // Tokenize let mut lexer = NftablesLexer::new(&source); @@ -278,7 +292,7 @@ fn process_single_file_format( } eprintln!(); } - parsed_ruleset.unwrap_or_else(|| crate::ast::Ruleset::new()) + parsed_ruleset.unwrap_or_else(crate::ast::Ruleset::new) } else { let mut parser = NftablesParser::new(tokens.clone()); parser @@ -399,7 +413,7 @@ fn process_single_file_lint( // Read file contents let source = - fs::read_to_string(&file).with_context(|| format!("Failed to read file: {}", file))?; + fs::read_to_string(file).with_context(|| format!("Failed to read file: {}", file))?; if debug { // Tokenize for debug output @@ -440,9 +454,9 @@ fn process_single_file_lint( let diagnostics = if let Some(modules) = &modules { let module_names: Vec<&str> = modules.iter().map(|s| s.as_str()).collect(); - analyzer.analyze_with_modules(&source, &file, &module_names) + analyzer.analyze_with_modules(&source, file, &module_names) } else { - analyzer.analyze(&source, &file) + analyzer.analyze(&source, file) }; if json { @@ -688,6 +702,51 @@ fn position_from_range(range: &text_size::TextRange, source: &str) -> (usize, us (1, 1) // fallback } +fn process_parse_command(file: String, tree: bool, verbose: bool, debug: bool) -> Result<()> { + let source = + fs::read_to_string(&file).with_context(|| format!("Failed to read file: {}", file))?; + + // Tokenize + let mut lexer = NftablesLexer::new(&source); + let tokens = lexer + .tokenize() + .map_err(|e| FormatterError::ParseError(format!("Tokenization failed: {}", e)))?; + + if debug { + eprintln!("=== TOKENS ==="); + for (i, token) in tokens.iter().enumerate() { + eprintln!( + "{:3}: {:?} @ {:?} = '{}'", + i, token.kind, token.range, token.text + ); + } + eprintln!(); + } + + // Build CST + let cst_tree = CstBuilder::build_tree(&tokens); + + // Validate CST + match CstBuilder::validate_tree(&cst_tree) { + Ok(()) => { + if debug { + eprintln!("CST validation passed"); + eprintln!(); + } + } + Err(e) => { + eprintln!("Warning: CST validation error: {}", e); + eprintln!(); + } + } + + // Display CST + let cst_display = CstBuilder::display_tree(&cst_tree, tree, verbose); + println!("{}", cst_display); + + Ok(()) +} + fn main() -> Result<()> { let args = Args::parse(); @@ -726,6 +785,11 @@ fn main() -> Result<()> { modules.clone(), args.debug, ), + Commands::Parse { + file, + tree, + verbose, + } => process_parse_command(file.clone(), *tree, *verbose, args.debug), }; if let Err(e) = result { -- 2.43.0 From cbfa512d75ca1ee9136bb3b6f782f2512b8fa0c0 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Mon, 2 Jun 2025 16:08:52 +0300 Subject: [PATCH 23/26] docs: improve project description and styling in README --- README.md | 62 ++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 41 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 66f23f0..bb49636 100644 --- a/README.md +++ b/README.md @@ -1,27 +1,33 @@ # nff -This is a high performance configuration parser and written in Rust. The goal is -to receive possibly jumbled up nftables rule files, and output ✨ pretty ✨ -human readable output in return. The main emphasis is on the syntax-aware -formatting with comprehensive grammar support. It is a future goal to allow -editors to hook into nff in order to format your rulesets directly from your -editor, or as a diagnostics source. +This is a high performance, low overhead configuration parser for nftables, +written in Rust. Syntax-aware parsing allows nff to provide a complete formatter +_and_ a linter for nftables. With the formatter the goal is to receive possibly +jumbled up nftables rule files, and output ✨ pretty ✨ human readable output in +return. The linter on another hand, will demonstrate possible syntax, +performance or stylistic errors. + +The main emphasis, however, is on the syntax-aware formatting with comprehensive +grammar support. ## Features -nff is in its early stages of development. While _most_ of the syntax is -supported, I cannot guarantee that _everything_ is supported just yet. +> [!NOTE] +> nff is in its early stages of development. While _most_ of the syntax is +> supported, I cannot guarantee that _everything_ is supported just yet. ### Core Functionality +Basic functionality of nff that most users will be interested in + - **Syntax-aware formatting** - Deep understanding of nftables grammar with semantic preservation -- **Multi-family support** - Handles `inet`, `ip`, `ip6`, `arp`, `bridge`, and - `netdev` table families + - **Multi-family support** - Handles `inet`, `ip`, `ip6`, `arp`, `bridge`, and + `netdev` table families + - **CIDR notation** - Proper handling of network addresses (`192.168.1.0/24`) + - **Chain properties** - Hooks, priorities (including negative), policies, + device bindings - **Flexible indentation** - Configurable tabs/spaces with custom depth -- **CIDR notation** - Proper handling of network addresses (`192.168.1.0/24`) -- **Chain properties** - Hooks, priorities (including negative), policies, - device bindings ### Advanced Features @@ -145,16 +151,26 @@ graph TD ## Installation -Recommended way of installing nff is to use Nix. +Recommended way of installing nff is to use Nix. Add `nff` to your flake inputs, +and add the package to your `environment.systemPackages`. Alternatively, on +non-NixOS systems, it is possible to use `nix profile install` to install nff. ### Editor Integration +> [!TIP] +> Your editor not here? Open an issue. I can only add support for editors I use +> but pull requests documenting alternative editor setups are appreciated! + #### Neovim Setup nff can be integrated into Neovim as a diagnostics source for nftables files. Here are several setup approaches: -##### Option 2: Using none-ls +##### Option 1: Using none-ls + +none-ls is the most common method of adding diagnostics sources in Neovim. While +I recommend using nvim-lint for its simplicity, below instructions document how +to set up null-ls. ```lua local null_ls = require("null-ls") @@ -182,6 +198,9 @@ null_ls.setup({ ##### Option 2: Using nvim-lint (recommended) +Recommended way of adding nff as a diagnostics source in Neovim. Simple, low +overhead and not as error-prone as null-ls. + ```lua -- ~/.config/nvim/lua/config/lint.lua require('lint').linters.nff = { @@ -224,7 +243,8 @@ vim.api.nvim_create_autocmd({ "BufEnter", "BufWritePost" }, { ##### Option 3: Custom Lua Function -For a simple custom solution: +Alternatively, if you don't want to use plugins, consider a setup such as this +one to do it without any reliance on plugins: ```lua -- ~/.config/nvim/lua/nff.lua @@ -564,6 +584,11 @@ style.nft:1:16: warning: Missing space after '{' [NFT503] ## Contributing +### Building + +Build with `cargo build` as usual. If you are using Nix, you will also want to +ensure that the Nix package builds as expected. + ### Code Style - Follow `cargo fmt` formatting @@ -578,11 +603,6 @@ style.nft:1:16: warning: Missing space after '{' [NFT503] - **Regression tests**: Known issue prevention - **Performance tests**: Benchmark critical paths -### Building - -Build with `cargo build` as usual. If you are using Nix, you will also want to -ensure that the Nix package builds as expected. - ## Technical Notes ### CST Implementation -- 2.43.0 From 4803788ffa411a31dd8b40002a1d6450faabaaf2 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Mon, 2 Jun 2025 17:21:26 +0300 Subject: [PATCH 24/26] ci: push nextest config --- .config/nextest.toml | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 .config/nextest.toml diff --git a/.config/nextest.toml b/.config/nextest.toml new file mode 100644 index 0000000..084d4e4 --- /dev/null +++ b/.config/nextest.toml @@ -0,0 +1,27 @@ +[test-groups] +nff = { max-threads = 1 } + +[profile.default] + +# "retries" defines the number of times a test should be retried. If set to a +# non-zero value, tests that succeed on a subsequent attempt will be marked as +# flaky. Can be overridden through the `--retries` option. +retries = 2 + +# This will display all of fail, retry, slow +# see https://nexte.st/book/other-options.html?highlight=failure-output#--status-level-and---final-status-level +status-level = "skip" + +# Treat a test that takes longer than this period as slow, and print a message. +# Given a non-zero positive integer, shutdown the tests when the number periods +# have passed. +slow-timeout = { period = "30s", terminate-after = 4 } + +# * "immediate-final": output failures as soon as they happen and at the end of +# the test run +failure-output = "immediate-final" + +# Do not cancel the test run on the first failure. +fail-fast = false + +test-threads = 2 -- 2.43.0 From ee54f8f8d31ff23ddff3027538fbd86e34e766b3 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Mon, 2 Jun 2025 19:03:12 +0300 Subject: [PATCH 25/26] docs: update rustdoc comments --- src/cst.rs | 6 +- src/diagnostic.rs | 462 ++++++++++++++++++++++++++++++++++++++++------ src/main.rs | 2 - 3 files changed, 410 insertions(+), 60 deletions(-) diff --git a/src/cst.rs b/src/cst.rs index 41ac715..835e23e 100644 --- a/src/cst.rs +++ b/src/cst.rs @@ -150,7 +150,7 @@ pub enum SyntaxKind { // Error recovery Error = 106, - // Additional protocol keywords + // Protocol keywords for nftables VmapKw = 107, NdRouterAdvertKw = 108, NdNeighborSolicitKw = 109, @@ -237,7 +237,7 @@ impl From for SyntaxKind { New => SyntaxKind::NewKw, Invalid => SyntaxKind::InvalidKw, - // Additional protocol keywords + // Protocol keywords for ICMP/ICMPv6 Vmap => SyntaxKind::VmapKw, NdRouterAdvert => SyntaxKind::NdRouterAdvertKw, NdNeighborSolicit => SyntaxKind::NdNeighborSolicitKw, @@ -358,7 +358,7 @@ pub enum CstError { /// Result type for CST operations pub type CstResult = Result; -/// Basic CST builder +/// CST builder for nftables syntax pub struct CstBuilder; impl CstBuilder { diff --git a/src/diagnostic.rs b/src/diagnostic.rs index a3a46b0..820c332 100644 --- a/src/diagnostic.rs +++ b/src/diagnostic.rs @@ -10,9 +10,9 @@ use crate::lexer::LexError; use crate::parser::{ParseError, Parser}; use serde::{Deserialize, Serialize}; -use serde_json; use std::collections::{HashMap, HashSet}; use std::fmt; +use std::net::IpAddr; use text_size::TextSize; /// Diagnostic severity levels following LSP specification @@ -171,10 +171,23 @@ pub struct Position { } impl Position { + /// Creates a new position with line and character coordinates + /// + /// # Parameters + /// * `line` - Zero-based line number + /// * `character` - Zero-based character offset in the line pub fn new(line: u32, character: u32) -> Self { Self { line, character } } + /// Converts a text offset to line and character coordinates + /// + /// # Parameters + /// * `text_size` - Byte offset in the source code + /// * `source` - Complete source text to analyze for line breaks + /// + /// # Returns + /// A position with line and character coordinates corresponding to the text offset pub fn from_text_size(text_size: TextSize, source: &str) -> Self { let mut line = 0; let mut character = 0; @@ -204,10 +217,21 @@ pub struct Range { } impl Range { + /// Creates a new range with start and end positions + /// + /// # Parameters + /// * `start` - Starting position (line and character) + /// * `end` - Ending position (line and character) pub fn new(start: Position, end: Position) -> Self { Self { start, end } } + /// Creates a range that covers only a single position + /// + /// Useful for diagnostics that point to a specific location rather than a range + /// + /// # Parameters + /// * `position` - The position to create a single-point range for pub fn single_position(position: Position) -> Self { Self { start: position.clone(), @@ -266,6 +290,16 @@ pub struct Diagnostic { } impl Diagnostic { + /// Creates a new diagnostic with essential information + /// + /// # Parameters + /// * `range` - The source code range the diagnostic applies to + /// * `severity` - The severity level of the diagnostic + /// * `code` - The diagnostic code indicating the type of issue + /// * `message` - A human-readable description of the diagnostic + /// + /// # Returns + /// A new diagnostic with default values for other fields pub fn new( range: Range, severity: DiagnosticSeverity, @@ -294,6 +328,11 @@ pub struct DiagnosticCollection { } impl DiagnosticCollection { + /// Creates a new diagnostic collection for a file + /// + /// # Parameters + /// * `file_path` - Path to the file being analyzed + /// * `source_text` - The content of the file pub fn new(file_path: String, source_text: String) -> Self { Self { diagnostics: Vec::new(), @@ -302,26 +341,44 @@ impl DiagnosticCollection { } } + /// Adds multiple diagnostics to the collection + /// + /// # Parameters + /// * `diagnostics` - Vector of diagnostics to add pub fn extend(&mut self, diagnostics: Vec) { self.diagnostics.extend(diagnostics); } + /// Returns an iterator over all error-level diagnostics in the collection pub fn errors(&self) -> impl Iterator { self.diagnostics .iter() .filter(|d| d.severity == DiagnosticSeverity::Error) } + /// Checks if the collection contains any error-level diagnostics pub fn has_errors(&self) -> bool { self.errors().count() > 0 } - /// Convert to JSON for LSP or tooling integration + /// Converts the diagnostic collection to JSON format + /// + /// Useful for integrating with Language Server Protocol (LSP) clients + /// or other tools that consume structured diagnostic data. + /// + /// # Returns + /// A JSON string representation of the diagnostic collection, or an error if serialization fails pub fn to_json(&self) -> serde_json::Result { serde_json::to_string_pretty(self) } - /// Convert to a human-readable format + /// Converts the diagnostic collection to a human-readable text format + /// + /// Produces a formatted report suitable for display in a terminal, + /// with file locations, error codes, and code snippets for context. + /// + /// # Returns + /// A formatted string containing all diagnostics with relevant context pub fn to_human_readable(&self) -> String { let mut output = String::new(); @@ -359,6 +416,18 @@ impl DiagnosticCollection { output } + /// Extracts source code lines around a diagnostic location + /// + /// Provides context for a diagnostic by showing the lines of code surrounding + /// the issue, with line numbers and a marker pointing to the problematic line. + /// + /// # Parameters + /// * `range` - The range in the source code to provide context for + /// * `context_lines` - Number of lines to include before and after the range + /// + /// # Returns + /// A vector of formatted strings containing the context lines with line numbers, + /// or None if the range is invalid fn get_context_lines(&self, range: &Range, context_lines: usize) -> Option> { let lines: Vec<&str> = self.source_text.lines().collect(); let start_line = range.start.line as usize; @@ -435,9 +504,7 @@ impl AnalyzerModule for LexicalAnalyzer { let mut lexer = NftablesLexer::new(source); match lexer.tokenize() { - Ok(_) => { - // No lexical errors - } + Ok(_) => {} Err(lex_error) => { let diagnostic = Self::lex_error_to_diagnostic(&lex_error, source); diagnostics.push(diagnostic); @@ -453,6 +520,17 @@ impl AnalyzerModule for LexicalAnalyzer { } impl LexicalAnalyzer { + /// Converts a lexical error to a diagnostic + /// + /// Translates lexer-specific errors into the generic diagnostic format + /// with appropriate severity, code, and location information. + /// + /// # Parameters + /// * `error` - The lexical error to convert + /// * `source` - Source code for position calculation + /// + /// # Returns + /// A diagnostic describing the lexical error pub fn lex_error_to_diagnostic(error: &LexError, source: &str) -> Diagnostic { match error { LexError::InvalidToken { position, text } => { @@ -508,18 +586,14 @@ impl AnalyzerModule for SyntaxAnalyzer { Ok(tokens) => { let mut parser = Parser::new(tokens); match parser.parse() { - Ok(_) => { - // No parse errors - } + Ok(_) => {} Err(parse_error) => { let diagnostic = Self::parse_error_to_diagnostic(&parse_error, source); diagnostics.push(diagnostic); } } } - Err(_) => { - // Already handled in lexical analysis - } + Err(_) => {} } diagnostics @@ -531,6 +605,17 @@ impl AnalyzerModule for SyntaxAnalyzer { } impl SyntaxAnalyzer { + /// Converts a parse error to a diagnostic + /// + /// Translates parser-specific errors into the generic diagnostic format + /// with appropriate severity, code, and meaningful error messages. + /// + /// # Parameters + /// * `error` - The parse error to convert + /// * `_source` - Source code for position calculation + /// + /// # Returns + /// A diagnostic describing the syntax error fn parse_error_to_diagnostic(error: &ParseError, _source: &str) -> Diagnostic { match error { ParseError::UnexpectedToken { @@ -607,12 +692,9 @@ impl AnalyzerModule for StyleAnalyzer { fn analyze(&self, source: &str, config: &DiagnosticConfig) -> Vec { 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)); let diagnostic = Diagnostic::new( @@ -637,6 +719,11 @@ impl AnalyzerModule for StyleAnalyzer { } impl StyleAnalyzer { + /// Analyzes line issues in nftables configuration + /// + /// Detects: + /// - Lines exceeding maximum length + /// - Trailing whitespace at end of lines fn analyze_line_issues(&self, source: &str, config: &DiagnosticConfig) -> Vec { let mut diagnostics = Vec::new(); @@ -680,6 +767,11 @@ impl StyleAnalyzer { diagnostics } + /// Analyzes whitespace issues in nftables configuration + /// + /// Detects: + /// - Too many consecutive empty lines + /// - Trailing empty lines at end of file fn analyze_whitespace_issues( &self, source: &str, @@ -736,6 +828,12 @@ impl StyleAnalyzer { diagnostics } + /// Analyzes indentation consistency in nftables configuration + /// + /// Checks for: + /// - Mixed tabs and spaces in a single line + /// - Inconsistent indentation styles across the file + /// - Adherence to preferred indentation style if specified fn analyze_indentation(&self, source: &str, config: &DiagnosticConfig) -> Vec { let mut diagnostics = Vec::new(); let mut has_tabs = false; @@ -810,23 +908,19 @@ impl AnalyzerModule for SemanticAnalyzer { fn analyze(&self, source: &str, config: &DiagnosticConfig) -> Vec { let mut diagnostics = Vec::new(); - // Always run semantic validation (syntax/semantic errors) diagnostics.extend(self.validate_table_declarations(source)); diagnostics.extend(self.validate_chain_declarations_semantic(source)); diagnostics.extend(self.validate_cidr_notation(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)); } @@ -840,6 +934,11 @@ impl AnalyzerModule for SemanticAnalyzer { } impl SemanticAnalyzer { + /// Validates table declarations in nftables configuration + /// + /// Checks for: + /// - Valid table family (ip, ip6, inet, arp, bridge, netdev) + /// - Duplicate table names fn validate_table_declarations(&self, source: &str) -> Vec { let mut diagnostics = Vec::new(); let mut seen_tables = HashSet::new(); @@ -854,11 +953,8 @@ impl SemanticAnalyzer { let family = parts[1]; let name = parts[2]; - // Validate family match family { - "ip" | "ip6" | "inet" | "arp" | "bridge" | "netdev" => { - // Valid family - } + "ip" | "ip6" | "inet" | "arp" | "bridge" | "netdev" => {} _ => { let start_col = line.find(family).unwrap_or(0); let range = Range::new( @@ -900,6 +996,11 @@ impl SemanticAnalyzer { diagnostics } + /// Validates chain declarations for correct hook types + /// + /// Checks for valid hooks in chain declarations: + /// - input, output, forward, prerouting, postrouting + /// - Reports unknown hooks as errors fn validate_chain_declarations_semantic(&self, source: &str) -> Vec { let mut diagnostics = Vec::new(); @@ -908,7 +1009,6 @@ impl SemanticAnalyzer { let trimmed = line.trim(); if trimmed.starts_with("type ") && trimmed.contains("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,9 +1016,7 @@ impl SemanticAnalyzer { if hook_words.len() >= 2 { let hook = hook_words[1]; match hook { - "input" | "output" | "forward" | "prerouting" | "postrouting" => { - // Valid hook - } + "input" | "output" | "forward" | "prerouting" | "postrouting" => {} _ => { let start_col = line.find(hook).unwrap_or(0); let range = Range::new( @@ -942,6 +1040,10 @@ impl SemanticAnalyzer { diagnostics } + /// Checks for best practices in nftables chain definitions + /// + /// Verifies that: + /// - Filter chains have explicit policies defined fn validate_chain_best_practices(&self, source: &str) -> Vec { let mut diagnostics = Vec::new(); @@ -949,7 +1051,6 @@ impl SemanticAnalyzer { 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), @@ -968,6 +1069,11 @@ impl SemanticAnalyzer { diagnostics } + /// Analyzes nftables configuration for performance optimizations + /// + /// Checks for: + /// - Large sets without timeouts + /// - Rules without counters for monitoring fn check_performance_hints(&self, source: &str) -> Vec { let mut diagnostics = Vec::new(); @@ -975,9 +1081,7 @@ impl SemanticAnalyzer { 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), @@ -1015,6 +1119,10 @@ impl SemanticAnalyzer { diagnostics } + /// Checks for security risks in nftables configuration + /// + /// Identifies: + /// - Overly permissive rules that accept traffic from anywhere (0.0.0.0/0 or ::/0) fn check_security_warnings(&self, source: &str) -> Vec { let mut diagnostics = Vec::new(); @@ -1044,35 +1152,62 @@ impl SemanticAnalyzer { diagnostics } + /// Validates CIDR notation in nftables configuration + /// + /// Checks for: + /// - Valid IP address format (both IPv4 and IPv6) + /// - Valid prefix length (0-32 for IPv4, 0-128 for IPv6) + /// - Valid numeric prefix format fn validate_cidr_notation(&self, source: &str) -> Vec { let mut diagnostics = Vec::new(); for (line_idx, line) in source.lines().enumerate() { let line_num = line_idx as u32; - // Simple CIDR validation without regex dependency + // Look for potential CIDR notation patterns let words: Vec<&str> = line.split_whitespace().collect(); for word in words { if word.contains('/') && word.chars().any(|c| c.is_ascii_digit()) { if let Some(slash_pos) = word.find('/') { let (ip_part, prefix_part) = word.split_at(slash_pos); - let prefix_part = &prefix_part[1..]; // Remove the '/' + let prefix_part = &prefix_part[1..]; - // Basic IPv4 validation - let ip_parts: Vec<&str> = ip_part.split('.').collect(); - if ip_parts.len() == 4 { - let mut valid_ip = true; - for part in ip_parts { - if part.parse::().is_err() { - valid_ip = false; - break; + match ip_part.parse::() { + Ok(ip_addr) => match prefix_part.parse::() { + Ok(prefix) => { + let max_prefix = match ip_addr { + IpAddr::V4(_) => 32, + IpAddr::V6(_) => 128, + }; + + if prefix > max_prefix { + if let Some(start_col) = line.find(word) { + let range = Range::new( + Position::new(line_num, start_col as u32), + Position::new( + line_num, + (start_col + word.len()) as u32, + ), + ); + let ip_type = match ip_addr { + IpAddr::V4(_) => "IPv4", + IpAddr::V6(_) => "IPv6", + }; + let diagnostic = Diagnostic::new( + range, + DiagnosticSeverity::Error, + DiagnosticCode::InvalidCidrNotation, + format!( + "Invalid CIDR prefix length: '{}' (max {} for {})", + prefix, max_prefix, ip_type + ), + ); + diagnostics.push(diagnostic); + } + } } - } - - // Validate prefix length - if valid_ip { - if let Ok(prefix) = prefix_part.parse::() { - if prefix > 32 { + Err(_) => { + if !prefix_part.is_empty() { if let Some(start_col) = line.find(word) { let range = Range::new( Position::new(line_num, start_col as u32), @@ -1086,14 +1221,23 @@ impl SemanticAnalyzer { DiagnosticSeverity::Error, DiagnosticCode::InvalidCidrNotation, format!( - "Invalid CIDR prefix length: '{}' (max 32 for IPv4)", - prefix + "Invalid CIDR prefix: '{}' must be a number", + prefix_part ), ); diagnostics.push(diagnostic); } } - } else if !prefix_part.is_empty() { + } + }, + Err(_) => { + if (ip_part.contains('.') + && ip_part.chars().any(|c| c.is_ascii_digit())) + || (ip_part.contains(':') + && ip_part + .chars() + .any(|c| c.is_ascii_digit() || c.is_ascii_hexdigit())) + { if let Some(start_col) = line.find(word) { let range = Range::new( Position::new(line_num, start_col as u32), @@ -1106,7 +1250,10 @@ impl SemanticAnalyzer { range, DiagnosticSeverity::Error, DiagnosticCode::InvalidCidrNotation, - format!("Invalid CIDR notation: '{}'", word), + format!( + "Invalid IP address in CIDR notation: '{}'", + ip_part + ), ); diagnostics.push(diagnostic); } @@ -1121,6 +1268,10 @@ impl SemanticAnalyzer { diagnostics } + /// Identifies redundant rules in nftables configuration + /// + /// Detects: + /// - Duplicate accept/drop/reject rules fn check_for_redundant_rules(&self, source: &str) -> Vec { let mut diagnostics = Vec::new(); let mut seen_rules = HashSet::new(); @@ -1129,12 +1280,10 @@ impl SemanticAnalyzer { let line_num = line_idx as u32; let trimmed = line.trim(); - // Simple rule detection (lines that contain actions) if trimmed.contains(" accept") || trimmed.contains(" drop") || trimmed.contains(" reject") { - // Check if rule already exists in the HashSet if seen_rules.contains(trimmed) { let range = Range::new( Position::new(line_num, 0), @@ -1148,7 +1297,6 @@ impl SemanticAnalyzer { ); diagnostics.push(diagnostic); } else { - // Add the rule to the HashSet if it's not a duplicate seen_rules.insert(trimmed.to_string()); } } @@ -1164,11 +1312,25 @@ pub struct DiagnosticAnalyzer { } impl DiagnosticAnalyzer { + /// Creates a new diagnostic analyzer with the specified configuration + /// + /// # Parameters + /// * `config` - Configuration settings for the analyzer pub fn new(config: DiagnosticConfig) -> Self { Self { config } } - /// Analyze source code with all available modules + /// Analyzes source code with all standard analysis modules + /// + /// This is the main entry point for a complete analysis of nftables configurations. + /// It runs lexical, syntax, style, and semantic analysis on the provided source. + /// + /// # Parameters + /// * `source` - Source code to analyze + /// * `file_path` - Path to the file being analyzed + /// + /// # Returns + /// A collection of all diagnostics found in the source pub fn analyze(&self, source: &str, file_path: &str) -> DiagnosticCollection { self.analyze_with_modules( source, @@ -1177,7 +1339,17 @@ impl DiagnosticAnalyzer { ) } - /// Analyze source code with specific modules + /// Analyzes source code with specific analysis modules + /// + /// Allows running only selected analysis modules for more targeted diagnostics. + /// + /// # Parameters + /// * `source` - Source code to analyze + /// * `file_path` - Path to the file being analyzed + /// * `module_names` - Names of modules to run ("lexical", "syntax", "style", "semantic") + /// + /// # Returns + /// A collection of diagnostics from the selected modules pub fn analyze_with_modules( &self, source: &str, @@ -1258,4 +1430,184 @@ mod tests { .any(|d| d.code == DiagnosticCode::TrailingWhitespace) ); } + + #[test] + fn test_cidr_validation_ipv4_valid() { + let analyzer = SemanticAnalyzer; + let source = "ip saddr 192.168.1.0/24 accept"; + let diagnostics = analyzer.validate_cidr_notation(source); + + // Verify valid IPv4 CIDR notation doesn't produce errors + assert!( + !diagnostics + .iter() + .any(|d| d.code == DiagnosticCode::InvalidCidrNotation) + ); + } + + #[test] + fn test_cidr_validation_ipv4_invalid_prefix() { + let analyzer = SemanticAnalyzer; + let source = "ip saddr 192.168.1.0/33 accept"; + let diagnostics = analyzer.validate_cidr_notation(source); + + // Verify detection of IPv4 prefix exceeding max (32) + assert!(diagnostics.iter().any(|d| { + d.code == DiagnosticCode::InvalidCidrNotation + && d.message + .contains("Invalid CIDR prefix length: '33' (max 32 for IPv4)") + })); + } + + #[test] + fn test_cidr_validation_ipv6_valid() { + let analyzer = SemanticAnalyzer; + let source = "ip6 saddr 2001:db8::/32 accept"; + let diagnostics = analyzer.validate_cidr_notation(source); + + // Verify valid IPv6 CIDR notation doesn't produce errors + assert!( + !diagnostics + .iter() + .any(|d| d.code == DiagnosticCode::InvalidCidrNotation) + ); + } + + #[test] + fn test_cidr_validation_ipv6_invalid_prefix() { + let analyzer = SemanticAnalyzer; + let source = "ip6 saddr 2001:db8::/129 accept"; + let diagnostics = analyzer.validate_cidr_notation(source); + + // Verify detection of IPv6 prefix exceeding max (128) + assert!(diagnostics.iter().any(|d| { + d.code == DiagnosticCode::InvalidCidrNotation + && d.message + .contains("Invalid CIDR prefix length: '129' (max 128 for IPv6)") + })); + } + + #[test] + fn test_cidr_validation_invalid_ip_address() { + let analyzer = SemanticAnalyzer; + let source = "ip saddr 192.168.1.256/24 accept"; + let diagnostics = analyzer.validate_cidr_notation(source); + + // Verify detection of invalid IP address format + assert!(diagnostics.iter().any(|d| { + d.code == DiagnosticCode::InvalidCidrNotation + && d.message + .contains("Invalid IP address in CIDR notation: '192.168.1.256'") + })); + } + + #[test] + fn test_cidr_validation_invalid_prefix_format() { + let analyzer = SemanticAnalyzer; + let source = "ip saddr 192.168.1.0/abc accept"; + let diagnostics = analyzer.validate_cidr_notation(source); + + // Verify detection of non-numeric CIDR prefix + assert!(diagnostics.iter().any(|d| { + d.code == DiagnosticCode::InvalidCidrNotation + && d.message + .contains("Invalid CIDR prefix: 'abc' must be a number") + })); + } + + #[test] + fn test_cidr_validation_ipv6_compressed_notation() { + let analyzer = SemanticAnalyzer; + let source = "ip6 saddr ::1/128 accept"; + let diagnostics = analyzer.validate_cidr_notation(source); + + // Verify compressed IPv6 notation is properly handled + assert!( + !diagnostics + .iter() + .any(|d| d.code == DiagnosticCode::InvalidCidrNotation) + ); + } + + #[test] + fn test_cidr_validation_multiple_cidrs() { + let analyzer = SemanticAnalyzer; + let source = r#" + ip saddr 192.168.1.0/24 accept + ip6 saddr 2001:db8::/32 accept + ip saddr 10.0.0.0/8 drop + ip6 saddr fe80::/64 accept + "#; + let diagnostics = analyzer.validate_cidr_notation(source); + + // Verify multiple valid CIDRs are properly parsed + assert!( + !diagnostics + .iter() + .any(|d| d.code == DiagnosticCode::InvalidCidrNotation) + ); + } + + #[test] + fn test_cidr_validation_mixed_valid_invalid() { + let analyzer = SemanticAnalyzer; + let source = r#" + ip saddr 192.168.1.0/24 accept + ip saddr 192.168.1.0/33 drop + ip6 saddr 2001:db8::/32 accept + ip6 saddr 2001:db8::/129 drop + "#; + let diagnostics = analyzer.validate_cidr_notation(source); + + // Verify detection of specific invalid prefixes in a mixed content + let cidr_errors: Vec<_> = diagnostics + .iter() + .filter(|d| d.code == DiagnosticCode::InvalidCidrNotation) + .collect(); + assert_eq!(cidr_errors.len(), 2); + + // Check for IPv4 error + assert!( + cidr_errors + .iter() + .any(|d| d.message.contains("33") && d.message.contains("IPv4")) + ); + // Check for IPv6 error + assert!( + cidr_errors + .iter() + .any(|d| d.message.contains("129") && d.message.contains("IPv6")) + ); + } + + #[test] + fn test_cidr_validation_edge_cases() { + let analyzer = SemanticAnalyzer; + + // Test edge case: maximum allowed prefix lengths + let source_ipv4_max = "ip saddr 192.168.1.0/32 accept"; + let diagnostics_ipv4 = analyzer.validate_cidr_notation(source_ipv4_max); + assert!( + !diagnostics_ipv4 + .iter() + .any(|d| d.code == DiagnosticCode::InvalidCidrNotation) + ); + + let source_ipv6_max = "ip6 saddr 2001:db8::/128 accept"; + let diagnostics_ipv6 = analyzer.validate_cidr_notation(source_ipv6_max); + assert!( + !diagnostics_ipv6 + .iter() + .any(|d| d.code == DiagnosticCode::InvalidCidrNotation) + ); + + // Test edge case: minimum prefix (catch-all address) + let source_zero = "ip saddr 0.0.0.0/0 accept"; + let diagnostics_zero = analyzer.validate_cidr_notation(source_zero); + assert!( + !diagnostics_zero + .iter() + .any(|d| d.code == DiagnosticCode::InvalidCidrNotation) + ); + } } diff --git a/src/main.rs b/src/main.rs index ea6be4f..ae1f85c 100755 --- a/src/main.rs +++ b/src/main.rs @@ -506,7 +506,6 @@ fn convert_parse_error_to_formatter_error( suggestion: None, }, ParseError::MissingToken { expected } => { - // Try to find current position from last token let (line, column) = if let Some(last_token) = tokens.last() { position_from_range(&last_token.range, source) } else { @@ -520,7 +519,6 @@ fn convert_parse_error_to_formatter_error( } } ParseError::InvalidExpression { message } => { - // Try to find the current token position let (line, column) = find_current_parse_position(tokens, source); FormatterError::SyntaxError { line, -- 2.43.0 From 03fe062973f8e42dbc3de07638e87591e8f349c5 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Tue, 3 Jun 2025 07:38:10 +0300 Subject: [PATCH 26/26] diagnostic: add test for IPv6 minimum prefix --- src/diagnostic.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/diagnostic.rs b/src/diagnostic.rs index 820c332..4d0c6c9 100644 --- a/src/diagnostic.rs +++ b/src/diagnostic.rs @@ -720,7 +720,7 @@ impl AnalyzerModule for StyleAnalyzer { impl StyleAnalyzer { /// Analyzes line issues in nftables configuration - /// + /// /// Detects: /// - Lines exceeding maximum length /// - Trailing whitespace at end of lines @@ -768,7 +768,7 @@ impl StyleAnalyzer { } /// Analyzes whitespace issues in nftables configuration - /// + /// /// Detects: /// - Too many consecutive empty lines /// - Trailing empty lines at end of file @@ -935,7 +935,7 @@ impl AnalyzerModule for SemanticAnalyzer { impl SemanticAnalyzer { /// Validates table declarations in nftables configuration - /// + /// /// Checks for: /// - Valid table family (ip, ip6, inet, arp, bridge, netdev) /// - Duplicate table names @@ -1609,5 +1609,14 @@ mod tests { .iter() .any(|d| d.code == DiagnosticCode::InvalidCidrNotation) ); + + // Test edge case: IPv6 minimum prefix + let source_ipv6_zero = "ip6 saddr ::/0 accept"; + let diagnostics_ipv6_zero = analyzer.validate_cidr_notation(source_ipv6_zero); + assert!( + !diagnostics_ipv6_zero + .iter() + .any(|d| d.code == DiagnosticCode::InvalidCidrNotation) + ); } } -- 2.43.0