diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f572da6bca9..66c86aea6ca9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -104,6 +104,20 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b - `noExtraNonNullAssertion` no longer reports a single non-null assertion enclosed in parentheses ([#3352](https://github.com/biomejs/biome/issues/3352)). Contributed by @Conaclos +- `useAdjacentOverloadSignatures` no longer reports a `#private` class member and a public class member that share the same name ([#3309](https://github.com/biomejs/biome/issues/3309)). + + The following code is no longer reported: + + ```js + class C { + #f() {} + g() {} + f() {} + } + ``` + + Contributed by @Conaclos + ### Parser diff --git a/crates/biome_js_analyze/src/lint/nursery/use_adjacent_overload_signatures.rs b/crates/biome_js_analyze/src/lint/nursery/use_adjacent_overload_signatures.rs index a42a034a3f6d..69c5cc885c80 100644 --- a/crates/biome_js_analyze/src/lint/nursery/use_adjacent_overload_signatures.rs +++ b/crates/biome_js_analyze/src/lint/nursery/use_adjacent_overload_signatures.rs @@ -4,7 +4,7 @@ use biome_analyze::{ use biome_console::markup; use biome_js_syntax::{ AnyJsModuleItem, JsClassDeclaration, JsFunctionDeclaration, JsModule, JsModuleItemList, - TsDeclareStatement, TsInterfaceDeclaration, TsTypeAliasDeclaration, + TsDeclareStatement, TsInterfaceDeclaration, TsTypeAliasDeclaration, TsTypeMemberList, }; use biome_rowan::{declare_node_union, AstNode, TextRange, TokenText}; use rustc_hash::FxHashSet; @@ -104,40 +104,36 @@ impl Rule for UseAdjacentOverloadSignatures { type Options = (); fn run(ctx: &RuleContext) -> Self::Signals { - let node = ctx.query(); - let mut methods: Vec<(TokenText, TextRange)> = Vec::new(); - let mut seen_methods = FxHashSet::default(); - let mut last_method = None; - - match node { + let methods = match ctx.query() { // Handle export function foo() {} in declare namespace Foo {} DeclarationOrModuleNode::TsDeclareStatement(node) => { let declaration = node.declaration().ok()?; let items = declaration.as_ts_module_declaration()?.body().ok()?.items(); - collect_exports(&items, &mut methods, &mut seen_methods, &mut last_method); + collect_exports(&items) } // Handle interface Foo {} DeclarationOrModuleNode::TsInterfaceDeclaration(node) => { - collect_interface(node, &mut methods, &mut seen_methods, &mut last_method); + collect_type_member_list(&node.members()) } // Handle type Foo = {} DeclarationOrModuleNode::TsTypeAliasDeclaration(node) => { - collect_type(node, &mut methods, &mut seen_methods, &mut last_method); + let members = node + .ty() + .ok() + .and_then(|ty| ty.as_ts_object_type().cloned())? + .members(); + collect_type_member_list(&members) } // Handle class Foo {} - DeclarationOrModuleNode::JsClassDeclaration(node) => { - collect_class(node, &mut methods, &mut seen_methods, &mut last_method); - } + DeclarationOrModuleNode::JsClassDeclaration(node) => collect_class(node), // Handle export function foo() {} - DeclarationOrModuleNode::JsFunctionDeclaration(node) => { - collect_function(node, &mut methods, &mut seen_methods, &mut last_method); - } + DeclarationOrModuleNode::JsFunctionDeclaration(node) => collect_function(node), // Handle export function foo() {} DeclarationOrModuleNode::JsModule(node) => { let items = node.items(); - collect_exports(&items, &mut methods, &mut seen_methods, &mut last_method); + collect_exports(&items) } - } + }; if !methods.is_empty() { Some(methods) @@ -169,57 +165,33 @@ impl Rule for UseAdjacentOverloadSignatures { } } -fn collect_interface( - node: &TsInterfaceDeclaration, - methods: &mut Vec<(TokenText, TextRange)>, - seen_methods: &mut FxHashSet, - last_method: &mut Option, -) { - let members = node.members(); - for member in members { +fn collect_type_member_list(node: &TsTypeMemberList) -> Vec<(TokenText, TextRange)> { + let mut methods: Vec<(TokenText, TextRange)> = Vec::new(); + let mut seen_methods = FxHashSet::default(); + let mut last_method = None; + for member in node { if let Some(ts_method_signature) = member.as_ts_method_signature_type_member() { if let Ok(method_member) = ts_method_signature.name() { if let Some(text) = method_member.name() { let range = method_member.range(); - check_method(text, range, methods, seen_methods, last_method); - } - } - } - } -} - -fn collect_type( - node: &TsTypeAliasDeclaration, - methods: &mut Vec<(TokenText, TextRange)>, - seen_methods: &mut FxHashSet, - last_method: &mut Option, -) { - let ty = node - .ty() - .ok() - .and_then(|ty| ty.as_ts_object_type().cloned()); - if let Some(ts_object) = ty { - let members = ts_object.members(); - for member in members { - if let Some(method_member) = member - .as_ts_method_signature_type_member() - .and_then(|m| m.name().ok()) - { - if let Some(text) = method_member.name() { - let range = method_member.range(); - check_method(text, range, methods, seen_methods, last_method); + check_method( + text, + range, + &mut methods, + &mut seen_methods, + &mut last_method, + ); } } } } + methods } -fn collect_class( - node: &JsClassDeclaration, - methods: &mut Vec<(TokenText, TextRange)>, - seen_methods: &mut FxHashSet, - last_method: &mut Option, -) { +fn collect_class(node: &JsClassDeclaration) -> Vec<(TokenText, TextRange)> { + let mut methods: Vec<(TokenText, TextRange)> = Vec::new(); + let mut seen_methods = FxHashSet::default(); + let mut last_method = None; let members = node.members(); for member in members { if let Some(method_class) = member @@ -229,26 +201,37 @@ fn collect_class( if let Ok(method_member) = method_class.name() { if let Some(text) = method_member.name() { let range = method_member.range(); - check_method(text, range, methods, seen_methods, last_method); + check_method( + text, + range, + &mut methods, + &mut seen_methods, + &mut last_method, + ); } } } else if let Some(method_class) = member.as_ts_method_signature_class_member() { if let Ok(method_member) = method_class.name() { if let Some(text) = method_member.name() { let range = method_member.range(); - check_method(text, range, methods, seen_methods, last_method); + check_method( + text, + range, + &mut methods, + &mut seen_methods, + &mut last_method, + ); } } } } + methods } -fn collect_function( - node: &JsFunctionDeclaration, - methods: &mut Vec<(TokenText, TextRange)>, - seen_methods: &mut FxHashSet, - last_method: &mut Option, -) { +fn collect_function(node: &JsFunctionDeclaration) -> Vec<(TokenText, TextRange)> { + let mut methods: Vec<(TokenText, TextRange)> = Vec::new(); + let mut seen_methods = FxHashSet::default(); + let mut last_method = None; if let Some(return_type_annotation) = node.return_type_annotation() { if let Some(ty) = return_type_annotation .ty() @@ -264,21 +247,26 @@ fn collect_function( { if let Some(text) = method_member.name() { let range = method_member.range(); - check_method(text, range, methods, seen_methods, last_method); + check_method( + text, + range, + &mut methods, + &mut seen_methods, + &mut last_method, + ); } } } } } } + methods } -fn collect_exports( - items: &JsModuleItemList, - methods: &mut Vec<(TokenText, TextRange)>, - seen_methods: &mut FxHashSet, - last_method: &mut Option, -) { +fn collect_exports(items: &JsModuleItemList) -> Vec<(TokenText, TextRange)> { + let mut methods: Vec<(TokenText, TextRange)> = Vec::new(); + let mut seen_methods = FxHashSet::default(); + let mut last_method = None; for item in items { if let AnyJsModuleItem::JsExport(node) = item { if let Ok(export) = node.export_clause() { @@ -292,26 +280,33 @@ fn collect_exports( }) { let text = name_token.token_text_trimmed(); let range = name_token.text_range(); - check_method(text, range, methods, seen_methods, last_method); + check_method( + text, + range, + &mut methods, + &mut seen_methods, + &mut last_method, + ); } } } } } } + methods } // Check if the method is already seen and add it to the list of methods -fn check_method( - text: TokenText, +fn check_method>( + text: T, range: TextRange, methods: &mut Vec<(TokenText, TextRange)>, - seen_methods: &mut FxHashSet, - last_method: &mut Option, + seen_methods: &mut FxHashSet, + last_method: &mut Option, ) { if let Some(last) = last_method { if last != &text && seen_methods.contains(&text) { - methods.push((text.clone(), range)); + methods.push((text.clone().into(), range)); } else { seen_methods.insert(text.clone()); } diff --git a/crates/biome_js_analyze/src/lint/suspicious/no_misleading_instantiator.rs b/crates/biome_js_analyze/src/lint/suspicious/no_misleading_instantiator.rs index 6ced2bf9156f..83302975eb16 100644 --- a/crates/biome_js_analyze/src/lint/suspicious/no_misleading_instantiator.rs +++ b/crates/biome_js_analyze/src/lint/suspicious/no_misleading_instantiator.rs @@ -3,8 +3,9 @@ use biome_analyze::{ }; use biome_console::{markup, MarkupBuf}; use biome_js_syntax::{ - AnyJsClassMember, AnyTsType, AnyTsTypeMember, JsClassDeclaration, JsSyntaxToken, - TsDeclareStatement, TsInterfaceDeclaration, TsReferenceType, TsTypeAliasDeclaration, + AnyJsClassMember, AnyTsType, AnyTsTypeMember, ClassMemberName, JsClassDeclaration, + JsSyntaxToken, TsDeclareStatement, TsInterfaceDeclaration, TsReferenceType, + TsTypeAliasDeclaration, }; use biome_rowan::{declare_node_union, AstNode, TextRange}; @@ -189,27 +190,27 @@ fn check_class_methods(js_class_decl: &JsClassDeclaration) -> Option .as_js_identifier_binding()? .name_token() .ok()?; - for member in js_class_decl.members() { - match member { - AnyJsClassMember::TsMethodSignatureClassMember(method) - if method.name().ok()?.name()? == "new" => - { - let return_type = method.return_type_annotation()?.ty().ok()?; - match return_type.as_any_ts_type()? { - AnyTsType::TsReferenceType(ref_type) => { - let return_type_ident = extract_return_type_ident(ref_type)?; - if class_ident.text_trimmed() == return_type_ident.text_trimmed() { + if let AnyJsClassMember::TsMethodSignatureClassMember(method) = member { + if let Some(ClassMemberName::Public(name)) = method.name().ok()?.name() { + if name.text() == "new" { + let return_type = method.return_type_annotation()?.ty().ok()?; + match return_type.as_any_ts_type()? { + AnyTsType::TsReferenceType(ref_type) => { + let return_type_ident = extract_return_type_ident(ref_type)?; + if class_ident.text_trimmed() == return_type_ident.text_trimmed() { + return Some(RuleState::ClassMisleadingNew(method.range())); + } + } + AnyTsType::TsThisType(this_type) + if this_type.this_token().ok().is_some() => + { return Some(RuleState::ClassMisleadingNew(method.range())); } + _ => continue, } - AnyTsType::TsThisType(this_type) if this_type.this_token().ok().is_some() => { - return Some(RuleState::ClassMisleadingNew(method.range())); - } - _ => continue, } } - _ => continue, } } None diff --git a/crates/biome_js_analyze/src/lint/suspicious/no_then_property.rs b/crates/biome_js_analyze/src/lint/suspicious/no_then_property.rs index 36b221254353..bd782824c848 100644 --- a/crates/biome_js_analyze/src/lint/suspicious/no_then_property.rs +++ b/crates/biome_js_analyze/src/lint/suspicious/no_then_property.rs @@ -4,8 +4,8 @@ use biome_analyze::{context::RuleContext, declare_lint_rule, Rule, RuleDiagnosti use biome_console::{markup, MarkupBuf}; use biome_js_syntax::{ AnyJsArrayElement, AnyJsAssignment, AnyJsAssignmentPattern, AnyJsCallArgument, - AnyJsClassMemberName, AnyJsDeclarationClause, AnyJsExportClause, AnyJsExportNamedSpecifier, - AnyJsExpression, AnyJsObjectMemberName, AnyJsTemplateElement, JsMethodObjectMember, + AnyJsDeclarationClause, AnyJsExportClause, AnyJsExportNamedSpecifier, AnyJsExpression, + AnyJsObjectMemberName, AnyJsTemplateElement, ClassMemberName, JsMethodObjectMember, }; use biome_js_syntax::{ AnyJsClassMember, AnyJsObjectMember, JsAssignmentExpression, JsCallExpression, @@ -222,43 +222,14 @@ fn process_js_method_object_member(node: &JsMethodObjectMember) -> Option Option { - if let Some(AnyJsClassMemberName::JsPrivateClassMemberName(_)) = node.name().ok()? { - return None; - } - match node { - AnyJsClassMember::JsGetterClassMember(node) => { - if node.name().ok()?.name()? == "then" { - return Some(RuleState { - range: node.name().ok()?.range(), - message: NoThenPropertyMessage::Class, - }); - } - } - AnyJsClassMember::JsMethodClassMember(node) => { - if node.name().ok()?.name()? == "then" { - return Some(RuleState { - range: node.name().ok()?.range(), - message: NoThenPropertyMessage::Class, - }); - } + let any_class_member_name = node.name().ok()??; + if let Some(ClassMemberName::Public(name)) = any_class_member_name.name() { + if name == "then" { + return Some(RuleState { + range: any_class_member_name.range(), + message: NoThenPropertyMessage::Class, + }); } - AnyJsClassMember::JsPropertyClassMember(node) => { - if node.name().ok()?.name()? == "then" { - return Some(RuleState { - range: node.name().ok()?.range(), - message: NoThenPropertyMessage::Class, - }); - } - } - AnyJsClassMember::JsSetterClassMember(node) => { - if node.name().ok()?.name()? == "then" { - return Some(RuleState { - range: node.name().ok()?.range(), - message: NoThenPropertyMessage::Class, - }); - } - } - _ => return None, } None } diff --git a/crates/biome_js_analyze/tests/specs/nursery/useAdjacentOverloadSignatures/valid.ts b/crates/biome_js_analyze/tests/specs/nursery/useAdjacentOverloadSignatures/valid.ts index 100ed71eda29..e1c99f337053 100644 --- a/crates/biome_js_analyze/tests/specs/nursery/useAdjacentOverloadSignatures/valid.ts +++ b/crates/biome_js_analyze/tests/specs/nursery/useAdjacentOverloadSignatures/valid.ts @@ -62,3 +62,10 @@ function f (): { functionA(x: number): number, } { } + +// Issue https://github.com/biomejs/biome/issues/3309#issuecomment-2208870371 +class C { + #f(): void {} + g(): void {} + f(): void {} +} \ No newline at end of file diff --git a/crates/biome_js_analyze/tests/specs/nursery/useAdjacentOverloadSignatures/valid.ts.snap b/crates/biome_js_analyze/tests/specs/nursery/useAdjacentOverloadSignatures/valid.ts.snap index f0584362fe9b..91300ae131c5 100644 --- a/crates/biome_js_analyze/tests/specs/nursery/useAdjacentOverloadSignatures/valid.ts.snap +++ b/crates/biome_js_analyze/tests/specs/nursery/useAdjacentOverloadSignatures/valid.ts.snap @@ -69,4 +69,10 @@ function f (): { } { } +// Issue https://github.com/biomejs/biome/issues/3309#issuecomment-2208870371 +class C { + #f(): void {} + g(): void {} + f(): void {} +} ``` diff --git a/crates/biome_js_analyze/tests/specs/suspicious/noMisleadingInstantiator/valid.ts b/crates/biome_js_analyze/tests/specs/suspicious/noMisleadingInstantiator/valid.ts index 39a76d5d6170..4540f4f7859a 100644 --- a/crates/biome_js_analyze/tests/specs/suspicious/noMisleadingInstantiator/valid.ts +++ b/crates/biome_js_analyze/tests/specs/suspicious/noMisleadingInstantiator/valid.ts @@ -49,3 +49,7 @@ interface foo { interface foo { new (): 'x'; } + +class C { + #new(): C; +} diff --git a/crates/biome_js_analyze/tests/specs/suspicious/noMisleadingInstantiator/valid.ts.snap b/crates/biome_js_analyze/tests/specs/suspicious/noMisleadingInstantiator/valid.ts.snap index f791b67e060f..21b911fc7dba 100644 --- a/crates/biome_js_analyze/tests/specs/suspicious/noMisleadingInstantiator/valid.ts.snap +++ b/crates/biome_js_analyze/tests/specs/suspicious/noMisleadingInstantiator/valid.ts.snap @@ -56,6 +56,8 @@ interface foo { new (): 'x'; } -``` - +class C { + #new(): C; +} +``` diff --git a/crates/biome_js_syntax/src/expr_ext.rs b/crates/biome_js_syntax/src/expr_ext.rs index 4314645a7aa1..3afab340e8c8 100644 --- a/crates/biome_js_syntax/src/expr_ext.rs +++ b/crates/biome_js_syntax/src/expr_ext.rs @@ -1584,6 +1584,32 @@ impl AnyTsEnumMemberName { } } +#[derive(Clone, Debug, Eq, Hash, PartialEq)] +pub enum ClassMemberName { + /// Name that is preceded in the source code by the private marker `#`. + /// For example `class { #f(){} }` + Private(TokenText), + /// Name that is NOT preceded in the source code by the private marker `#`. + /// For example `class { f(){} }` + Public(TokenText), +} +impl ClassMemberName { + pub fn text(&self) -> &str { + match self { + Self::Private(name) => name.text(), + Self::Public(name) => name.text(), + } + } +} +impl From for TokenText { + fn from(value: ClassMemberName) -> Self { + match value { + ClassMemberName::Private(name) => name, + ClassMemberName::Public(name) => name, + } + } +} + impl AnyJsClassMemberName { /// Returns the member name of the current node /// if it is a literal, a computed, or a private class member with a literal value. @@ -1612,7 +1638,7 @@ impl AnyJsClassMemberName { /// let computed = AnyJsClassMemberName::JsComputedMemberName(computed); /// assert_eq!(computed.name().unwrap().text(), "a"); /// ``` - pub fn name(&self) -> Option { + pub fn name(&self) -> Option { let token = match self { AnyJsClassMemberName::JsComputedMemberName(expr) => { let expr = expr.expression().ok()?; @@ -1630,9 +1656,13 @@ impl AnyJsClassMemberName { } } AnyJsClassMemberName::JsLiteralMemberName(expr) => expr.value().ok()?, - AnyJsClassMemberName::JsPrivateClassMemberName(expr) => expr.id_token().ok()?, + AnyJsClassMemberName::JsPrivateClassMemberName(expr) => { + return Some(ClassMemberName::Private(inner_string_text( + &expr.id_token().ok()?, + ))); + } }; - Some(inner_string_text(&token)) + Some(ClassMemberName::Public(inner_string_text(&token))) } }