Skip to content

Conversation

@antoinezanardi
Copy link
Contributor

This is my first contribution to Oxc, aimed at improving code maintainability and standardizing documentation across rules.

This PR serves as a draft proposal for how I can contribute to rules, focusing on both code and documentation. Comments and suggestions are very welcome to refine the style in a way that satisfies everyone.

Note

This PR is focused solely on refactoring and documentation; no test behavior is modified.

🔩 Refactoring

Goals when refactoring rules:

  • Make the code more idiomatic Rust (use ?, match, if let, Option/Result, Iterator, etc. naturally).
  • Improve readability and maintainability through better structuring, naming, and relevant refactoring.
  • Follow Rust best practices (borrowing management, clarity of signatures, use of &/Cow/From/Into where appropriate).
  • Avoid adding complexity: prioritize clarity and conciseness.
  • Prefer early returns to reduce nesting and improve flow.
  • Prefer the functional programming style.

📚 Documentation

Main changes in the documentation:

  • Add relevant content from the original ESLint rule for context.
  • Move up the Options section and clarify all possibilities.
  • Clarify the examples with detailed sections for each option, including correct and incorrect usage.

Next steps (if accepted):

  • Apply this documentation format and refactoring approach to other rules in future PRs for consistency across the project.

📖 Rule documentation skeleton

# Rule Name

## What it does

_A concise description of what the rule enforces or prevents.  
Briefly mention the relevant language construct(s) and what aspects are controlled._

---

## Why is this bad?

_Explain why not following this rule can lead to issues such as readability, maintainability, or bugs._

---

## Options

**First option:**  
- Type: `string`  
- Enum: `"<option1>"`, `"<option2>"`, ...  
- Default: `"<defaultOption>"`  

_Possible values (add descriptions for each value):_
- `<option1>`: _Explain what this enforces or prevents._
- `<option2>`: _Explain what this enforces or prevents._
- ...

**Second option (if applicable):**  
- Type: `object`  
- Properties:  
    - `<propertyName>`: `<type>` (default: `<defaultValue>`) - _Explanation of what this property controls._  

_Note: Add any relevant caveats, such as when an option is only used with a particular mode/value._

**Example configuration:**

```json
{
    "<rule-name>": ["error", "<option>", { "<propertyName>": <value> }]
}
``
---

## Examples

_Organize examples under sections for each main option._

### `"<option>"`

#### Incorrect

```js
// Code that violates the rule with this option
``

#### Correct

```js
// Compliant code with this rule/option
``

_Repeat the above for each relevant option, such as:_

### `"<option>" with { "<propertyName>": true }`

#### Incorrect

```js
// Code sample
``

#### Correct

```js
// Code sample
``

Copilot AI review requested due to automatic review settings August 29, 2025 09:42
@graphite-app
Copy link
Contributor

graphite-app bot commented Aug 29, 2025

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

@github-actions github-actions bot added A-linter Area - Linter C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior labels Aug 29, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the arrow-body-style ESLint rule implementation to improve code organization and enhance documentation. The changes focus on making the code more idiomatic Rust while standardizing the documentation format.

Key Changes:

  • Restructured the rule implementation by extracting logic into separate methods for better maintainability
  • Enhanced documentation with detailed examples for each configuration option
  • Improved code organization using pattern matching and early returns

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Contributor

@camc314 camc314 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

I pushed another commit with a minor change.
In general it's preferable to have the file look like:

  1. diagnostic functions
  2. the rule struct + config
  3. the rule's proc macro
  4. the impl Rule for
  5. any helper methods

keeping this same order helps consistency across all rules

@codspeed-hq
Copy link

codspeed-hq bot commented Aug 29, 2025

CodSpeed Instrumentation Performance Report

Merging #13389 will not alter performance

Comparing antoinezanardi:refactor/arrow-body-style-cleanup (40eb553) with main (fd3233c)

Summary

✅ 38 untouched benchmarks

@antoinezanardi
Copy link
Contributor Author

thanks!

I pushed another commit with a minor change. In general it's preferable to have the file look like:

  1. diagnostic functions
  2. the rule struct + config
  3. the rule's proc macro
  4. the impl Rule for
  5. any helper methods

keeping this same order helps consistency across all rules

Alright, I'll keep this in mind in my future rules refactors, thanks for the feedback

@camc314 camc314 merged commit c9cb2a4 into oxc-project:main Aug 29, 2025
25 checks passed
camc314 added a commit that referenced this pull request Sep 2, 2025
…nce readability (#13516)

## Refactor and Standardize: `eslint/default-param-last`

This PR refactors the implementation of the `default-param-last` rule to
make the codebase more **idiomatic Rust**, improving readability,
maintainability, and alignment with Rust best practices.

In addition, the rule documentation has been **standardized and
enhanced** following the documentation skeleton described in #13389.

### Changes included

* Refactored rule logic with clearer naming, early returns, and
idiomatic Rust patterns (`if let`, `Option`, `Iterator`, etc.).
* Improved structure for better readability and maintainability.
* Focused on optimization and performances.
* Standardized rule documentation for consistency across the linter
rules.

> [!NOTE]
> This PR is focused solely on refactoring and documentation; no test
behavior is modified.

---------

Co-authored-by: Cameron <cameron.clark@hey.com>
camc314 added a commit that referenced this pull request Sep 2, 2025
…ce readability (#13515)

## Refactor and Standardize: `eslint/default-case-last`

This PR refactors the implementation of the `default-case-last` rule to
make the codebase more **idiomatic Rust**, improving readability,
maintainability, and alignment with Rust best practices.

In addition, the rule documentation has been **standardized and
enhanced** following the documentation skeleton described in #13389.

### Changes included

* Refactored rule logic with clearer naming, early returns, and
idiomatic Rust patterns (`if let`, `Option`, `Iterator`, etc.).
* Improved structure for better readability and maintainability.
* Focused on optimization and performances.
* Standardized rule documentation for consistency across the linter
rules.

> [!NOTE]
> This PR is focused solely on refactoring and documentation; no test
behavior is modified.

---------

Co-authored-by: Cameron <cameron.clark@hey.com>
camc314 pushed a commit that referenced this pull request Sep 2, 2025
…e documentation (#13417)

## Refactor and Standardize: `eslint/block-scoped-var`

This PR refactors the implementation of the `block-scoped-var` rule to
make the codebase more **idiomatic Rust**, improving readability,
maintainability, and alignment with Rust best practices.

In addition, the rule documentation has been **standardized and
enhanced** following the documentation skeleton described in #13389.

### Changes included

* Refactored rule logic with clearer naming, early returns, and
idiomatic Rust patterns (`if let`, `Option`, `Iterator`, etc.).
* Improved structure for better readability and maintainability.
* Standardized rule documentation for consistency across the linter
rules.

> [!NOTE]
> This PR is focused solely on refactoring and documentation; no test
behavior is modified.
camc314 pushed a commit that referenced this pull request Sep 2, 2025
…adability (#13430)

## Refactor and Standardize: `eslint/default-case`

This PR refactors the implementation of the `default-case` rule to make
the codebase more **idiomatic Rust**, improving readability,
maintainability, and alignment with Rust best practices.

In addition, the rule documentation has been **standardized and
enhanced** following the documentation skeleton described in #13389.

### Changes included

* Refactored rule logic with clearer naming, early returns, and
idiomatic Rust patterns (`if let`, `Option`, `Iterator`, etc.).
* Improved structure for better readability and maintainability.
* Focused on optimization and performances.
* Standardized rule documentation for consistency across the linter
rules.

> [!NOTE]
> This PR is focused solely on refactoring and documentation; no test
behavior is modified.
camc314 pushed a commit that referenced this pull request Sep 8, 2025
…ocumentation (#13532)

## Refactor and Standardize: `eslint/for-direction`

This PR refactors the implementation of the `for-direction` rule to make
the codebase more **idiomatic Rust**, improving readability,
maintainability, and alignment with Rust best practices.

In addition, the rule documentation has been **standardized and
enhanced** following the documentation skeleton described in #13389.

### Changes included

* Refactored rule logic with clearer naming, early returns, and
idiomatic Rust patterns (`if let`, `Option`, `Iterator`, etc.).
* Improved structure for better readability and maintainability.
* Focused on optimization and performances.
* Standardized rule documentation for consistency across the linter
rules.

> [!NOTE]
> This PR is focused solely on refactoring and documentation; no test
behavior is modified.
Copilot AI pushed a commit that referenced this pull request Sep 8, 2025
…ocumentation (#13532)

## Refactor and Standardize: `eslint/for-direction`

This PR refactors the implementation of the `for-direction` rule to make
the codebase more **idiomatic Rust**, improving readability,
maintainability, and alignment with Rust best practices.

In addition, the rule documentation has been **standardized and
enhanced** following the documentation skeleton described in #13389.

### Changes included

* Refactored rule logic with clearer naming, early returns, and
idiomatic Rust patterns (`if let`, `Option`, `Iterator`, etc.).
* Improved structure for better readability and maintainability.
* Focused on optimization and performances.
* Standardized rule documentation for consistency across the linter
rules.

> [!NOTE]
> This PR is focused solely on refactoring and documentation; no test
behavior is modified.
camc314 added a commit that referenced this pull request Sep 16, 2025
…mentation (#13601)

## Refactor and Standardize: `eslint/func-names`

This PR refactors the implementation of the `func-names` rule to make
the codebase more **idiomatic Rust**, improving readability,
maintainability, and alignment with Rust best practices.

In addition, the rule documentation has been **standardized and
enhanced** following the documentation skeleton described in #13389.

### Changes included

* Refactored rule logic with clearer naming, early returns, and
idiomatic Rust patterns (`if let`, `Option`, `Iterator`, etc.).
* Improved structure for better readability and maintainability.
* Focused on optimization and performances.
* Standardized rule documentation for consistency across the linter
rules.

> [!NOTE]
> This PR is focused solely on refactoring and documentation; no test
behavior is modified.

---------

Co-authored-by: Cameron Clark <cameron.clark@hey.com>
camc314 pushed a commit that referenced this pull request Sep 24, 2025
…ation (#13527)

## Refactor and Standardize: `eslint/eqeqeq`

This PR refactors the implementation of the `eqeqeq` rule to make the
codebase more **idiomatic Rust**, improving readability,
maintainability, and alignment with Rust best practices.

In addition, the rule documentation has been **standardized and
enhanced** following the documentation skeleton described in #13389.

### Changes included

* Refactored rule logic with clearer naming, early returns, and
idiomatic Rust patterns (`if let`, `Option`, `Iterator`, etc.).
* Improved structure for better readability and maintainability.
* Standardized rule documentation for consistency across the linter
rules.

> [!NOTE]
> This PR is focused solely on refactoring and documentation; no test
behavior is modified.
camc314 added a commit that referenced this pull request Sep 24, 2025
…13498)

## Refactor and Standardize: `eslint/curly`

This PR refactors the implementation of the `curly` rule to make the
codebase more **idiomatic Rust**, improving readability,
maintainability, and alignment with Rust best practices.

In addition, the rule documentation has been **standardized and
enhanced** following the documentation skeleton described in #13389.

### Changes included

* Refactored rule logic with clearer naming, early returns, and
idiomatic Rust patterns (`if let`, `Option`, `Iterator`, etc.).
* Improved structure for better readability and maintainability.
* Focused on optimization and performances.
* Standardized rule documentation for consistency across the linter
rules.

---

> [!NOTE]
> This PR is focused solely on refactoring and documentation; no test
behavior is modified.

---------

Co-authored-by: Cameron Clark <cameron.clark@hey.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-linter Area - Linter C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants