Skip to content

Commit

Permalink
Fix some funky unused code checks
Browse files Browse the repository at this point in the history
  • Loading branch information
muglug committed May 6, 2024
1 parent 7cb0ae7 commit 3b2175d
Show file tree
Hide file tree
Showing 15 changed files with 169 additions and 145 deletions.
32 changes: 14 additions & 18 deletions src/analyzer/expr/assertion_finder.rs
Expand Up @@ -58,15 +58,15 @@ pub(crate) fn scrape_assertions(
);
}
aast::Expr_::Call(call) => {
let functionlike_id = get_static_functionlike_id_from_call(
call,
if let Some((_, interner)) = assertion_context.codebase {
Some(interner)
} else {
None
},
assertion_context.resolved_names,
);
let functionlike_id = if let Some((_, interner)) = assertion_context.codebase {
get_static_functionlike_id_from_call(
call,
interner,
assertion_context.resolved_names,
)
} else {
None
};

if let Some(FunctionLikeIdentifier::Function(name)) = functionlike_id {
return scrape_function_assertions(
Expand Down Expand Up @@ -348,15 +348,11 @@ fn scrape_shapes_isset(
negated: bool,
) {
if let aast::Expr_::Call(call) = &var_expr.2 {
let functionlike_id = get_static_functionlike_id_from_call(
call,
if let Some((_, interner)) = assertion_context.codebase {
Some(interner)
} else {
None
},
assertion_context.resolved_names,
);
let functionlike_id = if let Some((_, interner)) = assertion_context.codebase {
get_static_functionlike_id_from_call(call, interner, assertion_context.resolved_names)
} else {
None
};

if let Some(FunctionLikeIdentifier::Method(class_name, member_name)) = functionlike_id {
if let Some((codebase, interner)) = assertion_context.codebase {
Expand Down
2 changes: 1 addition & 1 deletion src/analyzer/expr/call/arguments_analyzer.rs
Expand Up @@ -181,7 +181,7 @@ pub(crate) fn check_arguments_match(

if matches!(functionlike_info.effects, FnEffect::Some(_))
|| matches!(functionlike_info.effects, FnEffect::Arg(_))
|| functionlike_info.pure_can_throw
|| functionlike_info.has_throw
|| functionlike_info.user_defined
|| functionlike_info.method_info.is_some()
{
Expand Down
21 changes: 12 additions & 9 deletions src/analyzer/expr/call/existing_atomic_method_call_analyzer.rs
Expand Up @@ -9,7 +9,7 @@ use hakana_reflection_info::{
t_atomic::{DictKey, TAtomic},
t_union::TUnion,
};
use hakana_reflection_info::{GenericParent, VarId, EFFECT_WRITE_LOCAL, EFFECT_WRITE_PROPS};
use hakana_reflection_info::{GenericParent, VarId, EFFECT_WRITE_LOCAL};
use hakana_str::StrId;
use hakana_type::get_null;
use hakana_type::template::standin_type_replacer;
Expand Down Expand Up @@ -164,14 +164,6 @@ pub(crate) fn analyze(
}
}

// .hhi for NumberFormatter was incorrect
if classlike_name == StrId::NUMBER_FORMATTER {
analysis_data.expr_effects.insert(
(pos.start_offset() as u32, pos.end_offset() as u32),
EFFECT_WRITE_PROPS,
);
}

check_method_args(
statements_analyzer,
analysis_data,
Expand All @@ -185,6 +177,17 @@ pub(crate) fn analyze(
method_name_pos,
)?;

// .hhi for NumberFormatter was incorrect
// or if we're calling parent::__construct, make sure we set correct write props effect
if method_id.0 == StrId::NUMBER_FORMATTER || method_id.1 == StrId::CONSTRUCT {
if let Some(effects) = analysis_data
.expr_effects
.get_mut(&(pos.start_offset() as u32, pos.end_offset() as u32))
{
*effects |= EFFECT_WRITE_LOCAL;
}
}

if functionlike_storage.ignore_taints_if_true {
analysis_data.if_true_assertions.insert(
(pos.start_offset() as u32, pos.end_offset() as u32),
Expand Down
14 changes: 12 additions & 2 deletions src/analyzer/expr/call/expression_call_analyzer.rs
Expand Up @@ -68,7 +68,16 @@ pub(crate) fn analyze(
start_line: 0,
},
);
lambda_storage.user_defined = true;
let existing_storage = codebase
.functionlike_infos
.get(&(closure_id.0 .0, StrId(closure_id.1)));

if let Some(existing_storage) = existing_storage {
lambda_storage.has_throw = existing_storage.has_throw;
}

lambda_storage.effects = FnEffect::from_u8(effects);

lambda_storage.params = closure_params
.iter()
.map(|fn_param| {
Expand All @@ -84,7 +93,8 @@ pub(crate) fn analyze(
})
.collect();
lambda_storage.return_type = closure_return_type.clone().map(|t| (*t).clone());
lambda_storage.effects = FnEffect::from_u8(effects);

lambda_storage.user_defined = true;

let functionlike_id = FunctionLikeIdentifier::Closure(closure_id.0, closure_id.1);

Expand Down
9 changes: 3 additions & 6 deletions src/analyzer/expr/call/new_analyzer.rs
Expand Up @@ -227,8 +227,8 @@ fn analyze_atomic(
}
};

match statements_analyzer.get_interner().lookup(&classlike_name) {
"ReflectionClass" | "ReflectionTypeAlias" => {
match classlike_name {
StrId::REFLECTION_CLASS | StrId::REFLECTION_FUNCTION | StrId::REFLECTION_TYPE_ALIAS => {
analysis_data.expr_effects.insert(
(pos.start_offset() as u32, pos.end_offset() as u32),
EFFECT_WRITE_GLOBALS,
Expand Down Expand Up @@ -317,10 +317,7 @@ fn analyze_named_constructor(

let mut generic_type_params = None;

let method_name = statements_analyzer
.get_interner()
.get("__construct")
.unwrap();
let method_name = StrId::CONSTRUCT;
let method_id = MethodIdentifier(classlike_name, method_name);
let declaring_method_id = codebase.get_declaring_method_id(&method_id);

Expand Down
10 changes: 8 additions & 2 deletions src/analyzer/expr/call_analyzer.rs
Expand Up @@ -2,7 +2,9 @@ use std::sync::Arc;

use hakana_reflection_info::code_location::HPos;
use hakana_reflection_info::issue::{Issue, IssueKind};
use hakana_reflection_info::{GenericParent, EFFECT_IMPURE, EFFECT_PURE, EFFECT_WRITE_PROPS};
use hakana_reflection_info::{
GenericParent, EFFECT_CAN_THROW, EFFECT_IMPURE, EFFECT_PURE, EFFECT_WRITE_PROPS,
};
use hakana_str::StrId;
use hakana_type::template::standin_type_replacer::get_relevant_bounds;
use hakana_type::type_comparator::type_comparison_result::TypeComparisonResult;
Expand Down Expand Up @@ -506,7 +508,11 @@ pub(crate) fn apply_effects(
FnEffect::Pure => {
analysis_data.expr_effects.insert(
(pos.start_offset() as u32, pos.end_offset() as u32),
EFFECT_PURE,
if function_storage.has_throw {
EFFECT_CAN_THROW
} else {
EFFECT_PURE
},
);
}
FnEffect::Unknown => {
Expand Down
60 changes: 26 additions & 34 deletions src/analyzer/expr/expression_identifier.rs
Expand Up @@ -167,58 +167,50 @@ pub fn get_functionlike_id_from_call(
resolved_names: &FxHashMap<u32, StrId>,
expr_types: &FxHashMap<(u32, u32), Rc<TUnion>>,
) -> Option<FunctionLikeIdentifier> {
get_static_functionlike_id_from_call(call_expr, Some(interner), resolved_names)
get_static_functionlike_id_from_call(call_expr, interner, resolved_names)
.or_else(|| get_method_id_from_call(call_expr, interner, expr_types))
}

pub fn get_static_functionlike_id_from_call(
call: &oxidized::ast::CallExpr,
interner: Option<&Interner>,
interner: &Interner,
resolved_names: &FxHashMap<u32, StrId>,
) -> Option<FunctionLikeIdentifier> {
match &call.func.2 {
aast::Expr_::Id(boxed_id) => {
if let Some(interner) = interner {
let name = if boxed_id.1 == "isset" {
StrId::ISSET
} else if boxed_id.1 == "\\in_array" {
interner.get("in_array").unwrap()
} else if let Some(resolved_name) =
resolved_names.get(&(boxed_id.0.start_offset() as u32))
{
*resolved_name
} else {
return None;
};

Some(FunctionLikeIdentifier::Function(name))
let name = if boxed_id.1 == "isset" {
StrId::ISSET
} else if boxed_id.1 == "\\in_array" {
interner.get("in_array").unwrap()
} else if let Some(resolved_name) =
resolved_names.get(&(boxed_id.0.start_offset() as u32))
{
*resolved_name
} else {
None
}
return None;
};

Some(FunctionLikeIdentifier::Function(name))
}
aast::Expr_::ClassConst(boxed) => {
if let Some(interner) = interner {
let (class_id, rhs_expr) = (&boxed.0, &boxed.1);
let (class_id, rhs_expr) = (&boxed.0, &boxed.1);

match &class_id.2 {
aast::ClassId_::CIexpr(lhs_expr) => {
if let aast::Expr_::Id(id) = &lhs_expr.2 {
if let (Some(class_name), Some(method_name)) = (
resolved_names.get(&(id.0.start_offset() as u32)),
interner.get(&rhs_expr.1),
) {
Some(FunctionLikeIdentifier::Method(*class_name, method_name))
} else {
None
}
match &class_id.2 {
aast::ClassId_::CIexpr(lhs_expr) => {
if let aast::Expr_::Id(id) = &lhs_expr.2 {
if let (Some(class_name), Some(method_name)) = (
resolved_names.get(&(id.0.start_offset() as u32)),
interner.get(&rhs_expr.1),
) {
Some(FunctionLikeIdentifier::Method(*class_name, method_name))
} else {
None
}
} else {
None
}
_ => None,
}
} else {
None
_ => None,
}
}
_ => None,
Expand Down
63 changes: 16 additions & 47 deletions src/analyzer/stmt_analyzer.rs
@@ -1,5 +1,6 @@
use hakana_reflection_info::code_location::{HPos, StmtStart};
use hakana_reflection_info::functionlike_identifier::FunctionLikeIdentifier;
use hakana_reflection_info::functionlike_info::FnEffect;
use hakana_reflection_info::EFFECT_PURE;
use hakana_str::StrId;
use hakana_type::get_arrayish_params;
Expand All @@ -8,7 +9,9 @@ use rustc_hash::FxHashSet;
use crate::custom_hook::AfterStmtAnalysisData;
use crate::expr::binop::assignment_analyzer;

use crate::expr::expression_identifier::get_functionlike_id_from_call;
use crate::expr::expression_identifier::{
get_functionlike_id_from_call, get_static_functionlike_id_from_call,
};
use crate::expression_analyzer;
use crate::function_analysis_data::FunctionAnalysisData;
use crate::scope_analyzer::ScopeAnalyzer;
Expand Down Expand Up @@ -286,51 +289,12 @@ fn detect_unused_statement_expressions(
);
}

let functionlike_id = if let aast::Expr_::Call(boxed_call) = &boxed.2 {
get_functionlike_id_from_call(
boxed_call,
statements_analyzer.get_interner(),
statements_analyzer.get_file_analyzer().resolved_names,
&analysis_data.expr_types,
)
} else {
None
};

if let Some(effect) = analysis_data.expr_effects.get(&(
boxed.pos().start_offset() as u32,
boxed.pos().end_offset() as u32,
)) {
if effect == &EFFECT_PURE {
let mut is_constructor_call = false;
let mut fn_can_throw = false;

if let Some(functionlike_id) = functionlike_id {
match functionlike_id {
FunctionLikeIdentifier::Function(function_id) => {
let codebase = statements_analyzer.get_codebase();

if let Some(functionlike_info) = codebase
.functionlike_infos
.get(&(function_id, StrId::EMPTY))
{
fn_can_throw = functionlike_info.pure_can_throw
}
}
FunctionLikeIdentifier::Method(_, method_name_id) => {
if method_name_id == StrId::CONSTRUCT {
is_constructor_call = true;
}

if method_name_id == StrId::ASSERT {
fn_can_throw = true;
}
}
_ => (),
}
};

if !is_constructor_call && !fn_can_throw {
if !matches!(boxed.2, aast::Expr_::New(..)) {
analysis_data.maybe_add_issue(
Issue::new(
IssueKind::UnusedStatement,
Expand All @@ -346,19 +310,24 @@ fn detect_unused_statement_expressions(
}
}

if let Some(functionlike_id) = functionlike_id {
if let FunctionLikeIdentifier::Function(function_id) = functionlike_id {
if let aast::Expr_::Call(boxed_call) = &boxed.2 {
let functionlike_id = get_static_functionlike_id_from_call(
boxed_call,
statements_analyzer.get_interner(),
statements_analyzer.get_file_analyzer().resolved_names,
);

if let Some(FunctionLikeIdentifier::Function(function_id)) = functionlike_id {
let codebase = statements_analyzer.get_codebase();
if let Some(_functionlike_info) = codebase
if let Some(functionlike_info) = codebase
.functionlike_infos
.get(&(function_id, StrId::EMPTY))
{
if let Some(expr_type) = analysis_data.get_rc_expr_type(boxed.pos()).cloned() {
let function_name = statements_analyzer.get_interner().lookup(&function_id);

if (function_name.starts_with("HH\\Lib\\Keyset\\")
|| function_name.starts_with("HH\\Lib\\Vec\\")
|| function_name.starts_with("HH\\Lib\\Dict\\"))
if !functionlike_info.user_defined
&& matches!(functionlike_info.effects, FnEffect::Arg(..))
&& expr_type.is_single()
{
let array_types = get_arrayish_params(expr_type.get_single(), codebase);
Expand Down
4 changes: 2 additions & 2 deletions src/code_info/functionlike_info.rs
Expand Up @@ -99,7 +99,7 @@ pub struct FunctionLikeInfo {

pub effects: FnEffect,

pub pure_can_throw: bool,
pub has_throw: bool,

/**
* Whether or not the function output is dependent solely on input - a function can be
Expand Down Expand Up @@ -191,7 +191,7 @@ impl FunctionLikeInfo {
where_constraints: vec![],
async_version: None,
is_production_code: true,
pure_can_throw: false,
has_throw: false,
is_closure: false,
overriding: false,
}
Expand Down

0 comments on commit 3b2175d

Please sign in to comment.