Skip to content

Commit

Permalink
Prevent upcasting Awaitable
Browse files Browse the repository at this point in the history
  • Loading branch information
@muglug
muglug committed Jan 8, 2024
1 parent 0332cf7 commit d0e5f85
Show file tree
Hide file tree
Showing 7 changed files with 105 additions and 81 deletions.
17 changes: 17 additions & 0 deletions src/analyzer/expr/assignment/instance_property_assignment_analyzer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,23 @@ pub(crate) fn analyze(
upper_bounds.push(bound);
}
}

if union_comparison_result.upcasted_awaitable {
analysis_data.maybe_add_issue(
Issue::new(
IssueKind::UpcastAwaitable,
format!(
"{} contains Awaitable but was passed into a more general type {}",
assignment_type.get_id(Some(statements_analyzer.get_interner())),
class_property_type.get_id(Some(statements_analyzer.get_interner())),
),
statements_analyzer.get_hpos(&stmt_var.1),
&context.function_context.calling_functionlike_id,
),
statements_analyzer.get_config(),
statements_analyzer.get_file_path_actual(),
);
}
} else {
if union_comparison_result.type_coerced.unwrap_or(false) {
if union_comparison_result
Expand Down
133 changes: 52 additions & 81 deletions src/analyzer/expr/call/argument_analyzer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,87 +231,6 @@ pub(crate) fn verify_type(
) {
let codebase = statements_analyzer.get_codebase();

if param_type.is_mixed() {
if codebase.infer_types_from_usage && !input_type.is_mixed() && !param_type.had_template {
if let Some(method_call_info) = method_call_info {
if let Some(_declaring_method_id) = &method_call_info.declaring_method_id {
// todo log potential method param type
}
} else {
// todo log potential function param type
}
}

add_dataflow(
statements_analyzer,
functionlike_id,
argument_offset,
input_expr,
input_type,
param_type,
context,
analysis_data,
function_param,
method_call_info,
ignore_taints,
specialize_taint,
function_call_pos,
);

return;
}

let mut mixed_from_any = false;
if input_type.is_mixed_with_any(&mut mixed_from_any) {
for origin in &input_type.parent_nodes {
analysis_data
.data_flow_graph
.add_mixed_data(origin, input_expr.pos());
}

analysis_data.maybe_add_issue(
Issue::new(
if mixed_from_any {
IssueKind::MixedAnyArgument
} else {
IssueKind::MixedArgument
},
format!(
"Argument {} of {} expects {}, {} 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())),
),
statements_analyzer.get_hpos(input_expr.pos()),
&context.function_context.calling_functionlike_id,
),
statements_analyzer.get_config(),
statements_analyzer.get_file_path_actual(),
);

// todo handle mixed values, including coercing when passed into functions
// that have hard type expectations

add_dataflow(
statements_analyzer,
functionlike_id,
argument_offset,
input_expr,
input_type,
param_type,
context,
analysis_data,
function_param,
method_call_info,
ignore_taints,
specialize_taint,
function_call_pos,
);

return;
}

if input_type.is_nothing() {
analysis_data.maybe_add_issue(
Issue::new(
Expand Down Expand Up @@ -373,6 +292,58 @@ pub(crate) fn verify_type(
input_type.parent_nodes = FxHashMap::default();
}*/

if union_comparison_result.upcasted_awaitable && context.inside_general_use {
analysis_data.maybe_add_issue(
Issue::new(
IssueKind::UpcastAwaitable,
format!(
"{} contains Awaitable but was passed into a more general type {}",
input_type.get_id(Some(statements_analyzer.get_interner())),
param_type.get_id(Some(statements_analyzer.get_interner())),
),
statements_analyzer.get_hpos(input_expr.pos()),
&context.function_context.calling_functionlike_id,
),
statements_analyzer.get_config(),
statements_analyzer.get_file_path_actual(),
);
}

if !param_type.is_mixed() {
let mut mixed_from_any = false;

if input_type.is_mixed_with_any(&mut mixed_from_any) {
for origin in &input_type.parent_nodes {
analysis_data
.data_flow_graph
.add_mixed_data(origin, input_expr.pos());
}

analysis_data.maybe_add_issue(
Issue::new(
if mixed_from_any {
IssueKind::MixedAnyArgument
} else {
IssueKind::MixedArgument
},
format!(
"Argument {} of {} expects {}, {} 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())),
),
statements_analyzer.get_hpos(input_expr.pos()),
&context.function_context.calling_functionlike_id,
),
statements_analyzer.get_config(),
statements_analyzer.get_file_path_actual(),
);

return;
}
}

if union_comparison_result.type_coerced.unwrap_or(false) && !input_type.is_mixed() {
if union_comparison_result
.type_coerced_from_nested_any
Expand Down
17 changes: 17 additions & 0 deletions src/analyzer/stmt/return_analyzer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,23 @@ pub(crate) fn analyze(
);
}
} else {
if union_comparison_result.upcasted_awaitable {
analysis_data.maybe_add_issue(
Issue::new(
IssueKind::UpcastAwaitable,
format!(
"{} contains Awaitable but was passed into a more general type {}",
inferred_return_type.get_id(Some(interner)),
expected_return_type.get_id(Some(interner)),
),
statements_analyzer.get_hpos(&return_expr.1),
&context.function_context.calling_functionlike_id,
),
statements_analyzer.get_config(),
statements_analyzer.get_file_path_actual(),
);
}

for (name, mut bound) in union_comparison_result.type_variable_lower_bounds {
if let Some((lower_bounds, _)) =
analysis_data.type_variable_bounds.get_mut(&name)
Expand Down
1 change: 1 addition & 0 deletions src/code_info/issue.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ pub enum IssueKind {
UnusedTrait,
UnusedTypeDefinition,
UnusedXhpAttribute,
UpcastAwaitable,
}

impl IssueKind {
Expand Down
11 changes: 11 additions & 0 deletions src/ttype/type_comparator/atomic_type_comparator.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,17 @@ pub fn is_contained_by(
return false;
}

if matches!(
input_type_part,
&TAtomic::TNamedObject {
name: StrId::AWAITABLE,
..
}
) && container_type_part.is_mixed()
{
atomic_comparison_result.upcasted_awaitable = true;
}

return true;
}

Expand Down
3 changes: 3 additions & 0 deletions src/ttype/type_comparator/type_comparison_result.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ pub struct TypeComparisonResult {
/* type is coerced from a generic `as mixed` param e.g. dict<string, T> into dict<string, string> */
pub type_coerced_from_as_mixed: Option<bool>,

pub upcasted_awaitable: bool,

/**
* This is used for array access. In the function below
* we know that there are only two possible keys, 0 and 1,
Expand Down Expand Up @@ -53,6 +55,7 @@ impl TypeComparisonResult {
replacement_atomic_type: None,
type_variable_lower_bounds: vec![],
type_variable_upper_bounds: vec![],
upcasted_awaitable: false,
}
}
}
4 changes: 4 additions & 0 deletions src/ttype/type_comparator/union_type_comparator.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,10 @@ pub fn is_contained_by(
atomic_comparison_result.type_coerced_to_literal;
}

if atomic_comparison_result.upcasted_awaitable && !container_type.had_template {
union_comparison_result.upcasted_awaitable = true;
}

if is_atomic_contained_by {
if let Some(replacement_atomic_type) =
atomic_comparison_result.replacement_atomic_type
Expand Down

0 comments on commit d0e5f85

Please sign in to comment.