From a0bbb48592e6219fecac8324c3acc3967794b537 Mon Sep 17 00:00:00 2001 From: Matt Brown Date: Tue, 21 May 2024 11:24:25 -0400 Subject: [PATCH] Always assume awaiting is impure --- src/analyzer/expr/call/arguments_analyzer.rs | 14 ++++------- .../call/function_call_return_type_fetcher.rs | 5 ++-- .../call/method_call_return_type_fetcher.rs | 8 +------ src/analyzer/expr/variable_fetch_analyzer.rs | 23 +------------------ src/analyzer/expression_analyzer.rs | 4 ++-- src/analyzer/functionlike_analyzer.rs | 2 -- .../negated_assertion_reconciler.rs | 3 +++ src/analyzer/scope_context/mod.rs | 6 ----- src/analyzer/stmt/return_analyzer.rs | 1 - src/code_info/t_atomic.rs | 1 - src/code_info_builder/typehint_resolver.rs | 2 -- src/ttype/type_combination.rs | 6 ++--- src/ttype/type_combiner.rs | 21 +++++------------ src/ttype/type_expander.rs | 11 +-------- .../usedAwaitablePipedToJoin/output.txt | 12 ++++------ .../usedVecFromAsync/input.hack | 3 +++ 16 files changed, 30 insertions(+), 92 deletions(-) create mode 100644 tests/unused/UnusedExpression/usedVecFromAsync/input.hack diff --git a/src/analyzer/expr/call/arguments_analyzer.rs b/src/analyzer/expr/call/arguments_analyzer.rs index a749c1cc..c47f6f16 100644 --- a/src/analyzer/expr/call/arguments_analyzer.rs +++ b/src/analyzer/expr/call/arguments_analyzer.rs @@ -178,20 +178,18 @@ pub(crate) fn check_arguments_match( for (_, arg_expr) in args.iter() { let was_inside_call = context.inside_general_use; - let was_inside_asio_join = context.inside_asio_join; if matches!(functionlike_info.effects, FnEffect::Some(_)) || matches!(functionlike_info.effects, FnEffect::Arg(_)) || functionlike_info.has_throw || functionlike_info.user_defined || functionlike_info.method_info.is_some() + || matches!( + functionlike_id, + FunctionLikeIdentifier::Function(StrId::ASIO_JOIN) + ) { context.inside_general_use = true; - } else if matches!( - functionlike_id, - FunctionLikeIdentifier::Function(StrId::ASIO_JOIN) - ) { - context.inside_asio_join = true; } // don't analyse closures here @@ -208,10 +206,6 @@ pub(crate) fn check_arguments_match( if !was_inside_call { context.inside_general_use = false; } - - if !was_inside_asio_join { - context.inside_asio_join = false; - } } let mut reordered_args = args.iter().enumerate().collect::>(); diff --git a/src/analyzer/expr/call/function_call_return_type_fetcher.rs b/src/analyzer/expr/call/function_call_return_type_fetcher.rs index 19207ef5..57d9d67e 100644 --- a/src/analyzer/expr/call/function_call_return_type_fetcher.rs +++ b/src/analyzer/expr/call/function_call_return_type_fetcher.rs @@ -132,7 +132,6 @@ pub(crate) fn fetch( .get_file_source() .file_path, ), - effects: Some(function_storage.effects.to_u8().unwrap_or(EFFECT_IMPURE)), ..Default::default() }, &mut analysis_data.data_flow_graph, @@ -612,7 +611,7 @@ fn handle_special_functions( let mut new_types = vec![]; for atomic_type in awaited_types { - if let TAtomic::TAwaitable { value, effects } = atomic_type { + if let TAtomic::TAwaitable { value } = atomic_type { let inside_type = (*value).clone(); extend_dataflow_uniquely( &mut awaited_type.parent_nodes, @@ -622,7 +621,7 @@ fn handle_special_functions( analysis_data.expr_effects.insert( (pos.start_offset() as u32, pos.end_offset() as u32), - effects.unwrap_or(EFFECT_IMPURE), + EFFECT_IMPURE, ); } else { new_types.push(atomic_type); diff --git a/src/analyzer/expr/call/method_call_return_type_fetcher.rs b/src/analyzer/expr/call/method_call_return_type_fetcher.rs index 9238b592..bdde0758 100644 --- a/src/analyzer/expr/call/method_call_return_type_fetcher.rs +++ b/src/analyzer/expr/call/method_call_return_type_fetcher.rs @@ -1,7 +1,7 @@ use std::rc::Rc; use hakana_reflection_info::functionlike_identifier::FunctionLikeIdentifier; -use hakana_reflection_info::{ExprId, GenericParent, VarId, EFFECT_IMPURE}; +use hakana_reflection_info::{ExprId, GenericParent, VarId}; use hakana_str::{Interner, StrId}; use hakana_type::get_mixed; use oxidized::{aast, ast_defs}; @@ -133,12 +133,6 @@ pub(crate) fn fetch( .get_file_source() .file_path, ), - effects: Some( - functionlike_storage - .effects - .to_u8() - .unwrap_or(EFFECT_IMPURE), - ), ..Default::default() }, &mut analysis_data.data_flow_graph, diff --git a/src/analyzer/expr/variable_fetch_analyzer.rs b/src/analyzer/expr/variable_fetch_analyzer.rs index 502e1d9f..e29803fd 100644 --- a/src/analyzer/expr/variable_fetch_analyzer.rs +++ b/src/analyzer/expr/variable_fetch_analyzer.rs @@ -11,10 +11,9 @@ use hakana_reflection_info::{ path::PathKind, }, issue::{Issue, IssueKind}, - t_atomic::TAtomic, t_union::TUnion, taint::SourceType, - VarId, EFFECT_IMPURE, EFFECT_PURE, EFFECT_READ_GLOBALS, + VarId, EFFECT_READ_GLOBALS, }; use hakana_type::{get_int, get_mixed_any, get_mixed_dict}; use oxidized::{ast_defs::Pos, tast::Lid}; @@ -214,26 +213,6 @@ fn add_dataflow_to_variable( data_flow_graph, &mut stmt_type, ); - } else if data_flow_graph.kind == GraphKind::FunctionBody && context.inside_asio_join { - if let TAtomic::TAwaitable { effects, .. } = stmt_type.get_single() { - if effects.unwrap_or(EFFECT_IMPURE) != EFFECT_PURE { - add_dataflow_to_used_var( - statements_analyzer, - pos, - lid, - data_flow_graph, - &mut stmt_type, - ); - } - } else { - add_dataflow_to_used_var( - statements_analyzer, - pos, - lid, - data_flow_graph, - &mut stmt_type, - ); - } } stmt_type diff --git a/src/analyzer/expression_analyzer.rs b/src/analyzer/expression_analyzer.rs index d616d362..62c72a4f 100644 --- a/src/analyzer/expression_analyzer.rs +++ b/src/analyzer/expression_analyzer.rs @@ -416,7 +416,7 @@ pub(crate) fn analyze( let mut new_types = vec![]; for atomic_type in awaited_types { - if let TAtomic::TAwaitable { value, effects } = atomic_type { + if let TAtomic::TAwaitable { value } = atomic_type { let inside_type = (*value).clone(); extend_dataflow_uniquely( &mut awaited_stmt_type.parent_nodes, @@ -425,7 +425,7 @@ pub(crate) fn analyze( new_types.extend(inside_type.types); analysis_data.expr_effects.insert( (expr.1.start_offset() as u32, expr.1.end_offset() as u32), - effects.unwrap_or(EFFECT_IMPURE), + EFFECT_IMPURE, ); } else { new_types.push(atomic_type); diff --git a/src/analyzer/functionlike_analyzer.rs b/src/analyzer/functionlike_analyzer.rs index 37e8397a..6739278c 100644 --- a/src/analyzer/functionlike_analyzer.rs +++ b/src/analyzer/functionlike_analyzer.rs @@ -705,7 +705,6 @@ impl<'a> FunctionLikeAnalyzer<'a> { inferred_return_type = Some(if functionlike_storage.is_async { wrap_atomic(TAtomic::TAwaitable { value: Box::new(fn_return_value), - effects: functionlike_storage.effects.to_u8(), }) } else { fn_return_value @@ -739,7 +738,6 @@ impl<'a> FunctionLikeAnalyzer<'a> { inferred_return_type = Some(if functionlike_storage.is_async { wrap_atomic(TAtomic::TAwaitable { value: Box::new(get_void()), - effects: functionlike_storage.effects.to_u8(), }) } else { get_void() diff --git a/src/analyzer/reconciler/negated_assertion_reconciler.rs b/src/analyzer/reconciler/negated_assertion_reconciler.rs index 5915d5c2..f505d5e2 100644 --- a/src/analyzer/reconciler/negated_assertion_reconciler.rs +++ b/src/analyzer/reconciler/negated_assertion_reconciler.rs @@ -607,6 +607,9 @@ fn handle_literal_negated_equality( acceptable_types.push(existing_atomic_type); } + TAtomic::TAwaitable { .. } => { + acceptable_types.push(existing_atomic_type); + } TAtomic::TTypeVariable { .. } => { did_remove_type = true; acceptable_types.push(existing_atomic_type); diff --git a/src/analyzer/scope_context/mod.rs b/src/analyzer/scope_context/mod.rs index f3fcfdf6..6ad7155b 100644 --- a/src/analyzer/scope_context/mod.rs +++ b/src/analyzer/scope_context/mod.rs @@ -115,11 +115,6 @@ pub struct ScopeContext { pub inside_awaitall: bool, - /** - * Whether or not we're inside an HH\Asio\join - */ - pub inside_asio_join: bool, - pub include_location: Option, /** @@ -198,7 +193,6 @@ impl ScopeContext { inside_try: false, inside_awaitall: false, inside_loop_exprs: false, - inside_asio_join: false, inside_negation: false, has_returned: false, diff --git a/src/analyzer/stmt/return_analyzer.rs b/src/analyzer/stmt/return_analyzer.rs index 743fa2c8..53143350 100644 --- a/src/analyzer/stmt/return_analyzer.rs +++ b/src/analyzer/stmt/return_analyzer.rs @@ -135,7 +135,6 @@ pub(crate) fn analyze( let parent_nodes = inferred_return_type.parent_nodes.clone(); inferred_return_type = wrap_atomic(TAtomic::TAwaitable { value: Box::new(inferred_return_type), - effects: None, }); extend_dataflow_uniquely(&mut inferred_return_type.parent_nodes, parent_nodes); } diff --git a/src/code_info/t_atomic.rs b/src/code_info/t_atomic.rs index 4e4caf2e..ca6362ad 100644 --- a/src/code_info/t_atomic.rs +++ b/src/code_info/t_atomic.rs @@ -44,7 +44,6 @@ pub enum TAtomic { }, TAwaitable { value: Box, - effects: Option, }, TBool, TClassname { diff --git a/src/code_info_builder/typehint_resolver.rs b/src/code_info_builder/typehint_resolver.rs index 0e1f35ac..8dbd69be 100644 --- a/src/code_info_builder/typehint_resolver.rs +++ b/src/code_info_builder/typehint_resolver.rs @@ -634,7 +634,6 @@ pub fn get_type_from_hint( ) { TAtomic::TAwaitable { value: Box::new(inner_type), - effects: None, } } else { TAtomic::TMixedWithFlags(true, false, false, false) @@ -642,7 +641,6 @@ pub fn get_type_from_hint( } else { TAtomic::TAwaitable { value: Box::new(get_mixed_any()), - effects: None, } } } diff --git a/src/ttype/type_combination.rs b/src/ttype/type_combination.rs index a86fd4a7..e7b266f5 100644 --- a/src/ttype/type_combination.rs +++ b/src/ttype/type_combination.rs @@ -35,7 +35,7 @@ pub(crate) struct TypeCombination { pub dict_type_params: Option<(TUnion, TUnion)>, pub vec_type_param: Option, pub keyset_type_param: Option, - pub awaitable_info: Option<(TUnion, u8)>, + pub awaitable_param: Option, pub dict_alias_name: Option)>>, @@ -72,7 +72,7 @@ impl TypeCombination { dict_type_params: None, vec_type_param: None, keyset_type_param: None, - awaitable_info: None, + awaitable_param: None, dict_alias_name: None, falsy_mixed: None, truthy_mixed: None, @@ -96,7 +96,7 @@ impl TypeCombination { &self.dict_type_params, &self.vec_type_param, &self.keyset_type_param, - &self.awaitable_info, + &self.awaitable_param, ) { return self.dict_entries.is_empty() && self.vec_entries.is_empty() diff --git a/src/ttype/type_combiner.rs b/src/ttype/type_combiner.rs index 922f7f8f..06c0c6f0 100644 --- a/src/ttype/type_combiner.rs +++ b/src/ttype/type_combiner.rs @@ -5,7 +5,6 @@ use hakana_reflection_info::{ codebase_info::{symbols::SymbolKind, CodebaseInfo}, t_atomic::{DictKey, TAtomic}, t_union::TUnion, - EFFECT_IMPURE, }; use hakana_str::StrId; use itertools::Itertools; @@ -117,10 +116,9 @@ pub fn combine( }); } - if let Some((type_param, effects)) = combination.awaitable_info { + if let Some(type_param) = combination.awaitable_param { new_types.push(TAtomic::TAwaitable { value: Box::new(type_param), - effects: Some(effects), }); } @@ -674,19 +672,12 @@ fn scrape_type_properties( return; } - if let TAtomic::TAwaitable { - ref value, - ref effects, - } = atomic - { - combination.awaitable_info = Some( - if let Some(ref existing_info) = combination.awaitable_info { - ( - combine_union_types(&existing_info.0, value, codebase, overwrite_empty_array), - effects.unwrap_or(EFFECT_IMPURE) & existing_info.1, - ) + if let TAtomic::TAwaitable { ref value } = atomic { + combination.awaitable_param = Some( + if let Some(ref existing_info) = combination.awaitable_param { + combine_union_types(&existing_info, value, codebase, overwrite_empty_array) } else { - ((**value).clone(), effects.unwrap_or(EFFECT_IMPURE)) + (**value).clone() }, ); diff --git a/src/ttype/type_expander.rs b/src/ttype/type_expander.rs index a65fa5a0..eaa086b2 100644 --- a/src/ttype/type_expander.rs +++ b/src/ttype/type_expander.rs @@ -45,7 +45,6 @@ pub struct TypeExpansionOptions<'a> { pub expand_templates: bool, pub expand_hakana_types: bool, pub expand_all_type_aliases: bool, - pub effects: Option, } impl Default for TypeExpansionOptions<'_> { @@ -62,7 +61,6 @@ impl Default for TypeExpansionOptions<'_> { expand_templates: true, expand_hakana_types: true, expand_all_type_aliases: false, - effects: None, } } } @@ -178,14 +176,7 @@ fn expand_atomic( expand_union(codebase, interner, type_param, options, data_flow_graph); return; - } else if let TAtomic::TAwaitable { - ref mut value, - effects, - } = return_type_part - { - if let Some(inserted_effects) = options.effects { - *effects = Some(inserted_effects); - } + } else if let TAtomic::TAwaitable { ref mut value } = return_type_part { expand_union(codebase, interner, value, options, data_flow_graph); return; diff --git a/tests/unused/UnusedExpression/usedAwaitablePipedToJoin/output.txt b/tests/unused/UnusedExpression/usedAwaitablePipedToJoin/output.txt index 3d971166..8e8068fb 100644 --- a/tests/unused/UnusedExpression/usedAwaitablePipedToJoin/output.txt +++ b/tests/unused/UnusedExpression/usedAwaitablePipedToJoin/output.txt @@ -1,16 +1,12 @@ -ERROR: UnusedAssignmentStatement - input.hack:15:5 - Assignment to $a is unused, and this expression has no effect -ERROR: UnusedPipeVariable - input.hack:15:10 - The pipe data in this expression is not used anywhere +ERROR: UnusedAssignment - input.hack:15:5 - Assignment to $a is unused ERROR: UnusedAssignmentStatement - input.hack:16:5 - Assignment to $b is unused, and this expression has no effect ERROR: UnusedPipeVariable - input.hack:16:10 - The pipe data in this expression is not used anywhere -ERROR: UnusedAssignmentStatement - input.hack:17:5 - Assignment to $c is unused, and this expression has no effect +ERROR: UnusedAssignment - input.hack:17:5 - Assignment to $c is unused ERROR: UnusedPipeVariable - input.hack:17:10 - The pipe data in this expression is not used anywhere -ERROR: UnusedPipeVariable - input.hack:17:10 - The pipe data in this expression is not used anywhere -ERROR: UnusedAssignmentStatement - input.hack:18:5 - Assignment to $a is unused, and this expression has no effect -ERROR: UnusedPipeVariable - input.hack:18:10 - The pipe data in this expression is not used anywhere +ERROR: UnusedAssignment - input.hack:18:5 - Assignment to $a is unused ERROR: UnusedAssignmentStatement - input.hack:19:5 - Assignment to $b is unused, and this expression has no effect ERROR: UnusedPipeVariable - input.hack:19:10 - The pipe data in this expression is not used anywhere -ERROR: UnusedAssignmentStatement - input.hack:20:5 - Assignment to $c is unused, and this expression has no effect -ERROR: UnusedPipeVariable - input.hack:20:10 - The pipe data in this expression is not used anywhere +ERROR: UnusedAssignment - input.hack:20:5 - Assignment to $c is unused ERROR: UnusedPipeVariable - input.hack:20:10 - The pipe data in this expression is not used anywhere ERROR: UnusedAssignment - input.hack:21:5 - Assignment to $a is unused ERROR: UnusedAwaitable - input.hack:22:5 - Assignment to awaitable $b is unused diff --git a/tests/unused/UnusedExpression/usedVecFromAsync/input.hack b/tests/unused/UnusedExpression/usedVecFromAsync/input.hack new file mode 100644 index 00000000..c9cfd172 --- /dev/null +++ b/tests/unused/UnusedExpression/usedVecFromAsync/input.hack @@ -0,0 +1,3 @@ +async function foo(vec> $vec): Awaitable { + await HH\Lib\Vec\from_async($vec); +} \ No newline at end of file