Skip to content

Commit

Permalink
Merge pull request #1739 from fzyzcjy/feat/11840
Browse files Browse the repository at this point in the history
Improve hints when using non-meaningful `&mut`
  • Loading branch information
fzyzcjy authored Feb 8, 2024
2 parents f4d5f48 + 067a503 commit 7bebc95
Show file tree
Hide file tree
Showing 6 changed files with 113 additions and 13 deletions.
22 changes: 18 additions & 4 deletions frb_codegen/src/library/codegen/parser/function_parser/argument.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::codegen::ir::field::{IrField, IrFieldSettings};
use crate::codegen::ir::func::{IrFuncMode, IrFuncOwnerInfo};
use crate::codegen::ir::ident::IrIdent;
use crate::codegen::ir::ty::boxed::IrTypeBoxed;
use crate::codegen::ir::ty::rust_auto_opaque::OwnershipMode;
use crate::codegen::ir::ty::IrType;
use crate::codegen::ir::ty::IrType::Boxed;
use crate::codegen::parser::attribute_parser::FrbAttributes;
Expand All @@ -11,7 +12,7 @@ use crate::codegen::parser::function_parser::{
use crate::codegen::parser::type_parser::misc::parse_comments;
use crate::codegen::parser::type_parser::TypeParserParsingContext;
use crate::if_then_some;
use anyhow::{bail, Context};
use anyhow::{bail, ensure, Context};
use syn::*;

impl<'a, 'b> FunctionParser<'a, 'b> {
Expand Down Expand Up @@ -76,6 +77,11 @@ impl<'a, 'b> FunctionParser<'a, 'b> {
..Default::default()
});
}

ensure!(
parse_receiver_ownership_mode(receiver) == OwnershipMode::Ref,
"If you want to use `self`/`&mut self`, please make the struct opaque (by adding `#[frb(opaque)]` on the struct)."
);
}

partial_info_for_normal_type_raw(ty, &receiver.attrs, name)
Expand Down Expand Up @@ -184,13 +190,21 @@ fn parse_name_from_pat_type(pat_type: &PatType) -> anyhow::Result<String> {
}

fn generate_ref_type_considering_reference(raw: &str, receiver: &Receiver) -> String {
match parse_receiver_ownership_mode(receiver) {
OwnershipMode::Owned => raw.to_owned(),
OwnershipMode::RefMut => format!("&mut {raw}"),
OwnershipMode::Ref => format!("&{raw}"),
}
}

fn parse_receiver_ownership_mode(receiver: &Receiver) -> OwnershipMode {
if receiver.reference.is_some() {
if receiver.mutability.is_some() {
format!("&mut {raw}")
OwnershipMode::RefMut
} else {
format!("&{raw}")
OwnershipMode::Ref
}
} else {
raw.to_owned()
OwnershipMode::Owned
}
}
39 changes: 30 additions & 9 deletions frb_codegen/src/library/codegen/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,13 +145,14 @@ mod tests {
use crate::codegen::parser::parse;
use crate::codegen::parser::reader::CachedRustReader;
use crate::codegen::parser::source_graph::crates::Crate;
use crate::codegen::parser::IrPack;
use crate::utils::logs::configure_opinionated_test_logging;
use crate::utils::test_utils::{
create_path_sanitizers, get_test_fixture_dir, json_golden_test,
};
use log::info;
use serial_test::serial;
use std::path::Path;
use std::path::{Path, PathBuf};

#[test]
#[serial]
Expand Down Expand Up @@ -204,11 +205,35 @@ mod tests {
body("library/codegen/parser/mod/generics", None)
}

#[test]
#[serial]
fn test_error_non_opaque_mut() -> anyhow::Result<()> {
let result = execute_parse("library/codegen/parser/mod/error_non_opaque_mut", None);
assert!(format!("{:#?}", result.err().unwrap())
.contains("If you want to use `self`/`&mut self`"));
Ok(())
}

#[allow(clippy::type_complexity)]
fn body(
fixture_name: &str,
rust_input_path_pack: Option<Box<dyn Fn(&Path) -> RustInputPathPack>>,
) -> anyhow::Result<()> {
let (actual_ir, rust_crate_dir) = execute_parse(fixture_name, rust_input_path_pack)?;
json_golden_test(
&serde_json::to_value(actual_ir)?,
&rust_crate_dir.join("expect_ir.json"),
&[],
)?;

Ok(())
}

#[allow(clippy::type_complexity)]
fn execute_parse(
fixture_name: &str,
rust_input_path_pack: Option<Box<dyn Fn(&Path) -> RustInputPathPack>>,
) -> anyhow::Result<(IrPack, PathBuf)> {
configure_opinionated_test_logging();
let test_fixture_dir = get_test_fixture_dir(fixture_name);
let rust_crate_dir = test_fixture_dir.clone();
Expand All @@ -223,9 +248,10 @@ mod tests {
&serde_json::to_value(crate_map)?,
&rust_crate_dir.join("expect_source_graph.json"),
&create_path_sanitizers(&test_fixture_dir),
)?;
)
.unwrap();

let actual_ir = parse(
let pack = parse(
&ParserInternalConfig {
rust_input_path_pack: rust_input_path_pack.map(|f| f(&rust_crate_dir)).unwrap_or(
RustInputPathPack {
Expand All @@ -240,12 +266,7 @@ mod tests {
&Dumper(&Default::default()),
&GeneratorProgressBarPack::new(),
)?;
json_golden_test(
&serde_json::to_value(actual_ir)?,
&rust_crate_dir.join("expect_ir.json"),
&[],
)?;

Ok(())
Ok((pack, rust_crate_dir))
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[package]
name = "example"
version = "0.1.0"
edition = "2018"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]

[workspace]
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
{
"manifest_path": "{the-working-directory}/Cargo.toml",
"name": "example",
"root_module": {
"info": {
"file_path": "{the-working-directory}/src/lib.rs",
"module_path": [
"crate"
],
"visibility": "Public"
},
"scope": {
"enums": [],
"modules": [
{
"info": {
"file_path": "{the-working-directory}/src/api.rs",
"module_path": [
"crate",
"api"
],
"visibility": "Inherited"
},
"scope": {
"enums": [],
"modules": [],
"structs": [
{
"ident": "MyStruct",
"mirror": false,
"path": [
"crate",
"api",
"MyStruct"
],
"visibility": "Public"
}
],
"type_alias": []
}
}
],
"structs": [],
"type_alias": []
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
pub struct MyStruct {
my_field: String,
}

impl MyStruct {
pub fn my_mut_method(&mut self) {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
mod api;

0 comments on commit 7bebc95

Please sign in to comment.