Skip to content

Commit

Permalink
Improve data flow to arrays
Browse files Browse the repository at this point in the history
  • Loading branch information
@muglug
muglug committed Jun 15, 2024
1 parent 971bc0d commit 63d619a
Show file tree
Hide file tree
Showing 5 changed files with 155 additions and 65 deletions.
10 changes: 5 additions & 5 deletions src/analyzer/expr/binop/assignment_analyzer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,11 +192,11 @@ pub(crate) fn analyze(
let mut origin_node_ids = vec![];

for parent_node in &existing_var_type.parent_nodes {
origin_node_ids.extend(
analysis_data
.data_flow_graph
.get_origin_node_ids(&parent_node.id, vec![]),
);
origin_node_ids.extend(analysis_data.data_flow_graph.get_origin_node_ids(
&parent_node.id,
vec![],
false,
));
}

origin_node_ids.retain(|id| {
Expand Down
55 changes: 52 additions & 3 deletions src/analyzer/expr/collection_analyzer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,23 @@ pub(crate) fn analyze_vals(
}
});

new_vec.parent_nodes = array_creation_info.parent_nodes;
if !array_creation_info.parent_nodes.is_empty() {
let vec_node = DataFlowNode::get_for_composition(statements_analyzer.get_hpos(pos));

for child_node in array_creation_info.parent_nodes {
analysis_data.data_flow_graph.add_path(
&child_node,
&vec_node,
PathKind::Default,
vec![],
vec![],
);
}

analysis_data.data_flow_graph.add_node(vec_node.clone());

new_vec.parent_nodes = vec![vec_node];
}

analysis_data.set_expr_type(pos, new_vec);
}
Expand All @@ -184,7 +200,24 @@ pub(crate) fn analyze_vals(

let mut keyset = get_keyset(item_value_type);

keyset.parent_nodes = array_creation_info.parent_nodes;
if !array_creation_info.parent_nodes.is_empty() {
let keyset_node =
DataFlowNode::get_for_composition(statements_analyzer.get_hpos(pos));

for child_node in array_creation_info.parent_nodes {
analysis_data.data_flow_graph.add_path(
&child_node,
&keyset_node,
PathKind::Default,
vec![],
vec![],
);
}

analysis_data.data_flow_graph.add_node(keyset_node.clone());

keyset.parent_nodes = vec![keyset_node];
}

analysis_data.set_expr_type(pos, keyset);
}
Expand Down Expand Up @@ -296,7 +329,23 @@ pub(crate) fn analyze_keyvals(
shape_name: None,
});

new_dict.parent_nodes = array_creation_info.parent_nodes;
if !array_creation_info.parent_nodes.is_empty() {
let dict_node = DataFlowNode::get_for_composition(statements_analyzer.get_hpos(pos));

for child_node in array_creation_info.parent_nodes {
analysis_data.data_flow_graph.add_path(
&child_node,
&dict_node,
PathKind::Default,
vec![],
vec![],
);
}

analysis_data.data_flow_graph.add_node(dict_node.clone());

new_dict.parent_nodes = vec![dict_node];
}

analysis_data.set_expr_type(pos, new_dict);

Expand Down
18 changes: 17 additions & 1 deletion src/analyzer/expr/shape_analyzer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,23 @@ pub(crate) fn analyze(
shape_name: None,
});

new_dict.parent_nodes = parent_nodes;
if !parent_nodes.is_empty() {
let dict_node = DataFlowNode::get_for_composition(statements_analyzer.get_hpos(pos));

for child_node in parent_nodes {
analysis_data.data_flow_graph.add_path(
&child_node,
&dict_node,
PathKind::Default,
vec![],
vec![],
);
}

analysis_data.data_flow_graph.add_node(dict_node.clone());

new_dict.parent_nodes = vec![dict_node];
}

analysis_data.set_expr_type(pos, new_dict);

Expand Down
127 changes: 72 additions & 55 deletions src/analyzer/stmt_analyzer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -308,66 +308,87 @@ fn detect_unused_statement_expressions(
}
}

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,
);
match &boxed.2 {
aast::Expr_::Call(boxed_call) => {
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
.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 !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);

if let Some((_, value_type)) = array_types {
if !value_type.is_null() && !value_type.is_void() {
analysis_data.maybe_add_issue(
Issue::new(
IssueKind::UnusedBuiltinReturnValue,
format!(
"The value {} returned from {} should be consumed",
expr_type
.get_id(Some(statements_analyzer.get_interner())),
function_name
if let Some(FunctionLikeIdentifier::Function(function_id)) = functionlike_id {
let codebase = statements_analyzer.get_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 !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);

if let Some((_, value_type)) = array_types {
if !value_type.is_null() && !value_type.is_void() {
analysis_data.maybe_add_issue(
Issue::new(
IssueKind::UnusedBuiltinReturnValue,
format!(
"The value {} returned from {} should be consumed",
expr_type.get_id(Some(
statements_analyzer.get_interner()
)),
function_name
),
statements_analyzer.get_hpos(&stmt.0),
&context.function_context.calling_functionlike_id,
),
statements_analyzer.get_hpos(&stmt.0),
&context.function_context.calling_functionlike_id,
),
statements_analyzer.get_config(),
statements_analyzer.get_file_path_actual(),
);
statements_analyzer.get_config(),
statements_analyzer.get_file_path_actual(),
);
}
}
}
}
}
}
}

if let Some(expr_type) = analysis_data.get_rc_expr_type(boxed.pos()).cloned() {
if expr_type.has_awaitable_types() {
analysis_data.maybe_add_issue(
Issue::new(
IssueKind::UnusedAwaitable,
"This awaitable is never awaited".to_string(),
statements_analyzer.get_hpos(&stmt.0),
&context.function_context.calling_functionlike_id,
),
statements_analyzer.get_config(),
statements_analyzer.get_file_path_actual(),
);
if let Some(expr_type) = analysis_data.get_rc_expr_type(boxed.pos()).cloned() {
if expr_type.has_awaitable_types() {
analysis_data.maybe_add_issue(
Issue::new(
IssueKind::UnusedAwaitable,
"This awaitable is never awaited".to_string(),
statements_analyzer.get_hpos(&stmt.0),
&context.function_context.calling_functionlike_id,
),
statements_analyzer.get_config(),
statements_analyzer.get_file_path_actual(),
);
}
}
}
aast::Expr_::Collection(_)
| aast::Expr_::ValCollection(_)
| aast::Expr_::KeyValCollection(_)
| aast::Expr_::ArrayGet(_)
| aast::Expr_::Shape(_)
| aast::Expr_::Tuple(_) => {
analysis_data.maybe_add_issue(
Issue::new(
IssueKind::UnusedStatement,
"This statement includes an expression that has no effect".to_string(),
statements_analyzer.get_hpos(&stmt.0),
&context.function_context.calling_functionlike_id,
),
statements_analyzer.get_config(),
statements_analyzer.get_file_path_actual(),
);
}
_ => (),
}
}

Expand Down Expand Up @@ -458,11 +479,7 @@ fn analyze_awaitall(
let parent_nodes = t.parent_nodes.clone();
if t.is_single() {
let inner = t.get_single();
if let TAtomic::TAwaitable {
value,
..
} = inner
{
if let TAtomic::TAwaitable { value, .. } = inner {
let mut new = (**value).clone();

new.parent_nodes = parent_nodes;
Expand Down
10 changes: 9 additions & 1 deletion src/code_info/data_flow/graph.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ impl DataFlowGraph {
&self,
assignment_node_id: &DataFlowNodeId,
ignore_paths: Vec<PathKind>,
var_ids_only: bool,
) -> Vec<DataFlowNodeId> {
let mut visited_child_ids = FxHashSet::default();

Expand All @@ -175,6 +176,13 @@ impl DataFlowGraph {

visited_child_ids.insert(child_node_id.clone());

if var_ids_only {
if let DataFlowNodeId::Var(..) | DataFlowNodeId::Param(..) = child_node_id {
origin_nodes.push(child_node_id);
continue;
}
}

let mut new_parent_nodes = FxHashSet::default();
let mut has_visited_a_parent_already = false;

Expand Down Expand Up @@ -233,7 +241,7 @@ impl DataFlowGraph {
}

pub fn add_mixed_data(&mut self, assignment_node: &DataFlowNode, pos: &Pos) {
let origin_node_ids = self.get_origin_node_ids(&assignment_node.id, vec![]);
let origin_node_ids = self.get_origin_node_ids(&assignment_node.id, vec![], false);

for origin_node_id in origin_node_ids {
if let DataFlowNodeId::CallTo(..) | DataFlowNodeId::SpecializedCallTo(..) =
Expand Down

0 comments on commit 63d619a

Please sign in to comment.