diff --git a/crates/does-it-throw/src/call_finder.rs b/crates/does-it-throw/src/call_finder.rs index 24361a1..bf914fb 100644 --- a/crates/does-it-throw/src/call_finder.rs +++ b/crates/does-it-throw/src/call_finder.rs @@ -3,7 +3,8 @@ extern crate swc_ecma_ast; extern crate swc_ecma_parser; extern crate swc_ecma_visit; -use std::collections::HashSet; +use std::collections::hash_map::DefaultHasher; +use std::collections::{HashMap, HashSet}; use std::hash::{Hash, Hasher}; use swc_ecma_ast::{ @@ -47,6 +48,7 @@ impl Hash for CallToThrowMap { pub struct InstantiationsMap { pub class_name: String, pub variable_name: String, + pub instantiation_span: Span, } impl PartialEq for InstantiationsMap { @@ -87,12 +89,36 @@ pub struct CallFinder { pub calls: HashSet, pub functions_with_throws: HashSet, pub current_class_name: Option, - pub instantiations: HashSet, + pub instantiations: HashMap, pub function_name_stack: Vec, pub object_property_stack: Vec, + pub processed_calls: HashSet, } impl CallFinder { + // use the lo and hi of the Span to generate a unique ID so that we don't + // record duplicate calls + // We also use the instantiation span for instances to ensure uniqueness + // for calls to methods on instances + fn generate_unique_call_id(&self, call: &CallExpr) -> u64 { + let mut hasher = DefaultHasher::new(); + + if let Callee::Expr(expr) = &call.callee { + if let Expr::Member(member_expr) = &**expr { + if let Expr::Ident(instance_ident) = &*member_expr.obj { + if let Some(inst_map) = self.instantiations.get(&instance_ident.sym.to_string()) { + // Use the instantiation span for instances + inst_map.instantiation_span.hash(&mut hasher); + } + } + } + } + + // Also hash the call span to ensure uniqueness + call.span.hash(&mut hasher); + hasher.finish() + } + fn handle_bin_expr(&mut self, bin_expr: &BinExpr) { if let Expr::Call(call_expr) = &*bin_expr.left { self.visit_call_expr(call_expr); @@ -204,6 +230,12 @@ impl Visit for CallFinder { fn visit_call_expr(&mut self, call: &CallExpr) { if let Callee::Expr(expr) = &call.callee { + let call_id = self.generate_unique_call_id(call); + // If we've already processed this call, skip it + if !self.processed_calls.insert(call_id) { + // This call was already processed, so return early + return; + } match &**expr { Expr::Member(member_expr) => { let mut possible_class_name = None; @@ -214,10 +246,8 @@ impl Visit for CallFinder { } if let Some(ref obj_name) = possible_class_name { let mut new_class_name = None; - for instantiation in self.instantiations.iter() { - if &instantiation.variable_name == obj_name { - new_class_name = Some(instantiation.class_name.clone()); - } + if let Some(instantiation) = self.instantiations.get(obj_name) { + new_class_name = Some(instantiation.class_name.clone()); } if let Some(class_name) = new_class_name { possible_class_name = Some(class_name); @@ -322,6 +352,7 @@ impl Visit for CallFinder { _ => {} } } + } fn visit_var_declarator(&mut self, var_declarator: &VarDeclarator) { @@ -332,9 +363,11 @@ impl Visit for CallFinder { let class_name = expr.sym.to_string(); if let Some(var_ident) = &var_declarator.name.as_ident() { let var_name = var_ident.sym.to_string(); - self.instantiations.insert(InstantiationsMap { + let instantiation_span = var_ident.span; + self.instantiations.insert(var_name.clone(), InstantiationsMap { class_name, variable_name: var_name, + instantiation_span, }); } } @@ -439,16 +472,59 @@ impl Visit for CallFinder { self.visit_stmt(alt); } } + Stmt::Return(return_stmt) => { + if let Some(expr) = &return_stmt.arg { + self.visit_expr(expr); + } + // Handle returning an object expression + if let Some(block_stmt) = &return_stmt.arg.as_ref().and_then(|arg| match arg.as_ref() { + Expr::Object(object_expr) => Some(Box::new(object_expr.clone())), + _ => None, + }) { + for prop_or_spread in &block_stmt.props { + match prop_or_spread { + swc_ecma_ast::PropOrSpread::Prop(boxed_prop) => { + match &**boxed_prop { + swc_ecma_ast::Prop::KeyValue(key_value_prop) => { + // Handle KeyValue + self.visit_expr(&key_value_prop.value); + } + swc_ecma_ast::Prop::Assign(assign_prop) => { + // Handle Assign + self.visit_expr(&assign_prop.value); + } + swc_ecma_ast::Prop::Getter(getter_prop) => { + // Handle Getter + if let Some(body) = &getter_prop.body { + for stmt in &body.stmts { + self.visit_stmt(stmt); + } + } + } + swc_ecma_ast::Prop::Setter(setter_prop) => { + // Handle Setter + if let Some(body) = &setter_prop.body { + for stmt in &body.stmts { + self.visit_stmt(stmt); + } + } + } + _ => {} + } + } + swc_ecma_ast::PropOrSpread::Spread(spread) => { + // Handle Spread syntax + self.visit_expr(&spread.expr); + } + _ => {} + } + } + } + } _ => { // For other kinds of statements, we continue with the default implementation (for now) swc_ecma_visit::visit_stmt(self, stmt); } } } - - fn visit_expr(&mut self, expr: &Expr) { - if let Expr::Call(call_expr) = expr { - self.visit_call_expr(call_expr) - } - } } diff --git a/crates/does-it-throw/src/fixtures/callExpr.ts b/crates/does-it-throw/src/fixtures/callExpr.ts index fcca192..bbd3e8c 100644 --- a/crates/does-it-throw/src/fixtures/callExpr.ts +++ b/crates/does-it-throw/src/fixtures/callExpr.ts @@ -39,3 +39,11 @@ SomeRandomCall2(() => { connection.oneWithASecondArg({}, () => { throw new Error('hi khue') }) + +const testGetter = { + get test() { + SomeThrow() + } +} + +const array = [SomeThrow(), SomeThrow2()] diff --git a/crates/does-it-throw/src/fixtures/returnStatement.ts b/crates/does-it-throw/src/fixtures/returnStatement.ts new file mode 100644 index 0000000..0c3c70a --- /dev/null +++ b/crates/does-it-throw/src/fixtures/returnStatement.ts @@ -0,0 +1,32 @@ +// @ts-nocheck +const someThrow = () => { + if (something) { + while (true) { + throw new Error("oh no"); + } + } else { + for (let i = 0; i < 10; i++) { + throw new Error("oh no"); + } + } +} +class Test { + badMethod() { + throw new Error("oh no"); + } +} + +const callToSomeThrow = () => { + const testMethod = new Test(); + return { + test: someThrow(), + testing: () => someThrow(), + array: [someThrow(), someThrow()], + object: { test: someThrow() }, + class: testMethod.badMethod(), + } +} + +function test() { + return someThrow(); +} \ No newline at end of file diff --git a/crates/does-it-throw/src/lib.rs b/crates/does-it-throw/src/lib.rs index f06a5eb..159ee10 100644 --- a/crates/does-it-throw/src/lib.rs +++ b/crates/does-it-throw/src/lib.rs @@ -10,7 +10,7 @@ extern crate swc_ecma_ast; extern crate swc_ecma_parser; extern crate swc_ecma_visit; -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::vec; @@ -94,10 +94,11 @@ pub fn analyze_code( }; throw_collector.visit_module(&module); let mut call_collector = CallFinder { + processed_calls: HashSet::new(), functions_with_throws: throw_collector.functions_with_throws.clone(), calls: HashSet::new(), current_class_name: None, - instantiations: HashSet::new(), + instantiations: HashMap::new(), function_name_stack: vec![], object_property_stack: vec![], }; diff --git a/crates/does-it-throw/src/main.rs b/crates/does-it-throw/src/main.rs index b6433f4..a0f5ce2 100644 --- a/crates/does-it-throw/src/main.rs +++ b/crates/does-it-throw/src/main.rs @@ -438,7 +438,7 @@ mod integration_tests { // general result assertions assert_eq!(result.functions_with_throws.len(), 5); - assert_eq!(result.calls_to_throws.len(), 7); + assert_eq!(result.calls_to_throws.len(), 10); assert_eq!(result.imported_identifier_usages.len(), 0); assert_eq!(result.import_sources.len(), 0); @@ -467,18 +467,20 @@ mod integration_tests { .iter() .map(|c| c.id.clone()) .collect(); - fn calls_to_throws_contains(calls_to_throws: &Vec, call_to_throw: &str) -> bool { calls_to_throws.iter().any(|c| c == call_to_throw) } [ - "NOT_SET-onInitialized", - "NOT_SET-SomeRandomCall2", - "NOT_SET-", "connection-", + "NOT_SET-", "NOT_SET-onInitialized", + "NOT_SET-", "NOT_SET-SomeRandomCall2", + "NOT_SET-SomeRandomCall2", + "NOT_SET-onInitialized", + "NOT_SET-", "connection-", + "NOT_SET-", ] .iter() .for_each(|f| assert!(calls_to_throws_contains(&calls_to_throws, f))); @@ -939,7 +941,7 @@ mod integration_tests { } #[test] - fn test_should_not_include_throws_for_ignore_statements () { + fn test_should_not_include_throws_for_ignore_statements() { let manifest_dir = env::var("CARGO_MANIFEST_DIR").unwrap(); let file_path = format!("{}/src/fixtures/ignoreStatements.ts", manifest_dir); // Read sample code from file @@ -961,7 +963,7 @@ mod integration_tests { } #[test] - fn test_should_not_include_throws_for_ignore_statements_js () { + fn test_should_not_include_throws_for_ignore_statements_js() { let manifest_dir = env::var("CARGO_MANIFEST_DIR").unwrap(); let file_path = format!("{}/src/fixtures/ignoreStatements.js", manifest_dir); // Read sample code from file @@ -981,4 +983,59 @@ mod integration_tests { assert_eq!(result.functions_with_throws.len(), 0); assert_eq!(result.calls_to_throws.len(), 0); } + + #[test] + fn test_should_handle_return_statements() { + let manifest_dir = env::var("CARGO_MANIFEST_DIR").unwrap(); + let file_path = format!("{}/src/fixtures/returnStatement.ts", manifest_dir); + // Read sample code from file + let sample_code = fs::read_to_string(file_path).expect("Something went wrong reading the file"); + let ignore_statements = vec![ + "@it-throws".to_string(), + "@it-throws-ignore".to_string(), + "@some-random-ignore".to_string(), + ]; + let cm: Lrc = Default::default(); + let user_settings = UserSettings { + include_try_statement_throws: true, + ignore_statements, + }; + let (result, _cm) = analyze_code(&sample_code, cm, &user_settings); + + assert_eq!(result.functions_with_throws.len(), 2); + assert_eq!(result.calls_to_throws.len(), 7); + + let function_names: Vec = result + .functions_with_throws + .clone() + .into_iter() + .map(|f| f.function_or_method_name) + .collect(); + + fn function_names_contains(function_names: &Vec, function_name: &str) -> bool { + function_names.iter().any(|f| f == function_name) + } + + ["someThrow", "badMethod"] + .iter() + .for_each(|f| assert!(function_names_contains(&function_names, f))); + + let calls_to_throws: Vec = result.calls_to_throws.into_iter().map(|c| c.id).collect(); + + fn calls_to_throws_contains(calls_to_throws: &Vec, call_to_throw: &str) -> bool { + calls_to_throws.iter().any(|c| c == call_to_throw) + } + + [ + "NOT_SET-callToSomeThrow", + "NOT_SET-callToSomeThrow", + "NOT_SET-callToSomeThrow", + "NOT_SET-test", + "NOT_SET-callToSomeThrow", + "NOT_SET-callToSomeThrow", + "Test-callToSomeThrow", + ] + .iter() + .for_each(|f| assert!(calls_to_throws_contains(&calls_to_throws, f))); + } } diff --git a/package.json b/package.json index c635d04..536e3f4 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,12 @@ "throw finder", "throw", "javascript", - "typescript" + "typescript", + "lsp", + "language server", + "exceptions", + "extension", + "exception finder" ], "galleryBanner": { "color": "#050b1f", diff --git a/server/package.json b/server/package.json index 5070315..2e5e3fc 100644 --- a/server/package.json +++ b/server/package.json @@ -32,7 +32,8 @@ "lsp", "language server", "exceptions", - "extension" + "extension", + "exception finder" ], "qna": "https://github.com/michaelangeloio/does-it-throw/discussions", "dependencies": {