Skip to content

Commit

Permalink
Merge pull request #1779 from fzyzcjy/feat/11874
Browse files Browse the repository at this point in the history
Improve hints when types are exported but not used
  • Loading branch information
fzyzcjy authored Feb 26, 2024
2 parents 5b0b200 + aac2f76 commit f7bd620
Show file tree
Hide file tree
Showing 80 changed files with 378 additions and 126 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ pub(crate) struct ApiDartOutputSpecItem {
pub funcs: Vec<ApiDartGeneratedFunction>,
pub classes: Vec<ApiDartGeneratedClass>,
pub imports: DartBasicHeaderCode,
pub unused_types: Vec<String>,
pub needs_freezed: bool,
}

Expand Down Expand Up @@ -118,12 +119,18 @@ fn generate_item(
})
.unwrap_or_default();

let unused_types = (context.ir_pack.unused_types.iter())
.filter(|t| &t.namespace == namespace)
.map(|t| t.name.to_owned())
.collect_vec();

let needs_freezed = classes.iter().any(|class| class.needs_freezed);

Ok(ApiDartOutputSpecItem {
funcs,
classes,
imports,
unused_types,
needs_freezed,
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,21 @@ fn generate_end_api_text(

let header = header.all_code();

let unused_types = item
.unused_types
.iter()
.sorted()
.map(|t| {
format!("// The type `{t}` is not used by any `pub` functions, thus it is ignored.\n")
})
.join("");

format!(
"
{header}
{unused_types}
{funcs}
{classes}
Expand Down
2 changes: 2 additions & 0 deletions frb_codegen/src/library/codegen/ir/pack.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::codegen::generator::codec::structs::CodecMode;
use crate::codegen::ir::func::IrFunc;
use crate::codegen::ir::namespace::NamespacedName;
use crate::codegen::ir::ty::enumeration::{IrEnum, IrEnumIdent};
use crate::codegen::ir::ty::structure::{IrStruct, IrStructIdent};
use crate::codegen::ir::ty::IrType;
Expand All @@ -17,6 +18,7 @@ pub struct IrPack {
pub struct_pool: IrStructPool,
pub enum_pool: IrEnumPool,
pub has_executor: bool,
pub unused_types: Vec<NamespacedName>,
}

impl IrPack {
Expand Down
49 changes: 12 additions & 37 deletions frb_codegen/src/library/codegen/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,22 @@ pub(crate) mod reader;
pub(crate) mod source_graph;
pub(crate) mod type_alias_resolver;
pub(crate) mod type_parser;
mod unused_checker;

use crate::codegen::dumper::Dumper;
use crate::codegen::ir::pack::IrPack;
use crate::codegen::ir::ty::IrType;
use crate::codegen::misc::GeneratorProgressBarPack;
use crate::codegen::parser::function_extractor::extract_generalized_functions_from_file;
use crate::codegen::parser::function_parser::FunctionParser;
use crate::codegen::parser::internal_config::ParserInternalConfig;
use crate::codegen::parser::misc::parse_has_executor;
use crate::codegen::parser::reader::CachedRustReader;
use crate::codegen::parser::source_graph::modules::{Enum, Struct};
use crate::codegen::parser::type_alias_resolver::resolve_type_aliases;
use crate::codegen::parser::type_parser::TypeParser;
use crate::codegen::parser::unused_checker::get_unused_types;
use crate::codegen::ConfigDumpContent;
use itertools::Itertools;
use log::{trace, warn};
use std::collections::{HashMap, HashSet};
use log::trace;
use std::path::{Path, PathBuf};
use syn::File;
use ConfigDumpContent::SourceGraph;
Expand Down Expand Up @@ -93,14 +92,21 @@ pub(crate) fn parse(

let (struct_pool, enum_pool) = type_parser.consume();

let ans = IrPack {
let mut ans = IrPack {
funcs: ir_funcs,
struct_pool,
enum_pool,
has_executor,
unused_types: vec![],
};

sanity_check_unused_struct_enum(&ans, &src_structs, &src_enums);
ans.unused_types = get_unused_types(
&ans,
&src_structs,
&src_enums,
&config.rust_input_path_pack,
&config.rust_crate_dir,
)?;

Ok(ans)
}
Expand Down Expand Up @@ -141,37 +147,6 @@ fn read_files(
.collect()
}

fn sanity_check_unused_struct_enum(
pack: &IrPack,
src_structs: &HashMap<String, &Struct>,
src_enums: &HashMap<String, &Enum>,
) {
let all_types: HashSet<String> = [
src_structs.keys().map_into().collect_vec(),
src_enums.keys().map_into().collect_vec(),
]
.concat()
.into_iter()
.collect();

let used_types: HashSet<String> = pack
.distinct_types(None)
.into_iter()
.filter_map(|ty| match ty {
IrType::StructRef(ty) => Some(ty.ident.0.name.clone()),
IrType::EnumRef(ty) => Some(ty.ident.0.name.clone()),
_ => None,
})
.collect();

if all_types != used_types {
warn!(
"Some structs/enums are exported as `pub`, but are never used in any `pub` functions, thus they are ignored: {:?}",
all_types.difference(&used_types),
)
}
}

#[cfg(test)]
mod tests {
use crate::codegen::config::internal_config::RustInputPathPack;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::codegen::ir::namespace::Namespace;
use derivative::Derivative;
use quote::ToTokens;
use serde::{Serialize, Serializer};
Expand Down Expand Up @@ -59,6 +60,14 @@ pub struct StructOrEnum<Item> {
}
// frb-coverage:ignore-end

impl<Item> StructOrEnum<Item> {
pub(crate) fn namespace(&self) -> Namespace {
let mut p = self.path.clone();
p.pop();
Namespace::new(p)
}
}

#[derive(Clone, Debug, Serialize)]
pub struct Struct(pub StructOrEnum<ItemStruct>);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ where
if let Some(src_object) = self.src_objects().get(*name) {
let src_object = (*src_object).clone();

let namespace = Namespace::new(pop_last(src_object.inner().path.clone()));
let namespace = src_object.inner().namespace();
let namespaced_name = NamespacedName::new(namespace, name.to_string());

let attrs = FrbAttributes::parse(src_object.attrs())?;
Expand Down Expand Up @@ -73,11 +73,6 @@ where
) -> anyhow::Result<IrType>;
}

fn pop_last(mut v: Vec<String>) -> Vec<String> {
v.pop();
v
}

#[derive(Clone, Debug, Default)]
pub(super) struct EnumOrStructParserInfo<Id, Obj> {
parsing_or_parsed_objects: HashSet<NamespacedName>,
Expand Down
3 changes: 2 additions & 1 deletion frb_codegen/src/library/codegen/parser/type_parser/path.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::codegen::ir::ty::IrType;
use crate::codegen::parser::type_parser::path_data::extract_path_data;
use crate::codegen::parser::type_parser::unencodable::splay_segments;
use crate::codegen::parser::type_parser::TypeParserWithContext;
use anyhow::bail;
Expand Down Expand Up @@ -28,7 +29,7 @@ impl<'a, 'b, 'c> TypeParserWithContext<'a, 'b, 'c> {
type_path: &TypePath,
path: &Path,
) -> anyhow::Result<IrType> {
let segments = self.extract_path_data(path)?;
let segments = extract_path_data(path)?;
let splayed_segments = splay_segments(&segments);

if let Some(last_segment) = splayed_segments.last() {
Expand Down
118 changes: 54 additions & 64 deletions frb_codegen/src/library/codegen/parser/type_parser/path_data.rs
Original file line number Diff line number Diff line change
@@ -1,75 +1,65 @@
use crate::codegen::ir::ty::rust_opaque::NameComponent;
use crate::codegen::parser::type_parser::TypeParserWithContext;
use crate::if_then_some;
use anyhow::Result;
use syn::{
AngleBracketedGenericArguments, GenericArgument, Path, PathArguments, PathSegment, Type,
};

impl<'a, 'b, 'c> TypeParserWithContext<'a, 'b, 'c> {
pub(crate) fn extract_path_data(&mut self, path: &Path) -> Result<Vec<NameComponent>> {
path.segments
.iter()
.map(|segment| self.parse_path_segment(segment))
.collect()
}

fn parse_path_segment(&mut self, segment: &PathSegment) -> Result<NameComponent> {
let ident = segment.ident.to_string();
let args = match &segment.arguments {
PathArguments::None => vec![],
PathArguments::AngleBracketed(args) => {
self.parse_angle_bracketed_generic_arguments(args)
// .with_context(|| {
// // This will stop the whole generator and tell the users, so we do not care about testing it
// // frb-coverage:ignore-start
// anyhow!("\"{ident}\" of \"{}\" is not valid", path.to_token_stream())
// // frb-coverage:ignore-end
// })?
}
// frb-coverage:ignore-start
_ => unreachable!(),
// frb-coverage:ignore-end
pub(crate) fn extract_path_data(path: &Path) -> Result<Vec<NameComponent>> {
path.segments.iter().map(parse_path_segment).collect()
}

// not used yet (detected by codecov)
// syn doc says "The `(A, B) -> C` in `Fn(A, B) -> C`",
// thus it seems we will not use it here.
//
// PathArguments::Parenthesized(args) => Some(Args::Signature(
// self.parse_parenthesized_generic_arguments(args)?,
// )),
};
Ok(NameComponent { ident, args })
}
fn parse_path_segment(segment: &PathSegment) -> Result<NameComponent> {
let ident = segment.ident.to_string();
let args = match &segment.arguments {
PathArguments::None => vec![],
PathArguments::AngleBracketed(args) => {
parse_angle_bracketed_generic_arguments(args)
// .with_context(|| {
// // This will stop the whole generator and tell the users, so we do not care about testing it
// // frb-coverage:ignore-start
// anyhow!("\"{ident}\" of \"{}\" is not valid", path.to_token_stream())
// // frb-coverage:ignore-end
// })?
}
// frb-coverage:ignore-start
_ => unreachable!(),
// frb-coverage:ignore-end

fn parse_angle_bracketed_generic_arguments(
&mut self,
args: &AngleBracketedGenericArguments,
) -> Vec<Type> {
args.args
.iter()
.filter_map(|arg| if_then_some!(let GenericArgument::Type(ty) = arg, ty.to_owned()))
// .map(|ty| self.parse_type(ty))
.collect()
}
// not used yet (detected by codecov)
// syn doc says "The `(A, B) -> C` in `Fn(A, B) -> C`",
// thus it seems we will not use it here.
//
// PathArguments::Parenthesized(args) => Some(Args::Signature(
// self.parse_parenthesized_generic_arguments(args)?,
// )),
};
Ok(NameComponent { ident, args })
}

// not used yet
// fn parse_parenthesized_generic_arguments(
// &mut self,
// args: &ParenthesizedGenericArguments,
// ) -> Result<Vec<IrType>> {
// let input_types = args
// .inputs
// .iter()
// .map(|ty| self.parse_type(ty))
// .collect::<Result<Vec<_>>>()?;
//
// let output_type = self.parse_return_type(&args.output)?;
//
// Ok({
// let mut ans = vec![output_type];
// ans.extend(input_types);
// ans
// })
// }
fn parse_angle_bracketed_generic_arguments(args: &AngleBracketedGenericArguments) -> Vec<Type> {
args.args
.iter()
.filter_map(|arg| if_then_some!(let GenericArgument::Type(ty) = arg, ty.to_owned()))
.collect()
}

// not used yet
// fn parse_parenthesized_generic_arguments(
// &mut self,
// args: &ParenthesizedGenericArguments,
// ) -> Result<Vec<IrType>> {
// let input_types = args
// .inputs
// .iter()
// .map(|ty| self.parse_type(ty))
// .collect::<Result<Vec<_>>>()?;
//
// let output_type = self.parse_return_type(&args.output)?;
//
// Ok({
// let mut ans = vec![output_type];
// ans.extend(input_types);
// ans
// })
// }
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use crate::codegen::ir::ty::rust_opaque::{
IrRustOpaqueInner, IrTypeRustOpaque, RustOpaqueCodecMode,
};
use crate::codegen::ir::ty::{IrType, IrTypeTrait};
use crate::codegen::parser::type_parser::path_data::extract_path_data;
use crate::codegen::parser::type_parser::rust_opaque::{
GeneralizedRustOpaqueParserInfo, RustOpaqueParserTypeInfo,
};
Expand Down Expand Up @@ -38,7 +39,7 @@ impl<'a, 'b, 'c> TypeParserWithContext<'a, 'b, 'c> {
let info = self.get_or_insert_rust_auto_opaque_info(&inner_str, namespace, None);

let raw_segments = match inner {
Type::Path(inner) => self.extract_path_data(&inner.path)?,
Type::Path(inner) => extract_path_data(&inner.path)?,
_ => vec![],
};

Expand Down
Loading

0 comments on commit f7bd620

Please sign in to comment.