From f29ddb735e3116a5fca5eebd877307b76e1acee6 Mon Sep 17 00:00:00 2001 From: Matt Brown Date: Mon, 7 Oct 2024 11:26:19 -0400 Subject: [PATCH] Fix a bunch of minor bugs --- .../expr/binop/arithmetic_analyzer.rs | 4 +- src/analyzer/expr/call/argument_analyzer.rs | 24 ++++++++++- src/analyzer/function_analysis_data.rs | 1 + src/code_info/data_flow/node.rs | 13 +++--- src/code_info/t_atomic.rs | 26 ++++++----- src/code_info/t_union.rs | 43 +++++++++++++------ .../ttype/comparison/dict_type_comparator.rs | 12 ++++++ .../comparison/type_comparison_result.rs | 6 +++ .../ttype/comparison/union_type_comparator.rs | 4 ++ src/code_info_builder/lib.rs | 4 +- .../BinaryOperation/loopAddition/input.hack | 21 +++++++++ .../possiblyInvalidShapeKey/input.hack | 9 ++++ .../possiblyInvalidShapeKey/output.txt | 1 + .../Class/enumMemberMatchesEnum/input.hack | 21 +++++++++ .../Generics/Class/newtypeAs/defs.hack | 1 + .../Generics/Class/newtypeAs/input.hack | 10 +++++ .../Generics/Class/newtypeAs/other_defs.hack | 1 + 17 files changed, 165 insertions(+), 36 deletions(-) create mode 100644 tests/inference/BinaryOperation/loopAddition/input.hack create mode 100644 tests/inference/DictVecKeyset/possiblyInvalidShapeKey/input.hack create mode 100644 tests/inference/DictVecKeyset/possiblyInvalidShapeKey/output.txt create mode 100644 tests/inference/Generics/Class/enumMemberMatchesEnum/input.hack create mode 100644 tests/inference/Generics/Class/newtypeAs/defs.hack create mode 100644 tests/inference/Generics/Class/newtypeAs/input.hack create mode 100644 tests/inference/Generics/Class/newtypeAs/other_defs.hack diff --git a/src/analyzer/expr/binop/arithmetic_analyzer.rs b/src/analyzer/expr/binop/arithmetic_analyzer.rs index 29055905..4ac76762 100644 --- a/src/analyzer/expr/binop/arithmetic_analyzer.rs +++ b/src/analyzer/expr/binop/arithmetic_analyzer.rs @@ -186,8 +186,8 @@ pub(crate) fn analyze<'expr: 'tast, 'tast>( results.push(if has_loop_variable { match (&e1_type_atomic, &e2_type_atomic) { ( - TAtomic::TInt | TAtomic::TLiteralInt { .. }, - TAtomic::TInt | TAtomic::TLiteralInt { .. }, + TAtomic::TInt | TAtomic::TLiteralInt { .. } | TAtomic::TNothing, + TAtomic::TInt | TAtomic::TLiteralInt { .. } | TAtomic::TNothing, ) => match operator { oxidized::ast_defs::Bop::Slash => TAtomic::TNum, _ => TAtomic::TInt, diff --git a/src/analyzer/expr/call/argument_analyzer.rs b/src/analyzer/expr/call/argument_analyzer.rs index 813f14dd..c26c9d96 100644 --- a/src/analyzer/expr/call/argument_analyzer.rs +++ b/src/analyzer/expr/call/argument_analyzer.rs @@ -423,11 +423,33 @@ pub(crate) fn verify_type( Issue::new( IssueKind::PossiblyInvalidArgument, format!( - "Argument {} of {} expects {}, possibly different type {} provided", + "Argument {} of {} expects {}, possibly different type {} provided{}", (argument_offset + 1), functionlike_id.to_string(statements_analyzer.get_interner()), param_type.get_id(Some(statements_analyzer.get_interner())), input_type.get_id(Some(statements_analyzer.get_interner())), + if let Some(type_mismatch_parent_nodes) = + union_comparison_result.type_mismatch_parents + { + if !type_mismatch_parent_nodes.0.is_empty() { + if let Some(pos) = type_mismatch_parent_nodes.0[0].get_pos() { + format!( + " in :{}:{} is a mismatch with {}", + pos.start_line, + pos.start_column, + type_mismatch_parent_nodes + .1 + .get_id(Some(statements_analyzer.get_interner())) + ) + } else { + "".to_string() + } + } else { + "".to_string() + } + } else { + "".to_string() + } ), statements_analyzer.get_hpos(input_expr.pos()), &context.function_context.calling_functionlike_id, diff --git a/src/analyzer/function_analysis_data.rs b/src/analyzer/function_analysis_data.rs index 6430a57b..b46c5b7d 100644 --- a/src/analyzer/function_analysis_data.rs +++ b/src/analyzer/function_analysis_data.rs @@ -340,6 +340,7 @@ impl FunctionAnalysisData { // missing shape field or shape field unknown 4057 | 4138 => match &issue_kind { IssueKind::InvalidArgument + | IssueKind::PossiblyInvalidArgument | IssueKind::LessSpecificArgument | IssueKind::LessSpecificReturnStatement | IssueKind::InvalidReturnStatement => return true, diff --git a/src/code_info/data_flow/node.rs b/src/code_info/data_flow/node.rs index bf60d283..f6be7160 100644 --- a/src/code_info/data_flow/node.rs +++ b/src/code_info/data_flow/node.rs @@ -824,14 +824,13 @@ impl DataFlowNode { #[inline] pub fn get_pos(&self) -> Option { match &self.kind { - DataFlowNodeKind::Vertex { pos, .. } | DataFlowNodeKind::TaintSource { pos, .. } => *pos, - DataFlowNodeKind::TaintSink { pos, .. } => Some(*pos), - DataFlowNodeKind::VariableUseSource { .. } - | DataFlowNodeKind::ForLoopInit { .. } - | DataFlowNodeKind::VariableUseSink { .. } - | DataFlowNodeKind::DataSource { .. } => { - panic!() + DataFlowNodeKind::Vertex { pos, .. } | DataFlowNodeKind::TaintSource { pos, .. } => { + *pos } + DataFlowNodeKind::TaintSink { pos, .. } + | DataFlowNodeKind::VariableUseSource { pos, .. } + | DataFlowNodeKind::DataSource { pos, .. } => Some(*pos), + DataFlowNodeKind::ForLoopInit { .. } | DataFlowNodeKind::VariableUseSink { .. } => None, } } } diff --git a/src/code_info/t_atomic.rs b/src/code_info/t_atomic.rs index b3f5a879..81f284ad 100644 --- a/src/code_info/t_atomic.rs +++ b/src/code_info/t_atomic.rs @@ -1006,18 +1006,24 @@ impl TAtomic { ) } - #[inline] pub fn is_string_subtype(&self) -> bool { - matches!( - self, + match self { TAtomic::TLiteralClassname { .. } - | TAtomic::TLiteralString { .. } - | TAtomic::TClassname { .. } - | TAtomic::TTypename { .. } - | TAtomic::TGenericClassname { .. } - | TAtomic::TGenericTypename { .. } - | TAtomic::TStringWithFlags { .. } - ) + | TAtomic::TLiteralString { .. } + | TAtomic::TClassname { .. } + | TAtomic::TTypename { .. } + | TAtomic::TGenericClassname { .. } + | TAtomic::TGenericTypename { .. } + | TAtomic::TStringWithFlags { .. } => true, + TAtomic::TTypeAlias { + as_type: Some(as_type), + .. + } => { + as_type.is_single() + && (as_type.types[0].is_string() || as_type.types[0].is_string_subtype()) + } + _ => false, + } } #[inline] diff --git a/src/code_info/t_union.rs b/src/code_info/t_union.rs index 7a48320e..d7431b61 100644 --- a/src/code_info/t_union.rs +++ b/src/code_info/t_union.rs @@ -417,28 +417,43 @@ impl TUnion { pub fn is_literal_of(&self, other: &TUnion) -> bool { let other_atomic_type = other.types.first().unwrap(); - if let TAtomic::TString = other_atomic_type { - for self_atomic_type in &self.types { - if self_atomic_type.is_string_subtype() { - continue; + match other_atomic_type { + TAtomic::TString => { + for self_atomic_type in &self.types { + if self_atomic_type.is_string_subtype() { + continue; + } + + return false; } - return false; + true } + TAtomic::TInt => { + for self_atomic_type in &self.types { + if let TAtomic::TLiteralInt { .. } = self_atomic_type { + continue; + } - true - } else if let TAtomic::TInt = other_atomic_type { - for self_atomic_type in &self.types { - if let TAtomic::TLiteralInt { .. } = self_atomic_type { - continue; + return false; } - return false; + true } + TAtomic::TEnum { name, .. } => { + for self_atomic_type in &self.types { + if let TAtomic::TEnumLiteralCase { enum_name, .. } = self_atomic_type { + if enum_name == name { + continue; + } + } - true - } else { - false + return false; + } + + true + } + _ => false, } } diff --git a/src/code_info/ttype/comparison/dict_type_comparator.rs b/src/code_info/ttype/comparison/dict_type_comparator.rs index 35af85a0..2c868fb4 100644 --- a/src/code_info/ttype/comparison/dict_type_comparator.rs +++ b/src/code_info/ttype/comparison/dict_type_comparator.rs @@ -33,6 +33,12 @@ pub(crate) fn is_contained_by( for (key, (c_u, container_property_type)) in container_known_items { if let Some((i_u, input_property_type)) = input_known_items.get(key) { if *i_u && !c_u { + if atomic_comparison_result.type_mismatch_parents.is_none() { + atomic_comparison_result.type_mismatch_parents = Some(( + input_property_type.parent_nodes.clone(), + container_property_type.clone(), + )); + } all_types_contain = false; } @@ -45,6 +51,12 @@ pub(crate) fn is_contained_by( inside_assertion, atomic_comparison_result, ) { + if atomic_comparison_result.type_mismatch_parents.is_none() { + atomic_comparison_result.type_mismatch_parents = Some(( + input_property_type.parent_nodes.clone(), + container_property_type.clone(), + )); + } all_types_contain = false; } } else if !c_u { diff --git a/src/code_info/ttype/comparison/type_comparison_result.rs b/src/code_info/ttype/comparison/type_comparison_result.rs index f8bc7517..6d5c4ba8 100644 --- a/src/code_info/ttype/comparison/type_comparison_result.rs +++ b/src/code_info/ttype/comparison/type_comparison_result.rs @@ -1,3 +1,6 @@ +use std::sync::Arc; + +use crate::data_flow::node::DataFlowNode; use crate::{t_atomic::TAtomic, t_union::TUnion}; use crate::ttype::template::TemplateBound; @@ -34,6 +37,8 @@ pub struct TypeComparisonResult { pub type_variable_lower_bounds: Vec<(String, TemplateBound)>, pub type_variable_upper_bounds: Vec<(String, TemplateBound)>, + + pub type_mismatch_parents: Option<(Vec, Arc)>, } impl Default for TypeComparisonResult { @@ -55,6 +60,7 @@ impl TypeComparisonResult { type_variable_lower_bounds: vec![], type_variable_upper_bounds: vec![], upcasted_awaitable: false, + type_mismatch_parents: None, } } } diff --git a/src/code_info/ttype/comparison/union_type_comparator.rs b/src/code_info/ttype/comparison/union_type_comparator.rs index 500ca2ab..841ac186 100644 --- a/src/code_info/ttype/comparison/union_type_comparator.rs +++ b/src/code_info/ttype/comparison/union_type_comparator.rs @@ -179,6 +179,10 @@ pub fn is_contained_by( union_comparison_result .type_variable_upper_bounds .extend(atomic_comparison_result.type_variable_upper_bounds); + } else { + if union_comparison_result.type_mismatch_parents.is_none() { + union_comparison_result.type_mismatch_parents = atomic_comparison_result.type_mismatch_parents; + } } if atomic_comparison_result.type_coerced.unwrap_or(false) { diff --git a/src/code_info_builder/lib.rs b/src/code_info_builder/lib.rs index 9054bbb2..466db1bd 100644 --- a/src/code_info_builder/lib.rs +++ b/src/code_info_builder/lib.rs @@ -771,14 +771,14 @@ fn fix_function_return_type(function_name: StrId, functionlike_storage: &mut Fun | StrId::STRTOLOWER | StrId::STRTOUPPER | StrId::MB_STRTOLOWER - | StrId::MB_STRTOUPPER => functionlike_storage.return_type = Some(get_string()), + | StrId::MB_STRTOUPPER + | StrId::DATE => functionlike_storage.return_type = Some(get_string()), // falsable strings StrId::JSON_ENCODE | StrId::FILE_GET_CONTENTS | StrId::HEX2BIN | StrId::REALPATH - | StrId::DATE | StrId::BASE64_DECODE | StrId::DATE_FORMAT | StrId::HASH_HMAC => { diff --git a/tests/inference/BinaryOperation/loopAddition/input.hack b/tests/inference/BinaryOperation/loopAddition/input.hack new file mode 100644 index 00000000..71a220a8 --- /dev/null +++ b/tests/inference/BinaryOperation/loopAddition/input.hack @@ -0,0 +1,21 @@ +function foo(): void { + $p1 = 0; + + while (rand(0, 1)) { + if (rand(0, 1)) { + if ($p1 === 0) { + bar($p1); + } else { + $p1 = $p1 - 1; + } + } + + if (rand(0, 1)) { + $p1 = $p1 - 1; + } + } + + bar($p1); +} + +function bar(int $i): void {} \ No newline at end of file diff --git a/tests/inference/DictVecKeyset/possiblyInvalidShapeKey/input.hack b/tests/inference/DictVecKeyset/possiblyInvalidShapeKey/input.hack new file mode 100644 index 00000000..7111b336 --- /dev/null +++ b/tests/inference/DictVecKeyset/possiblyInvalidShapeKey/input.hack @@ -0,0 +1,9 @@ +function foo(shape('a' => string) $arr): void { +} + +function bar(shape('a' => string) $arr, int $i): void { + if (rand(0, 1)) { + $arr['a'] = $i; + } + foo($arr); +} \ No newline at end of file diff --git a/tests/inference/DictVecKeyset/possiblyInvalidShapeKey/output.txt b/tests/inference/DictVecKeyset/possiblyInvalidShapeKey/output.txt new file mode 100644 index 00000000..97066869 --- /dev/null +++ b/tests/inference/DictVecKeyset/possiblyInvalidShapeKey/output.txt @@ -0,0 +1 @@ +ERROR: PossiblyInvalidArgument - input.hack:8:9 - Argument 1 of foo expects shape('a' => string), possibly different type shape('a' => string|int) provided in :4:45 is a mismatch with string \ No newline at end of file diff --git a/tests/inference/Generics/Class/enumMemberMatchesEnum/input.hack b/tests/inference/Generics/Class/enumMemberMatchesEnum/input.hack new file mode 100644 index 00000000..2ac3d0fd --- /dev/null +++ b/tests/inference/Generics/Class/enumMemberMatchesEnum/input.hack @@ -0,0 +1,21 @@ +final class A { + public function __construct(public T $t) {} +} + +function makeA(T $t): A { + return new A($t); +} + +enum B: int as int { + A = 1; + B = 2; + C = 3; +} + +function foo(): void { + $a = makeA(B::B); + bar($a); +} + +function bar(A $_a): void { +} \ No newline at end of file diff --git a/tests/inference/Generics/Class/newtypeAs/defs.hack b/tests/inference/Generics/Class/newtypeAs/defs.hack new file mode 100644 index 00000000..69c9e062 --- /dev/null +++ b/tests/inference/Generics/Class/newtypeAs/defs.hack @@ -0,0 +1 @@ +newtype foo_t as other_t = other_t; \ No newline at end of file diff --git a/tests/inference/Generics/Class/newtypeAs/input.hack b/tests/inference/Generics/Class/newtypeAs/input.hack new file mode 100644 index 00000000..55d06bb9 --- /dev/null +++ b/tests/inference/Generics/Class/newtypeAs/input.hack @@ -0,0 +1,10 @@ +final class A { + public function __construct(public T $t) {} +} + +function foo(A $a): void { + bar($a); +} + +function bar(A $_a): void { +} \ No newline at end of file diff --git a/tests/inference/Generics/Class/newtypeAs/other_defs.hack b/tests/inference/Generics/Class/newtypeAs/other_defs.hack new file mode 100644 index 00000000..a1d69c55 --- /dev/null +++ b/tests/inference/Generics/Class/newtypeAs/other_defs.hack @@ -0,0 +1 @@ +newtype other_t as string = string; \ No newline at end of file