diff --git a/.vscode/settings.json b/.vscode/settings.json index 57b3355..7d0c7d0 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -21,4 +21,4 @@ "editor.defaultFormatter": "biomejs.biome" }, "doesItThrow.trace.server": "verbose" -} \ No newline at end of file +} \ No newline at end of file diff --git a/crates/does-it-throw-wasm/src/lib.rs b/crates/does-it-throw-wasm/src/lib.rs index a3f11fb..1dbdc3b 100644 --- a/crates/does-it-throw-wasm/src/lib.rs +++ b/crates/does-it-throw-wasm/src/lib.rs @@ -16,7 +16,7 @@ use wasm_bindgen::prelude::*; use does_it_throw::call_finder::CallToThrowMap; use does_it_throw::throw_finder::{IdentifierUsage, ThrowMap}; -use does_it_throw::{analyze_code, AnalysisResult}; +use does_it_throw::{analyze_code, AnalysisResult, UserSettings}; // Define an extern block with the `console.log` function. #[wasm_bindgen] @@ -403,6 +403,7 @@ interface InputData { function_throw_severity?: DiagnosticSeverityInput; call_to_throw_severity?: DiagnosticSeverityInput; call_to_imported_throw_severity?: DiagnosticSeverityInput; + include_try_statement_throws?: boolean; } "#; @@ -441,12 +442,13 @@ pub struct InputData { // uri: String, // typescript_settings: Option, // ids_to_check: Vec, - file_content: String, - debug: Option, - throw_statement_severity: Option, - function_throw_severity: Option, - call_to_throw_severity: Option, - call_to_imported_throw_severity: Option, + pub file_content: String, + pub debug: Option, + pub throw_statement_severity: Option, + pub function_throw_severity: Option, + pub call_to_throw_severity: Option, + pub call_to_imported_throw_severity: Option, + pub include_try_statement_throws: Option, } #[wasm_bindgen] @@ -456,7 +458,11 @@ pub fn parse_js(data: JsValue) -> JsValue { let cm: Lrc = Default::default(); - let (results, cm) = analyze_code(&input_data.file_content, cm); + let user_settings = UserSettings { + include_try_statement_throws: input_data.include_try_statement_throws.unwrap_or(false), + }; + + let (results, cm) = analyze_code(&input_data.file_content, cm, &user_settings); let parse_result = ParseResult::into(results, &cm, input_data.debug, input_data); @@ -870,4 +876,43 @@ mod tests { "Function imported that may throw." ); } + + #[test] + fn test_should_include_throws_in_try_statement() { + // smoke test to ensure backwards compatibility + let cm = Lrc::new(SourceMap::default()); + let source_file = cm.new_source_file( + FileName::Custom("test_file".into()), + "function foo() {\n try {\n throw new Error();\n } catch (e) {\n throw e;\n }\n}".into(), + ); + + let throw_span = Span::new( + source_file.start_pos + BytePos(34), + source_file.start_pos + BytePos(51), + Default::default(), + ); + + let functions_with_throws = HashSet::from([ThrowMap { + throw_statement: throw_span, + throw_spans: vec![throw_span], + function_or_method_name: "foo".to_string(), + class_name: None, + id: "foo".to_string(), + }]); + + let mut diagnostics: Vec = Vec::new(); + + add_diagnostics_for_functions_that_throw( + &mut diagnostics, + functions_with_throws, + &cm, + None, + DiagnosticSeverity::Hint, + DiagnosticSeverity::Hint, + ); + + assert_eq!(diagnostics.len(), 2); + assert_eq!(diagnostics[0].severity, DiagnosticSeverity::Hint.to_int()); + assert_eq!(diagnostics[0].message, "Function that may throw."); + } } diff --git a/crates/does-it-throw/src/fixtures/.vscode/settings.json b/crates/does-it-throw/src/fixtures/.vscode/settings.json index 06b0432..f6e25aa 100644 --- a/crates/does-it-throw/src/fixtures/.vscode/settings.json +++ b/crates/does-it-throw/src/fixtures/.vscode/settings.json @@ -1,3 +1,4 @@ { - "doesItThrow.trace.server": "verbose" + "doesItThrow.trace.server": "verbose", + "doesItThrow.includeTryStatementThrows": false } \ No newline at end of file diff --git a/crates/does-it-throw/src/fixtures/something.ts b/crates/does-it-throw/src/fixtures/something.ts new file mode 100644 index 0000000..06f1074 --- /dev/null +++ b/crates/does-it-throw/src/fixtures/something.ts @@ -0,0 +1,7 @@ +export const SomeThrow = () => { + throw new Error('never gonna let you down'); +} + +export function Something () { + throw new Error('never gonna run around and desert you') +} \ No newline at end of file diff --git a/crates/does-it-throw/src/fixtures/tryStatement.ts b/crates/does-it-throw/src/fixtures/tryStatement.ts new file mode 100644 index 0000000..50a3eb0 --- /dev/null +++ b/crates/does-it-throw/src/fixtures/tryStatement.ts @@ -0,0 +1,148 @@ +// @ts-nocheck +export const someConstThatThrows = () => { + try { + throw new Error('never gonna give you up') + } catch (e) { + console.log(e) + } +} + +function callToConstThatThrows4() { + someConstThatThrows() +} + +const someCondition = true +export class Something { + constructor() { + try { + throw new Error('hi khue') + } catch (e) { + console.log(e) + } + } + + someMethodThatThrows() { + try { + throw new Error('hi khue') + } catch (e) { + console.log(e) + } + } + + someMethodThatDoesNotThrow() { + console.log('hi khue') + } + + someMethodThatThrows2() { + if (someCondition) { + throw new Error('hi khue') + } + } + + nestedThrow() { + try { + if (someCondition) { + return true + } + throw new Error('hi khue') + } catch (e) { + console.log(e) + } + } + + callNestedThrow() { + if (someCondition) { + return true + } + if (someCondition) { + return true + } + this.nestedThrow() + } +} + +const _somethingCall = () => { + const something = new Something() + something.someMethodThatThrows() +} + +export const somethingCall = () => { + const something = new Something() + something.someMethodThatThrows() +} + +function _somethingCall2() { + const something = new Something() + something.someMethodThatThrows() +} + +export function somethingCall2() { + const something = new Something() + something.someMethodThatThrows() +} + +// @ts-nocheck +// should work for private class +class SomeClass { + constructor(public x: number) {} + + async _contextFromWorkflow() { + try { + throw new Error('Some error') + } catch (e) { + console.log(e) + } + } + + async someCallToThrow() { + const { user, stravaUser, streakContext } = opts?.contextFromWorkFlow ?? (await this._contextFromWorkflow(job)) + } +} + +// should work for exported class +// biome-ignore lint/suspicious/noRedeclare: +export class SomeClass2 { + constructor(public x: number) {} + + async _contextFromWorkflow() { + try { + throw new Error('Some error') + } catch (e) { + console.log(e) + } + } + + async someCallToThrow() { + const { user, stravaUser, streakContext } = opts?.contextFromWorkFlow ?? (await this._contextFromWorkflow(job)) + } +} + +const server = http.createServer(async (req, res) => { + switch (req.url) { + case '/api/pong': + console.log('pong!', INSTANCE_ID, PRIVATE_IP) + try { + throw new Error('') + } catch (e) { + console.log(e) + } + break + case '/api/ping': + console.log('ping!', INSTANCE_ID, PRIVATE_IP) + const ips = await SomeThrow() + someObjectLiteral.objectLiteralThrow() + const others = ips.filter((ip) => ip !== PRIVATE_IP) + + others.forEach((ip) => { + http.get(`http://[${ip}]:8080/api/pong`) + }) + break + case '/api/throw': + someRandomThrow() + break + } + + res.end() +}) + +const wss = new WebSocketServer({ noServer: true }) diff --git a/crates/does-it-throw/src/lib.rs b/crates/does-it-throw/src/lib.rs index 6094610..dc12f10 100644 --- a/crates/does-it-throw/src/lib.rs +++ b/crates/does-it-throw/src/lib.rs @@ -49,7 +49,15 @@ impl From for AnalysisResult { } } -pub fn analyze_code(content: &str, cm: Lrc) -> (AnalysisResult, Lrc) { +pub struct UserSettings { + pub include_try_statement_throws: bool, +} + +pub fn analyze_code( + content: &str, + cm: Lrc, + user_settings: &UserSettings, +) -> (AnalysisResult, Lrc) { let fm = cm.new_source_file(swc_common::FileName::Anon, content.into()); let lexer = Lexer::new( Syntax::Typescript(swc_ecma_parser::TsConfig { @@ -75,6 +83,7 @@ pub fn analyze_code(content: &str, cm: Lrc) -> (AnalysisResult, Lrc = Default::default(); - - let (result, _cm) = analyze_code(&sample_code, cm); + let user_settings = UserSettings { + include_try_statement_throws: false, + }; + let (result, _cm) = analyze_code(&sample_code, cm, &user_settings); for import in result.import_sources.into_iter() { println!("Imported {}", import); } @@ -75,8 +77,10 @@ mod integration_tests { // Read sample code from file let sample_code = fs::read_to_string(file_path).expect("Something went wrong reading the file"); let cm: Lrc = Default::default(); - - let (result, _cm) = analyze_code(&sample_code, cm); + let user_settings = UserSettings { + include_try_statement_throws: false, + }; + let (result, _cm) = analyze_code(&sample_code, cm, &user_settings); // general result assertions assert_eq!(result.functions_with_throws.len(), 4); @@ -134,8 +138,10 @@ mod integration_tests { // Read sample code from file let sample_code = fs::read_to_string(file_path).expect("Something went wrong reading the file"); let cm: Lrc = Default::default(); - - let (result, _cm) = analyze_code(&sample_code, cm); + let user_settings = UserSettings { + include_try_statement_throws: false, + }; + let (result, _cm) = analyze_code(&sample_code, cm, &user_settings); // general result assertions assert_eq!(result.functions_with_throws.len(), 4); @@ -193,8 +199,10 @@ mod integration_tests { // Read sample code from file let sample_code = fs::read_to_string(file_path).expect("Something went wrong reading the file"); let cm: Lrc = Default::default(); - - let (result, _cm) = analyze_code(&sample_code, cm); + let user_settings = UserSettings { + include_try_statement_throws: false, + }; + let (result, _cm) = analyze_code(&sample_code, cm, &user_settings); // general result assertions assert_eq!(result.functions_with_throws.len(), 4); @@ -247,8 +255,10 @@ mod integration_tests { // Read sample code from file let sample_code = fs::read_to_string(file_path).expect("Something went wrong reading the file"); let cm: Lrc = Default::default(); - - let (result, _cm) = analyze_code(&sample_code, cm); + let user_settings = UserSettings { + include_try_statement_throws: false, + }; + let (result, _cm) = analyze_code(&sample_code, cm, &user_settings); // general result assertions assert_eq!(result.functions_with_throws.len(), 4); @@ -302,8 +312,10 @@ mod integration_tests { // Read sample code from file let sample_code = fs::read_to_string(file_path).expect("Something went wrong reading the file"); let cm: Lrc = Default::default(); - - let (result, _cm) = analyze_code(&sample_code, cm); + let user_settings = UserSettings { + include_try_statement_throws: false, + }; + let (result, _cm) = analyze_code(&sample_code, cm, &user_settings); // general result assertions assert_eq!(result.functions_with_throws.len(), 3); @@ -356,8 +368,10 @@ mod integration_tests { // Read sample code from file let sample_code = fs::read_to_string(file_path).expect("Something went wrong reading the file"); let cm: Lrc = Default::default(); - - let (result, _cm) = analyze_code(&sample_code, cm); + let user_settings = UserSettings { + include_try_statement_throws: false, + }; + let (result, _cm) = analyze_code(&sample_code, cm, &user_settings); // general result assertions assert_eq!(result.functions_with_throws.len(), 3); @@ -409,8 +423,10 @@ mod integration_tests { // Read sample code from file let sample_code = fs::read_to_string(file_path).expect("Something went wrong reading the file"); let cm: Lrc = Default::default(); - - let (result, _cm) = analyze_code(&sample_code, cm); + let user_settings = UserSettings { + include_try_statement_throws: false, + }; + let (result, _cm) = analyze_code(&sample_code, cm, &user_settings); // general result assertions assert_eq!(result.functions_with_throws.len(), 5); @@ -467,8 +483,10 @@ mod integration_tests { // Read sample code from file let sample_code = fs::read_to_string(file_path).expect("Something went wrong reading the file"); let cm: Lrc = Default::default(); - - let (result, _cm) = analyze_code(&sample_code, cm); + let user_settings = UserSettings { + include_try_statement_throws: false, + }; + let (result, _cm) = analyze_code(&sample_code, cm, &user_settings); // general result assertions assert_eq!(result.functions_with_throws.len(), 4); @@ -517,8 +535,10 @@ mod integration_tests { // Read sample code from file let sample_code = fs::read_to_string(file_path).expect("Something went wrong reading the file"); let cm: Lrc = Default::default(); - - let (result, _cm) = analyze_code(&sample_code, cm); + let user_settings = UserSettings { + include_try_statement_throws: false, + }; + let (result, _cm) = analyze_code(&sample_code, cm, &user_settings); // general result assertions assert_eq!(result.functions_with_throws.len(), 4); @@ -567,8 +587,10 @@ mod integration_tests { // Read sample code from file let sample_code = fs::read_to_string(file_path).expect("Something went wrong reading the file"); let cm: Lrc = Default::default(); - - let (result, _cm) = analyze_code(&sample_code, cm); + let user_settings = UserSettings { + include_try_statement_throws: false, + }; + let (result, _cm) = analyze_code(&sample_code, cm, &user_settings); // general result assertions assert_eq!(result.functions_with_throws.len(), 0); @@ -616,8 +638,10 @@ mod integration_tests { // Read sample code from file let sample_code = fs::read_to_string(file_path).expect("Something went wrong reading the file"); let cm: Lrc = Default::default(); - - let (result, _cm) = analyze_code(&sample_code, cm); + let user_settings = UserSettings { + include_try_statement_throws: false, + }; + let (result, _cm) = analyze_code(&sample_code, cm, &user_settings); // general result assertions assert_eq!(result.functions_with_throws.len(), 2); @@ -660,8 +684,10 @@ mod integration_tests { // Read sample code from file let sample_code = fs::read_to_string(file_path).expect("Something went wrong reading the file"); let cm: Lrc = Default::default(); - - let (result, _cm) = analyze_code(&sample_code, cm); + let user_settings = UserSettings { + include_try_statement_throws: false, + }; + let (result, _cm) = analyze_code(&sample_code, cm, &user_settings); // general result assertions assert_eq!(result.functions_with_throws.len(), 2); @@ -721,4 +747,69 @@ mod integration_tests { .iter() .for_each(|f| assert!(import_identifiers_contains(&import_identifiers, f))); } + + #[test] + fn test_try_statement() { + let manifest_dir = env::var("CARGO_MANIFEST_DIR").unwrap(); + let file_path = format!("{}/src/fixtures/tryStatement.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 cm: Lrc = Default::default(); + let user_settings = UserSettings { + include_try_statement_throws: true, + }; + let (result, _cm) = analyze_code(&sample_code, cm, &user_settings); + + // general result assertions + assert_eq!(result.functions_with_throws.len(), 8); + assert_eq!(result.calls_to_throws.len(), 9); + assert_eq!(result.imported_identifier_usages.len(), 0); + assert_eq!(result.import_sources.len(), 0); + + // function names + 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) + } + + [ + "someMethodThatThrows", + "_contextFromWorkflow", + "createServer", + "", + "someConstThatThrows", + "nestedThrow", + "_contextFromWorkflow", + "someMethodThatThrows2", + ] + .iter() + .for_each(|f| assert!(function_names_contains(&function_names, f))); + + // calls to throws + 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) + } + + [ + "Something-somethingCall", + "Something-_somethingCall2", + "NOT_SET-someCallToThrow", + "Something-somethingCall2", + "http-", + "Something-_somethingCall", + "NOT_SET-callNestedThrow", + "NOT_SET-callToConstThatThrows4", + "NOT_SET-someCallToThrow", + ] + .iter() + .for_each(|f| assert!(calls_to_throws_contains(&calls_to_throws, f))); + } } diff --git a/crates/does-it-throw/src/throw_finder.rs b/crates/does-it-throw/src/throw_finder.rs index f24a42b..d63ebf3 100644 --- a/crates/does-it-throw/src/throw_finder.rs +++ b/crates/does-it-throw/src/throw_finder.rs @@ -29,12 +29,24 @@ fn prop_name_to_string(prop_name: &PropName) -> String { } pub struct ThrowFinder { - throw_spans: Vec, + pub throw_spans: Vec, + pub inside_try_statement: bool, + pub include_try_statements: bool, } impl Visit for ThrowFinder { fn visit_throw_stmt(&mut self, node: &ThrowStmt) { - self.throw_spans.push(node.span) + if !self.inside_try_statement { + self.throw_spans.push(node.span); + } + if self.inside_try_statement && self.include_try_statements { + self.throw_spans.push(node.span); + } + } + fn visit_try_stmt(&mut self,n: &swc_ecma_ast::TryStmt) { + self.inside_try_statement = true; + swc_ecma_visit::visit_try_stmt(self, n); + self.inside_try_statement = false; } } @@ -107,12 +119,15 @@ pub struct ThrowAnalyzer { pub function_name_stack: Vec, pub current_class_name: Option, pub current_method_name: Option, + pub include_try_statement: bool, } impl ThrowAnalyzer { fn check_function_for_throws(&mut self, function: &Function) { let mut throw_finder = ThrowFinder { throw_spans: vec![], + inside_try_statement: false, + include_try_statements: self.include_try_statement, }; throw_finder.visit_function(function); if !throw_finder.throw_spans.is_empty() { @@ -145,6 +160,8 @@ impl ThrowAnalyzer { fn check_arrow_function_for_throws(&mut self, arrow_function: &ArrowExpr) { let mut throw_finder = ThrowFinder { throw_spans: vec![], + inside_try_statement: false, + include_try_statements: self.include_try_statement, }; throw_finder.visit_arrow_expr(arrow_function); if !throw_finder.throw_spans.is_empty() { @@ -177,6 +194,8 @@ impl ThrowAnalyzer { fn check_constructor_for_throws(&mut self, constructor: &Constructor) { let mut throw_finder = ThrowFinder { throw_spans: vec![], + inside_try_statement: false, + include_try_statements: self.include_try_statement, }; throw_finder.visit_constructor(constructor); if !throw_finder.throw_spans.is_empty() { @@ -289,6 +308,8 @@ impl Visit for ThrowAnalyzer { Expr::Arrow(arrow_expr) => { let mut throw_finder = ThrowFinder { throw_spans: vec![], + inside_try_statement: false, + include_try_statements: self.include_try_statement, }; throw_finder.visit_arrow_expr(arrow_expr); if !throw_finder.throw_spans.is_empty() { @@ -345,6 +366,8 @@ impl Visit for ThrowAnalyzer { let mut throw_finder = ThrowFinder { throw_spans: vec![], + inside_try_statement: false, + include_try_statements: self.include_try_statement, }; throw_finder.visit_function(&method_prop.function); @@ -374,6 +397,8 @@ impl Visit for ThrowAnalyzer { Expr::Fn(fn_expr) => { let mut throw_finder = ThrowFinder { throw_spans: vec![], + inside_try_statement: false, + include_try_statements: self.include_try_statement, }; throw_finder.visit_function(&fn_expr.function); let function_name = prop_name_to_string(&key_value_prop.key); @@ -399,6 +424,8 @@ impl Visit for ThrowAnalyzer { Expr::Arrow(arrow_expr) => { let mut throw_finder = ThrowFinder { throw_spans: vec![], + inside_try_statement: false, + include_try_statements: self.include_try_statement, }; throw_finder.visit_arrow_expr(arrow_expr); let function_name = prop_name_to_string(&key_value_prop.key); @@ -437,6 +464,8 @@ impl Visit for ThrowAnalyzer { let function_name = ident.sym.to_string(); let mut throw_finder = ThrowFinder { throw_spans: vec![], + inside_try_statement: false, + include_try_statements: self.include_try_statement, }; // Check if the init is a function expression or arrow function @@ -585,6 +614,8 @@ impl Visit for ThrowAnalyzer { let mut throw_finder = ThrowFinder { throw_spans: vec![], + inside_try_statement: false, + include_try_statements: self.include_try_statement, }; throw_finder.visit_class_method(class_method); diff --git a/docs/neovim.md b/docs/neovim.md index 3e4e2dd..183105b 100644 --- a/docs/neovim.md +++ b/docs/neovim.md @@ -53,6 +53,7 @@ local server_config = { functionThrowSeverity = "Hint", callToThrowSeverity = "Hint", callToImportedThrowSeverity = "Hint", + includeTryStatementThrows = false, maxNumberOfProblems = 10000 } } diff --git a/package.json b/package.json index 5bbfc7e..988a56f 100644 --- a/package.json +++ b/package.json @@ -127,6 +127,12 @@ "default": 100, "description": "Controls the maximum number of problems produced by the server." }, + "doesItThrow.includeTryStatementThrows": { + "scope": "resource", + "type": "boolean", + "default": false, + "description": "Include throw statements inside try statements." + }, "doesItThrow.trace.server": { "scope": "window", "type": "string", diff --git a/server/src/server.ts b/server/src/server.ts index e0f05d4..67d9274 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -81,6 +81,7 @@ interface Settings { functionThrowSeverity: DiagnosticSeverity callToThrowSeverity: DiagnosticSeverity callToImportedThrowSeverity: DiagnosticSeverity + includeTryStatementThrows: boolean } // The global settings, used when the `workspace/configuration` request is not supported by the client. @@ -91,7 +92,8 @@ const defaultSettings: Settings = { throwStatementSeverity: 'Hint', functionThrowSeverity: 'Hint', callToThrowSeverity: 'Hint', - callToImportedThrowSeverity: 'Hint' + callToImportedThrowSeverity: 'Hint', + includeTryStatementThrows: false } // 👆 very unlikely someone will have more than 1 million throw statements, lol // if you do, might want to rethink your code? @@ -180,10 +182,12 @@ async function validateTextDocument(textDocument: TextDocument): Promise { typescript_settings: { decorators: true }, - function_throw_severity: settings.functionThrowSeverity, - throw_statement_severity: settings.throwStatementSeverity, - call_to_imported_throw_severity: settings.callToImportedThrowSeverity, - call_to_throw_severity: settings.callToThrowSeverity + function_throw_severity: settings?.functionThrowSeverity ?? defaultSettings.functionThrowSeverity, + throw_statement_severity: settings?.throwStatementSeverity ?? defaultSettings.throwStatementSeverity, + call_to_imported_throw_severity: + settings?.callToImportedThrowSeverity ?? defaultSettings.callToImportedThrowSeverity, + call_to_throw_severity: settings?.callToThrowSeverity ?? defaultSettings.callToThrowSeverity, + include_try_statement_throws: settings?.includeTryStatementThrows ?? defaultSettings.includeTryStatementThrows } satisfies InputData const analysis = parse_js(opts) as ParseResult