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

Commit

Permalink
feat(rome_js_analyze): relax useLiteralEnumMembers
Browse files Browse the repository at this point in the history
  • Loading branch information
@Conaclos
Conaclos committed Jul 23, 2023
1 parent 9360776 commit 1a8dccb
Show file tree
Hide file tree
Showing 8 changed files with 519 additions and 405 deletions.
25 changes: 25 additions & 0 deletions CHANGELOG.md
View file
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
View file
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()
}
59 changes: 17 additions & 42 deletions crates/rome_js_analyze/tests/specs/nursery/useLiteralEnumMembers/invalid.ts
View file
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

0 comments on commit 1a8dccb

Please sign in to comment.