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

fix(useAdjacentOverloadSignatures): don't report #private and public members with same name #3459

Merged
merged 1 commit into from
Jul 17, 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
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
Loading