Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for UnusedMethodCall #18

Merged
merged 2 commits into from
Apr 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/analyzer/expr/assertion_finder.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::expression_identifier::{get_dim_id, get_var_id};
use crate::expr::expression_identifier::get_functionlike_id_from_call;
use crate::expr::expression_identifier::get_static_functionlike_id_from_call;
use crate::{formula_generator::AssertionContext, function_analysis_data::FunctionAnalysisData};
use hakana_reflection_info::code_location::HPos;
use hakana_reflection_info::function_context::FunctionLikeIdentifier;
Expand Down Expand Up @@ -58,7 +58,7 @@ pub(crate) fn scrape_assertions(
);
}
aast::Expr_::Call(call) => {
let functionlike_id = get_functionlike_id_from_call(
let functionlike_id = get_static_functionlike_id_from_call(
call,
if let Some((_, interner)) = assertion_context.codebase {
Some(interner)
Expand Down Expand Up @@ -348,7 +348,7 @@ fn scrape_shapes_isset(
negated: bool,
) {
if let aast::Expr_::Call(call) = &var_expr.2 {
let functionlike_id = get_functionlike_id_from_call(
let functionlike_id = get_static_functionlike_id_from_call(
call,
if let Some((_, interner)) = assertion_context.codebase {
Some(interner)
Expand Down
59 changes: 57 additions & 2 deletions src/analyzer/expr/expression_identifier.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
use std::rc::Rc;

use hakana_reflection_info::{
ast::get_id_name, codebase_info::CodebaseInfo, functionlike_identifier::FunctionLikeIdentifier,
ExprId, VarId,
t_atomic::TAtomic, t_union::TUnion, ExprId, VarId,
};
use hakana_str::{Interner, StrId};
use rustc_hash::{FxHashMap, FxHashSet};

use oxidized::{aast, ast_defs};
use oxidized::{aast, ast::PropOrMethod, ast_defs};

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

Expand Down Expand Up @@ -160,6 +162,16 @@ pub(crate) fn get_dim_id(
}

pub fn get_functionlike_id_from_call(
call_expr: &oxidized::ast::CallExpr,
interner: &Interner,
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)
.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>,
resolved_names: &FxHashMap<u32, StrId>,
Expand Down Expand Up @@ -213,6 +225,49 @@ pub fn get_functionlike_id_from_call(
}
}

pub fn get_method_id_from_call(
call_expr: &oxidized::ast::CallExpr,
interner: &Interner,
expr_types: &FxHashMap<(u32, u32), Rc<TUnion>>,
) -> Option<FunctionLikeIdentifier> {
// Instance method call
match &call_expr.func.2 {
aast::Expr_::ObjGet(boxed) => {
let (lhs_expr, rhs_expr, _nullfetch, prop_or_method) =
(&boxed.0, &boxed.1, &boxed.2, &boxed.3);

if *prop_or_method == PropOrMethod::IsProp {
return None;
}

let class_id = if let Some(lhs_expr_type) = expr_types.get(&(
lhs_expr.1.start_offset() as u32,
lhs_expr.1.end_offset() as u32,
)) {
let t = lhs_expr_type.types.first().unwrap();
if let TAtomic::TNamedObject { name, .. } = t {
name
} else {
return None;
}
} else {
return None;
};

if let aast::Expr_::Id(method_name_node) = &rhs_expr.2 {
if let Some(method_id) = interner.get(&method_name_node.1) {
return Some(FunctionLikeIdentifier::Method(*class_id, method_id));
} else {
return None;
}
} else {
return None;
}
}
_ => return None,
}
}

pub fn get_expr_id(
conditional: &aast::Expr<(), ()>,
statements_analyzer: &StatementsAnalyzer,
Expand Down
76 changes: 51 additions & 25 deletions src/analyzer/stmt_analyzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,12 +273,11 @@ fn detect_unused_statement_expressions(
stmt: &aast::Stmt<(), ()>,
context: &mut ScopeContext,
) {
if has_unused_must_use(boxed, statements_analyzer) {
if let Some(issue_kind) = has_unused_must_use(boxed, statements_analyzer, analysis_data) {
analysis_data.maybe_add_issue(
Issue::new(
IssueKind::UnusedFunctionCall,
"This function is annotated with MustUse but the returned value is not used"
.to_string(),
issue_kind,
"This is annotated with MustUse but the returned value is not used".to_string(),
statements_analyzer.get_hpos(&stmt.0),
&context.function_context.calling_functionlike_id,
),
Expand All @@ -290,8 +289,9 @@ fn detect_unused_statement_expressions(
let functionlike_id = if let aast::Expr_::Call(boxed_call) = &boxed.2 {
get_functionlike_id_from_call(
boxed_call,
Some(statements_analyzer.get_interner()),
statements_analyzer.get_interner(),
statements_analyzer.get_file_analyzer().resolved_names,
&analysis_data.expr_types,
)
} else {
None
Expand Down Expand Up @@ -406,40 +406,66 @@ fn detect_unused_statement_expressions(
fn has_unused_must_use(
boxed: &aast::Expr<(), ()>,
statements_analyzer: &StatementsAnalyzer,
) -> bool {
analysis_data: &mut FunctionAnalysisData,
) -> Option<IssueKind> {
match &boxed.2 {
aast::Expr_::Call(boxed_call) => {
let functionlike_id = get_functionlike_id_from_call(
let functionlike_id_from_call = get_functionlike_id_from_call(
boxed_call,
Some(statements_analyzer.get_interner()),
statements_analyzer.get_interner(),
statements_analyzer.get_file_analyzer().resolved_names,
&analysis_data.expr_types,
);
if let Some(FunctionLikeIdentifier::Function(function_id)) = functionlike_id {
// For statements like "Asio\join(some_fn());"
// Asio\join does not count as "using" the value
if function_id == StrId::ASIO_JOIN {
return boxed_call
.args
.iter()
.any(|arg| has_unused_must_use(&arg.1, statements_analyzer));
}

if let Some(functionlike_id) = functionlike_id_from_call {
let codebase = statements_analyzer.get_codebase();
if let Some(functionlike_info) = codebase
.functionlike_infos
.get(&(function_id, StrId::EMPTY))
{
return functionlike_info.must_use;
match functionlike_id {
FunctionLikeIdentifier::Function(function_id) => {
// For statements like "Asio\join(some_fn());"
// Asio\join does not count as "using" the value
if function_id == StrId::ASIO_JOIN {
for arg in boxed_call.args.iter() {
let has_unused =
has_unused_must_use(&arg.1, statements_analyzer, analysis_data);
if has_unused.is_some() {
return has_unused;
}
}
}

if let Some(functionlike_info) = codebase
.functionlike_infos
.get(&(function_id, StrId::EMPTY))
{
return if functionlike_info.must_use {
Some(IssueKind::UnusedFunctionCall)
} else {
None
};
}
}
FunctionLikeIdentifier::Method(method_class, method_name) => {
if let Some(functionlike_info) = codebase
.functionlike_infos
.get(&(method_class, method_name))
{
return if functionlike_info.must_use {
Some(IssueKind::UnusedMethodCall)
} else {
None
};
}
}
FunctionLikeIdentifier::Closure(_, _) => (),
}
}
}
aast::Expr_::Await(await_expr) => {
return has_unused_must_use(await_expr, statements_analyzer)
return has_unused_must_use(await_expr, statements_analyzer, analysis_data)
}
_ => (),
}

false
None
}

fn analyze_awaitall(
Expand Down
3 changes: 2 additions & 1 deletion src/code_info/issue.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -127,6 +127,7 @@ pub enum IssueKind {
UnusedClosureParameter,
UnusedFunction,
UnusedFunctionCall,
UnusedMethodCall,
UnusedInheritedMethod,
UnusedInterface,
UnusedParameter,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
ERROR: UnusedFunctionCall - input.hack:7:5 - This function is annotated with MustUse but the returned value is not used
ERROR: UnusedFunctionCall - input.hack:11:5 - This function is annotated with MustUse but the returned value is not used
ERROR: UnusedFunctionCall - input.hack:7:5 - This is annotated with MustUse but the returned value is not used
ERROR: UnusedFunctionCall - input.hack:11:5 - This is annotated with MustUse but the returned value is not used
16 changes: 16 additions & 0 deletions tests/unused/UnusedExpression/unusedMethodCall/input.hack
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
class UnusedMethodClass {
<<Hakana\MustUse>>
public function getEncodedId(): string {
return '';
}

public function doWork(): string {
return '';
}
}

function foo(): void {
$c = new UnusedMethodClass();
$c->getEncodedId();
$c->doWork();
}
1 change: 1 addition & 0 deletions tests/unused/UnusedExpression/unusedMethodCall/output.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
UnusedMethodCall
22 changes: 22 additions & 0 deletions tests/unused/UnusedExpression/unusedMethodCallAsync/input.hack
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
class UnusedMethodClass {
<<Hakana\MustUse>>
public async function getEncodedId(): Awaitable<string> {
return '';
}

public async function doWork(): Awaitable<string> {
return '';
}
}

async function foo(): Awaitable<void> {
$c = new UnusedMethodClass();
await $c->getEncodedId();
await $c->doWork();
}

function foo2(): void {
$c = new UnusedMethodClass();
Asio\join($c->getEncodedId());
Asio\join($c->doWork());
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ERROR: UnusedMethodCall - input.hack:14:5 - This is annotated with MustUse but the returned value is not used
ERROR: UnusedMethodCall - input.hack:20:5 - This is annotated with MustUse but the returned value is not used
Loading