Skip to content

Commit

Permalink
fix(useAdjacentOverloadSignatures): don't report #private and public …
Browse files Browse the repository at this point in the history
…members with same name (#3459)
  • Loading branch information
Conaclos committed Jul 17, 2024
1 parent 7188856 commit ba8aabc
Show file tree
Hide file tree
Showing 9 changed files with 170 additions and 140 deletions.
14 changes: 14 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -104,40 +104,36 @@ impl Rule for UseAdjacentOverloadSignatures {
type Options = ();

fn run(ctx: &RuleContext<Self>) -> 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)
Expand Down Expand Up @@ -169,57 +165,33 @@ impl Rule for UseAdjacentOverloadSignatures {
}
}

fn collect_interface(
node: &TsInterfaceDeclaration,
methods: &mut Vec<(TokenText, TextRange)>,
seen_methods: &mut FxHashSet<TokenText>,
last_method: &mut Option<TokenText>,
) {
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<TokenText>,
last_method: &mut Option<TokenText>,
) {
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<TokenText>,
last_method: &mut Option<TokenText>,
) {
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
Expand All @@ -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<TokenText>,
last_method: &mut Option<TokenText>,
) {
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()
Expand All @@ -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<TokenText>,
last_method: &mut Option<TokenText>,
) {
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() {
Expand All @@ -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<T: Clone + Eq + std::hash::Hash + Into<TokenText>>(
text: T,
range: TextRange,
methods: &mut Vec<(TokenText, TextRange)>,
seen_methods: &mut FxHashSet<TokenText>,
last_method: &mut Option<TokenText>,
seen_methods: &mut FxHashSet<T>,
last_method: &mut Option<T>,
) {
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());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -189,27 +190,27 @@ fn check_class_methods(js_class_decl: &JsClassDeclaration) -> Option<RuleState>
.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
Expand Down
47 changes: 9 additions & 38 deletions crates/biome_js_analyze/src/lint/suspicious/no_then_property.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -222,43 +222,14 @@ fn process_js_method_object_member(node: &JsMethodObjectMember) -> Option<RuleSt
}

fn process_js_class_member(node: &AnyJsClassMember) -> Option<RuleState> {
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
}
Expand Down
Loading

0 comments on commit ba8aabc

Please sign in to comment.