forge_lint/sol/info/
literal_instead_of_constant.rs

1use super::LiteralInsteadOfConstant;
2use crate::{
3    linter::{LateLintPass, LintContext},
4    sol::{Severity, SolLint},
5};
6use alloy_primitives::{Address, U256};
7use solar::{
8    ast::{LitKind, StrKind, UnOpKind},
9    interface::Span,
10    sema::{
11        Gcx,
12        hir::{self, Expr, ExprKind, Hir, Lit, Stmt, StmtKind, Visit},
13        ty::TyKind,
14    },
15};
16use std::{collections::HashMap, convert::Infallible, ops::ControlFlow};
17
18declare_forge_lint!(
19    LITERAL_INSTEAD_OF_CONSTANT,
20    Severity::Info,
21    "literal-instead-of-constant",
22    "this literal appears multiple times in the contract"
23);
24
25impl<'gcx> LateLintPass<'gcx> for LiteralInsteadOfConstant {
26    fn check_nested_contract(&mut self, ctx: &LintContext, gcx: Gcx<'gcx>, id: hir::ContractId) {
27        // Group the literals of the contract's own functions and modifiers by semantic value;
28        // inherited items group with their declaring contract. Collection covers the executable
29        // expressions: the body statements, and the modifier and base-constructor arguments of
30        // the header. Parameter and return types stay out, so a fixed array size in a signature
31        // is a type annotation rather than a repeated value.
32        let mut collector = LiteralCollector { gcx, groups: HashMap::new() };
33        let functions = gcx.hir.contract(id).functions();
34        for function in functions.map(|id| gcx.hir.function(id)) {
35            for modifier in function.modifiers {
36                let _ = collector.visit_modifier(modifier);
37            }
38            for stmt in function.body.iter().flat_map(|body| body.stmts) {
39                let _ = collector.visit_stmt(stmt);
40            }
41        }
42        // A value used in one single place is fine: only repetitions report. Emissions are
43        // sorted by position so the output does not depend on the map's iteration order.
44        let mut repeated: Vec<Span> =
45            collector.groups.into_values().filter(|spans| spans.len() > 1).flatten().collect();
46        repeated.sort_by_key(|span| span.lo());
47        for span in repeated {
48            ctx.emit(&LITERAL_INSTEAD_OF_CONSTANT, span);
49        }
50    }
51}
52
53/// The semantic value of a literal, the grouping key: two spellings of the same number
54/// (`100`, `0x64`, `1e2`) or the same unit-scaled amount (`1 ether`, `1e18`) are one value.
55/// A numeric literal under a value-changing unary operator denotes a DISTINCT constant, so
56/// `-5` and `~5` never group with the bare `5`.
57#[derive(PartialEq, Eq, Hash)]
58enum LiteralValue {
59    Number(Option<UnOpKind>, U256),
60    Address(Address),
61    HexString(Vec<u8>),
62}
63
64/// Collects the grouping-relevant literals of a subtree: numbers above 2, address literals
65/// and hex string literals. A bare literal indexing an array-like value or bounding a slice
66/// stays out as positional, matching Aderyn; a mapping key counts, it is configuration data.
67struct LiteralCollector<'gcx> {
68    gcx: Gcx<'gcx>,
69    groups: HashMap<LiteralValue, Vec<Span>>,
70}
71
72impl<'gcx> LiteralCollector<'gcx> {
73    /// Records one literal under its semantic grouping key, `op` being the value-changing
74    /// unary operator applied to it, if any.
75    fn record_lit(&mut self, lit: &Lit<'_>, span: Span, op: Option<UnOpKind>) {
76        let key = match &lit.kind {
77            // `0`, `1` and `2` are structural rather than configuration values.
78            LitKind::Number(v) if *v > U256::from(2u64) => LiteralValue::Number(op, *v),
79            LitKind::Address(address) => LiteralValue::Address(*address),
80            LitKind::Str(StrKind::Hex, bytes, _) => {
81                LiteralValue::HexString(bytes.as_byte_str().to_vec())
82            }
83            _ => return,
84        };
85        self.groups.entry(key).or_default().push(span);
86    }
87
88    /// Walks `expr` unless it is a bare literal in a positional role.
89    fn visit_unless_bare_lit(&mut self, expr: &'gcx Expr<'gcx>) {
90        if !is_lit(expr) {
91            let _ = self.visit_expr(expr);
92        }
93    }
94}
95
96fn is_lit(expr: &Expr<'_>) -> bool {
97    matches!(expr.peel_parens().kind, ExprKind::Lit(..))
98}
99
100impl<'gcx> Visit<'gcx> for LiteralCollector<'gcx> {
101    type BreakValue = Infallible;
102
103    fn hir(&self) -> &'gcx Hir<'gcx> {
104        &self.gcx.hir
105    }
106
107    fn visit_stmt(&mut self, stmt: &'gcx Stmt<'gcx>) -> ControlFlow<Self::BreakValue> {
108        // Yul literals commonly encode structural values such as memory offsets, masks, and
109        // selectors, where extracting a constant would not necessarily improve readability.
110        if matches!(stmt.kind, StmtKind::AssemblyBlock(_)) {
111            return ControlFlow::Continue(());
112        }
113        self.walk_stmt(stmt)
114    }
115
116    fn visit_expr(&mut self, expr: &'gcx Expr<'gcx>) -> ControlFlow<Self::BreakValue> {
117        let is_value_changing =
118            |op: &hir::UnOp| matches!(op.kind, UnOpKind::Neg | UnOpKind::BitNot);
119        match &expr.kind {
120            // A bare literal indexing an array-like value (`arr[3]`) is positional, not a
121            // magic value; a mapping key (`m[500]`) is configuration data and counts.
122            ExprKind::Index(base, index) => {
123                let _ = self.visit_expr(base);
124                if let Some(index) = index {
125                    let is_mapping = matches!(
126                        self.gcx.type_of_expr(base.peel_parens().id).map(|ty| ty.peel_refs().kind),
127                        Some(TyKind::Mapping(..))
128                    );
129                    if is_mapping {
130                        let _ = self.visit_expr(index);
131                    } else {
132                        self.visit_unless_bare_lit(index);
133                    }
134                }
135            }
136            // A bare literal shift amount (`x << 128`, `acc >>= 128`) and bare slice bounds
137            // (`d[555:600]`) are structural too: slices only exist on array-like values.
138            ExprKind::Binary(lhs, op, rhs) | ExprKind::Assign(lhs, Some(op), rhs)
139                if op.kind.is_shift() =>
140            {
141                let _ = self.visit_expr(lhs);
142                self.visit_unless_bare_lit(rhs);
143            }
144            ExprKind::Slice(base, start, end) => {
145                let _ = self.visit_expr(base);
146                for bound in [start, end].into_iter().flatten() {
147                    self.visit_unless_bare_lit(bound);
148                }
149            }
150            ExprKind::Unary(op, operand) if is_value_changing(op) => {
151                match &operand.peel_parens().kind {
152                    // `-5` / `~5`: record the operator-qualified value without descending into the
153                    // operand, which would re-record the bare magnitude.
154                    ExprKind::Lit(lit) => self.record_lit(lit, expr.span, Some(op.kind)),
155                    // A nested unary over a literal (`-(-5)`, `~~5`) folds to a value that is
156                    // neither this operator's nor the bare literal's; canonicalizing it is not
157                    // worth it, so the chain is skipped rather than miss-keyed. A non-literal
158                    // operand deeper down (`-(-(x + 500))`) still records its own literals.
159                    ExprKind::Unary(inner, inner_operand)
160                        if is_value_changing(inner) && is_lit(inner_operand) => {}
161                    _ => return self.walk_expr(expr),
162                }
163            }
164            ExprKind::Lit(lit) => self.record_lit(lit, lit.span, None),
165            _ => return self.walk_expr(expr),
166        }
167        ControlFlow::Continue(())
168    }
169}
forge_lint/sol/info/literal_instead_of_constant.rs

forge_lint/sol/info/
literal_instead_of_constant.rs
