Add new issue for StrictObjectEquality checks (#17)

slackhq · Apr 26, 2024 · 2e95cc6 · 2e95cc6
1 parent df7f9d7
commit 2e95cc6
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 1 deletion.
diff --git a/src/analyzer/expr/binop_analyzer.rs b/src/analyzer/expr/binop_analyzer.rs
@@ -6,6 +6,7 @@ use crate::statements_analyzer::StatementsAnalyzer;
 use crate::stmt_analyzer::AnalysisError;
 
 use hakana_reflection_info::issue::{Issue, IssueKind};
+use hakana_reflection_info::t_atomic::TAtomic;
 use hakana_type::type_comparator::union_type_comparator;
 use hakana_type::{get_bool, get_int};
 use oxidized::pos::Pos;
@@ -142,6 +143,26 @@ pub(crate) fn analyze(
                         statements_analyzer.get_config(),
                         statements_analyzer.get_file_path_actual(),
                     );
+                } else if matches!(
+                    expr.0,
+                    oxidized::ast_defs::Bop::Eqeqeq | oxidized::ast_defs::Bop::Diff2
+                ) && lhs_type.types.len() == 1
+                    && lhs_type.types[0] == rhs_type.types[0]
+                    && matches!(lhs_type.types[0], TAtomic::TNamedObject { .. })
+                {
+                    analysis_data.maybe_add_issue(
+                        Issue::new(
+                            IssueKind::StrictObjectEquality,
+                            format!(
+                                "Strict equality compares {} objects by reference rather than value",
+                                lhs_type.get_id(Some(interner)),
+                            ),
+                            statements_analyzer.get_hpos(pos),
+                            &context.function_context.calling_functionlike_id,
+                        ),
+                        statements_analyzer.get_config(),
+                        statements_analyzer.get_file_path_actual(),
+                    );
                 }
             }
 

diff --git a/src/code_info/issue.rs b/src/code_info/issue.rs
@@ -1,5 +1,5 @@
-use std::{hash::Hasher, str::FromStr};
 use core::hash::Hash;
+use std::{hash::Hasher, str::FromStr};
 
 use hakana_str::StrId;
 use rustc_hash::FxHashSet;
@@ -107,6 +107,7 @@ pub enum IssueKind {
     RedundantTruthinessCheck,
     RedundantTypeComparison,
     ShadowedLoopVar,
+    StrictObjectEquality,
     TaintedData(Box<SinkType>),
     TestOnlyCall,
     UndefinedIntArrayOffset,

diff --git a/tests/inference/TypeReconciliation/Conditional/preventStrictEqualityObjectType/input.hack b/tests/inference/TypeReconciliation/Conditional/preventStrictEqualityObjectType/input.hack
@@ -0,0 +1,8 @@
+class A {}
+function foo(A $a, A $b) : bool {
+    return $a === $b;
+}
+function foo2(A $a, A $b) : bool {
+    return $a !== $b;
+}
+$a = foo(new A(), new A());
diff --git a/tests/inference/TypeReconciliation/Conditional/preventStrictEqualityObjectType/output.txt b/tests/inference/TypeReconciliation/Conditional/preventStrictEqualityObjectType/output.txt
@@ -0,0 +1,2 @@
+ERROR: StrictObjectEquality - input.hack:3:12 - Strict equality compares A objects by reference rather than value
+ERROR: StrictObjectEquality - input.hack:6:12 - Strict equality compares A objects by reference rather than value