Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

feat(rome_js_analyze): relax useLiteralEnumMembers #4720

Merged
merged 1 commit into from
Jul 23, 2023
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
25 changes: 25 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,31 @@ if no error diagnostics are emitted.
var x = a => 1 ? 2 : 3;
```

- Relax [`useLiteralEnumMembers`](https://docs.rome.tools/lint/rules/useLiteralEnumMembers/)

Enum members that refers to previous enum members are now allowed.
This allows common pattern in enum flags like in the following example:

```ts
enum FileAccess {
None = 0,
Read = 1,
Write = 1 << 1,
All = Read | Write,
}
```

Arbitrary numeric constant expressions are also allowed:

```ts
enum FileAccess {
None = 0,
Read = 2**0,
Write = 2**1,
All = Read | Write,
}
```

- Improve [useLiteralKeys](https://docs.rome.tools/lint/rules/useLiteralKeys/).

Now, the rule suggests simplifying computed properties to string literal properties:
Expand Down
187 changes: 133 additions & 54 deletions crates/rome_js_analyze/src/analyzers/nursery/use_literal_enum_members.rs
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
use rome_analyze::{context::RuleContext, declare_rule, Ast, Rule, RuleDiagnostic};
use rome_console::markup;
use rome_js_syntax::{
AnyJsExpression, JsBinaryExpression, JsSyntaxKind, JsUnaryExpression, JsUnaryOperator,
TsEnumMember,
AnyJsExpression, AnyJsLiteralExpression, AnyJsMemberExpression, JsUnaryOperator,
TsEnumDeclaration,
};
use rome_rowan::AstNode;
use rome_rowan::{AstNode, TextRange};
use rustc_hash::FxHashSet;

declare_rule! {
/// Require all enum members to be literal values.
///
/// Usually, an enum member is initialized with a literal number or a literal string.
/// However, _TypeScript_ allows the value of an enum member to be many different kinds of expressions.
/// Using a computed enum member is often error-prone and confusing.
/// This rule requires the initialization of enum members with literal values.
/// It allows bitwise expressions for supporting [enum flags](https://stackoverflow.com/questions/39359740/what-are-enum-flags-in-typescript/39359953#39359953).
///
/// In contrast to the equivalent _ESLint_ rule, this rule allows arbitrary bitwise constant expressions.
/// This rule requires the initialization of enum members with constant expressions.
/// It allows numeric and bitwise expressions for supporting [enum flags](https://stackoverflow.com/questions/39359740/what-are-enum-flags-in-typescript/39359953#39359953).
/// It also allows referencing previous enum members.
///
/// Source: https://typescript-eslint.io/rules/prefer-literal-enum-member/
///
Expand All @@ -31,14 +31,6 @@ declare_rule! {
/// }
/// ```
///
/// ```ts,expect_diagnostic
/// const x = 2;
/// enum Invalid {
/// A,
/// B = 2**3,
/// }
/// ```
///
/// ## Valid
///
/// ```ts
Expand Down Expand Up @@ -68,7 +60,7 @@ declare_rule! {
/// None = 0,
/// Read = 1,
/// Write = 1 << 1,
/// All = 1 | (1 << 1)
/// All = Read | Write
/// }
/// ```
pub(crate) UseLiteralEnumMembers {
Expand All @@ -79,61 +71,148 @@ declare_rule! {
}

impl Rule for UseLiteralEnumMembers {
type Query = Ast<TsEnumMember>;
type State = ();
type Signals = Option<Self::State>;
type Query = Ast<TsEnumDeclaration>;
type State = TextRange;
type Signals = Vec<Self::State>;
type Options = ();

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let enum_member = ctx.query();
let Some(initializer) = enum_member.initializer() else {
// no initializer => sequentially assigned literal integer
return None;
let enum_declaration = ctx.query();
let mut result = Vec::new();
let mut enum_member_names = FxHashSet::default();
let Ok(enum_name) = enum_declaration.id() else {
return result;
};
let expr = initializer.expression().ok()?.omit_parentheses();
if expr.as_any_js_literal_expression().is_some() || is_bitwise_constant_expression(&expr) {
return None;
} else if let Some(expr) = expr.as_js_unary_expression() {
if expr.is_signed_numeric_literal().ok()? {
return None;
}
} else if let Some(expr) = expr.as_js_template_expression() {
if expr.is_constant() {
return None;
let Some(enum_name) = enum_name.as_js_identifier_binding()
.and_then(|x| x.name_token().ok()) else {
return result;
};
let enum_name = enum_name.text_trimmed();
for enum_member in enum_declaration.members() {
let Ok(enum_member) = enum_member else {
continue;
};
// no initializer => sequentially assigned literal integer
if let Some(initializer) = enum_member.initializer() {
if let Ok(initializer) = initializer.expression() {
let range = initializer.range();
if !is_constant_enum_expression(initializer, enum_name, &enum_member_names) {
result.push(range);
}
}
};
if let Ok(name) = enum_member.name() {
if let Some(name) = name.name() {
enum_member_names.insert(name.text().to_string());
}
}
}
Some(())
result
}

fn diagnostic(ctx: &RuleContext<Self>, _: &Self::State) -> Option<RuleDiagnostic> {
let enum_member = ctx.query();
fn diagnostic(
_: &RuleContext<Self>,
initializer_range: &Self::State,
) -> Option<RuleDiagnostic> {
Some(RuleDiagnostic::new(
rule_category!(),
enum_member.initializer()?.expression().ok()?.range(),
initializer_range,
markup! {
"The enum member should be initialized with a literal value such as a number or a string."
},
))
}
}

/// Returns true if `expr` is an expression that only includes literal numbers and bitwise operations.
fn is_bitwise_constant_expression(expr: &AnyJsExpression) -> bool {
for node in expr.syntax().descendants() {
if let Some(exp) = JsUnaryExpression::cast_ref(&node) {
if exp.operator() != Ok(JsUnaryOperator::BitwiseNot) {
return false;
}
} else if let Some(exp) = JsBinaryExpression::cast_ref(&node) {
if !exp.is_binary_operator() {
return false;
/// Returns true if `expr` is a constant enum expression.
/// A constant enum expression can contain numbers, string literals, and reference to
/// one of the enum member of `enum_member_names` of the enum name `enum_name`.
/// These values can be combined thanks to numeric, bitwise, and concatenation operations.
fn is_constant_enum_expression(
expr: AnyJsExpression,
enum_name: &str,
enum_member_names: &FxHashSet<String>,
) -> bool {
(move || {
// stack that holds expressions to validate.
let mut stack = Vec::new();
stack.push(expr);
while let Some(expr) = stack.pop() {
match expr.omit_parentheses() {
AnyJsExpression::AnyJsLiteralExpression(expr) => {
if !matches!(
expr,
AnyJsLiteralExpression::JsNumberLiteralExpression(_)
| AnyJsLiteralExpression::JsStringLiteralExpression(_)
) {
return Some(false);
}
}
AnyJsExpression::JsTemplateExpression(expr) => {
if !expr.is_constant() {
return Some(false);
}
}
AnyJsExpression::JsUnaryExpression(expr) => {
if !matches!(
expr.operator(),
Ok(JsUnaryOperator::BitwiseNot
| JsUnaryOperator::Minus
| JsUnaryOperator::Plus)
) {
return Some(false);
}
stack.push(expr.argument().ok()?)
}
AnyJsExpression::JsBinaryExpression(expr) => {
if !expr.is_binary_operation() && !expr.is_numeric_operation() {
return Some(false);
}
stack.push(expr.left().ok()?);
stack.push(expr.right().ok()?);
}
AnyJsExpression::JsIdentifierExpression(expr) => {
// Allow reference to previous member name
let name = expr.name().ok()?;
if !enum_member_names.contains(name.value_token().ok()?.text_trimmed()) {
return Some(false);
}
}
AnyJsExpression::JsStaticMemberExpression(expr) => {
if !is_enum_member_reference(expr.into(), enum_name, enum_member_names) {
return Some(false);
}
}
AnyJsExpression::JsComputedMemberExpression(expr) => {
if !is_enum_member_reference(expr.into(), enum_name, enum_member_names) {
return Some(false);
}
}
_ => {
return Some(false);
}
}
} else if !matches!(
node.kind(),
JsSyntaxKind::JS_NUMBER_LITERAL_EXPRESSION | JsSyntaxKind::JS_PARENTHESIZED_EXPRESSION
) {
return false;
}
}
true
Some(true)
})()
.unwrap_or_default()
}

// Return true if `expr` is a reference to one of the enum member `enum_member_names`
// of the enum named `enum_name`.
fn is_enum_member_reference(
expr: AnyJsMemberExpression,
enum_name: &str,
enum_member_names: &FxHashSet<String>,
) -> bool {
(move || {
// Allow reference to previous member name namespaced by the enum name
let object = expr.object().ok()?.omit_parentheses();
let object = object.as_js_identifier_expression()?;
Some(
object.name().ok()?.has_name(enum_name)
&& enum_member_names.contains(expr.member_name()?.text()),
)
})()
.unwrap_or_default()
}
Original file line number Diff line number Diff line change
@@ -1,74 +1,38 @@
enum InvalidObject {
enum InvalidLiterals {
A = {},
B = [],
C = true,
D = 1n,
}


enum InvalidArray {
A = [],
}


enum InvalidTemplateLiteral {
A = `foo ${0}`,
}


enum InvalidConstructor {
A = new Set(),
}


enum InvalidExpression {
A = 2 + 2,
}

enum InvalidExpression {
A = delete 2,
B = -a,
C = void 2,
D = ~2,
E = !0,
D = !0,
}


const variable = 'Test';
enum InvalidVariable {
A = 'TestStr',
B = 2,
C,
V = variable,
}


enum InvalidEnumMember {
A = 'TestStr',
B = A,
}


const Valid = { A: 2 };
enum InvalidObjectMember {
A = 'TestStr',
B = Valid.A,
}


enum Valid {
A,
}
enum InvalidEnumMember {
A = 'TestStr',
B = Valid.A,
A = Valid.A,
}


const obj = { a: 1 };
enum InvalidSpread {
A = 'TestStr',
B = { ...a },
}


const x = 1;
enum Foo {
A = x << 0,
Expand All @@ -80,3 +44,14 @@ enum Foo {
G = ~x,
}

enum InvalidRef {
A = A,
B = InvalidRef.B,
C = InvalidRef["C"],
D = E,
E = InvalidRef.F,
F = InvalidRef["G"],
G
}

export {}
Loading