feat(lint): add UnsafeTypecast lint #11046

TropicalDog17 · 2025-07-19T10:35:31Z

Motivation

Closes #11029

Solution

PR Checklist

Added Tests
Added Documentation
Breaking changes

Cargo.toml

crates/lint/testdata/UnsafeTypecast.sol

0xClandestine · 2025-07-19T16:17:46Z

crates/lint/testdata/UnsafeTypecast.stderr

+warning[unsafe-typecast]: typecasts that can truncate values should be avoided
+ --> ROOT/testdata/UnsafeTypecast.sol:LL:CC
+  |
+5 |         uint128 b = uint128(a);


uint128 -> uint128 is safe
uintN -> uintN is always safe
intN -> intN is always safe

a is an address type so i think it still unsafe

Definitely ensure the compiler isn't catching some of these, cannot directly convert an address to uint128 without a compile error.

0xClandestine · 2025-07-19T16:18:37Z

crates/lint/testdata/UnsafeTypecast.stderr

+warning[unsafe-typecast]: typecasts that can truncate values should be avoided
+  --> ROOT/testdata/UnsafeTypecast.sol:LL:CC
+   |
+94 |         uint128 a = uint128(1000000000000000000000000000000000000000);


Pretty sure the compiler prevents you from downcasting a literal that's smaller than the target type size.

0xClandestine · 2025-07-19T16:20:57Z

crates/lint/testdata/UnsafeTypecast.stderr

+warning[unsafe-typecast]: typecasts that can truncate values should be avoided
+ --> ROOT/testdata/UnsafeTypecast.sol:LL:CC
+  |
+8 |         uint64 c = uint64(a);
+  |                    ---------
+  |
+  = help: https://book.getfoundry.sh/reference/forge/forge-lint#unsafe-typecast


We'll need to emit a suggested fix as outlined in #11046

Can checkout my UnwrappedModifierLogic lint for reference #10967

crates/lint/testdata/UnsafeTypecast.sol

0xClandestine · 2025-07-22T19:33:22Z

Any updates, can update the tests for you if needed @TropicalDog17

…foundry into feat/unsafe-typecast-lint

TropicalDog17 · 2025-07-23T07:09:47Z

Hi @0xClandestine, I've updated the test, but not sure how to emit a helpful fix for the lint, can you help me on that? Thank a lot!

0xrusowsky · 2025-07-23T08:24:35Z

Hi @0xClandestine, I've updated the test, but not sure how to emit a helpful fix for the lint, can you help me on that? Thank a lot!

maybe in this case a warning is enough, and no fix suggestion is required, but you could tell them to consider adding an explicit check like:

if (a > type(uint32).max) revert TypecastOverflow();

0xrusowsky

please address the feedback

crates/lint/src/sol/med/unsafe_typecast.rs

0xrusowsky · 2025-07-23T09:31:12Z

crates/lint/src/sol/med/unsafe_typecast.rs

+            solar_ast::LitKind::Number(num) => {
+                if is_negative_number_literal(num) {
+                    Some(hir::ElementaryType::Int(solar_ast::TypeSize::ZERO))
+                } else {
+                    Some(hir::ElementaryType::UInt(solar_ast::TypeSize::ZERO))
+                }
+            }


the compiler performs checks when assigning literal values, i think it is safe to remove this check, which would also remove the need of the bigint dep.

Please return None and add a comment explaining the reason why

Assigning values which cannot fit into the type gives a compiler error. For example:
uint8 foo = 300;

The largest value an uint8 can hold is (2 8) - 1 = 255. So, the compiler says:
value 300 does not fit into type uint8

ref: https://solang.readthedocs.io/en/latest/language/types.html

crates/lint/testdata/UnsafeTypecast.sol

Cargo.toml

crates/lint/testdata/UnsafeTypecast.sol

…foundry into feat/unsafe-typecast-lint

0xClandestine

Looking good, some nits 🤝

0xClandestine · 2025-07-24T15:01:20Z

crates/lint/testdata/UnsafeTypecast.stderr

+   = note: Consider disabling this lint only if you're certain the cast is safe:
+
+           - // Ensure the value fits in the target type before casting
+           - // forge-lint: disable-next-line(unsafe-typecast)
+           - // Cast is safe because [explain why]


Nice, some small nits.

Let's make the snippet an addition rather than deletion.

Minor changes to note and suggestion.

Suggested change

= note: Consider disabling this lint only if you're certain the cast is safe:

- // Ensure the value fits in the target type before casting

- // forge-lint: disable-next-line(unsafe-typecast)

- // Cast is safe because [explain why]

= note: Consider disabling this lint if you're certain the cast is safe:

+ // Cast is safe because [explain why]

+ // forge-lint: disable-next-line(unsafe-typecast)

0xClandestine · 2025-07-24T15:02:06Z

crates/lint/src/sol/med/unsafe_typecast.rs

+    UNSAFE_TYPECAST,
+    Severity::Med,
+    "unsafe-typecast",
+    "typecasts that can truncate values should be avoided"


Suggested change

"typecasts that can truncate values should be avoided"

"typecasts that can truncate values should be checked"

0xClandestine · 2025-07-25T16:15:01Z

crates/lint/src/sol/med/unsafe_typecast.rs

+        desc: Some("Consider disabling this lint only if you're certain the cast is safe:"),
+        code: "// Ensure the value fits in the target type before casting\n// forge-lint: disable-next-line(unsafe-typecast)\n// Cast is safe because [explain why]".to_string(),


Suggested change

desc: Some("Consider disabling this lint only if you're certain the cast is safe:"),

code: "// Ensure the value fits in the target type before casting\n// forge-lint: disable-next-line(unsafe-typecast)\n// Cast is safe because [explain why]".to_string(),

desc: Some("Consider disabling this lint if you're certain the cast is safe:"),

code: "// Cast is safe because [explain why]".to_string()\n// forge-lint: disable-next-line(unsafe-typecast),

0xClandestine · 2025-07-25T16:26:43Z

crates/lint/testdata/UnsafeTypecast.sol

@@ -0,0 +1,219 @@
+contract UnsafeTypecast {
+    function upcastSafeUint() public pure {


Mb, can we get a bytes32 version of this as well?

0xClandestine · 2025-07-25T16:27:06Z

crates/lint/testdata/UnsafeTypecast.sol

+        int248 E = int248(D);
+        int256 F = int256(E);
+    }
+    function downcastUnsafeUint() public pure {


Can we also get a bytes32 version of this?

crates/lint/testdata/UnsafeTypecast.sol

bet

e8aafe0

TropicalDog17 requested review from DaniPopes, klkvr and mattsse as code owners July 19, 2025 10:35

github-project-automation bot added this to Foundry Jul 19, 2025

TropicalDog17 requested review from grandizzy, yash-atreya and zerosnacks as code owners July 19, 2025 10:35

Merge branch 'master' into feat/unsafe-typecast-lint

0455b31

0xClandestine suggested changes Jul 19, 2025

View reviewed changes

0xClandestine reviewed Jul 19, 2025

View reviewed changes

crates/lint/testdata/UnsafeTypecast.sol Show resolved Hide resolved

TropicalDog17 added 4 commits July 23, 2025 06:57

add more tests, emit fix

befa78a

Merge branch 'feat/unsafe-typecast-lint' of github.com:TropicalDog17/…

34e6a68

…foundry into feat/unsafe-typecast-lint

remove unused deps

45f5bda

test: remove compiler error test

dff52fa

TropicalDog17 added 2 commits July 23, 2025 14:09

Merge branch 'master' into feat/unsafe-typecast-lint

2768187

lint

47e80e4

0xrusowsky requested changes Jul 23, 2025

View reviewed changes

0xrusowsky reviewed Jul 23, 2025

View reviewed changes

crates/lint/testdata/UnsafeTypecast.sol Outdated Show resolved Hide resolved

0xrusowsky reviewed Jul 23, 2025

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

0xClandestine reviewed Jul 23, 2025

View reviewed changes

crates/lint/testdata/UnsafeTypecast.sol Outdated Show resolved Hide resolved

0xClandestine reviewed Jul 23, 2025

View reviewed changes

crates/lint/testdata/UnsafeTypecast.sol Outdated Show resolved Hide resolved

TropicalDog17 added 2 commits July 24, 2025 07:40

refactor

3614115

Merge branch 'feat/unsafe-typecast-lint' of github.com:TropicalDog17/…

01210d8

…foundry into feat/unsafe-typecast-lint

TropicalDog17 requested review from 0xrusowsky and 0xClandestine July 24, 2025 07:44

Merge branch 'master' into feat/unsafe-typecast-lint

3d4f355

0xClandestine suggested changes Jul 25, 2025

View reviewed changes

	"typecasts that can truncate values should be avoided"
	"typecasts that can truncate values should be checked"

		desc: Some("Consider disabling this lint only if you're certain the cast is safe:"),
		code: "// Ensure the value fits in the target type before casting\n// forge-lint: disable-next-line(unsafe-typecast)\n// Cast is safe because [explain why]".to_string(),

		@@ -0,0 +1,219 @@
		contract UnsafeTypecast {
		function upcastSafeUint() public pure {

feat(lint): add UnsafeTypecast lint #11046

Are you sure you want to change the base?

feat(lint): add UnsafeTypecast lint #11046

Conversation

TropicalDog17 commented Jul 19, 2025

Motivation

Solution

PR Checklist

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xClandestine Jul 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

0xClandestine commented Jul 22, 2025

Uh oh!

TropicalDog17 commented Jul 23, 2025

Uh oh!

0xrusowsky commented Jul 23, 2025

Uh oh!

0xrusowsky left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

0xrusowsky Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

0xClandestine left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

0xClandestine Jul 19, 2025 •

edited

Loading

0xrusowsky Jul 23, 2025 •

edited

Loading