Fix a bunch of minor bugs

slackhq · Oct 7, 2024 · f29ddb7 · f29ddb7
1 parent 580c18e
commit f29ddb7
Show file tree

Hide file tree

Showing 17 changed files with 165 additions and 36 deletions.
diff --git 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
@@ -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
@@ -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
@@ -824,14 +824,13 @@ impl DataFlowNode {
     #[inline]
     pub fn get_pos(&self) -> Option<HPos> {
         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
@@ -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
@@ -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
@@ -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
@@ -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<DataFlowNode>, Arc<TUnion>)>,
 }
 
 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
@@ -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
@@ -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
@@ -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 {}
diff --git a/tests/inference/DictVecKeyset/possiblyInvalidShapeKey/input.hack 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);
+}
diff --git a/tests/inference/DictVecKeyset/possiblyInvalidShapeKey/output.txt 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
diff --git a/tests/inference/Generics/Class/enumMemberMatchesEnum/input.hack b/tests/inference/Generics/Class/enumMemberMatchesEnum/input.hack
@@ -0,0 +1,21 @@
+final class A<T> {
+    public function __construct(public T $t) {}
+}
+
+function makeA<T>(T $t): A<T> {
+    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<B> $_a): void {
+}
diff --git a/tests/inference/Generics/Class/newtypeAs/defs.hack b/tests/inference/Generics/Class/newtypeAs/defs.hack
@@ -0,0 +1 @@
+newtype foo_t as other_t = other_t;
diff --git a/tests/inference/Generics/Class/newtypeAs/input.hack b/tests/inference/Generics/Class/newtypeAs/input.hack
@@ -0,0 +1,10 @@
+final class A<T> {
+    public function __construct(public T $t) {}
+}
+
+function foo(A<foo_t> $a): void {
+    bar($a);
+}
+
+function bar(A<string> $_a): void {
+}
diff --git a/tests/inference/Generics/Class/newtypeAs/other_defs.hack b/tests/inference/Generics/Class/newtypeAs/other_defs.hack
@@ -0,0 +1 @@
+newtype other_t as string = string;
Original file line number	Diff line number	Diff line change
		@@ -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