Skip to content
Merged

fix #814

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
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::{
FileId,
db_index::{LuaFunctionType, LuaType},
};
use crate::{SemanticModel, VariadicType};
use crate::{SemanticModel, VariadicType, first_param_may_not_self};

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Adding first_param_may_not_self to the use statement increases code readability by explicitly stating the dependencies of this module. This makes it easier to understand the module's functionality without having to search for the function's definition.


#[derive(Debug)]
pub struct LuaSignature {
Expand Down Expand Up @@ -137,11 +137,8 @@ impl LuaSignature {
match owner_type {
Some(owner_type) => {
// 一些类型不应该被视为 method
if let (LuaType::Ref(_) | LuaType::Def(_), _) = (owner_type, param_type)
&& (param_type.is_any()
|| param_type.is_table()
|| param_type.is_class_tpl()
|| param_type.is_str_tpl_ref())
if matches!(owner_type, LuaType::Ref(_) | LuaType::Def(_))
&& first_param_may_not_self(param_type)
{
return false;
}
Comment on lines +140 to 144

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The original code used a complex conditional statement to check the type. The refactored code uses matches! which is more concise and readable for pattern matching.

Consider adding a comment explaining the purpose of this check to improve maintainability.

Suggested change
if matches!(owner_type, LuaType::Ref(_) | LuaType::Def(_))
&& first_param_may_not_self(param_type)
{
return false;
}
if matches!(owner_type, LuaType::Ref(_) | LuaType::Def(_)) {
// These types should not be considered methods.
if first_param_may_not_self(param_type) {
return false;
}
}

Expand Down
12 changes: 12 additions & 0 deletions crates/emmylua_code_analysis/src/db_index/type/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -301,3 +301,15 @@ fn get_real_type_with_depth<'a>(
_ => Some(typ),
}
}

// 第一个参数是否不应该视为 self
pub fn first_param_may_not_self(typ: &LuaType) -> bool {
if typ.is_table() || matches!(typ, LuaType::Ref(_) | LuaType::Def(_) | LuaType::Any) {
return false;
}
if let LuaType::Union(u) = typ {
return u.into_vec().iter().any(first_param_may_not_self);
}

true
}
Comment on lines +305 to +315

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This function determines whether the first parameter should not be considered as self. It checks if the type is a table, Ref, Def, or Any, and returns false if it is. If the type is a union, it recursively checks each type in the union. Otherwise, it returns true.

Consider adding a more detailed comment explaining the rationale behind this logic, especially why certain types are excluded from being considered self.

8 changes: 3 additions & 5 deletions crates/emmylua_code_analysis/src/db_index/type/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use smol_str::SmolStr;
use crate::{
AsyncState, DbIndex, FileId, InFiled, SemanticModel,
db_index::{LuaMemberKey, LuaSignatureId, r#type::type_visit_trait::TypeVisitTrait},
first_param_may_not_self,
};

use super::{TypeOps, type_decl::LuaTypeDeclId};
Expand Down Expand Up @@ -686,11 +687,8 @@ impl LuaFunctionType {
match owner_type {
Some(owner_type) => {
// 一些类型不应该被视为 method
if let (LuaType::Ref(_) | LuaType::Def(_), _) = (owner_type, t)
&& (t.is_any()
|| t.is_table()
|| t.is_class_tpl()
|| t.is_str_tpl_ref())
if matches!(owner_type, LuaType::Ref(_) | LuaType::Def(_))
&& first_param_may_not_self(t)
{
return false;
}
Comment on lines +690 to 694

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The original code used a complex conditional statement to check the type. The refactored code uses matches! which is more concise and readable for pattern matching.

Consider adding a comment explaining the purpose of this check to improve maintainability.

Suggested change
if matches!(owner_type, LuaType::Ref(_) | LuaType::Def(_))
&& first_param_may_not_self(t)
{
return false;
}
if matches!(owner_type, LuaType::Ref(_) | LuaType::Def(_)) {
// These types should not be considered methods.
if first_param_may_not_self(t) {
return false;
}
}

Expand Down
6 changes: 5 additions & 1 deletion crates/emmylua_ls/src/handlers/hover/function/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,11 @@ fn hover_doc_function_type(
if index == 0 && is_method && !func.is_colon_define() {
"".to_string()
} else if let Some(ty) = &param.1 {
format!("{}: {}", name, humanize_type(db, ty, RenderLevel::Normal))
format!(
"{}: {}",
name,
humanize_type(db, ty, builder.detail_render_level)
)
} else {
name.to_string()
}
Comment on lines 361 to 371

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The code was reformatted to improve readability by explicitly formatting the string with name, db, ty, and builder.detail_render_level on separate lines. This change enhances maintainability by making it easier to modify the formatting in the future.

Expand Down
20 changes: 20 additions & 0 deletions crates/emmylua_ls/src/handlers/test/hover_function_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -564,4 +564,24 @@ mod tests {
));
Ok(())
}

#[gtest]
fn test_fix_method_1() -> Result<()> {
let mut ws = ProviderVirtualWorkspace::new();
check!(ws.check_hover(
r#"
---@class ClassControl
local ClassControl = {}

---@generic T
---@param name `T`|T
function ClassControl.ne<??>w(name)
end
"#,
VirtualHoverResult {
value: "```lua\nfunction ClassControl.new(name: T)\n```".to_string(),
},
));
Ok(())
}
}