Merge pull request 1Password#209 from 1Password/dr/context-types

Fix duplicate root added to walker.

Author: darrell-roberts

cli/src/main.rs

@@ -23,6 +23,7 @@ use rayon::iter::ParallelBridge;
 #[cfg(feature = "go")]
 use typeshare_core::language::Go;
 use typeshare_core::{
+    context::ParseContext,
     language::{
         CrateName, GenericConstraints, Kotlin, Language, Scala, SupportedLanguage, Swift,
         TypeScript,
@@ -72,6 +73,8 @@ fn main() -> anyhow::Result<()> {
 
     let directories = options.directories.as_slice();
 
+    info!("Using directories: {directories:?}");
+
     let language_type = match options.language {
         None => panic!("no language specified; `clap` should have guaranteed its presence"),
         Some(language) => match language {
@@ -110,7 +113,7 @@ fn main() -> anyhow::Result<()> {
         .overrides(overrides)
         .follow_links(options.follow_links);
 
-    for root in directories {
+    for root in directories.iter().skip(1) {
         walker_builder.add(root);
     }
 
@@ -127,9 +130,13 @@ fn main() -> anyhow::Result<()> {
 
     let multi_file = matches!(destination, Output::Folder(_));
     let target_os = config.target_os.clone();
-
     let mut lang = language(language_type, config, multi_file);
-    let ignored_types = lang.ignored_reference_types();
+
+    let parse_context = ParseContext {
+        ignored_types: lang.ignored_reference_types(),
+        multi_file,
+        target_os,
+    };
 
     // The walker ignores directories that are git-ignored. If you need
     // a git-ignored directory to be processed, add the specific directory to
@@ -141,9 +148,7 @@ fn main() -> anyhow::Result<()> {
     // https://docs.rs/ignore/latest/ignore/struct.WalkParallel.html
     let crate_parsed_data = parse_input(
         parser_inputs(walker_builder, language_type, multi_file).par_bridge(),
-        &ignored_types,
-        multi_file,
-        &target_os,
+        &parse_context,
     )?;
 
     // Collect all the types into a map of the file name they
@@ -157,6 +162,8 @@ fn main() -> anyhow::Result<()> {
 
     check_parse_errors(&crate_parsed_data)?;
 
+    info!("typeshare started writing generated types");
+
     write_generated(
         destination,
         lang.as_mut(),

cli/src/parse.rs

@@ -7,6 +7,7 @@ use std::{
     path::PathBuf,
 };
 use typeshare_core::{
+    context::{ParseContext, ParseFileContext},
     language::{CrateName, CrateTypes, SupportedLanguage, SINGLE_FILE_CRATE_NAME},
     parser::ParsedData,
     RenameExt,
@@ -89,9 +90,7 @@ pub fn all_types(file_mappings: &BTreeMap<CrateName, ParsedData>) -> CrateTypes
 /// Collect all the parsed sources into a mapping of crate name to parsed data.
 pub fn parse_input(
     inputs: impl ParallelIterator<Item = ParserInput>,
-    ignored_types: &[&str],
-    multi_file: bool,
-    target_os: &[String],
+    parse_context: &ParseContext,
 ) -> anyhow::Result<BTreeMap<CrateName, ParsedData>> {
     inputs
         .into_par_iter()
@@ -103,17 +102,17 @@ pub fn parse_input(
                  file_name,
                  crate_name,
              }| {
-                let parsed_result = typeshare_core::parser::parse(
-                    &std::fs::read_to_string(&file_path)
+                let parse_file_context = ParseFileContext {
+                    source_code: std::fs::read_to_string(&file_path)
                         .with_context(|| format!("Failed to read input: {file_name}"))?,
-                    crate_name.clone(),
-                    file_name.clone(),
+                    crate_name: crate_name.clone(),
+                    file_name: file_name.clone(),
                     file_path,
-                    ignored_types,
-                    multi_file,
-                    target_os,
-                )
-                .with_context(|| format!("Failed to parse: {file_name}"))?;
+                };
+
+                let parsed_result =
+                    typeshare_core::parser::parse(parse_context, parse_file_context)
+                        .with_context(|| format!("Failed to parse: {file_name}"))?;
 
                 if let Some(parsed_data) = parsed_result {
                     parsed_crates

core/src/context.rs

@@ -0,0 +1,27 @@
+//! Context types for parsing.
+//!
+use crate::language::CrateName;
+use std::path::PathBuf;
+
+/// Context for parsing rust source files.
+#[derive(Default)]
+pub struct ParseContext<'a> {
+    /// Types to ignore
+    pub ignored_types: Vec<&'a str>,
+    /// Multi file output enabled.
+    pub multi_file: bool,
+    /// `target_os` filtering.
+    pub target_os: Vec<String>,
+}
+
+/// Parsing context for a single rust source file.
+pub struct ParseFileContext {
+    /// Source code content
+    pub source_code: String,
+    /// Name of the crate this file belongs to.
+    pub crate_name: CrateName,
+    /// File name.
+    pub file_name: String,
+    /// Full path to the source file.
+    pub file_path: PathBuf,
+}

core/src/lib.rs

@@ -1,14 +1,13 @@
 //! The core library for typeshare.
 //! Contains the parser and language converters.
-
 use thiserror::Error;
 
-mod rename;
-
+pub mod context;
 /// Implementations for each language converter
 pub mod language;
 /// Parsing Rust code into a format the `language` modules can understand
 pub mod parser;
+mod rename;
 /// Codifying Rust types and how they convert to various languages.
 pub mod rust_types;
 mod target_os_check;

core/src/parser.rs

@@ -1,4 +1,5 @@
 use crate::{
+    context::{ParseContext, ParseFileContext},
     language::{CrateName, SupportedLanguage},
     rename::RenameExt,
     rust_types::{
@@ -15,7 +16,6 @@ use proc_macro2::Ident;
 use std::{
     collections::{BTreeSet, HashMap, HashSet},
     convert::TryFrom,
-    path::PathBuf,
 };
 use syn::{
     ext::IdentExt, parse::ParseBuffer, punctuated::Punctuated, visit::Visit, Attribute, Expr,
@@ -162,32 +162,27 @@ impl ParsedData {
 
 /// Parse the given Rust source string into `ParsedData`.
 pub fn parse(
-    source_code: &str,
-    crate_name: CrateName,
-    file_name: String,
-    file_path: PathBuf,
-    ignored_types: &[&str],
-    mult_file: bool,
-    target_os: &[String],
+    parse_context: &ParseContext,
+    parse_file_context: ParseFileContext,
 ) -> Result<Option<ParsedData>, ParseError> {
     // We will only produce output for files that contain the `#[typeshare]`
     // attribute, so this is a quick and easy performance win
-    if !source_code.contains("#[typeshare") {
+    if !parse_file_context.source_code.contains("#[typeshare") {
         return Ok(None);
     }
 
-    debug!("parsing {file_name}");
-    // Parse and process the input, ensuring we parse only items marked with
-    // `#[typeshare]`
-    let mut import_visitor = TypeShareVisitor::new(
+    let ParseFileContext {
+        source_code,
         crate_name,
         file_name,
         file_path,
-        ignored_types,
-        mult_file,
-        target_os,
-    );
-    import_visitor.visit_file(&syn::parse_file(source_code)?);
+    } = parse_file_context;
+
+    debug!("parsing {file_name}");
+    // Parse and process the input, ensuring we parse only items marked with
+    // `#[typeshare]`
+    let mut import_visitor = TypeShareVisitor::new(parse_context, crate_name, file_name, file_path);
+    import_visitor.visit_file(&syn::parse_file(&source_code)?);
 
     Ok(import_visitor.parsed_data())
 }

core/src/visitors.rs

@@ -1,5 +1,6 @@
 //! Visitors to collect various items from the AST.
 use crate::{
+    context::ParseContext,
     language::CrateName,
     parser::{
         has_typeshare_annotation, parse_enum, parse_struct, parse_type_alias, ErrorInfo,
@@ -46,29 +47,24 @@ const IGNORED_TYPES: &[&str] = &["Option", "String", "Vec", "HashMap", "T", "I54
 
 /// An import visitor that collects all use or
 /// qualified referenced items.
-#[derive(Default)]
 pub struct TypeShareVisitor<'a> {
     parsed_data: ParsedData,
     file_path: PathBuf,
-    ignored_types: &'a [&'a str],
-    target_os: &'a [String],
+    parse_context: &'a ParseContext<'a>,
 }
 
 impl<'a> TypeShareVisitor<'a> {
     /// Create an import visitor for a given crate name.
     pub fn new(
+        parse_context: &'a ParseContext<'a>,
         crate_name: CrateName,
         file_name: String,
         file_path: PathBuf,
-        ignored_types: &'a [&'a str],
-        multi_file: bool,
-        target_os: &'a [String],
     ) -> Self {
         Self {
-            parsed_data: ParsedData::new(crate_name, file_name, multi_file),
+            parsed_data: ParsedData::new(crate_name, file_name, parse_context.multi_file),
             file_path,
-            ignored_types,
-            target_os,
+            parse_context,
         }
     }
 
@@ -193,7 +189,7 @@ impl<'a> TypeShareVisitor<'a> {
     /// not match `--target-os` argument?
     #[inline(always)]
     fn target_os_accepted(&self, attrs: &[Attribute]) -> bool {
-        accept_target_os(attrs, self.target_os)
+        accept_target_os(attrs, &self.parse_context.target_os)
     }
 }
 
@@ -224,7 +220,10 @@ impl<'ast, 'a> Visit<'ast> for TypeShareVisitor<'a> {
 
             (accept_crate(&crate_candidate)
                 && accept_type(&type_candidate)
-                && !self.ignored_types.contains(&type_candidate.as_str())
+                && !self
+                    .parse_context
+                    .ignored_types
+                    .contains(&type_candidate.as_str())
                 && crate_candidate != type_candidate)
                 .then(|| {
                     // resolve crate and super aliases into the crate name.
@@ -254,10 +253,14 @@ impl<'ast, 'a> Visit<'ast> for TypeShareVisitor<'a> {
         if !self.parsed_data.multi_file {
             return;
         }
-        self.parsed_data.import_types.extend(
-            parse_import(i, &self.parsed_data.crate_name)
-                .filter(|imp| !self.ignored_types.contains(&imp.type_name.as_str())),
-        );
+        self.parsed_data
+            .import_types
+            .extend(parse_import(i, &self.parsed_data.crate_name).filter(|imp| {
+                !self
+                    .parse_context
+                    .ignored_types
+                    .contains(&imp.type_name.as_str())
+            }));
         syn::visit::visit_item_use(self, i);
     }
 
@@ -266,7 +269,7 @@ impl<'ast, 'a> Visit<'ast> for TypeShareVisitor<'a> {
         debug!("Visiting {}", i.ident);
         if has_typeshare_annotation(&i.attrs) && self.target_os_accepted(&i.attrs) {
             debug!("\tParsing {}", i.ident);
-            self.collect_result(parse_struct(i, self.target_os));
+            self.collect_result(parse_struct(i, &self.parse_context.target_os));
         }
 
         syn::visit::visit_item_struct(self, i);
@@ -277,7 +280,7 @@ impl<'ast, 'a> Visit<'ast> for TypeShareVisitor<'a> {
         debug!("Visiting {}", i.ident);
         if has_typeshare_annotation(&i.attrs) && self.target_os_accepted(&i.attrs) {
             debug!("\tParsing {}", i.ident);
-            self.collect_result(parse_enum(i, self.target_os));
+            self.collect_result(parse_enum(i, &self.parse_context.target_os));
         }
 
         syn::visit::visit_item_enum(self, i);
@@ -432,7 +435,7 @@ fn parse_import<'a>(
 #[cfg(test)]
 mod test {
     use super::{parse_import, TypeShareVisitor};
-    use crate::visitors::ImportedType;
+    use crate::{context::ParseContext, visitors::ImportedType};
     use cool_asserts::assert_matches;
     use itertools::Itertools;
     use syn::{visit::Visit, File};
@@ -603,14 +606,18 @@ mod test {
             }
             ";
 
+        let parse_context = ParseContext {
+            ignored_types: Vec::new(),
+            multi_file: true,
+            target_os: Vec::new(),
+        };
+
         let file: File = syn::parse_str(rust_code).unwrap();
         let mut visitor = TypeShareVisitor::new(
+            &parse_context,
             "my_crate".into(),
             "my_file".into(),
             "file_path".into(),
-            &[],
-            true,
-            &[],
         );
         visitor.visit_file(&file);
 

core/tests/agnostic_tests.rs

@@ -1,5 +1,6 @@
 use std::io::Write;
 use typeshare_core::{
+    context::{ParseContext, ParseFileContext},
     language::{CrateTypes, Language, TypeScript},
     parser::{self, ParseError},
     rust_types::RustTypeParseError,
@@ -12,14 +13,16 @@ pub fn process_input(
     imports: &CrateTypes,
     out: &mut dyn Write,
 ) -> Result<(), ProcessInputError> {
+    let parse_context = ParseContext::default();
+
     let mut parsed_data = parser::parse(
-        input,
-        "default_name".into(),
-        "file_name".into(),
-        "file_path".into(),
-        &[],
-        false,
-        &[],
+        &parse_context,
+        ParseFileContext {
+            source_code: input.to_string(),
+            crate_name: "default_name".into(),
+            file_name: "file_name".into(),
+            file_path: "file_path".into(),
+        },
     )?
     .unwrap();