Skip to content

Commit

Permalink
Temporary state with beefed up java matching (#32)
Browse files Browse the repository at this point in the history
* Temporary state with beefed up java matching

* Ensure good test coverage
  • Loading branch information
ianoc authored Nov 12, 2020
1 parent 74ee368 commit 7723ab9
Show file tree
Hide file tree
Showing 5 changed files with 159 additions and 59 deletions.
127 changes: 108 additions & 19 deletions bazelfe-core/src/error_extraction/java/error_cannot_find_symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,22 +65,22 @@ fn extract_symbol_with_package(
}
}

fn extract_symbol_with_class(
fn extract_symbol(
lines: &Vec<String>,
src_file_name: &String,
parsed_file: &ParsedFile,
result: &mut Vec<JavaClassImportRequest>,
) {
lazy_static! {
// We match a slew of things like locations around methods, or otherwise, we don't mind
// where these come from. We will attempt to use the symbol info to figure out the lookup
static ref SYMBOL_RE: Regex =
Regex::new(r"^\s*symbol:\s*(class|variable)\s*(.*)$").unwrap();
static ref CLASS_RE: Regex = Regex::new(r"^\s*location:\s*class\s*(.*)$").unwrap();
}

let symbol_capture = SYMBOL_RE.captures(&lines[2]);
let class_capture = CLASS_RE.captures(&lines[3]);

if let (Some(ref capture), Some(_)) = (symbol_capture, class_capture) {
if let Some(ref capture) = symbol_capture {
let missing_symbol = capture.get(2).unwrap().as_str();
let mut packages: Vec<String> = Vec::new();
if let Some(ref package_name) = &parsed_file.package_name {
Expand All @@ -102,13 +102,17 @@ fn extract_symbol_with_class(
match import.suffix {
crate::source_dependencies::SelectorType::SelectorList(_) => (),
crate::source_dependencies::SelectorType::NoSelector => {
if let Some(ref matches_end) = import
.prefix_section
.strip_suffix(missing_symbol)
.and_then(|e| e.strip_suffix("."))
{
if import.prefix_section.ends_with(missing_symbol) {
// If we find matching imports, these are ~pretty high quality signals of what we are looking for is this.
// Zero out the lower quality signals and just inject these with higher priority
packages.clear();
packages.push(matches_end.to_string());
result.push(JavaClassImportRequest {
src_file_name: src_file_name.to_string(),
class_name: import.prefix_section.clone(),
exact_only: false,
src_fn: "cannot_find_symbol_matched_import",
priority: 6,
})
}
}
crate::source_dependencies::SelectorType::WildcardSelector => (),
Expand All @@ -135,19 +139,37 @@ pub(in crate::error_extraction) fn extract(
}

let mut result = Vec::default();
let mut batch_result = Vec::default();
let mut process_batch: Option<(Vec<String>, String)> = None;
for ln in input.lines() {
let captures = RE.captures(ln);
if let Some((ref mut vec, ref src_file_name)) = process_batch {
if vec.len() < 3 {
vec.push(ln.to_string());
// on ~just len == 3
// the other branch of the outer if may never fire if there is only 3 lines.
if vec.len() == 3 {
// we only need 3 lines of an error to try extract the symbol information
if let Some(file_data) = file_parse_cache.load_file(src_file_name) {
extract_symbol(vec, src_file_name, file_data, &mut batch_result);
}
}
} else {
vec.push(ln.to_string());
extract_symbol_with_package(vec, src_file_name, &mut result);

if let Some(file_data) = file_parse_cache.load_file(src_file_name) {
extract_symbol_with_class(vec, src_file_name, file_data, &mut result);
let mut tmp = Vec::default();
// If given the use of the 4th line we extracted with a package too, this is higher priority
// so ditch the symbol info and use this instead!
extract_symbol_with_package(vec, src_file_name, &mut tmp);

// panic!("{:#?}", tmp);
if tmp.is_empty() {
result.extend(batch_result.drain(..));
} else {
result.extend(tmp.drain(..));
}
batch_result.clear();

process_batch = None;
}
}
Expand All @@ -167,12 +189,15 @@ pub(in crate::error_extraction) fn extract(
e.prefix_section.to_string(),
);
result.push(class_import_request);
// Don't bother using fallback mechanisms, we have an import matched.
process_batch = None;
}
}
}
}
}
}
result.extend(batch_result.drain(..));
result.sort();
result.dedup();
Some(result)
Expand All @@ -182,21 +207,52 @@ pub(in crate::error_extraction) fn extract(
mod tests {

use super::*;

#[test]
fn test_not_a_member_of_package_error() {
fn test_not_a_member_of_package_on_import_line() {
let mut file_cache = super::super::FileParseCache::init_from_par(
String::from("src/main/java/com/example/foo/Example.java"),
crate::source_dependencies::ParsedFile {
package_name: None,
imports: vec![crate::source_dependencies::Import {
line_number: 16,
prefix_section: String::from("javax.annotation.Nullable"),
// this should get ignorred since we are on a package item.
prefix_section: String::from("javax.foo.bar.baz.annotation.Nullable"),
suffix: crate::source_dependencies::SelectorType::NoSelector,
}],
},
);
let sample_output =
"src/main/java/com/example/foo/Example.java:16: error: cannot find symbol
import javax.annotation.Nullable;
^
symbol: class Nullable
location: package javax.annotation";
assert_eq!(
extract(sample_output, &mut file_cache),
Some(vec![build_class_import_request(
String::from("src/main/java/com/example/foo/Example.java"),
"javax.foo.bar.baz.annotation.Nullable".to_string()
)])
);
}

#[test]
fn test_not_a_member_of_package_error() {
let mut file_cache = super::super::FileParseCache::init_from_par(
String::from("src/main/java/com/example/foo/Example.java"),
crate::source_dependencies::ParsedFile {
package_name: None,
imports: vec![crate::source_dependencies::Import {
line_number: 16,
// this should get ignorred since we are on a package item.
prefix_section: String::from("javax.foo.bar.baz.annotation.Nullable"),
suffix: crate::source_dependencies::SelectorType::NoSelector,
}],
},
);
let sample_output =
"src/main/java/com/example/foo/Example.java:22: error: cannot find symbol
import javax.annotation.Nullable;
^
symbol: class Nullable
Expand Down Expand Up @@ -313,10 +369,43 @@ mod tests {
location: class UsingClass";
assert_eq!(
extract(sample_output, &mut file_cache),
Some(vec![build_class_import_request_low_priority(
String::from("src/main/java/com/example/foo/Example.java"),
"javax.annotation.Nullable".to_string()
)])
Some(vec![JavaClassImportRequest {
src_file_name: String::from("src/main/java/com/example/foo/Example.java"),
class_name: String::from("javax.annotation.Nullable"),
exact_only: false,
src_fn: "cannot_find_symbol_matched_import",
priority: 6,
}])
);
}

#[test]
fn test_not_a_member_of_package_symbol_with_matching_import() {
let mut file_cache = super::super::FileParseCache::init_from_par(
String::from("src/main/java/com/example/foo/Example.java"),
crate::source_dependencies::ParsedFile {
package_name: Some(String::from("com.example.foo")),
imports: vec![crate::source_dependencies::Import {
line_number: 16,
prefix_section: String::from("javax.annotation.Nullable"),
suffix: crate::source_dependencies::SelectorType::NoSelector,
}],
},
);
let sample_output =
"src/main/java/com/example/foo/Example.java:19: error: cannot find symbol
Nullable.class,
^
symbol: class Nullable";
assert_eq!(
extract(sample_output, &mut file_cache),
Some(vec![JavaClassImportRequest {
src_file_name: String::from("src/main/java/com/example/foo/Example.java"),
class_name: String::from("javax.annotation.Nullable"),
exact_only: false,
src_fn: "cannot_find_symbol_matched_import",
priority: 6,
}])
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ pub(in crate::error_extraction) fn extract(
}

let mut result = None;
for ln in input.lines() {
let lines: Vec<&str> = input.lines().collect();
for (pos, ln) in lines.iter().enumerate() {
let captures = RE.captures(ln);

match captures {
Expand All @@ -55,6 +56,22 @@ pub(in crate::error_extraction) fn extract(
));
}
}
} else {
if pos < lines.len() - 1 {
let target_line = lines[pos + 1];
match crate::source_dependencies::java::parse_imports(target_line) {
Ok(matched) => {
if let Some(e) = matched.into_iter().next() {
class_import_request = Some(build_class_import_request(
src_file_name.to_string(),
e.prefix_section.to_string(),
30,
));
}
}
Err(_) => (),
}
}
}

let class_import_request = match class_import_request {
Expand Down Expand Up @@ -133,4 +150,33 @@ mod tests {
)])
);
}

#[test]
fn test_not_a_member_of_package_error_with_no_file_parse_import() {
let mut file_cache = super::super::FileParseCache::init_from_par(
String::from("src/main/java/com/example/Example.java"),
crate::source_dependencies::ParsedFile {
package_name: None,
imports: vec![crate::source_dependencies::Import {
line_number: 3,
prefix_section: String::from("com.google.common.base.Preconditions"),
suffix: crate::source_dependencies::SelectorType::NoSelector,
}],
},
);

// non existant path.
let sample_output =
"foo/bar/baz/doh/src/main/java/com/example/Example.java:3: error: package com.google.common.base does not exist
import com.google.common.base.Preconditions;
";
assert_eq!(
extract(sample_output, &mut file_cache),
Some(vec![build_class_import_request(
String::from("foo/bar/baz/doh/src/main/java/com/example/Example.java"),
"com.google.common.base.Preconditions".to_string(),
30
)])
);
}
}
9 changes: 1 addition & 8 deletions bazelfe-core/src/error_extraction/java/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ impl FileParseCache {
}
pub fn extract_errors(input: &str) -> Vec<super::ClassImportRequest> {
let mut file_parse_cache: FileParseCache = FileParseCache::new();
let combined_vec: Vec<super::ClassImportRequest> = vec![
let mut combined_vec: Vec<super::ClassImportRequest> = vec![
error_package_does_not_exist::extract(input, &mut file_parse_cache),
error_indirect_dependency::extract(input),
error_does_not_represent_a_declared_type::extract(input),
Expand All @@ -84,10 +84,3 @@ pub fn extract_errors(input: &str) -> Vec<super::ClassImportRequest> {

combined_vec
}

pub fn extract_suffix_errors(input: &str) -> Vec<super::ClassSuffixMatch> {
vec![error_cannot_access::extract(input)]
.into_iter()
.flat_map(|e| e)
.collect()
}
18 changes: 3 additions & 15 deletions bazelfe-core/src/error_extraction/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,7 @@ pub fn extract_errors(target_kind: &Option<String>, input: &str) -> Vec<ClassImp
}
}

pub fn extract_suffix_errors(target_kind: &Option<String>, input: &str) -> Vec<ClassSuffixMatch> {
match target_kind.as_ref() {
None => Vec::default(),
Some(kind) => match kind.as_ref() {
"scala_library" => scala::extract_suffix_errors(input),
"scala_test" => scala::extract_suffix_errors(input),
"java_library" => java::extract_suffix_errors(input),
"java_test" => java::extract_suffix_errors(input),
_ => {
let mut v = scala::extract_suffix_errors(input);
v.extend(java::extract_suffix_errors(input).into_iter());
v
}
},
}
pub fn extract_suffix_errors(_target_kind: &Option<String>, _input: &str) -> Vec<ClassSuffixMatch> {
// TODO this is being deprecated/removed
Vec::default()
}
Original file line number Diff line number Diff line change
Expand Up @@ -365,18 +365,6 @@ mod tests {
Vec::default()
).await;

// we are just testing that we load the file and invoke the paths, so we just need ~any error types in here.
test_content_to_expected_result(
"src/main/java/com/example/foo/bar/Baz.java:205: error: cannot access JSONObject
Blah key = Blah.myfun(jwk);",
"java_library",
Vec::default(),
vec![ClassSuffixMatch {
suffix: String::from("JSONObject"),
src_fn: String::from("java::error_cannot_access"),
}],
)
.await
}

#[tokio::test]
Expand Down Expand Up @@ -442,10 +430,6 @@ src/main/java/com/example/foo/Example.java:16: error: cannot find symbol
ActionRequest::Prefix(String::from("javax.annotation.foo.bar.baz")),
ActionRequest::Prefix(String::from("javax.annotation.foo.bar")),
],
vec![ActionRequest::Suffix(ClassSuffixMatch {
suffix: String::from("JSONObject"),
src_fn: String::from("java::error_cannot_access"),
})],
],
)
.await
Expand Down

0 comments on commit 7723ab9

Please sign in to comment.