name: transient should prefix with t_

README.md

@@ -45,6 +45,7 @@ This command ensures that development [best practices](https://book.getfoundry.s
 
 - Test names follow a convention of `^test(Fork)?(Fuzz)?(_Revert(If|When|On))?_(\w+)*$`. (To see a list of example valid test names, see [here](https://github.com/ScopeLift/scopelint/blob/1857e3940bfe92ac5a136827374f4b27ff083971/src/check/validators/test_names.rs#L106-L127)).
 - Constants and immutables are in `ALL_CAPS`.
+- Transient storage variables are prefixed with `t_` (e.g. `t_reentrancyFlag`).
 - Function names and visibility in forge scripts only have 1 public `run` method per script.
 - Internal or private functions in the source directory start with a leading underscore.
 

src/check/validators/variable_names.rs

@@ -19,6 +19,7 @@ fn is_matching_file(parsed: &Parsed) -> bool {
 /// - Storage variables should NOT have an underscore prefix
 /// - Non-storage variables (local variables, parameters) should have an underscore prefix
 /// - Variables that reference storage/storages should NOT have an underscore prefix
+/// - Transient storage variables should be prefixed with `t_` (e.g. `t_reentrancyFlag`)
 pub fn validate(parsed: &Parsed) -> Vec<InvalidItem> {
     if !is_matching_file(parsed) {
         return Vec::new();
@@ -90,7 +91,20 @@ fn validate_function(parsed: &Parsed, f: &FunctionDefinition) -> Vec<InvalidItem
 fn validate_state_variable(parsed: &Parsed, v: &VariableDefinition) -> Option<InvalidItem> {
     v.name.as_ref().and_then(|name| {
         let name_str = &name.name;
-        if is_valid_state_variable_name(name_str) {
+        let is_transient = is_transient_state_variable(&parsed.src, name.loc);
+
+        if is_transient {
+            if name_str.starts_with("t_") {
+                None
+            } else {
+                Some(InvalidItem::new(
+                    ValidatorKind::Variable,
+                    parsed,
+                    name.loc,
+                    format!("Transient state variable '{name_str}' should be prefixed with 't_'"),
+                ))
+            }
+        } else if is_valid_state_variable_name(name_str) {
             None
         } else {
             Some(InvalidItem::new(
@@ -183,6 +197,190 @@ fn is_valid_state_variable_name(name: &str) -> bool {
     !name.starts_with('_')
 }
 
+/// Detects whether a state variable declaration uses the `transient` keyword by scanning the
+/// original source before the variable name.
+fn is_transient_state_variable(src: &str, name_loc: solang_parser::pt::Loc) -> bool {
+    let start = name_loc.start();
+    if start > src.len() {
+        return false;
+    }
+
+    let decl_start = declaration_start_before_name(src, start);
+    contains_keyword_before_name(src, decl_start, start, "transient")
+}
+
+#[derive(Clone, Copy, Eq, PartialEq)]
+enum ScanState {
+    Code,
+    LineComment,
+    BlockComment,
+    SingleQuote,
+    DoubleQuote,
+}
+
+fn declaration_start_before_name(src: &str, end: usize) -> usize {
+    let bytes = src.as_bytes();
+    let mut idx = 0;
+    let mut last_boundary = 0;
+    let mut state = ScanState::Code;
+
+    while idx < end {
+        match state {
+            ScanState::Code => match bytes[idx] {
+                b'/' if idx + 1 < end && bytes[idx + 1] == b'/' => {
+                    state = ScanState::LineComment;
+                    idx += 2;
+                }
+                b'/' if idx + 1 < end && bytes[idx + 1] == b'*' => {
+                    state = ScanState::BlockComment;
+                    idx += 2;
+                }
+                b'\'' => {
+                    state = ScanState::SingleQuote;
+                    idx += 1;
+                }
+                b'"' => {
+                    state = ScanState::DoubleQuote;
+                    idx += 1;
+                }
+                b';' | b'{' | b'}' => {
+                    last_boundary = idx + 1;
+                    idx += 1;
+                }
+                _ => idx += 1,
+            },
+            ScanState::LineComment => {
+                if bytes[idx] == b'\n' {
+                    state = ScanState::Code;
+                }
+                idx += 1;
+            }
+            ScanState::BlockComment => {
+                if bytes[idx] == b'*' && idx + 1 < end && bytes[idx + 1] == b'/' {
+                    state = ScanState::Code;
+                    idx += 2;
+                } else {
+                    idx += 1;
+                }
+            }
+            ScanState::SingleQuote => {
+                if bytes[idx] == b'\\' {
+                    idx = (idx + 2).min(end);
+                } else {
+                    if bytes[idx] == b'\'' {
+                        state = ScanState::Code;
+                    }
+                    idx += 1;
+                }
+            }
+            ScanState::DoubleQuote => {
+                if bytes[idx] == b'\\' {
+                    idx = (idx + 2).min(end);
+                } else {
+                    if bytes[idx] == b'"' {
+                        state = ScanState::Code;
+                    }
+                    idx += 1;
+                }
+            }
+        }
+    }
+
+    last_boundary
+}
+
+fn contains_keyword_before_name(src: &str, start: usize, end: usize, keyword: &str) -> bool {
+    let bytes = src.as_bytes();
+    let keyword = keyword.as_bytes();
+    let mut idx = start;
+    let mut state = ScanState::Code;
+
+    while idx < end {
+        match state {
+            ScanState::Code => {
+                if idx + keyword.len() <= end &&
+                    &bytes[idx..idx + keyword.len()] == keyword &&
+                    is_keyword_boundary(bytes, start, idx, idx + keyword.len(), end)
+                {
+                    return true;
+                }
+
+                match bytes[idx] {
+                    b'/' if idx + 1 < end && bytes[idx + 1] == b'/' => {
+                        state = ScanState::LineComment;
+                        idx += 2;
+                    }
+                    b'/' if idx + 1 < end && bytes[idx + 1] == b'*' => {
+                        state = ScanState::BlockComment;
+                        idx += 2;
+                    }
+                    b'\'' => {
+                        state = ScanState::SingleQuote;
+                        idx += 1;
+                    }
+                    b'"' => {
+                        state = ScanState::DoubleQuote;
+                        idx += 1;
+                    }
+                    _ => idx += 1,
+                }
+            }
+            ScanState::LineComment => {
+                if bytes[idx] == b'\n' {
+                    state = ScanState::Code;
+                }
+                idx += 1;
+            }
+            ScanState::BlockComment => {
+                if bytes[idx] == b'*' && idx + 1 < end && bytes[idx + 1] == b'/' {
+                    state = ScanState::Code;
+                    idx += 2;
+                } else {
+                    idx += 1;
+                }
+            }
+            ScanState::SingleQuote => {
+                if bytes[idx] == b'\\' {
+                    idx = (idx + 2).min(end);
+                } else {
+                    if bytes[idx] == b'\'' {
+                        state = ScanState::Code;
+                    }
+                    idx += 1;
+                }
+            }
+            ScanState::DoubleQuote => {
+                if bytes[idx] == b'\\' {
+                    idx = (idx + 2).min(end);
+                } else {
+                    if bytes[idx] == b'"' {
+                        state = ScanState::Code;
+                    }
+                    idx += 1;
+                }
+            }
+        }
+    }
+
+    false
+}
+
+const fn is_keyword_boundary(
+    bytes: &[u8],
+    search_start: usize,
+    keyword_start: usize,
+    keyword_end: usize,
+    search_end: usize,
+) -> bool {
+    let before = keyword_start == search_start || !is_identifier_char(bytes[keyword_start - 1]);
+    let after = keyword_end == search_end || !is_identifier_char(bytes[keyword_end]);
+    before && after
+}
+
+const fn is_identifier_char(byte: u8) -> bool {
+    byte.is_ascii_alphanumeric() || byte == b'_'
+}
+
 fn is_valid_local_variable_name(name: &str, is_storage: bool) -> bool {
     if is_storage {
         // Storage variables should NOT have underscore prefix
@@ -320,4 +518,83 @@ mod tests {
         };
         expected_findings.assert_eq(content, &validate);
     }
+
+    #[test]
+    fn test_transient_state_variable_with_correct_prefix() {
+        let content = r"
+            contract MyContract {
+                uint128 transient t_reentrancyFlag;
+            }
+        ";
+
+        let expected_findings = ExpectedFindings::new(0);
+        expected_findings.assert_eq(content, &validate);
+    }
+
+    #[test]
+    fn test_transient_state_variable_without_prefix() {
+        let content = r"
+            contract MyContract {
+                uint128 transient reentrancyFlag;
+            }
+        ";
+
+        let expected_findings = ExpectedFindings {
+            src: 1,
+            test: 1,
+            handler: 1,
+            script: 1,
+            ..ExpectedFindings::default()
+        };
+        expected_findings.assert_eq(content, &validate);
+    }
+
+    #[test]
+    fn test_same_line_state_variable_after_transient() {
+        let content = r"
+            contract MyContract {
+                uint128 transient t_reentrancyFlag; uint128 normalFlag;
+            }
+        ";
+
+        let expected_findings = ExpectedFindings::new(0);
+        expected_findings.assert_eq(content, &validate);
+    }
+
+    #[test]
+    fn test_transient_state_variable_ignored_with_rule_directive() {
+        let content = r"
+            contract MyContract {
+                // scopelint: ignore-variable-next-line
+                uint128 transient reentrancyFlag;
+            }
+        ";
+
+        let expected_findings = ExpectedFindings::new(0);
+        expected_findings.assert_eq(content, &validate);
+    }
+
+    #[test]
+    fn test_transient_state_variable_ignored_with_ignore_line() {
+        let content = r"
+            contract MyContract {
+                uint128 transient reentrancyFlag; // scopelint: ignore-line
+            }
+        ";
+
+        let expected_findings = ExpectedFindings::new(0);
+        expected_findings.assert_eq(content, &validate);
+    }
+
+    #[test]
+    fn test_comment_text_does_not_mark_state_variable_transient() {
+        let content = r"
+            contract MyContract {
+                uint128 /* transient */ reentrancyFlag;
+            }
+        ";
+
+        let expected_findings = ExpectedFindings::new(0);
+        expected_findings.assert_eq(content, &validate);
+    }
 }

src/parser.rs

@@ -8,6 +8,9 @@ use std::sync::LazyLock;
 static TRANSIENT_KEYWORD: LazyLock<Regex> =
     LazyLock::new(|| Regex::new(r"\btransient\b").expect("transient regex is valid"));
 const TRANSIENT_REPLACEMENT: &str = "         ";
+// Replacement must be the same length as the keyword so all byte offsets (line numbers,
+// comment and inline-config locations) remain valid against the original source.
+const _: () = assert!(TRANSIENT_REPLACEMENT.len() == "transient".len());
 
 /// Parses Solidity source code, with a fallback that strips unsupported keywords (e.g.
 /// `transient`).

tests/check-proj1-AllFindings/test/Counter.t.sol

@@ -7,6 +7,7 @@ contract CounterTest is Test {
   uint256 constant testVal = 123;
 
   Counter public counter;
+  uint128 transient reentrancyFlag;
 
   function setUp() public {
     counter = new Counter();

tests/check-proj2-NoFindings/test/Counter.t.sol

@@ -7,7 +7,7 @@ contract CounterTest is Test {
   uint256 constant TEST_VAL = 123;
 
   Counter public counter;
-  uint128 transient transientVar;
+  uint128 transient t_reentrancyFlag;
 
   function setUp() public {
     counter = new Counter();

tests/check.rs

@@ -52,7 +52,7 @@ fn test_check_proj1_all_findings() {
         "Invalid src method name in ./src/CounterIgnored3.sol on line 1: Missing SPDX-License-Identifier header",
         "Invalid src method name in ./src/CounterIgnored4.sol on line 1: Missing SPDX-License-Identifier header",
         "Invalid src method name in ./src/CounterIgnored4.sol on line 29: missingLeadingUnderscoreAndNotIgnored",
-        "Invalid test name in ./test/Counter.t.sol on line 16: testIncrementBadName",
+        "Invalid test name in ./test/Counter.t.sol on line 17: testIncrementBadName",
         "Invalid directive in ./src/Counter.sol: Invalid inline config item: this directive is invalid",
         "Invalid variable name in ./script/Counter.s.sol on line 25: Local variable 'x' should have underscore prefix",
         "Invalid variable name in ./src/Counter.sol on line 19: Parameter 'newNumber' should have underscore prefix",
@@ -68,8 +68,9 @@ fn test_check_proj1_all_findings() {
         "Invalid variable name in ./src/CounterIgnored3.sol on line 39: Parameter 'someImportantUser' should have underscore prefix",
         "Invalid variable name in ./src/CounterIgnored3.sol on line 48: Parameter 'someImportantUser' should have underscore prefix",
         "Invalid variable name in ./src/CounterIgnored3.sol on line 7: State variable '_GOOD__IMMUTABLE_' should NOT have underscore prefix",
-        "Invalid variable name in ./test/Counter.t.sol on line 31: Local variable 'x' should have underscore prefix",
-        "Invalid variable name in ./test/Counter.t.sol on line 21: Parameter 'x' should have underscore prefix",
+        "Invalid variable name in ./test/Counter.t.sol on line 32: Local variable 'x' should have underscore prefix",
+        "Invalid variable name in ./test/Counter.t.sol on line 22: Parameter 'x' should have underscore prefix",
+        "Invalid variable name in ./test/Counter.t.sol on line 10: Transient state variable 'reentrancyFlag' should be prefixed with 't_'",
         "Invalid error name in ./src/Counter.sol on line 40: Error 'AnotherInvalidError' should be prefixed with 'Counter_'",
         "Invalid error name in ./src/Counter.sol on line 39: Error 'InvalidError' should be prefixed with 'Counter_'",
         "Invalid EIP712 typehash in ./src/Counter.sol: EIP712 typehash 'PERMIT_TYPEHASH' parameter mismatch: typehash defines 5 parameters but abi.encode usage uses 3 parameters",