Skip to content

Commit c99bda4

Browse files
committed
feat(v2): diagnose duplicate override specifiers
Reject functions, fallback/receive functions, modifiers, and state variables that carry more than one `override` specifier, matching solc's ParserError 1827/9125/9102. Closes the SDR[9] and SDR[12] validation TODOs and the `override` part of SDR[1730] in the IR builder.
1 parent 4534d6a commit c99bda4

20 files changed

Lines changed: 261 additions & 19 deletions

File tree

crates/solidity-v2/outputs/cargo/common/generated/public_api.txt

Lines changed: 28 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/solidity-v2/outputs/cargo/common/src/diagnostics/kinds/syntax/mod.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
mod extra_terminal;
22
mod multiple_mutability_specifiers;
3+
mod multiple_override_specifiers;
34
mod multiple_virtual_specifiers;
45
mod unexpected_eof;
56
mod unexpected_terminal;
67
mod unsupported_syntax;
78

89
pub use extra_terminal::ExtraTerminal;
910
pub use multiple_mutability_specifiers::MultipleMutabilitySpecifiers;
11+
pub use multiple_override_specifiers::MultipleOverrideSpecifiers;
1012
pub use multiple_virtual_specifiers::MultipleVirtualSpecifiers;
1113
use serde::Serialize;
1214
pub use unexpected_eof::UnexpectedEof;
@@ -37,5 +39,8 @@ define_diagnostic_kind! {
3739

3840
/// More than one `virtual` specifier was provided on a definition.
3941
MultipleVirtualSpecifiers(MultipleVirtualSpecifiers),
42+
43+
/// More than one `override` specifier was provided on a definition.
44+
MultipleOverrideSpecifiers(MultipleOverrideSpecifiers),
4045
}
4146
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
use serde::Serialize;
2+
3+
use crate::diagnostics::extensions::DiagnosticExtensions;
4+
use crate::diagnostics::severity::DiagnosticSeverity;
5+
6+
/// Diagnostic emitted when a definition carries more than one `override`
7+
/// specifier.
8+
#[derive(Clone, Debug, Eq, PartialEq, Serialize)]
9+
pub struct MultipleOverrideSpecifiers;
10+
11+
impl DiagnosticExtensions for MultipleOverrideSpecifiers {
12+
fn severity(&self) -> DiagnosticSeverity {
13+
DiagnosticSeverity::Error
14+
}
15+
16+
fn code(&self) -> &'static str {
17+
"syntax/multiple-override-specifiers"
18+
}
19+
20+
fn message(&self) -> String {
21+
"Only a single override specifier can be provided.".to_string()
22+
}
23+
}

crates/solidity-v2/outputs/cargo/ir/src/ir/builder/mod.rs

Lines changed: 36 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use std::sync::Arc;
22

33
use slang_solidity_v2_common::diagnostics::kinds::syntax::{
4-
MultipleMutabilitySpecifiers, MultipleVirtualSpecifiers,
4+
MultipleMutabilitySpecifiers, MultipleOverrideSpecifiers, MultipleVirtualSpecifiers,
55
};
66
use slang_solidity_v2_common::diagnostics::DiagnosticCollection;
77
use slang_solidity_v2_cst::structured_cst::nodes as input;
@@ -226,7 +226,6 @@ impl<S: Source> CstToIrBuilder<'_, S> {
226226
None
227227
}
228228
}));
229-
// TODO(validation) SDR[9]: function definitions can have only a single override specifier
230229
let override_specifier = self.function_override_specifier(&source.attributes);
231230
let modifier_invocations = self.function_modifier_invocations(&source.attributes);
232231
let returns = source
@@ -470,13 +469,13 @@ impl<S: Source> CstToIrBuilder<'_, S> {
470469
&mut self,
471470
attributes: &input::FunctionAttributes,
472471
) -> Option<output::OverridePaths> {
473-
attributes.elements.iter().find_map(|attribute| {
472+
self.extract_single_override(attributes.elements.iter().filter_map(|attribute| {
474473
if let input::FunctionAttribute::OverrideSpecifier(specifier) = attribute {
475-
Some(self.build_override_specifier_as_paths(specifier))
474+
Some(specifier)
476475
} else {
477476
None
478477
}
479-
})
478+
}))
480479
}
481480

482481
fn function_modifier_invocations(
@@ -646,13 +645,13 @@ impl<S: Source> CstToIrBuilder<'_, S> {
646645
&mut self,
647646
attributes: &input::FallbackFunctionAttributes,
648647
) -> Option<output::OverridePaths> {
649-
attributes.elements.iter().find_map(|attribute| {
648+
self.extract_single_override(attributes.elements.iter().filter_map(|attribute| {
650649
if let input::FallbackFunctionAttribute::OverrideSpecifier(specifier) = attribute {
651-
Some(self.build_override_specifier_as_paths(specifier))
650+
Some(specifier)
652651
} else {
653652
None
654653
}
655-
})
654+
}))
656655
}
657656

658657
fn fallback_function_modifier_invocations(
@@ -721,13 +720,13 @@ impl<S: Source> CstToIrBuilder<'_, S> {
721720
&mut self,
722721
attributes: &input::ReceiveFunctionAttributes,
723722
) -> Option<output::OverridePaths> {
724-
attributes.elements.iter().find_map(|attribute| {
723+
self.extract_single_override(attributes.elements.iter().filter_map(|attribute| {
725724
if let input::ReceiveFunctionAttribute::OverrideSpecifier(specifier) = attribute {
726-
Some(self.build_override_specifier_as_paths(specifier))
725+
Some(specifier)
727726
} else {
728727
None
729728
}
730-
})
729+
}))
731730
}
732731

733732
fn receive_function_modifier_invocations(
@@ -761,7 +760,6 @@ impl<S: Source> CstToIrBuilder<'_, S> {
761760
let visibility = output::FunctionVisibility::Internal;
762761
// mutability is irrelevant for modifiers
763762
let mutability = output::FunctionMutability::NonPayable;
764-
// TODO(validation) SDR[1730]: modifiers can have only a single override specifier
765763
let virtual_keyword =
766764
self.extract_first_virtual(source.attributes.elements.iter().filter_map(|attribute| {
767765
if let input::ModifierAttribute::VirtualKeyword(virtual_keyword) = attribute {
@@ -795,13 +793,13 @@ impl<S: Source> CstToIrBuilder<'_, S> {
795793
&mut self,
796794
attributes: &input::ModifierAttributes,
797795
) -> Option<output::OverridePaths> {
798-
attributes.elements.iter().find_map(|attribute| {
796+
self.extract_single_override(attributes.elements.iter().filter_map(|attribute| {
799797
if let input::ModifierAttribute::OverrideSpecifier(specifier) = attribute {
800-
Some(self.build_override_specifier_as_paths(specifier))
798+
Some(specifier)
801799
} else {
802800
None
803801
}
804-
})
802+
}))
805803
}
806804

807805
//
@@ -833,14 +831,13 @@ impl<S: Source> CstToIrBuilder<'_, S> {
833831
&mut self,
834832
attributes: &input::StateVariableAttributes,
835833
) -> Option<output::OverridePaths> {
836-
// TODO(validation) SDR[12]: only one override specifier is allowed
837-
attributes.elements.iter().find_map(|attribute| {
834+
self.extract_single_override(attributes.elements.iter().filter_map(|attribute| {
838835
if let input::StateVariableAttribute::OverrideSpecifier(specifier) = attribute {
839-
Some(self.build_override_specifier_as_paths(specifier))
836+
Some(specifier)
840837
} else {
841838
None
842839
}
843-
})
840+
}))
844841
}
845842

846843
fn state_variable_as_constant_definition(
@@ -1029,4 +1026,24 @@ impl<S: Source> CstToIrBuilder<'_, S> {
10291026

10301027
Some(first)
10311028
}
1029+
1030+
/// Extracts and transforms the first `override` specifier, emitting an
1031+
/// error for any subsequent ones.
1032+
fn extract_single_override<'a>(
1033+
&mut self,
1034+
specifiers: impl IntoIterator<Item = &'a input::OverrideSpecifier>,
1035+
) -> Option<output::OverridePaths> {
1036+
let mut specifiers = specifiers.into_iter();
1037+
let first = self.build_override_specifier_as_paths(specifiers.next()?);
1038+
1039+
for specifier in specifiers {
1040+
self.diagnostics.push(
1041+
self.file_id.to_owned(),
1042+
specifier.calculate_text_range().unwrap(),
1043+
MultipleOverrideSpecifiers,
1044+
);
1045+
}
1046+
1047+
Some(first)
1048+
}
10321049
}

crates/solidity-v2/outputs/cargo/tests/src/diagnostics_output/snapshots.generated.rs

Lines changed: 29 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/solidity-v2/testing/snapshots/diagnostics_output/syntax/multiple_override_specifiers/fallback_functions/generated/slang/0.8.0-failure.txt

Lines changed: 11 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/solidity-v2/testing/snapshots/diagnostics_output/syntax/multiple_override_specifiers/fallback_functions/generated/solc/0.8.0-failure.txt

Lines changed: 11 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
// SPDX-License-Identifier: MIT
2+
pragma solidity *;
3+
4+
contract C {
5+
fallback() external override override {}
6+
}

crates/solidity-v2/testing/snapshots/diagnostics_output/syntax/multiple_override_specifiers/functions/generated/slang/0.8.0-failure.txt

Lines changed: 11 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/solidity-v2/testing/snapshots/diagnostics_output/syntax/multiple_override_specifiers/functions/generated/solc/0.8.0-failure.txt

Lines changed: 11 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)