Skip to content

refactor(rust)!: clippy avoid-breaking-exported-api = false#11088

Merged
graphite-app[bot] merged 1 commit intomainfrom
05-17-refactor_rust_clippy_avoid-breaking-exported-api_false
May 17, 2025
Merged

refactor(rust)!: clippy avoid-breaking-exported-api = false#11088
graphite-app[bot] merged 1 commit intomainfrom
05-17-refactor_rust_clippy_avoid-breaking-exported-api_false

Conversation

@Boshen
Copy link
Member

@Boshen Boshen commented May 17, 2025

No description provided.

Copilot AI review requested due to automatic review settings May 17, 2025 02:07
Copy link
Member Author

Boshen commented May 17, 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.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions github-actions bot added A-linter Area - Linter A-semantic Area - Semantic A-cli Area - CLI A-ast Area - AST A-transformer Area - Transformer / Transpiler A-isolated-declarations Isolated Declarations A-ast-tools Area - AST tools A-editor Area - Editor and Language Server A-formatter Area - Formatter C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior labels May 17, 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 several parts of the codebase, primarily updating function signatures and simplifying return types, while also removing some error‐handling paths to streamline configuration building. Key changes include:

  • Replacing chains using .and_then with .map in multiple rule modules to eliminate Option wrapping.
  • Changing build() methods to return configuration objects directly instead of Results.
  • Updating several methods’ receivers from &self to self in const contexts and making minor clippy suppression adjustments.

Reviewed Changes

Copilot reviewed 47 out of 47 changed files in this pull request and generated no comments.

Show a summary per file
File Description
crates/oxc_linter/src/rules/jest/no_restricted_matchers.rs Updated matcher compilation from optional to direct hash map return.
crates/oxc_linter/src/rules/jest/no_restricted_jest_methods.rs Similar refactor for jest methods matcher compilation.
crates/oxc_linter/src/rules/jest/no_large_snapshots.rs Changed snapshot allowed matchers chain accordingly.
crates/oxc_linter/src/loader/mod.rs Removed load_str function to simplify file loading.
crates/oxc_linter/src/config/config_builder.rs Switched build() to return Config directly, updating error handling.
crates/oxc_language_server/src/linter/server_linter.rs Adjusted build() usage by removing expect() calls.
crates/oxc_isolated_declarations/src/signatures.rs Changed transform_ts_signatures receiver from mutable to immutable.
crates/oxc_isolated_declarations/src/enum.rs Updated transform_ts_enum_declaration to always return a Declaration.
crates/oxc_formatter/src/options.rs & crates/oxc_ast Changed const method signatures to take self instead of &self.
apps/oxlint/src/lint.rs Updated config builder handling by removing Result unwrapping.
.clippy.toml Set avoid-breaking-exported-api to false.
Comments suppressed due to low confidence (11)

crates/oxc_linter/src/loader/mod.rs:22

  • The removal of the load_str function streamlines file loading; ensure that any external dependencies or tests relying on it are updated accordingly.
pub fn load_str<'a, P: AsRef<Path>>(...) -> Result<Vec<JavaScriptSource<'a>>, LoadError> {

crates/oxc_linter/src/rules/jest/no_restricted_matchers.rs:85

  • The change from .and_then to .map with Self::compile_restricted_matchers removes the Option wrapper; verify that downstream code correctly handles the always-present hash map.
.map(Self::compile_restricted_matchers)

crates/oxc_linter/src/rules/jest/no_restricted_jest_methods.rs:69

  • Replacing .and_then with .map in the restricted jest methods chain alters the expected return type; ensure that the direct hash map fits all intended usages.
.map(Self::compile_restricted_jest_methods)

crates/oxc_linter/src/rules/jest/no_large_snapshots.rs:158

  • Refactoring from .and_then to .map in compiling allowed snapshots simplifies the chain; confirm that this change does not bypass any required error handling.
.map(Self::compile_allowed_snapshots)

crates/oxc_linter/src/config/config_builder.rs:295

  • Changing build() to return a Config directly instead of a Result affects error propagation; verify that configuration errors are handled appropriately at higher layers.
pub fn build(self) -> Config {

crates/oxc_language_server/src/linter/server_linter.rs:64

  • Removing the expect() call in favor of directly calling build() aligns with the updated build() signature; ensure that configuration errors are managed downstream.
let base_config = config_builder.build();

crates/oxc_isolated_declarations/src/signatures.rs:14

  • Changing the method receiver from mutable to immutable suggests that transform_ts_signatures does not modify internal state; please confirm that this change is intended and does not affect functionality.
pub fn transform_ts_signatures(&self, signatures: &mut ArenaVec<'a, TSSignature<'a>>) {

crates/oxc_isolated_declarations/src/enum.rs:21

  • Modifying transform_ts_enum_declaration to return a Declaration directly (instead of Option) simplifies its usage; ensure that all callers adjust to the non-optional return value.
pub fn transform_ts_enum_declaration(&self, decl: &TSEnumDeclaration<'a>) -> Declaration<'a> {

crates/oxc_formatter/src/options.rs:118

  • Refactoring const methods to take self instead of &self improves clarity if the type implements Copy; verify that these type changes are safe and cause no semantic differences.
pub const fn is_tab(self) -> bool {

apps/oxlint/src/lint.rs:251

  • Switching from handling a Result via expect() to a direct build() call alters error handling; ensure that any configuration failures are properly addressed in the call chain.
let lint_config = config_builder.build();

.clippy.toml:1

  • Setting avoid-breaking-exported-api to false can affect API stability; confirm that this decision is documented and aligns with the project’s long-term design goals.
avoid-breaking-exported-api = false

@Boshen Boshen changed the title refactor(rust): clippy avoid-breaking-exported-api = false refactor(rust)!: clippy avoid-breaking-exported-api = false May 17, 2025
@graphite-app
Copy link
Contributor

graphite-app bot commented May 17, 2025

Merge activity

@graphite-app graphite-app bot force-pushed the 05-17-refactor_rust_clippy_avoid-breaking-exported-api_false branch from 5a0dac5 to f1d3b0f Compare May 17, 2025 02:14
@codspeed-hq
Copy link

codspeed-hq bot commented May 17, 2025

CodSpeed Instrumentation Performance Report

Merging #11088 will not alter performance

Comparing 05-17-refactor_rust_clippy_avoid-breaking-exported-api_false (5d9344f) with main (8f8d823)

Summary

✅ 36 untouched benchmarks

@Boshen Boshen force-pushed the 05-17-refactor_rust_clippy_avoid-breaking-exported-api_false branch from f1d3b0f to 51c505e Compare May 17, 2025 02:18
@graphite-app graphite-app bot force-pushed the 05-17-refactor_rust_clippy_avoid-breaking-exported-api_false branch from 51c505e to 2928a80 Compare May 17, 2025 02:19
@Boshen Boshen force-pushed the 05-17-refactor_rust_clippy_avoid-breaking-exported-api_false branch from 2928a80 to 3e3d855 Compare May 17, 2025 02:20
@graphite-app graphite-app bot force-pushed the 05-17-refactor_rust_clippy_avoid-breaking-exported-api_false branch from 3e3d855 to 5d9344f Compare May 17, 2025 02:21
@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label May 17, 2025
@graphite-app graphite-app bot merged commit 5d9344f into main May 17, 2025
29 of 30 checks passed
@graphite-app graphite-app bot deleted the 05-17-refactor_rust_clippy_avoid-breaking-exported-api_false branch May 17, 2025 02:28
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label May 17, 2025
overlookmotel added a commit that referenced this pull request May 17, 2025
Miri is failing on main. It seems to have started with #11088, though
that could be a coincidence - maybe it's just latest nightly made some
change which broke Miri.

2 things are causing the fails:

1. "fatal error: cross-interpreting doctests is not currently supported
by Miri".
2. A lint error that only occurs on nightly in `Vec::append_elements`.

Fix (1) by disabling doc tests.

Fix (2) by changing the code. The `#[allow]` attribute is needed because
the amended code triggers a *different* clippy warning on stable!
This was referenced May 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ast Area - AST A-ast-tools Area - AST tools A-cli Area - CLI A-editor Area - Editor and Language Server A-formatter Area - Formatter A-isolated-declarations Isolated Declarations A-linter Area - Linter A-semantic Area - Semantic A-transformer Area - Transformer / Transpiler 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