Skip to content

Commit

Permalink
Improved unused expression discovery
Browse files Browse the repository at this point in the history
  • Loading branch information
muglug committed Dec 5, 2023
1 parent dd1e735 commit 1da1b6a
Show file tree
Hide file tree
Showing 40 changed files with 232 additions and 173 deletions.
4 changes: 3 additions & 1 deletion src/aast_utils/naming_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,9 @@ impl<'ast> Visitor<'ast> for Scanner<'_> {
// );

if nc.in_class_id || nc.in_function_id || nc.in_xhp_id || nc.in_constant_id {
if let std::collections::hash_map::Entry::Vacant(e) = self.resolved_names.entry(id.0.start_offset()) {
if let std::collections::hash_map::Entry::Vacant(e) =
self.resolved_names.entry(id.0.start_offset())
{
let resolved_name = if nc.in_xhp_id {
nc.get_resolved_name(
self.interner,
Expand Down
4 changes: 2 additions & 2 deletions src/analyzer/algebra_analyzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ use oxidized::ast::Pos;
use rustc_hash::FxHashSet;

use crate::{
scope_analyzer::ScopeAnalyzer, statements_analyzer::StatementsAnalyzer,
function_analysis_data::FunctionAnalysisData,
function_analysis_data::FunctionAnalysisData, scope_analyzer::ScopeAnalyzer,
statements_analyzer::StatementsAnalyzer,
};

pub(crate) fn check_for_paradox(
Expand Down
17 changes: 13 additions & 4 deletions src/analyzer/expr/assertion_finder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,13 +166,19 @@ fn process_custom_assertions(
) -> FxHashMap<String, Vec<Vec<Assertion>>> {
let mut if_true_assertions = analysis_data
.if_true_assertions
.get(&(conditional_pos.start_offset() as u32, conditional_pos.end_offset() as u32))
.get(&(
conditional_pos.start_offset() as u32,
conditional_pos.end_offset() as u32,
))
.cloned()
.unwrap_or(FxHashMap::default());

let if_false_assertions = analysis_data
.if_false_assertions
.get(&(conditional_pos.start_offset() as u32, conditional_pos.end_offset() as u32))
.get(&(
conditional_pos.start_offset() as u32,
conditional_pos.end_offset() as u32,
))
.cloned()
.unwrap_or(FxHashMap::default());

Expand Down Expand Up @@ -216,7 +222,7 @@ fn get_is_assertions(
) {
t
} else {
return vec![]
return vec![];
};

if let Some((codebase, _)) = assertion_context.codebase {
Expand Down Expand Up @@ -271,7 +277,10 @@ fn get_is_assertions(
if let (Some(lhs_type), Some((codebase, interner))) = (
analysis_data
.expr_types
.get(&(var_expr.1.start_offset() as u32, var_expr.1.end_offset() as u32))
.get(&(
var_expr.1.start_offset() as u32,
var_expr.1.end_offset() as u32,
))
.clone(),
assertion_context.codebase,
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,9 @@ pub(crate) fn analyze(
}
}

if let Some((property_id, invalid_class_property_type)) = invalid_assignment_value_types.iter().next() {
if let Some((property_id, invalid_class_property_type)) =
invalid_assignment_value_types.iter().next()
{
analysis_data.maybe_add_issue(
Issue::new(
IssueKind::InvalidPropertyAssignmentValue,
Expand Down Expand Up @@ -213,9 +215,10 @@ pub(crate) fn analyze_regular_assignment(

context.inside_general_use = was_inside_general_use;

analysis_data
.expr_effects
.insert((pos.start_offset() as u32, pos.end_offset() as u32), EFFECT_WRITE_PROPS);
analysis_data.expr_effects.insert(
(pos.start_offset() as u32, pos.end_offset() as u32),
EFFECT_WRITE_PROPS,
);

let lhs_type = analysis_data.get_rc_expr_type(&stmt_var.pos()).cloned();

Expand Down
16 changes: 8 additions & 8 deletions src/analyzer/expr/binop/arithmetic_analyzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,10 +249,10 @@ pub(crate) fn assign_arithmetic_type(
.data_flow_graph
.add_node(decision_node.clone());

if let Some(lhs_type) = analysis_data
.expr_types
.get(&(lhs_expr.1.start_offset() as u32, lhs_expr.1.end_offset() as u32))
{
if let Some(lhs_type) = analysis_data.expr_types.get(&(
lhs_expr.1.start_offset() as u32,
lhs_expr.1.end_offset() as u32,
)) {
cond_type.parent_nodes.insert(decision_node.clone());

for old_parent_node in &lhs_type.parent_nodes {
Expand All @@ -266,10 +266,10 @@ pub(crate) fn assign_arithmetic_type(
}
}

if let Some(rhs_type) = analysis_data
.expr_types
.get(&(rhs_expr.1.start_offset() as u32, rhs_expr.1.end_offset() as u32))
{
if let Some(rhs_type) = analysis_data.expr_types.get(&(
rhs_expr.1.start_offset() as u32,
rhs_expr.1.end_offset() as u32,
)) {
cond_type.parent_nodes.insert(decision_node.clone());

for old_parent_node in &rhs_type.parent_nodes {
Expand Down
63 changes: 31 additions & 32 deletions src/analyzer/expr/call/argument_analyzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -737,25 +737,26 @@ fn add_dataflow(
};
// TODO add plugin hooks for adding/removing taints

let argument_value_node = if data_flow_graph.kind == GraphKind::FunctionBody {
DataFlowNode {
id: "call to ".to_string()
+ functionlike_id
.to_string(statements_analyzer.get_interner())
.as_str(),
kind: DataFlowNodeKind::VariableUseSink {
pos: statements_analyzer.get_hpos(input_expr.pos()),
},
}
} else {
DataFlowNode::get_for_assignment(
"call to ".to_string()
+ functionlike_id
.to_string(statements_analyzer.get_interner())
.as_str(),
statements_analyzer.get_hpos(input_expr.pos()),
)
};
let argument_value_node =
if data_flow_graph.kind == GraphKind::FunctionBody && context.inside_general_use {
DataFlowNode {
id: "call to ".to_string()
+ functionlike_id
.to_string(statements_analyzer.get_interner())
.as_str(),
kind: DataFlowNodeKind::VariableUseSink {
pos: statements_analyzer.get_hpos(input_expr.pos()),
},
}
} else {
DataFlowNode::get_for_assignment(
"call to ".to_string()
+ functionlike_id
.to_string(statements_analyzer.get_interner())
.as_str(),
statements_analyzer.get_hpos(input_expr.pos()),
)
};

for parent_node in &input_type.parent_nodes {
data_flow_graph.add_path(
Expand All @@ -771,9 +772,15 @@ fn add_dataflow(
);
}

if data_flow_graph.kind == GraphKind::FunctionBody {
data_flow_graph.add_node(argument_value_node);
} else {
data_flow_graph.add_path(
&argument_value_node,
&method_node,
PathKind::Default,
None,
None,
);

if matches!(data_flow_graph.kind, GraphKind::WholeProgram(_)) {
let mut taints = get_argument_taints(
functionlike_id,
argument_offset,
Expand All @@ -784,16 +791,6 @@ fn add_dataflow(
taints.extend(sinks.clone());
}

data_flow_graph.add_node(argument_value_node.clone());

data_flow_graph.add_path(
&argument_value_node,
&method_node,
PathKind::Default,
None,
None,
);

if !taints.is_empty() {
let method_node_sink = DataFlowNode {
id: method_node.get_id().clone(),
Expand All @@ -809,6 +806,8 @@ fn add_dataflow(

data_flow_graph.add_node(method_node);
}

data_flow_graph.add_node(argument_value_node);
}

pub(crate) fn get_removed_taints_in_comments(
Expand Down
24 changes: 22 additions & 2 deletions src/analyzer/expr/call/arguments_analyzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use hakana_reflection_info::classlike_info::ClassLikeInfo;
use hakana_reflection_info::codebase_info::CodebaseInfo;
use hakana_reflection_info::data_flow::graph::GraphKind;
use hakana_reflection_info::functionlike_identifier::FunctionLikeIdentifier;
use hakana_reflection_info::functionlike_info::FunctionLikeInfo;
use hakana_reflection_info::functionlike_info::{FnEffect, FunctionLikeInfo};
use hakana_reflection_info::functionlike_parameter::{DefaultType, FunctionLikeParameter};
use hakana_reflection_info::t_atomic::TAtomic;
use hakana_reflection_info::t_union::{populate_union_type, TUnion};
Expand Down Expand Up @@ -175,7 +175,9 @@ pub(crate) fn check_arguments_match(
for (_, arg_expr) in args.iter() {
let was_inside_call = context.inside_general_use;

context.inside_general_use = true;
if matches!(functionlike_info.effects, FnEffect::Some(_)) {
context.inside_general_use = true;
}

// don't analyse closures here
if !matches!(arg_expr.2, aast::Expr_::Lfun(_) | aast::Expr_::Efun(_)) {
Expand Down Expand Up @@ -407,6 +409,12 @@ pub(crate) fn check_arguments_match(
continue;
};

let was_inside_call = context.inside_general_use;

if matches!(functionlike_info.effects, FnEffect::Some(_)) {
context.inside_general_use = true;
}

argument_analyzer::check_argument_matches(
statements_analyzer,
functionlike_id,
Expand All @@ -425,6 +433,10 @@ pub(crate) fn check_arguments_match(
function_name_pos,
);

if !was_inside_call {
context.inside_general_use = false;
}

if let GraphKind::WholeProgram(_) = &analysis_data.data_flow_graph.kind {
if let Some(removed_taints) = &function_param.removed_taints_when_returning_true {
if let Some(expr_var_id) = expression_identifier::get_var_id(
Expand Down Expand Up @@ -473,6 +485,12 @@ pub(crate) fn check_arguments_match(
get_mixed_any()
};

let was_inside_call = context.inside_general_use;

if matches!(functionlike_info.effects, FnEffect::Some(_)) {
context.inside_general_use = true;
}

argument_analyzer::check_argument_matches(
statements_analyzer,
functionlike_id,
Expand All @@ -490,6 +508,8 @@ pub(crate) fn check_arguments_match(
function_call_pos,
function_name_pos,
);

context.inside_general_use = was_inside_call;
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/analyzer/expr/call/atomic_static_call_analyzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,8 @@ pub(crate) fn analyze(
&method_name.unwrap(),
(expr.2, expr.3, expr.4),
lhs_type_part,
pos,Some(&expr.1.0),
pos,
Some(&expr.1 .0),
analysis_data,
context,
if_body_context,
Expand Down
7 changes: 4 additions & 3 deletions src/analyzer/expr/call/new_analyzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,9 +230,10 @@ fn analyze_atomic(

match statements_analyzer.get_interner().lookup(&classlike_name) {
"ReflectionClass" | "ReflectionTypeAlias" => {
analysis_data
.expr_effects
.insert((pos.start_offset() as u32, pos.end_offset() as u32), EFFECT_WRITE_GLOBALS);
analysis_data.expr_effects.insert(
(pos.start_offset() as u32, pos.end_offset() as u32),
EFFECT_WRITE_GLOBALS,
);
}
_ => {}
}
Expand Down
47 changes: 31 additions & 16 deletions src/analyzer/expr/call_analyzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,13 @@ pub(crate) fn analyze(
ast_defs::PropOrMethod::IsMethod => {
return instance_call_analyzer::analyze(
statements_analyzer,
(lhs_expr, rhs_expr, &expr.targs, &expr.args, &expr.unpacked_arg),
(
lhs_expr,
rhs_expr,
&expr.targs,
&expr.args,
&expr.unpacked_arg,
),
&pos,
analysis_data,
context,
Expand All @@ -91,7 +97,13 @@ pub(crate) fn analyze(

return static_call_analyzer::analyze(
statements_analyzer,
(class_id, rhs_expr, &expr.targs, &expr.args, &expr.unpacked_arg),
(
class_id,
rhs_expr,
&expr.targs,
&expr.args,
&expr.unpacked_arg,
),
&pos,
analysis_data,
context,
Expand Down Expand Up @@ -339,7 +351,7 @@ pub(crate) fn check_method_args(
if_body_context,
template_result,
pos,
method_name_pos
method_name_pos,
)?;

apply_effects(functionlike_storage, analysis_data, pos, &call_expr.1);
Expand All @@ -358,26 +370,28 @@ pub(crate) fn apply_effects(
expr_args: &Vec<(ast_defs::ParamKind, aast::Expr<(), ()>)>,
) {
if function_storage.name == STR_ASIO_JOIN {
analysis_data
.expr_effects
.insert((pos.start_offset() as u32, pos.end_offset() as u32), EFFECT_IMPURE);
analysis_data.expr_effects.insert(
(pos.start_offset() as u32, pos.end_offset() as u32),
EFFECT_IMPURE,
);
return;
}

match function_storage.effects {
FnEffect::Some(stored_effects) => {
if stored_effects > EFFECT_WRITE_PROPS {
analysis_data
.expr_effects
.insert((pos.start_offset() as u32, pos.end_offset() as u32), stored_effects);
analysis_data.expr_effects.insert(
(pos.start_offset() as u32, pos.end_offset() as u32),
stored_effects,
);
}
}
FnEffect::Arg(arg_offset) => {
if let Some((_, arg_expr)) = expr_args.get(arg_offset as usize) {
if let Some(arg_type) = analysis_data
.expr_types
.get(&(arg_expr.pos().start_offset() as u32, arg_expr.pos().end_offset() as u32))
{
if let Some(arg_type) = analysis_data.expr_types.get(&(
arg_expr.pos().start_offset() as u32,
arg_expr.pos().end_offset() as u32,
)) {
for arg_atomic_type in &arg_type.types {
if let TAtomic::TClosure { effects, .. } = arg_atomic_type {
if let Some(evaluated_effects) = effects {
Expand All @@ -386,9 +400,10 @@ pub(crate) fn apply_effects(
*evaluated_effects,
);
} else {
analysis_data
.expr_effects
.insert((pos.start_offset() as u32, pos.end_offset() as u32), EFFECT_IMPURE);
analysis_data.expr_effects.insert(
(pos.start_offset() as u32, pos.end_offset() as u32),
EFFECT_IMPURE,
);
}
}
}
Expand Down
Loading

0 comments on commit 1da1b6a

Please sign in to comment.