Repository: google/autocxx (rust)

Issue:
Assume a value for AUTOCXX_RS
Following #171 our macro now simply `include!`s a file generated in an earlier codegen phase.

It requires `AUTOCXX_RS` to be set to educate it where to find such files, but in a `cargo` context it should be able to assume a default.

It seems quite likely that `rust-analyzer` and friends will be able to parse and understand the FFI if we manage to do htis.

Pull request description:
Better expansion in a rust-analyzer IDE context
Fixes #178.

Patch:
diff --git a/engine/src/builder.rs b/engine/src/builder.rs
index 147cb08c..8121c2e0 100644
--- a/engine/src/builder.rs
+++ b/engine/src/builder.rs
@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use autocxx_parser::file_locations::FileLocationStrategy;
 use proc_macro2::TokenStream;
 
 use crate::{ParseError, ParsedFile};
@@ -67,7 +68,7 @@ where
     I: IntoIterator<Item = T>,
     T: AsRef<OsStr>,
 {
-    build_to_custom_directory(rs_file, autocxx_incs, out_dir())
+    build_to_custom_directory(rs_file, autocxx_incs, None)
 }
 
 /// Builds successfully, or exits the process displaying a suitable
@@ -93,19 +94,22 @@ fn report(err: BuilderError) -> String {
 pub(crate) fn build_to_custom_directory<P1, I, T>(
     rs_file: P1,
     autocxx_incs: I,
-    outdir: PathBuf,
+    custom_gendir: Option<PathBuf>,
 ) -> BuilderResult
 where
     P1: AsRef<Path>,
     I: IntoIterator<Item = T>,
     T: AsRef<OsStr>,
 {
-    let gendir = outdir.join("autocxx-build");
-    let incdir = gendir.join("include");
+    let gen_location_strategy = match custom_gendir {
+        None => FileLocationStrategy::new(),
+        Some(custom_dir) => FileLocationStrategy::Custom(custom_dir),
+    };
+    let incdir = gen_location_strategy.get_include_dir();
     ensure_created(&incdir)?;
-    let cxxdir = gendir.join("cxx");
+    let cxxdir = gen_location_strategy.get_cxx_dir();
     ensure_created(&cxxdir)?;
-    let rsdir = gendir.join("rs");
+    let rsdir = gen_location_strategy.get_rs_dir();
     ensure_created(&rsdir)?;
     // We are incredibly unsophisticated in our directory arrangement here
     // compared to cxx. I have no doubt that we will need to replicate just
@@ -115,9 +119,8 @@ where
     write_to_file(&incdir, "cxx.h", crate::HEADER.as_bytes())?;
 
     let autocxx_inc = build_autocxx_inc(autocxx_incs, &incdir);
-    // Configure cargo to tell the procedural macro how to find the
-    // Rust files we may have written out.
-    println!("cargo:rustc-env=AUTOCXX_RS={}", rsdir.to_str().unwrap());
+    // pass on th
+    gen_location_strategy.set_cargo_env_vars_for_build();
 
     let mut parsed_file = crate::parse_file(rs_file).map_err(BuilderError::ParseError)?;
     parsed_file
@@ -166,10 +169,6 @@ fn ensure_created(dir: &PathBuf) -> Result<(), BuilderError> {
     std::fs::create_dir_all(dir).map_err(|e| BuilderError::UnableToCreateDirectory(e, dir.clone()))
 }
 
-fn out_dir() -> PathBuf {
-    std::env::var_os("OUT_DIR").map(PathBuf::from).unwrap()
-}
-
 fn build_autocxx_inc<I, T>(paths: I, extra_path: &PathBuf) -> String
 where
     I: IntoIterator<Item = T>,
diff --git a/engine/src/byvalue_checker.rs b/engine/src/byvalue_checker.rs
index 96da9eb3..af737c2e 100644
--- a/engine/src/byvalue_checker.rs
+++ b/engine/src/byvalue_checker.rs
@@ -183,11 +183,7 @@ impl ByValueChecker {
 
     fn has_vtable(def: &ItemStruct) -> bool {
         for f in &def.fields {
-            if f.ident
-                .as_ref()
-                .map(|id| id == "vtable_")
-                .unwrap_or(false)
-            {
+            if f.ident.as_ref().map(|id| id == "vtable_").unwrap_or(false) {
                 return true;
             }
         }
diff --git a/parser/src/file_locations.rs b/parser/src/file_locations.rs
new file mode 100644
index 00000000..002837af
--- /dev/null
+++ b/parser/src/file_locations.rs
@@ -0,0 +1,119 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use proc_macro2::TokenStream;
+use quote::quote;
+use std::path::PathBuf;
+
+/// The strategy used to generate, and to find generated, files.
+/// As standard, these are based off the OUT_DIR set by Cargo,
+/// but our code can't assume it can read OUT_DIR as it may
+/// be running in the rust-analyzer proc macro server where it's
+/// not available. We need to give provision for custom locations
+/// and we need our code generator build script to be able to pass
+/// locations through to the proc macro by env vars.
+pub enum FileLocationStrategy {
+    Custom(PathBuf),
+    FromAutocxxRs(PathBuf),
+    FromOutDir(PathBuf),
+    UnknownMaybeFromOutdir,
+}
+
+static BUILD_DIR_NAME: &str = "autocxx-build-dir";
+static RS_DIR_NAME: &str = "rs";
+static AUTOCXX_RS: &str = "AUTOCXX_RS";
+
+impl FileLocationStrategy {
+    pub fn new() -> Self {
+        match std::env::var_os(AUTOCXX_RS) {
+            None => match std::env::var_os("OUT_DIR") {
+                None => FileLocationStrategy::UnknownMaybeFromOutdir,
+                Some(od) => FileLocationStrategy::FromOutDir(PathBuf::from(od)),
+            },
+            Some(acrs) => FileLocationStrategy::FromAutocxxRs(PathBuf::from(acrs)),
+        }
+    }
+
+    pub fn new_custom(gen_dir: PathBuf) -> Self {
+        FileLocationStrategy::Custom(gen_dir)
+    }
+
+    /// Make a macro to include a given generated Rust file name.
+    /// This can't simply be calculated from `get_rs_dir` because
+    /// of limitations in rust-analyzer.
+    pub fn make_include(&self, fname: String) -> TokenStream {
+        match self {
+            FileLocationStrategy::FromAutocxxRs(custom_dir) => {
+                let fname = custom_dir.join(fname).to_str().unwrap().to_string();
+                quote! {
+                    include!( #fname );
+                }
+            }
+            FileLocationStrategy::Custom(_) => panic!("Should never happen in the macro"),
+            FileLocationStrategy::UnknownMaybeFromOutdir | FileLocationStrategy::FromOutDir(_) => {
+                let fname = format!("/{}/{}/{}", BUILD_DIR_NAME, RS_DIR_NAME, fname);
+                // rust-analyzer works better if we ask Rust to do the path
+                // concatenation rather than doing it in proc-macro code.
+                // proc-macro code does not itself have access to the value of
+                // OUT_DIR, but if we put it into a macro like the below,
+                // rust-analyzer can cope.
+                quote! {
+                    include!(concat!(env!("OUT_DIR"), #fname));
+                }
+            }
+        }
+    }
+
+    fn get_gen_dir(&self, suffix: &str) -> PathBuf {
+        let root = match self {
+            FileLocationStrategy::Custom(gen_dir)
+            | FileLocationStrategy::FromAutocxxRs(gen_dir) => gen_dir.clone(),
+            FileLocationStrategy::FromOutDir(out_dir) => out_dir.join(BUILD_DIR_NAME),
+            FileLocationStrategy::UnknownMaybeFromOutdir => {
+                panic!("Could not determine OUT_DIR or AUTOCXX_RS dir")
+            }
+        };
+        root.join(suffix)
+    }
+
+    /// Location to generate Rust files.
+    pub fn get_rs_dir(&self) -> PathBuf {
+        self.get_gen_dir(RS_DIR_NAME)
+    }
+
+    /// Location to generate C++ header files.
+    pub fn get_include_dir(&self) -> PathBuf {
+        self.get_gen_dir("include")
+    }
+
+    /// Location to generate C++ code.
+    pub fn get_cxx_dir(&self) -> PathBuf {
+        self.get_gen_dir("cxx")
+    }
+
+    /// From a build script, inform cargo how to set environment variables
+    /// to make them available to the procedural macro.
+    pub fn set_cargo_env_vars_for_build(&self) {
+        match self {
+            FileLocationStrategy::Custom(_) => {
+                println!(
+                    "cargo:rustc-env={}={}",
+                    AUTOCXX_RS,
+                    self.get_rs_dir().to_str().unwrap()
+                );
+            }
+            _ => {}
+        }
+    }
+}
diff --git a/parser/src/lib.rs b/parser/src/lib.rs
index 692cee2d..fabf4893 100644
--- a/parser/src/lib.rs
+++ b/parser/src/lib.rs
@@ -13,17 +13,17 @@
 // limitations under the License.
 
 mod config;
+pub mod file_locations;
 mod type_database;
 
 use std::{
     collections::hash_map::DefaultHasher,
     hash::{Hash, Hasher},
-    path::PathBuf,
 };
 
 pub use config::{CppInclusion, IncludeCppConfig, UnsafePolicy};
+use file_locations::FileLocationStrategy;
 use proc_macro2::TokenStream as TokenStream2;
-use quote::quote;
 use syn::Result as ParseResult;
 use syn::{
     parse::{Parse, ParseStream},
@@ -66,13 +66,8 @@ impl IncludeCpp {
         if self.config.parse_only {
             return TokenStream2::new();
         }
-        let rs_dir = std::env::var_os("AUTOCXX_RS").expect("No AUTOCXX_RS configured");
-        let rs_dir = PathBuf::from(rs_dir);
         let fname = self.get_rs_filename();
-        let fname = rs_dir.join(fname).to_str().unwrap().to_string();
-        quote! {
-            include!(#fname);
-        }
+        FileLocationStrategy::new().make_include(fname)
     }
 
     pub fn get_config(&self) -> &IncludeCppConfig {


Test patch:
diff --git a/engine/src/integration_tests.rs b/engine/src/integration_tests.rs
index 638a4201..c7a476ca 100644
--- a/engine/src/integration_tests.rs
+++ b/engine/src/integration_tests.rs
@@ -225,9 +225,12 @@ fn do_run_test(
     let rs_path = write_rust_to_file(&unexpanded_rust);
 
     info!("Path is {:?}", tdir.path());
-    let build_results =
-        crate::builder::build_to_custom_directory(&rs_path, &[tdir.path()], target_dir.clone())
-            .map_err(TestError::AutoCxx)?;
+    let build_results = crate::builder::build_to_custom_directory(
+        &rs_path,
+        &[tdir.path()],
+        Some(target_dir.clone()),
+    )
+    .map_err(TestError::AutoCxx)?;
     let mut b = build_results.0;
     let generated_rs_files = build_results.1;