Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix broken trait or type in impl ingot check #863

Merged
merged 2 commits into from
Mar 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions crates/analyzer/src/namespace/items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1789,12 +1789,14 @@ impl ImplId {
&self,
db: &dyn AnalyzerDb,
sink: &mut impl DiagnosticSink,
type_module: Option<ModuleId>,
type_ingot: Option<IngotId>,
) {
let impl_module = self.data(db).module;
let is_allowed = match type_module {
None => impl_module == self.data(db).trait_id.module(db),
Some(val) => val == impl_module || self.data(db).trait_id.module(db) == impl_module,
let impl_ingot = self.data(db).module.ingot(db);
let is_allowed = match type_ingot {
None => impl_ingot == self.data(db).trait_id.module(db).ingot(db),
Some(val) => {
val == impl_ingot || self.data(db).trait_id.module(db).ingot(db) == impl_ingot
}
};

if !is_allowed {
Expand Down Expand Up @@ -1831,10 +1833,10 @@ impl ImplId {
vec![],
)),
Type::Struct(id) => {
self.validate_type_or_trait_is_in_ingot(db, sink, Some(id.module(db)))
self.validate_type_or_trait_is_in_ingot(db, sink, Some(id.module(db).ingot(db)))
}
Type::Enum(id) => {
self.validate_type_or_trait_is_in_ingot(db, sink, Some(id.module(db)))
self.validate_type_or_trait_is_in_ingot(db, sink, Some(id.module(db).ingot(db)))
}
Type::Base(_) | Type::Array(_) | Type::Tuple(_) | Type::String(_) => {
self.validate_type_or_trait_is_in_ingot(db, sink, None)
Expand Down
2 changes: 1 addition & 1 deletion crates/analyzer/src/traversal/expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1381,7 +1381,7 @@ fn expr_call_pure(

if function.is_test(context.db()) {
context.fancy_error(
&format!("`{}` is a test function", fn_name),
&format!("`{fn_name}` is a test function"),
vec![Label::primary(call_span, "test functions are not callable")],
vec![],
);
Expand Down
16 changes: 8 additions & 8 deletions crates/fe/src/task/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ pub fn test(args: TestArgs) {
test_ingot(&args)
};

println!("{}", test_sink);
println!("{test_sink}");
if test_sink.failure_count() != 0 {
std::process::exit(1)
}
Expand All @@ -38,7 +38,7 @@ fn test_single_file(args: &TestArgs) -> TestSink {
let mut db = fe_driver::Db::default();
let content = match std::fs::read_to_string(input_path) {
Err(err) => {
eprintln!("Failed to load file: `{}`. Error: {}", input_path, err);
eprintln!("Failed to load file: `{input_path}`. Error: {err}");
std::process::exit(1)
}
Ok(content) => content,
Expand All @@ -51,7 +51,7 @@ fn test_single_file(args: &TestArgs) -> TestSink {
sink
}
Err(error) => {
eprintln!("Unable to compile {}.", input_path);
eprintln!("Unable to compile {input_path}.");
print_diagnostics(&db, &error.0);
std::process::exit(1)
}
Expand All @@ -60,7 +60,7 @@ fn test_single_file(args: &TestArgs) -> TestSink {

pub fn execute_tests(module_name: &str, tests: &[CompiledTest], sink: &mut TestSink) {
if tests.len() == 1 {
println!("executing 1 test in {}:", module_name);
println!("executing 1 test in {module_name}:");
} else {
println!("executing {} tests in {}:", tests.len(), module_name);
}
Expand All @@ -83,18 +83,18 @@ fn test_ingot(args: &TestArgs) -> TestSink {
let optimize = args.optimize.unwrap_or(true);

if !Path::new(input_path).exists() {
eprintln!("Input directory does not exist: `{}`.", input_path);
eprintln!("Input directory does not exist: `{input_path}`.");
std::process::exit(1)
}

let content = match load_files_from_dir(input_path) {
Ok(files) if files.is_empty() => {
eprintln!("Input directory is not an ingot: `{}`", input_path);
eprintln!("Input directory is not an ingot: `{input_path}`");
std::process::exit(1)
}
Ok(files) => files,
Err(err) => {
eprintln!("Failed to load project files. Error: {}", err);
eprintln!("Failed to load project files. Error: {err}");
std::process::exit(1)
}
};
Expand All @@ -110,7 +110,7 @@ fn test_ingot(args: &TestArgs) -> TestSink {
sink
}
Err(error) => {
eprintln!("Unable to compile {}.", input_path);
eprintln!("Unable to compile {input_path}.");
print_diagnostics(&db, &error.0);
std::process::exit(1)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
pub struct MyS {
fn x() -> u256 {
return 10
}
}

pub trait Trait {
fn x() -> u256;
}
13 changes: 13 additions & 0 deletions crates/test-files/fixtures/ingots/trait_ingot_check/src/main.fe
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
use foo::{MyS, Trait}

impl Trait for MyS {
fn x() -> u256 {
return 10
}
}

contract Foo {
pub fn main() -> u256 {
return MyS::x()
}
}
6 changes: 3 additions & 3 deletions crates/test-runner/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ impl Display for TestSink {

let test_description = |n: usize, status: &dyn Display| -> String {
if n == 1 {
format!("1 test {}", status)
format!("1 test {status}")
} else {
format!("{} tests {}", n, status)
format!("{n} tests {status}")
}
};

Expand Down Expand Up @@ -85,7 +85,7 @@ pub fn execute(name: &str, bytecode: &str, sink: &mut TestSink) -> bool {
if reverted {
sink.inc_success_count();
} else {
sink.insert_failure(name, &format!("{:?}", result));
sink.insert_failure(name, &format!("{result:?}"));
};

reverted
Expand Down
7 changes: 7 additions & 0 deletions crates/tests-legacy/src/ingots.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,13 @@ fn test_trait_no_ambiguity() {
})
}

#[test]
fn test_trait_ingot_check() {
with_executor(&|mut executor| {
let _harness = deploy_ingot(&mut executor, "trait_ingot_check", "Foo", &[]);
})
}

#[test]
fn test_ingot_pub_contract() {
with_executor(&|mut executor| {
Expand Down
8 changes: 8 additions & 0 deletions newsfragments/863.feature.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Fixed broken trait orphan rule

Fe has an orphan rule for Traits similar to Rust's that requires
that either the trait or the type that we are implementing the trait for
are located in the same ingot as the `impl`. This rule was implemented
incorrectly so that instead of requiring them to be in the same ingot,
they were required to be in the same module. This change fixes this
so that the orphan rule is enforced correctly.