Skip to content

Commit

Permalink
Support more granular unused method checks
Browse files Browse the repository at this point in the history
  • Loading branch information
muglug committed Apr 19, 2024
1 parent 3d81aa9 commit 5169d8b
Show file tree
Hide file tree
Showing 17 changed files with 80 additions and 61 deletions.
5 changes: 4 additions & 1 deletion src/code_info/functionlike_info.rs
Expand Up @@ -47,7 +47,7 @@ impl FnEffect {
}
}

#[derive(Debug, Clone, Serialize, Deserialize)]
#[derive(Debug, Clone, Copy, Serialize, Deserialize)]
pub struct MetaStart {
pub start_offset: u32,
pub start_line: u32,
Expand Down Expand Up @@ -150,6 +150,8 @@ pub struct FunctionLikeInfo {
pub is_production_code: bool,

pub is_closure: bool,

pub overriding: bool,
}

impl FunctionLikeInfo {
Expand Down Expand Up @@ -188,6 +190,7 @@ impl FunctionLikeInfo {
is_production_code: true,
pure_can_throw: false,
is_closure: false,
overriding: false,
}
}

Expand Down
1 change: 1 addition & 0 deletions src/code_info/issue.rs
Expand Up @@ -127,6 +127,7 @@ pub enum IssueKind {
UnusedClosureParameter,
UnusedFunction,
UnusedFunctionCall,
UnusedInheritedMethod,
UnusedInterface,
UnusedParameter,
UnusedPipeVariable,
Expand Down
24 changes: 0 additions & 24 deletions src/code_info/method_info.rs
@@ -1,6 +1,3 @@
use hakana_str::StrId;
use rustc_hash::FxHashSet;

use serde::{Deserialize, Serialize};

use crate::member_visibility::MemberVisibility;
Expand All @@ -14,20 +11,6 @@ pub struct MethodInfo {
pub is_final: bool,

pub is_abstract: bool,

pub defining_fqcln: Option<StrId>,

pub external_mutation_free: bool,

pub immutable: bool,

pub mutation_free_inferred: bool,

pub this_property_mutations: Option<FxHashSet<String>>,

pub stubbed: bool,

pub probably_fluent: bool,
}

impl Default for MethodInfo {
Expand All @@ -43,13 +26,6 @@ impl MethodInfo {
visibility: MemberVisibility::Public,
is_final: false,
is_abstract: false,
defining_fqcln: None,
external_mutation_free: false,
immutable: false,
mutation_free_inferred: false,
this_property_mutations: None,
stubbed: false,
probably_fluent: false,
}
}
}
4 changes: 3 additions & 1 deletion src/code_info_builder/functionlike_scanner.rs
Expand Up @@ -90,7 +90,6 @@ pub(crate) fn scan_method(

let mut method_info = MethodInfo::new();

method_info.defining_fqcln = Some(classlike_name);
method_info.is_static = m.static_;
method_info.is_final = m.final_ || classlike_storage.is_final;
method_info.is_abstract = m.abstract_;
Expand Down Expand Up @@ -404,6 +403,9 @@ pub(crate) fn get_functionlike(
StrId::CODEGEN => {
functionlike_info.generated = true;
}
StrId::OVERRIDE => {
functionlike_info.overriding = true;
}
_ => {}
}
}
Expand Down
13 changes: 0 additions & 13 deletions src/file_scanner_analyzer/populator.rs
Expand Up @@ -436,19 +436,6 @@ fn populate_classlike_storage(
// todo add file references for cache invalidation

if storage.immutable {
for method_name in &storage.methods {
let functionlike_storage = codebase
.functionlike_infos
.get_mut(&(storage.name, *method_name))
.unwrap();

if let Some(method_storage) = functionlike_storage.method_info.as_mut() {
if !method_storage.is_static {
method_storage.immutable = true;
}
}
}

for (_, property_storage) in storage.properties.iter_mut() {
if !property_storage.is_static {
property_storage.soft_readonly = true;
Expand Down
64 changes: 46 additions & 18 deletions src/file_scanner_analyzer/unused_symbols.rs
Expand Up @@ -97,10 +97,14 @@ pub(crate) fn find_unused_definitions(
continue;
}

if config
.migration_symbols
.contains_key(interner.lookup(&functionlike_name.0))
{
let issue = Issue::new(
IssueKind::UnusedFunction,
format!("Unused function {}", interner.lookup(&functionlike_name.0)),
*pos,
&Some(FunctionLikeIdentifier::Function(functionlike_name.0)),
);

if config.issues_to_fix.contains(&issue.kind) && !config.add_fixmes {
let meta_start = &functionlike_info.meta_start;
let def_pos = &functionlike_info.def_location;
analysis_result
Expand All @@ -115,13 +119,6 @@ pub(crate) fn find_unused_definitions(
);
}

let issue = Issue::new(
IssueKind::UnusedFunction,
format!("Unused function {}", interner.lookup(&functionlike_name.0)),
*pos,
&Some(FunctionLikeIdentifier::Function(functionlike_name.0)),
);

if config.can_add_issue(&issue) {
*analysis_result
.issue_counts
Expand Down Expand Up @@ -198,10 +195,7 @@ pub(crate) fn find_unused_definitions(
&Some(FunctionLikeIdentifier::Function(*classlike_name)),
);

if config
.migration_symbols
.contains_key(interner.lookup(classlike_name))
{
if config.issues_to_fix.contains(&issue.kind) && !config.add_fixmes {
let meta_start = &classlike_info.meta_start;
let def_pos = &classlike_info.def_location;
analysis_result
Expand Down Expand Up @@ -314,7 +308,13 @@ pub(crate) fn find_unused_definitions(
}

let issue =
if matches!(method_storage.visibility, MemberVisibility::Private) {
if matches!(method_storage.visibility, MemberVisibility::Private)
|| (matches!(
method_storage.visibility,
MemberVisibility::Protected
) && method_storage.is_final
&& !functionlike_storage.overriding)
{
Issue::new(
IssueKind::UnusedPrivateMethod,
format!(
Expand All @@ -328,11 +328,25 @@ pub(crate) fn find_unused_definitions(
*method_name_ptr,
)),
)
} else if functionlike_storage.overriding {
Issue::new(
IssueKind::UnusedInheritedMethod,
format!(
"Unused inherited method {}::{}",
interner.lookup(classlike_name),
interner.lookup(method_name_ptr)
),
functionlike_storage.name_location.unwrap(),
&Some(FunctionLikeIdentifier::Method(
*classlike_name,
*method_name_ptr,
)),
)
} else {
Issue::new(
IssueKind::UnusedPublicOrProtectedMethod,
format!(
"Possibly-unused method {}::{}",
"Unused public or protected method {}::{}",
interner.lookup(classlike_name),
interner.lookup(method_name_ptr)
),
Expand All @@ -350,7 +364,21 @@ pub(crate) fn find_unused_definitions(
continue;
}

if config.can_add_issue(&issue) {
if config.issues_to_fix.contains(&issue.kind) && !config.add_fixmes {
let meta_start = functionlike_storage.meta_start;
let def_pos = functionlike_storage.def_location;
analysis_result
.replacements
.entry(pos.file_path)
.or_default()
.insert(
(meta_start.start_offset, def_pos.end_offset),
Replacement::TrimPrecedingWhitespace(
meta_start.start_offset + 1
- meta_start.start_column as u32,
),
);
} else if config.can_add_issue(&issue) {
*analysis_result
.issue_counts
.entry(issue.kind.clone())
Expand Down
7 changes: 7 additions & 0 deletions src/language_server/lib.rs
Expand Up @@ -140,6 +140,13 @@ impl LanguageServer for Backend {
}
}

self.client
.log_message(
MessageType::INFO,
format!("receiving changes {:?}", new_file_statuses),
)
.await;

if let Some(ref mut existing_file_changes) = self.file_changes {
existing_file_changes.extend(new_file_statuses);
} else {
Expand Down
1 change: 1 addition & 0 deletions src/str/build.rs
Expand Up @@ -258,6 +258,7 @@ fn main() -> Result<()> {
"__EntryPoint",
"__FILE__",
"__FUNCTION__",
"__Override",
"__PHP_Incomplete_Class",
"__Sealed",
"__construct",
Expand Down
4 changes: 2 additions & 2 deletions tests/diff/xhpMissingAttribute/output.txt
@@ -1,3 +1,3 @@
ERROR: UnusedPublicOrProtectedMethod - input.hack:6:33 - Possibly-unused method MyElement::renderAsync
ERROR: UnusedPublicOrProtectedMethod - input.hack:6:33 - Unused public or protected method MyElement::renderAsync
ERROR: UnusedFunction - input.hack:11:10 - Unused function foo
ERROR: MissingRequiredXhpAttribute - input.hack:12:12 - XHP class MyElement is missing a required attribute: some-attr
ERROR: MissingRequiredXhpAttribute - input.hack:12:12 - XHP class MyElement is missing a required attribute: some-attr
9 changes: 9 additions & 0 deletions tests/fix/UnusedPrivateMethod/simple/input.hack
@@ -0,0 +1,9 @@
class Foo {
/** cool doc */
private function bar(): void {}
}

<<__EntryPoint>>
function main(): void {
new Foo();
}
7 changes: 7 additions & 0 deletions tests/fix/UnusedPrivateMethod/simple/output.txt
@@ -0,0 +1,7 @@
class Foo {
}

<<__EntryPoint>>
function main(): void {
new Foo();
}

This file was deleted.

This file was deleted.

0 comments on commit 5169d8b

Please sign in to comment.