Skip to main content

gobject_linter/rules/
strcmp_explicit_comparison.rs

1use gobject_ast::model::{BinaryOp, Expression, FileModel, FunctionDefItem, UnaryOp};
2
3use crate::{
4    ast_context::AstContext,
5    config::Config,
6    rules::{Fix, Rule, Violation},
7};
8
9pub struct StrcmpExplicitComparison;
10
11impl Rule for StrcmpExplicitComparison {
12    fn name(&self) -> &'static str {
13        "strcmp_explicit_comparison"
14    }
15
16    fn description(&self) -> &'static str {
17        "Require explicit comparison with 0 for strcmp/g_strcmp0 (returns 0 for equality, not TRUE)"
18    }
19
20    fn category(&self) -> crate::rules::Category {
21        crate::rules::Category::Correctness
22    }
23
24    fn fixable(&self) -> bool {
25        true
26    }
27
28    fn check_func_impl(
29        &self,
30        _ast_context: &AstContext,
31        _config: &Config,
32        func: &FunctionDefItem,
33        file: &FileModel,
34        violations: &mut Vec<Violation>,
35    ) {
36        for stmt in &func.body_statements {
37            for if_stmt in stmt.iter_if_statements() {
38                self.check_condition(&if_stmt.condition, file, violations);
39            }
40        }
41    }
42}
43
44impl StrcmpExplicitComparison {
45    fn check_condition(
46        &self,
47        condition: &Expression,
48        file: &FileModel,
49
50        violations: &mut Vec<Violation>,
51    ) {
52        match condition {
53            // Binary expression: check if it's a comparison with strcmp, or recurse for logical ops
54            Expression::Binary(binary) => {
55                // If it's a comparison operator, don't flag strcmp calls on either side
56                // (they already have explicit comparison)
57                match binary.operator {
58                    BinaryOp::Equal
59                    | BinaryOp::NotEqual
60                    | BinaryOp::Less
61                    | BinaryOp::LessEqual
62                    | BinaryOp::Greater
63                    | BinaryOp::GreaterEqual => {
64                        // Don't recurse - strcmp calls here are OK
65                    }
66                    // For logical operators, recurse into both sides
67                    BinaryOp::LogicalAnd | BinaryOp::LogicalOr => {
68                        self.check_condition(&binary.left, file, violations);
69                        self.check_condition(&binary.right, file, violations);
70                    }
71                    _ => {
72                        // For other binary operators, recurse
73                        self.check_condition(&binary.left, file, violations);
74                        self.check_condition(&binary.right, file, violations);
75                    }
76                }
77            }
78            // Bare call: if (strcmp(a, b)) or if (g_strcmp0(a, b))
79            Expression::Call(call)
80                if call
81                    .function_name_str()
82                    .is_some_and(|name| self.is_str_compare(name)) =>
83            {
84                let func_name = call.function_name();
85                // Fix: add "!= 0" after the call
86                let fix = Fix::new(
87                    call.location.end_byte,
88                    call.location.end_byte,
89                    " != 0".to_string(),
90                );
91
92                violations.push(self.violation_with_fix_at(
93                    &file.path,
94                    &call.location,
95                    format!(
96                        "{}() returns 0 for equality — add explicit comparison: '{}(...) != 0'",
97                        func_name, func_name
98                    ),
99                    fix,
100                ));
101            }
102            // Negated call: if (!strcmp(a, b)) or if (!g_strcmp0(a, b))
103            Expression::Unary(unary) if unary.operator == UnaryOp::Not => {
104                if let Expression::Call(call) = &*unary.operand
105                    && call
106                        .function_name_str()
107                        .is_some_and(|name| self.is_str_compare(name))
108                {
109                    let func_name = call.function_name();
110                    // Fix: remove the '!' and add ' == 0' after the call
111                    let fixes = vec![
112                        // Remove the '!' operator
113                        Fix::delete(unary.location.start_byte, call.location.start_byte),
114                        // Add ' == 0' after the call
115                        Fix::new(
116                            call.location.end_byte,
117                            call.location.end_byte,
118                            " == 0".to_string(),
119                        ),
120                    ];
121
122                    violations.push(self.violation_with_fixes_at(
123                        &file.path,
124                        &call.location,
125                        format!(
126                            "{}() returns 0 for equality — use '{}(...) == 0' instead of '!{}(...)'",
127                            func_name, func_name, func_name
128                        ),
129                        fixes,
130                    ));
131                }
132            }
133            _ => {}
134        }
135    }
136
137    fn is_str_compare(&self, func_name: &str) -> bool {
138        matches!(func_name, "strcmp" | "g_strcmp0")
139    }
140}