Skip to content

Commit

Permalink
feat(js_semantic): support enum member declaration and direct ref (#3347
Browse files Browse the repository at this point in the history
)
  • Loading branch information
Conaclos authored Jul 4, 2024
1 parent 7f3c86e commit 7e485c2
Show file tree
Hide file tree
Showing 40 changed files with 666 additions and 135 deletions.
28 changes: 28 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,21 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b

### Linter

#### Enhancements

- [noInvalidUseBeforeDeclaration](https://biomejs.dev/linter/rules/no-invalid-use-before-declaration) now reports direct use of an enum member before its declaration.

In the following code, `A` is reported as use before its declaration.

```ts
enum E {
B = A << 1,
A = 1,
}
```

Contributed by @Conaclos

#### Bug fixes

- Don't request alt text for elements hidden from assistive technologies ([#3316](https://github.com/biomejs/biome/issues/3316)). Contributed by @robintown
Expand All @@ -51,6 +66,19 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b
- Add [nursery/noDynamicNamespaceImportAccess](https://biomejs.dev/linter/no-dynamic-namespace-import-access/). Contributed by @minht11


- [noUndeclaredVariables](https://biomejs.dev/linter/rules/no-undeclared-variables/) n longer report a direct reference to an enum member ([#2974](https://github.com/biomejs/biome/issues/2974)).

In the following code, the `A` reference is no longer reported as an undeclared variable.

```ts
enum E {
A = 1,
B = A << 1,
}
```

Contributed by @Conaclos

### Parser

## v1.8.3 (2024-06-27)
Expand Down
37 changes: 22 additions & 15 deletions crates/biome_js_analyze/src/lint/complexity/use_literal_keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,12 @@ use biome_analyze::{
RuleSource,
};
use biome_console::markup;
use biome_js_factory::make::{
self, ident, js_literal_member_name, js_name, js_static_member_assignment,
js_static_member_expression, token,
};
use biome_js_factory::make;
use biome_js_syntax::{
AnyJsAssignment, AnyJsComputedMember, AnyJsMemberExpression, AnyJsName, AnyJsObjectMemberName,
JsComputedMemberName, T,
AnyTsEnumMemberName, JsComputedMemberName, JsSyntaxKind, T,
};
use biome_rowan::{declare_node_union, AstNode, BatchMutationExt, TextRange};
use biome_rowan::{declare_node_union, AstNode, BatchMutationExt, SyntaxNodeOptionExt, TextRange};
use biome_unicode_table::is_js_ident;

declare_lint_rule! {
Expand Down Expand Up @@ -129,12 +126,14 @@ impl Rule for UseLiteralKeys {
match node {
AnyJsMember::AnyJsComputedMember(node) => {
let object = node.object().ok()?;
let member = js_name(ident(identifier));
let dot_token = node.optional_chain_token().unwrap_or_else(|| token(T![.]));
let member = make::js_name(make::ident(identifier));
let dot_token = node
.optional_chain_token()
.unwrap_or_else(|| make::token(T![.]));

match node {
AnyJsComputedMember::JsComputedMemberExpression(node) => {
let static_expression = js_static_member_expression(
let static_expression = make::js_static_member_expression(
object,
dot_token,
AnyJsName::JsName(member),
Expand All @@ -145,7 +144,7 @@ impl Rule for UseLiteralKeys {
);
}
AnyJsComputedMember::JsComputedMemberAssignment(node) => {
let static_member = js_static_member_assignment(
let static_member = make::js_static_member_assignment(
object,
dot_token,
AnyJsName::JsName(member),
Expand All @@ -163,11 +162,19 @@ impl Rule for UseLiteralKeys {
} else {
make::js_string_literal_single_quotes(identifier)
};
let literal_member_name = js_literal_member_name(name_token);
mutation.replace_node(
AnyJsObjectMemberName::from(member.clone()),
literal_member_name.into(),
);
if member.syntax().parent().kind() == Some(JsSyntaxKind::TS_ENUM_MEMBER) {
let literal_enum_member_name = make::ts_literal_enum_member_name(name_token);
mutation.replace_node(
AnyTsEnumMemberName::from(member.clone()),
literal_enum_member_name.into(),
);
} else {
let literal_member_name = make::js_literal_member_name(name_token);
mutation.replace_node(
AnyJsObjectMemberName::from(member.clone()),
literal_member_name.into(),
);
}
}
}
Some(JsRuleAction::new(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,12 @@ impl Rule for NoInvalidUseBeforeDeclaration {
let model = ctx.model();
let mut result = vec![];
for binding in model.all_bindings() {
let AnyJsIdentifierBinding::JsIdentifierBinding(id) = binding.tree() else {
let id = binding.tree();
if matches!(
id,
AnyJsIdentifierBinding::TsIdentifierBinding(_)
| AnyJsIdentifierBinding::TsTypeParameterName(_)
) {
// Ignore type declarations (interfaces, type-aliases, ...)
continue;
};
Expand Down Expand Up @@ -162,6 +167,7 @@ impl Rule for NoInvalidUseBeforeDeclaration {
binding_range: declaration_range,
} = state;
let declaration_kind_text = match declaration_kind {
DeclarationKind::EnumMember => "enum member",
DeclarationKind::Parameter => "parameter",
DeclarationKind::Variable => "variable",
};
Expand All @@ -188,6 +194,7 @@ pub struct InvalidUseBeforeDeclaration {

#[derive(Debug, Copy, Clone)]
pub enum DeclarationKind {
EnumMember,
Parameter,
Variable,
}
Expand All @@ -197,6 +204,7 @@ impl TryFrom<&AnyJsBindingDeclaration> for DeclarationKind {

fn try_from(value: &AnyJsBindingDeclaration) -> Result<Self, Self::Error> {
match value {
AnyJsBindingDeclaration::TsEnumMember(_) => Ok(DeclarationKind::EnumMember),
// Variable declaration
AnyJsBindingDeclaration::JsArrayBindingPatternElement(_)
| AnyJsBindingDeclaration::JsArrayBindingPatternRestElement(_)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,8 @@ fn suggested_fix_if_unused(binding: &AnyJsIdentifierBinding) -> Option<Suggested
| AnyJsBindingDeclaration::JsClassExpression(_)
| AnyJsBindingDeclaration::JsFunctionExpression(_)
| AnyJsBindingDeclaration::TsIndexSignatureParameter(_)
| AnyJsBindingDeclaration::TsMappedType(_) => None,
| AnyJsBindingDeclaration::TsMappedType(_)
| AnyJsBindingDeclaration::TsEnumMember(_) => None,

// Some parameters are ok to not be used
AnyJsBindingDeclaration::JsArrowFunctionExpression(_) => {
Expand Down Expand Up @@ -288,6 +289,10 @@ impl Rule for NoUnusedVariables {
}

let binding = ctx.query();
if matches!(binding, AnyJsIdentifierBinding::TsLiteralEnumMemberName(_)) {
// Enum members can be unused.
return None;
}

if binding.name_token().ok()?.text_trimmed().starts_with('_') {
return None;
Expand Down Expand Up @@ -423,6 +428,9 @@ impl Rule for NoUnusedVariables {
AnyJsIdentifierBinding::TsTypeParameterName(binding) => {
binding.ident_token().ok()?
}
AnyJsIdentifierBinding::TsLiteralEnumMemberName(_) => {
return None;
}
};
let name_trimmed = name.text_trimmed();
let new_name = format!("_{name_trimmed}");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,8 @@ fn capture_needs_to_be_in_the_dependency_list(
| AnyJsBindingDeclaration::TsModuleDeclaration(_)
| AnyJsBindingDeclaration::TsInferType(_)
| AnyJsBindingDeclaration::TsMappedType(_)
| AnyJsBindingDeclaration::TsTypeParameter(_) => false,
| AnyJsBindingDeclaration::TsTypeParameter(_)
| AnyJsBindingDeclaration::TsEnumMember(_) => false,
// Function declarations are stable if ...
AnyJsBindingDeclaration::JsFunctionDeclaration(declaration) => {
let declaration_range = declaration.syntax().text_range();
Expand Down
14 changes: 8 additions & 6 deletions crates/biome_js_analyze/src/lint/style/use_naming_convention.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ use biome_js_syntax::{
binding_ext::AnyJsBindingDeclaration, AnyJsClassMember, AnyJsObjectMember,
AnyJsVariableDeclaration, AnyTsTypeMember, JsIdentifierBinding, JsLiteralExportName,
JsLiteralMemberName, JsMethodModifierList, JsPrivateClassMemberName, JsPropertyModifierList,
JsSyntaxKind, JsSyntaxToken, JsVariableDeclarator, JsVariableKind, Modifier, TsEnumMember,
TsIdentifierBinding, TsMethodSignatureModifierList, TsPropertySignatureModifierList,
TsTypeParameterName,
JsSyntaxKind, JsSyntaxToken, JsVariableDeclarator, JsVariableKind, Modifier,
TsIdentifierBinding, TsLiteralEnumMemberName, TsMethodSignatureModifierList,
TsPropertySignatureModifierList, TsTypeParameterName,
};
use biome_rowan::{
declare_node_union, AstNode, BatchMutationExt, SyntaxResult, TextRange, TextSize,
Expand Down Expand Up @@ -843,6 +843,7 @@ declare_node_union! {
JsPrivateClassMemberName |
JsLiteralExportName |
TsIdentifierBinding |
TsLiteralEnumMemberName |
TsTypeParameterName
}

Expand All @@ -856,6 +857,7 @@ impl AnyIdentifierBindingLike {
}
AnyIdentifierBindingLike::JsLiteralExportName(export_name) => export_name.value(),
AnyIdentifierBindingLike::TsIdentifierBinding(binding) => binding.name_token(),
AnyIdentifierBindingLike::TsLiteralEnumMemberName(member_name) => member_name.value(),
AnyIdentifierBindingLike::TsTypeParameterName(type_parameter) => {
type_parameter.ident_token()
}
Expand Down Expand Up @@ -1177,8 +1179,6 @@ impl Selector {
Selector::from_type_member(&member)
} else if let Some(member) = member_name.parent::<AnyJsObjectMember>() {
Selector::from_object_member(&member)
} else if member_name.parent::<TsEnumMember>().is_some() {
Some(Kind::EnumMember.into())
} else {
None
}
Expand All @@ -1202,6 +1202,7 @@ impl Selector {
_ => None,
}
}
AnyIdentifierBindingLike::TsLiteralEnumMemberName(_) => Some(Kind::EnumMember.into()),
AnyIdentifierBindingLike::TsTypeParameterName(_) => Some(Kind::TypeParameter.into()),
}
}
Expand Down Expand Up @@ -1298,7 +1299,8 @@ impl Selector {
// Type parameters should be handled at call site
| AnyJsBindingDeclaration::TsInferType(_)
| AnyJsBindingDeclaration::TsMappedType(_)
| AnyJsBindingDeclaration::TsTypeParameter(_) => None,
| AnyJsBindingDeclaration::TsTypeParameter(_)
| AnyJsBindingDeclaration::TsEnumMember(_) => None,
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,8 @@ export type T = {

[""]: number
}

export enum E {
["A"],
["B"],
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ export type T = {
[""]: number
}

export enum E {
["A"],
["B"],
}

```

# Diagnostics
Expand Down Expand Up @@ -218,3 +223,40 @@ invalid.ts:22:3 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━
│ - -
```

```
invalid.ts:26:3 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! The computed expression can be simplified to a string literal.
25 │ export enum E {
> 26 │ ["A"],
│ ^^^
27 │ ["B"],
28 │ }
i Unsafe fix: Use a literal key instead.
26 │ → ["A"],
│ - -
```

```
invalid.ts:27:3 lint/complexity/useLiteralKeys FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! The computed expression can be simplified to a string literal.
25 │ export enum E {
26 │ ["A"],
> 27 │ ["B"],
│ ^^^
28 │ }
29 │
i Unsafe fix: Use a literal key instead.
27 │ → ["B"],
│ - -
```
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
class C {
constructor(readonly a = b, readonly b = 0) {}
}

enum E {
A = B,
B,
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ class C {
constructor(readonly a = b, readonly b = 0) {}
}

enum E {
A = B,
B,
}

```

# Diagnostics
Expand All @@ -33,4 +38,25 @@ invalid.ts:2:30 lint/correctness/noInvalidUseBeforeDeclaration ━━━━━
```

```
invalid.ts:6:9 lint/correctness/noInvalidUseBeforeDeclaration ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This enum member is used before its declaration.
5 │ enum E {
> 6 │ A = B,
│ ^
7 │ B,
8 │ }
i The enum member is declared here:
5 │ enum E {
6 │ A = B,
> 7 │ B,
│ ^
8 │ }
9 │
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
enum E {
A = 1,
B = A << 1,
"C" = 3 << 1,
D = C << 1,
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: validEnumMemberRef.ts
---
# Input
```ts
enum E {
A = 1,
B = A << 1,
"C" = 3 << 1,
D = C << 1,
}

```
Loading

0 comments on commit 7e485c2

Please sign in to comment.