fix(rust_extractor): improve logic for finding the save analysis file (…

…#5354)

* fix(rust_extractor): use output file name as the name of the save analysis file

* chore(rust_indexer): add indexer test for inline test code

* fix(rust_extractor): improve save_analysis path detection logic

* fix(rust_extractor): add ok_or_else logic

kythe/rust/extractor/src/bin/extractor.rs

@@ -24,10 +24,10 @@ use glob::glob;
 use kythe_rust_extractor::vname_util::VNameRule;
 use protobuf::Message;
 use sha2::{Digest, Sha256};
+use std::env;
 use std::fs::File;
 use std::io::{Read, Write};
 use std::path::{Path, PathBuf};
-use std::{env, fs};
 use tempdir::TempDir;
 use zip::{write::FileOptions, ZipWriter};
 
@@ -67,13 +67,29 @@ fn main() -> Result<()> {
         }
     }
 
+    // Grab the build target's output path
+    let build_output_path = spawn_info
+        .get_output_file()
+        .get(0)
+        .ok_or_else(|| anyhow!("Failed to get output file from spawn info"))?;
+
+    // Get the name of the output file (without extension)
+    let output_file_name = PathBuf::from(build_output_path)
+        .with_extension("")
+        .file_name()
+        .ok_or_else(|| anyhow!("Failed to extract file name from build output path"))?
+        .to_str()
+        .ok_or_else(|| anyhow!("Failed to convert build output path file name to string"))?
+        .to_string();
+
     // Create temporary directory and run the analysis
     let tmp_dir = TempDir::new("rust_extractor")
         .with_context(|| "Failed to make temporary directory".to_string())?;
     let build_target_arguments: Vec<String> = spawn_info.get_argument().to_vec();
     save_analysis::generate_save_analysis(
         build_target_arguments.clone(),
         PathBuf::from(tmp_dir.path()),
+        &output_file_name,
     )?;
 
     // Create the output kzip
@@ -118,33 +134,9 @@ fn main() -> Result<()> {
         }
     }
 
-    // Grab the build target's output path
-    let build_output_path = spawn_info
-        .get_output_file()
-        .get(0)
-        .ok_or_else(|| anyhow!("Failed to get output file from spawn info"))?;
-
-    // Infer the crate name from the compiler args, falling back to the output file
-    // name if necessary
-    let crate_name = {
-        let crate_name_matches: Vec<_> =
-            build_target_arguments.iter().filter(|arg| arg.starts_with("--crate-name=")).collect();
-        if !crate_name_matches.is_empty() {
-            crate_name_matches.get(0).unwrap().replace("--crate-name=", "")
-        } else {
-            PathBuf::from(build_output_path)
-                .file_name()
-                .unwrap()
-                .to_str()
-                .ok_or_else(|| anyhow!("Failed to convert build output path file name to string"))?
-                .to_string()
-        }
-    };
-
     // Add save analysis to kzip
-    let save_analysis_path: String = analysis_path_string(&crate_name, tmp_dir.path())?;
-    let save_analysis_vname: VName =
-        create_vname(&mut vname_rules, &save_analysis_path, &default_corpus);
+    let save_analysis_path = analysis_path_string(&output_file_name, tmp_dir.path())?;
+    let save_analysis_vname = create_vname(&mut vname_rules, &save_analysis_path, &default_corpus);
     kzip_add_required_input(
         &save_analysis_path,
         save_analysis_vname,
@@ -304,27 +296,19 @@ fn kzip_add_file(
 }
 
 /// Find the path of the save_analysis file using the build target's
-/// crate name and the temporary base directory
-fn analysis_path_string(crate_name: &str, temp_dir_path: &Path) -> Result<String> {
-    let entries = fs::read_dir(temp_dir_path.join("save-analysis")).with_context(|| {
-        format!("Failed to read the save_analysis temporary directory: {:?}", temp_dir_path)
-    })?;
-
-    for entry in entries {
-        let entry = entry.with_context(|| "Failed to get information about directory entry")?;
-        let metadata = entry.metadata().with_context(|| "Failed to get entry metadata")?;
-        let path = entry.path();
-        let path_string = path
+/// output file name and the temporary base directory
+fn analysis_path_string(output_file_name: &str, temp_dir_path: &Path) -> Result<String> {
+    // The path should always be {tmp_dir}/save-analysis/{output_file_name}.json
+    let expected_path =
+        temp_dir_path.join("save-analysis").join(output_file_name).with_extension("json");
+    if expected_path.exists() {
+        expected_path
             .to_str()
             .ok_or_else(|| anyhow!("save_analysis file path is not valid UTF-8"))
-            .map(|path_str| path_str.to_string())?;
-        if metadata.is_file() && path_string.contains(crate_name) && path_string.ends_with(".json")
-        {
-            return Ok(path_string);
-        }
+            .map(String::from)
+    } else {
+        Err(anyhow!("Failed to find save-analysis file in {:?}", temp_dir_path))
     }
-
-    Err(anyhow!("Failed to find save-analysis file in {:?}", temp_dir_path))
 }
 
 fn create_vname(rules: &mut [VNameRule], path: &str, default_corpus: &str) -> VName {

kythe/rust/extractor/src/bin/save_analysis.rs

@@ -21,9 +21,13 @@ use std::path::{Path, PathBuf};
 ///
 /// * `arguments` - The Bazel arguments extracted from the extra action protobuf
 /// * `output_dir` - The base directory to output the save_analysis
-pub fn generate_save_analysis(arguments: Vec<String>, output_dir: PathBuf) -> Result<()> {
+pub fn generate_save_analysis(
+    arguments: Vec<String>,
+    output_dir: PathBuf,
+    output_file_name: &str,
+) -> Result<()> {
     let rustc_arguments = generate_arguments(arguments, &output_dir)?;
-    kythe_rust_extractor::generate_analysis(rustc_arguments, output_dir)
+    kythe_rust_extractor::generate_analysis(rustc_arguments, output_dir, output_file_name)
         .map_err(|_| anyhow!("Failed to generate save_analysis"))?;
     Ok(())
 }

kythe/rust/extractor/src/lib.rs

@@ -31,14 +31,18 @@ use std::path::PathBuf;
 /// first element must be an empty string.
 /// The save_analysis JSON output file will be located at
 /// {output_dir}/save-analysis/{crate_name}.json
-pub fn generate_analysis(rustc_arguments: Vec<String>, output_dir: PathBuf) -> Result<(), String> {
+pub fn generate_analysis(
+    rustc_arguments: Vec<String>,
+    output_dir: PathBuf,
+    output_file_name: &str,
+) -> Result<(), String> {
     let first_arg =
         rustc_arguments.get(0).ok_or_else(|| "Arguments vector should not be empty".to_string())?;
     if first_arg != &"".to_string() {
         return Err("The first argument must be an empty string".into());
     }
 
-    let mut callback_shim = CallbackShim::new(output_dir);
+    let mut callback_shim = CallbackShim::new(output_dir, output_file_name.to_string());
 
     rustc_driver::catch_fatal_errors(|| {
         RunCompiler::new(&rustc_arguments, &mut callback_shim).run()
@@ -53,12 +57,13 @@ pub fn generate_analysis(rustc_arguments: Vec<String>, output_dir: PathBuf) -> R
 #[derive(Default)]
 struct CallbackShim {
     output_dir: PathBuf,
+    output_file_name: String,
 }
 
 impl CallbackShim {
     /// Create a new CallbackShim that dumps save_analysis files to `output_dir`
-    pub fn new(output_dir: PathBuf) -> Self {
-        Self { output_dir }
+    pub fn new(output_dir: PathBuf, output_file_name: String) -> Self {
+        Self { output_dir, output_file_name }
     }
 }
 
@@ -96,7 +101,7 @@ impl Callbacks for CallbackShim {
                 &crate_name,
                 input,
                 None,
-                DumpHandler::new(Some(self.output_dir.as_path()), &crate_name),
+                DumpHandler::new(Some(self.output_dir.as_path()), &self.output_file_name),
             )
         });
 

kythe/rust/extractor/tests/integration_test.rs

@@ -55,7 +55,7 @@ fn main() -> Result<()> {
     let input_files: Vec<String> = vec![rust_test_source];
     spawn_info.set_input_file(protobuf::RepeatedField::from_vec(input_files));
 
-    let output_key = format!("{}/test_crate", temp_dir_str);
+    let output_key = format!("{}/libtest_crate-1234", temp_dir_str);
     let output_files: Vec<String> = vec![output_key.clone()];
     spawn_info.set_output_file(protobuf::RepeatedField::from_vec(output_files));
 
@@ -260,7 +260,7 @@ fn correct_arguments_succeed(
     let analysis_input = required_inputs.get(2).expect("Failed to get the third required input");
     let analysis_path = analysis_input.get_info().get_path();
     assert!(
-        analysis_path.contains("save-analysis/test_crate.json"),
+        analysis_path.contains("save-analysis/libtest_crate-1234.json"),
         "Unexpected file path for save_analysis file: {}",
         analysis_path
     );

kythe/rust/extractor/tests/test.rs

@@ -24,7 +24,7 @@ fn empty_args_fails() {
     let args: Vec<String> = Vec::new();
     let temp_dir = TempDir::new("extractor_test").expect("Could not create temporary directory");
     let analysis_directory = PathBuf::from(temp_dir.path());
-    let result = generate_analysis(args, analysis_directory);
+    let result = generate_analysis(args, analysis_directory, "lib");
     assert_eq!(result.unwrap_err(), "Arguments vector should not be empty".to_string());
 }
 
@@ -33,7 +33,7 @@ fn nonempty_string_first_fails() {
     let args: Vec<String> = vec!["nonempty".to_string()];
     let temp_dir = TempDir::new("extractor_test").expect("Could not create temporary directory");
     let analysis_directory = PathBuf::from(temp_dir.path());
-    let result = generate_analysis(args, analysis_directory);
+    let result = generate_analysis(args, analysis_directory, "lib");
     assert_eq!(result.unwrap_err(), "The first argument must be an empty string".to_string());
 }
 
@@ -55,11 +55,11 @@ fn correct_arguments_succeed() {
         format!("--out-dir={}", temp_dir.path().to_str().unwrap()),
     ];
     let analysis_directory = PathBuf::from(temp_dir.path());
-    let result = generate_analysis(args, analysis_directory);
+    let result = generate_analysis(args, analysis_directory, "lib");
     assert_eq!(result.unwrap(), (), "generate_analysis result wasn't void");
 
     // Ensure the save_analysis file exists
-    let _ = File::open(Path::new(temp_dir.path()).join("save-analysis/test_crate.json"))
+    let _ = File::open(Path::new(temp_dir.path()).join("save-analysis/lib.json"))
         .expect("save_analysis did not exist in the expected path");
 }
 

kythe/rust/indexer/BUILD

@@ -163,3 +163,9 @@ rust_indexer_test(
     srcs = ["testdata/out_dir/main.rs"],
     out_dir_files = ["testdata/out_dir/generated.rs"],
 )
+
+rust_indexer_test(
+    name = "test_lib_test",
+    srcs = ["testdata/test_lib.rs"],
+    is_test_lib = True,
+)

kythe/rust/indexer/testdata/test_lib.rs

@@ -0,0 +1,17 @@
+// Verifies that xrefs work for inline tests (must be compiled with --test)
+
+fn main() {
+    println!("Hello, world!");
+}
+
+//- @+4"test_fn" defines/binding TestFn
+//- TestFn.node/kind function
+//- TestFn.complete definition
+#[test]
+fn test_fn() {
+    //- @x defines/binding VarX
+    //- VarX.node/kind variable
+    //- VarX.subkind local
+    let x = 1;
+    assert_eq!(x, 1);
+}

tools/build_rules/verifier_test/rust_indexer_test.bzl

@@ -68,6 +68,9 @@ def _rust_extract_impl(ctx):
         "--linker=%s" % linker_path,
     ]
 
+    if ctx.attr.is_test_lib:
+        args.append("--test_lib")
+
     # If there were out_dir_files, set --out_dir_env
     if len(all_out_dir_files) != 0:
         args.append("--out_dir_env=${pwd}/%s" % paths.dirname(all_out_dir_files[0].path))
@@ -116,6 +119,9 @@ rust_extract = rule(
             mandatory = True,
             allow_files = [".rs"],
         ),
+        "is_test_lib": attr.bool(
+            mandatory = True,
+        ),
         "crate_name": attr.string(
             default = "test_crate",
         ),
@@ -206,13 +212,15 @@ def _rust_indexer(
         name,
         srcs,
         out_dir_files = [],
+        is_test_lib = False,
         has_marked_source = False,
         emit_anchor_scopes = False):
     kzip = name + "_units"
     rust_extract(
         name = kzip,
         srcs = srcs,
         out_dir_files = out_dir_files,
+        is_test_lib = is_test_lib,
     )
     entries = name + "_entries"
     rust_entries(
@@ -228,6 +236,7 @@ def rust_indexer_test(
         name,
         srcs,
         out_dir_files = [],
+        is_test_lib = False,
         size = None,
         tags = None,
         log_entries = False,
@@ -241,6 +250,7 @@ def rust_indexer_test(
       name: Rule name
       srcs: A list of Rust source files to index and verify
       out_dir_files: A list of files to include in $OUT_DIR
+      is_test_lib: Whether to compile the srcs as a Rust test
       size: The size to pass to the verifier_test macro
       tags: The tags to pass to the verifier_test macro
       log_entries: Enable to make the verifier log all indexer entries
@@ -254,6 +264,7 @@ def rust_indexer_test(
         name = name,
         srcs = srcs,
         out_dir_files = out_dir_files,
+        is_test_lib = is_test_lib,
         has_marked_source = has_marked_source,
         emit_anchor_scopes = emit_anchor_scopes,
     )

tools/rust/extra_action/src/main.rs

@@ -71,6 +71,12 @@ fn main() -> Result<()> {
                 .takes_value(true)
                 .help("The path that $OUT_DIR will be set to"),
         )
+        .arg(
+            Arg::with_name("test_lib")
+                .long("test_lib")
+                .required(false)
+                .help("Add \"--test\" to the compiler arguments"),
+        )
         .get_matches();
 
     let mut extra_action = ExtraActionInfo::new();
@@ -89,7 +95,7 @@ fn main() -> Result<()> {
             .position(|file_path| file_path.contains(&"main.rs") || file_path.contains(&"lib.rs"))
             .unwrap()]
     };
-    let arguments: Vec<String> = vec![
+    let mut arguments: Vec<String> = vec![
         "--".to_string(),
         "rustc".to_string(),
         // Only the main source file gets passed to the compiler
@@ -100,6 +106,9 @@ fn main() -> Result<()> {
         // This path gets replaced by the extractor so it doesn't matter
         "--out-dir=/tmp".to_string(),
     ];
+    if matches.is_present("test_lib") {
+        arguments.push("--test".to_string());
+    }
     spawn_info.set_argument(protobuf::RepeatedField::from_vec(arguments));
     spawn_info.set_input_file(protobuf::RepeatedField::from_vec(source_files));
     spawn_info.set_output_file(protobuf::RepeatedField::from_vec(vec![crate_name.to_string()]));