From 5babeedec1f37204031cb009c4a2a0711599aeb0 Mon Sep 17 00:00:00 2001
From: TilakMaddy <tilak.madichetti@gmail.com>
Date: Sun, 15 Dec 2024 16:30:01 +0530
Subject: [PATCH 1/7] feat: warn for payable fallback func without receive func
---
crates/sema/src/typeck/mod.rs | 24 +++++++++++++++++++
.../payable_fallback_without_receive.sol | 15 ++++++++++++
.../payable_fallback_without_receive.stderr | 18 ++++++++++++++
3 files changed, 57 insertions(+)
create mode 100644 tests/ui/parser/payable_fallback_without_receive.sol
create mode 100644 tests/ui/parser/payable_fallback_without_receive.stderr
diff --git a/crates/sema/src/typeck/mod.rs b/crates/sema/src/typeck/mod.rs
index c2bde199..248ed6d7 100644
--- a/crates/sema/src/typeck/mod.rs
+++ b/crates/sema/src/typeck/mod.rs
@@ -11,6 +11,7 @@ pub(crate) fn check(gcx: Gcx<'_>) {
gcx.sess,
gcx.hir.par_contract_ids().for_each(|id| {
check_duplicate_definitions(gcx, &gcx.symbol_resolver.contract_scopes[id]);
+ check_payable_fallback_without_receive(gcx, id);
}),
gcx.hir.par_source_ids().for_each(|id| {
check_duplicate_definitions(gcx, &gcx.symbol_resolver.source_scopes[id]);
@@ -18,6 +19,29 @@ pub(crate) fn check(gcx: Gcx<'_>) {
);
}
+fn check_payable_fallback_without_receive(gcx: Gcx<'_>, contract_id: hir::ContractId) {
+ let mut payable_fallback_found = false;
+ let mut receive_found = false;
+ for item in gcx.hir.contract_items(contract_id) {
+ if let hir::Item::Function(f) = item {
+ if f.kind.is_fallback() && f.state_mutability.is_payable() {
+ payable_fallback_found = true;
+ } else if f.kind.is_receive() {
+ receive_found = true;
+ }
+ }
+ }
+
+ if payable_fallback_found && !receive_found {
+ let contract = gcx.hir.contract(contract_id);
+ gcx.dcx()
+ .warn("contract has a payable fallback function, but no receive ether function")
+ .span(contract.name.span)
+ .help("consider adding a `receive() external payable { ... }`")
+ .emit();
+ }
+}
+
/// Checks for definitions that have the same name and parameter types in the given scope.
fn check_duplicate_definitions(gcx: Gcx<'_>, scope: &Declarations) {
let is_duplicate = |a: Declaration, b: Declaration| -> bool {
diff --git a/tests/ui/parser/payable_fallback_without_receive.sol b/tests/ui/parser/payable_fallback_without_receive.sol
new file mode 100644
index 00000000..f14287f8
--- /dev/null
+++ b/tests/ui/parser/payable_fallback_without_receive.sol
@@ -0,0 +1,15 @@
+contract P1 {
+ fallback() external payable {}
+}
+
+contract P2 is P1 {
+ receive() external payable {}
+}
+
+contract P3 {
+ fallback() external payable {}
+
+ receive() external payable {}
+}
+
+contract P4 is P1 {}
diff --git a/tests/ui/parser/payable_fallback_without_receive.stderr b/tests/ui/parser/payable_fallback_without_receive.stderr
new file mode 100644
index 00000000..40c8196b
--- /dev/null
+++ b/tests/ui/parser/payable_fallback_without_receive.stderr
@@ -0,0 +1,18 @@
+warning: contract has a payable fallback function, but no receive ether function
+ --> ROOT/tests/ui/parser/payable_fallback_without_receive.sol:LL:CC
+ |
+LL | contract P1 {
+ | --
+ |
+ = help: consider adding a `receive() external payable { ... }`
+
+warning: contract has a payable fallback function, but no receive ether function
+ --> ROOT/tests/ui/parser/payable_fallback_without_receive.sol:LL:CC
+ |
+LL | contract P4 is P1 {}
+ | --
+ |
+ = help: consider adding a `receive() external payable { ... }`
+
+warning: 2 warnings emitted
+
From 287a55bf98bf05c18270419f045703baf0d05af1 Mon Sep 17 00:00:00 2001
From: TilakMaddy <tilak.madichetti@gmail.com>
Date: Sun, 15 Dec 2024 20:49:34 +0530
Subject: [PATCH 2/7] fix
---
crates/sema/src/typeck/mod.rs | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/crates/sema/src/typeck/mod.rs b/crates/sema/src/typeck/mod.rs
index 248ed6d7..ab624675 100644
--- a/crates/sema/src/typeck/mod.rs
+++ b/crates/sema/src/typeck/mod.rs
@@ -20,20 +20,14 @@ pub(crate) fn check(gcx: Gcx<'_>) {
}
fn check_payable_fallback_without_receive(gcx: Gcx<'_>, contract_id: hir::ContractId) {
- let mut payable_fallback_found = false;
- let mut receive_found = false;
- for item in gcx.hir.contract_items(contract_id) {
- if let hir::Item::Function(f) = item {
- if f.kind.is_fallback() && f.state_mutability.is_payable() {
- payable_fallback_found = true;
- } else if f.kind.is_receive() {
- receive_found = true;
- }
- }
- }
+ let contract = gcx.hir.contract(contract_id);
+
+ let payable_fallback_found =
+ contract.fallback.is_some_and(|f| gcx.hir.function(f).state_mutability.is_payable());
+
+ let receive_found = contract.receive.is_some();
if payable_fallback_found && !receive_found {
- let contract = gcx.hir.contract(contract_id);
gcx.dcx()
.warn("contract has a payable fallback function, but no receive ether function")
.span(contract.name.span)
From adc0aae865fc74aecbf2f3eebf25b70284b97fa3 Mon Sep 17 00:00:00 2001
From: TilakMaddy <tilak.madichetti@gmail.com>
Date: Wed, 5 Feb 2025 23:22:09 +0530
Subject: [PATCH 3/7] fix
---
crates/sema/src/typeck/mod.rs | 4 +++-
tests/ui/parser/payable_fallback_without_receive.stderr | 8 ++++----
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/crates/sema/src/typeck/mod.rs b/crates/sema/src/typeck/mod.rs
index ab624675..48239015 100644
--- a/crates/sema/src/typeck/mod.rs
+++ b/crates/sema/src/typeck/mod.rs
@@ -5,6 +5,7 @@ use crate::{
};
use rayon::prelude::*;
use solar_data_structures::{map::FxHashSet, parallel};
+use solar_interface::error_code;
pub(crate) fn check(gcx: Gcx<'_>) {
parallel!(
@@ -31,7 +32,8 @@ fn check_payable_fallback_without_receive(gcx: Gcx<'_>, contract_id: hir::Contra
gcx.dcx()
.warn("contract has a payable fallback function, but no receive ether function")
.span(contract.name.span)
- .help("consider adding a `receive() external payable { ... }`")
+ .code(error_code!(3628))
+ .help("consider changing fallback to receive")
.emit();
}
}
diff --git a/tests/ui/parser/payable_fallback_without_receive.stderr b/tests/ui/parser/payable_fallback_without_receive.stderr
index 40c8196b..9fd80558 100644
--- a/tests/ui/parser/payable_fallback_without_receive.stderr
+++ b/tests/ui/parser/payable_fallback_without_receive.stderr
@@ -1,18 +1,18 @@
-warning: contract has a payable fallback function, but no receive ether function
+warning[3628]: contract has a payable fallback function, but no receive ether function
--> ROOT/tests/ui/parser/payable_fallback_without_receive.sol:LL:CC
|
LL | contract P1 {
| --
|
- = help: consider adding a `receive() external payable { ... }`
+ = help: consider changing fallback to receive
-warning: contract has a payable fallback function, but no receive ether function
+warning[3628]: contract has a payable fallback function, but no receive ether function
--> ROOT/tests/ui/parser/payable_fallback_without_receive.sol:LL:CC
|
LL | contract P4 is P1 {}
| --
|
- = help: consider adding a `receive() external payable { ... }`
+ = help: consider changing fallback to receive
warning: 2 warnings emitted
From dc874aa630dc09e7925b7c7de3d4010fcc735a08 Mon Sep 17 00:00:00 2001
From: TilakMaddy <tilak.madichetti@gmail.com>
Date: Wed, 5 Feb 2025 23:29:05 +0530
Subject: [PATCH 4/7] fix
---
crates/sema/src/typeck/mod.rs | 3 +--
tests/ui/parser/payable_fallback_without_receive.stderr | 6 ++----
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/crates/sema/src/typeck/mod.rs b/crates/sema/src/typeck/mod.rs
index 48239015..813a15ed 100644
--- a/crates/sema/src/typeck/mod.rs
+++ b/crates/sema/src/typeck/mod.rs
@@ -31,9 +31,8 @@ fn check_payable_fallback_without_receive(gcx: Gcx<'_>, contract_id: hir::Contra
if payable_fallback_found && !receive_found {
gcx.dcx()
.warn("contract has a payable fallback function, but no receive ether function")
- .span(contract.name.span)
.code(error_code!(3628))
- .help("consider changing fallback to receive")
+ .span_help(contract.name.span, "consider changing fallback to receive")
.emit();
}
}
diff --git a/tests/ui/parser/payable_fallback_without_receive.stderr b/tests/ui/parser/payable_fallback_without_receive.stderr
index 9fd80558..bedbf6b5 100644
--- a/tests/ui/parser/payable_fallback_without_receive.stderr
+++ b/tests/ui/parser/payable_fallback_without_receive.stderr
@@ -2,17 +2,15 @@ warning[3628]: contract has a payable fallback function, but no receive ether fu
--> ROOT/tests/ui/parser/payable_fallback_without_receive.sol:LL:CC
|
LL | contract P1 {
- | --
+ | -- help: consider changing fallback to receive
|
- = help: consider changing fallback to receive
warning[3628]: contract has a payable fallback function, but no receive ether function
--> ROOT/tests/ui/parser/payable_fallback_without_receive.sol:LL:CC
|
LL | contract P4 is P1 {}
- | --
+ | -- help: consider changing fallback to receive
|
- = help: consider changing fallback to receive
warning: 2 warnings emitted
From 5ad42eb05b4444737c6fff5ed8cf956093e335f4 Mon Sep 17 00:00:00 2001
From: TilakMaddy <tilak.madichetti@gmail.com>
Date: Fri, 7 Feb 2025 20:06:35 +0530
Subject: [PATCH 5/7] fix: add warnings to test solidity file
---
tests/ui/parser/payable_fallback_without_receive.sol | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tests/ui/parser/payable_fallback_without_receive.sol b/tests/ui/parser/payable_fallback_without_receive.sol
index f14287f8..90d5bd71 100644
--- a/tests/ui/parser/payable_fallback_without_receive.sol
+++ b/tests/ui/parser/payable_fallback_without_receive.sol
@@ -1,4 +1,5 @@
contract P1 {
+//~^ WARN: contract has a payable fallback function, but no receive ether function
fallback() external payable {}
}
@@ -13,3 +14,5 @@ contract P3 {
}
contract P4 is P1 {}
+//~^ WARN: contract has a payable fallback function, but no receive ether function
+
From c3f1f43478317dea2f698fec565d7cfa5f41f08d Mon Sep 17 00:00:00 2001
From: TilakMaddy <tilak.madichetti@gmail.com>
Date: Fri, 7 Feb 2025 20:14:47 +0530
Subject: [PATCH 6/7] remove space
---
tests/ui/parser/payable_fallback_without_receive.sol | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/ui/parser/payable_fallback_without_receive.sol b/tests/ui/parser/payable_fallback_without_receive.sol
index 90d5bd71..9a5edff5 100644
--- a/tests/ui/parser/payable_fallback_without_receive.sol
+++ b/tests/ui/parser/payable_fallback_without_receive.sol
@@ -1,5 +1,5 @@
contract P1 {
-//~^ WARN: contract has a payable fallback function, but no receive ether function
+//~^WARN: contract has a payable fallback function, but no receive ether function
fallback() external payable {}
}
@@ -14,5 +14,5 @@ contract P3 {
}
contract P4 is P1 {}
-//~^ WARN: contract has a payable fallback function, but no receive ether function
+//~^WARN: contract has a payable fallback function, but no receive ether function
From 843313581ad4202911c2f2b954b823d2756b80f4 Mon Sep 17 00:00:00 2001
From: DaniPopes <57450786+DaniPopes@users.noreply.github.com>
Date: Fri, 7 Feb 2025 16:17:40 +0100
Subject: [PATCH 7/7] fix span and ui_test workaround
---
crates/interface/src/diagnostics/context.rs | 5 ++++-
crates/sema/src/hir/mod.rs | 5 +++++
crates/sema/src/typeck/mod.rs | 21 +++++++++----------
.../payable_fallback_without_receive.sol | 5 ++---
.../payable_fallback_without_receive.stderr | 14 +++++++++----
5 files changed, 31 insertions(+), 19 deletions(-)
diff --git a/crates/interface/src/diagnostics/context.rs b/crates/interface/src/diagnostics/context.rs
index 5b22c1c4..9bb4828e 100644
--- a/crates/interface/src/diagnostics/context.rs
+++ b/crates/interface/src/diagnostics/context.rs
@@ -345,7 +345,10 @@ impl DiagCtxtInner {
match (self.deduplicated_err_count, self.deduplicated_warn_count) {
(0, 0) => Ok(()),
(0, w) => {
- self.emitter.emit_diagnostic(&Diag::new(Level::Warning, warnings(w)));
+ // TODO: Don't emit in tests since it's not handled by `ui_test`.
+ if self.flags.track_diagnostics {
+ self.emitter.emit_diagnostic(&Diag::new(Level::Warning, warnings(w)));
+ }
Ok(())
}
(e, 0) => self.emit_diagnostic(Diag::new(Level::Error, errors(e))),
diff --git a/crates/sema/src/hir/mod.rs b/crates/sema/src/hir/mod.rs
index 26cbcb65..7395d2b9 100644
--- a/crates/sema/src/hir/mod.rs
+++ b/crates/sema/src/hir/mod.rs
@@ -612,6 +612,11 @@ pub struct Function<'hir> {
}
impl Function<'_> {
+ /// Returns the span of the `kind` keyword.
+ pub fn keyword_span(&self) -> Span {
+ self.span.with_hi(self.span.lo() + self.kind.to_str().len() as u32)
+ }
+
/// Returns `true` if this is a free function, meaning it is not part of a contract.
pub fn is_free(&self) -> bool {
self.contract.is_none()
diff --git a/crates/sema/src/typeck/mod.rs b/crates/sema/src/typeck/mod.rs
index 813a15ed..2adf6795 100644
--- a/crates/sema/src/typeck/mod.rs
+++ b/crates/sema/src/typeck/mod.rs
@@ -23,17 +23,16 @@ pub(crate) fn check(gcx: Gcx<'_>) {
fn check_payable_fallback_without_receive(gcx: Gcx<'_>, contract_id: hir::ContractId) {
let contract = gcx.hir.contract(contract_id);
- let payable_fallback_found =
- contract.fallback.is_some_and(|f| gcx.hir.function(f).state_mutability.is_payable());
-
- let receive_found = contract.receive.is_some();
-
- if payable_fallback_found && !receive_found {
- gcx.dcx()
- .warn("contract has a payable fallback function, but no receive ether function")
- .code(error_code!(3628))
- .span_help(contract.name.span, "consider changing fallback to receive")
- .emit();
+ if let Some(fallback) = contract.fallback {
+ let fallback = gcx.hir.function(fallback);
+ if fallback.state_mutability.is_payable() && contract.receive.is_none() {
+ gcx.dcx()
+ .warn("contract has a payable fallback function, but no receive ether function")
+ .span(contract.name.span)
+ .code(error_code!(3628))
+ .span_help(fallback.keyword_span(), "consider changing `fallback` to `receive`")
+ .emit();
+ }
}
}
diff --git a/tests/ui/parser/payable_fallback_without_receive.sol b/tests/ui/parser/payable_fallback_without_receive.sol
index 9a5edff5..2e6254ad 100644
--- a/tests/ui/parser/payable_fallback_without_receive.sol
+++ b/tests/ui/parser/payable_fallback_without_receive.sol
@@ -1,5 +1,5 @@
contract P1 {
-//~^WARN: contract has a payable fallback function, but no receive ether function
+ //~^ WARN: contract has a payable fallback function, but no receive ether function
fallback() external payable {}
}
@@ -14,5 +14,4 @@ contract P3 {
}
contract P4 is P1 {}
-//~^WARN: contract has a payable fallback function, but no receive ether function
-
+//~^ WARN: contract has a payable fallback function, but no receive ether function
diff --git a/tests/ui/parser/payable_fallback_without_receive.stderr b/tests/ui/parser/payable_fallback_without_receive.stderr
index bedbf6b5..9ce0a9fb 100644
--- a/tests/ui/parser/payable_fallback_without_receive.stderr
+++ b/tests/ui/parser/payable_fallback_without_receive.stderr
@@ -2,15 +2,21 @@ warning[3628]: contract has a payable fallback function, but no receive ether fu
--> ROOT/tests/ui/parser/payable_fallback_without_receive.sol:LL:CC
|
LL | contract P1 {
- | -- help: consider changing fallback to receive
+ | --
+LL |
+LL | fallback() external payable {}
+ | -------- help: consider changing `fallback` to `receive`
|
warning[3628]: contract has a payable fallback function, but no receive ether function
--> ROOT/tests/ui/parser/payable_fallback_without_receive.sol:LL:CC
|
+LL | fallback() external payable {}
+ | -------- help: consider changing `fallback` to `receive`
+LL | }
+...
+LL |
LL | contract P4 is P1 {}
- | -- help: consider changing fallback to receive
+ | --
|
-warning: 2 warnings emitted
-