refactor: Add lint rules for no unused vars and unused import #9940

coratgerl · 2025-11-22T14:42:02Z

Pull Request

Add rules for no-unused-vars and no-unused-import

Summary by CodeRabbit

Chores
- Added a linting plugin to detect unused imports/vars and updated lint rules accordingly.
- Updated linting directives across the codebase to use the new rules.
Refactor
- Simplified error handling by removing unused error variable bindings in catch blocks throughout the codebase.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

parse-github-assistant · 2025-11-22T14:42:06Z

🚀 Thanks for opening this pull request!

coderabbitai · 2025-11-22T14:42:13Z

📝 Walkthrough

Walkthrough

Adds the eslint-plugin-unused-imports and updates lint directives; removes unused error-variable bindings from many catch clauses; adjusts an import and ESLint header in the benchmark file. No functional or API changes are introduced.

Changes

Cohort / File(s)	Summary
ESLint Plugin Setup `eslint.config.js`, `package.json`	Adds `eslint-plugin-unused-imports` (devDependency) and registers it in ESLint config; enables `unused-imports/no-unused-imports` and `unused-imports/no-unused-vars` rules.
Adapter Files — ESLint Directive Updates `src/Adapters/Analytics/AnalyticsAdapter.js`, `src/Adapters/Auth/AuthAdapter.js`, `src/Adapters/Cache/CacheAdapter.js`, `src/Adapters/Email/MailAdapter.js`, `src/Adapters/Files/FilesAdapter.js`, `src/Adapters/Logger/LoggerAdapter.js`, `src/Adapters/PubSub/PubSubAdapter.js`, `src/Adapters/Push/PushAdapter.js`, `src/Adapters/WebSocketServer/WSAdapter.js`, `src/Adapters/WebSocketServer/WSSAdapter.js`	Replaces top-of-file ESLint directives that disabled `no-unused-vars` with directives targeting `unused-imports/no-unused-vars`. No runtime or API changes.
Catch Block Error Binding Removals `ci/nodeEngineCheck.js`, `src/Adapters/Auth/apple.js`, `src/Adapters/Auth/facebook.js`, `src/Adapters/Files/GridFSBucketAdapter.js`, `src/Adapters/Logger/WinstonLogger.js`, `src/Controllers/HooksController.js`, `src/Controllers/index.js`, `src/GraphQL/loaders/filesMutations.js`, `src/Options/parsers.js`, `src/Routers/ClassesRouter.js`, `src/Routers/FilesRouter.js`, `src/Routers/PagesRouter.js`, `src/Utils.js`, `src/batch.js`, `src/cli/utils/runner.js`, `src/middlewares.js`, `src/password.js`, `src/request.js`	Replaces catch clauses that previously captured an unused error variable (e.g., `catch (e)` / `catch (err)`) with parameterless `catch` blocks. Behavior and control flow unchanged; error object no longer bound.
Import & Header Cleanup `benchmark/performance.js`	Removes ESLint directive lines and changes import to use `node:perf_hooks`, dropping the unused `PerformanceObserver` import.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Spot-check that none of the modified catch bodies reference the removed error variable.
Review files where catches swallow errors (e.g., WinstonLogger.js, GridFSBucketAdapter.js, HooksController.js, PagesRouter.js) to ensure ignoring the error is acceptable.
Confirm ESLint config and directives are syntactically consistent and the plugin version in package.json matches expectations.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is incomplete relative to the template requirements. It lacks the required Issue link, detailed Approach section, and Task checklist completion.	Fill in the required template sections: add the issue link under 'Closes:', expand the Approach section with details about the changes made, and complete the Tasks checklist.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check	✅ Passed	The title 'refactor: Add lint rules for no unused vars and unused import' directly and accurately describes the main objective of the changeset—adding ESLint rules for detecting unused variables and imports.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

parseplatformorg · 2025-11-22T14:42:15Z

✅ Snyk checks have passed. No issues have been found so far.

Status	Scanner	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (2)

src/Adapters/Files/GridFSBucketAdapter.js (1)

166-176: Catch binding removal in key rotation loop is safe

Switching to a bare catch here is behavior‑neutral since the error variable was never read and the loop already just continues on failure.

If you ever revisit this function, consider logging or accumulating the specific errors for fileNamesNotRotated to aid debugging key‑rotation issues, but that’s orthogonal to this lint‑driven change.

src/Routers/FilesRouter.js (1)

305-316: Metadata handler catch cleanup (optional: consider logging)

Removing the unused catch parameter keeps the current contract of always returning 200 {} on failure to fetch metadata, so behavior is unchanged. If you ever revisit this path, you might consider logging the underlying error before responding to help diagnose backend issues while still returning an empty payload to clients.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9ed9af4 and 5d1ba2d.

⛔ Files ignored due to path filters (1)

package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (31)

benchmark/performance.js (1 hunks)
ci/nodeEngineCheck.js (1 hunks)
eslint.config.js (2 hunks)
package.json (1 hunks)
src/Adapters/Analytics/AnalyticsAdapter.js (1 hunks)
src/Adapters/Auth/AuthAdapter.js (1 hunks)
src/Adapters/Auth/apple.js (1 hunks)
src/Adapters/Auth/facebook.js (1 hunks)
src/Adapters/Cache/CacheAdapter.js (1 hunks)
src/Adapters/Email/MailAdapter.js (1 hunks)
src/Adapters/Files/FilesAdapter.js (1 hunks)
src/Adapters/Files/GridFSBucketAdapter.js (1 hunks)
src/Adapters/Logger/LoggerAdapter.js (1 hunks)
src/Adapters/Logger/WinstonLogger.js (2 hunks)
src/Adapters/PubSub/PubSubAdapter.js (1 hunks)
src/Adapters/Push/PushAdapter.js (1 hunks)
src/Adapters/WebSocketServer/WSAdapter.js (2 hunks)
src/Adapters/WebSocketServer/WSSAdapter.js (1 hunks)
src/Controllers/HooksController.js (1 hunks)
src/Controllers/index.js (1 hunks)
src/GraphQL/loaders/filesMutations.js (1 hunks)
src/Options/parsers.js (1 hunks)
src/Routers/ClassesRouter.js (1 hunks)
src/Routers/FilesRouter.js (1 hunks)
src/Routers/PagesRouter.js (3 hunks)
src/Utils.js (1 hunks)
src/batch.js (1 hunks)
src/cli/utils/runner.js (1 hunks)
src/middlewares.js (3 hunks)
src/password.js (1 hunks)
src/request.js (1 hunks)

🧰 Additional context used

🧠 Learnings (3)

📓 Common learnings

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.

📚 Learning: 2025-05-09T09:59:06.289Z

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: Tests in the parse-server repository should use promise-based approaches rather than callback patterns with `done()`. Use a pattern where a Promise is created that resolves when the event occurs, then await that promise.

Applied to files:

src/GraphQL/loaders/filesMutations.js

📚 Learning: 2025-05-09T09:59:06.289Z

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`. The preferred pattern is to create a Promise that resolves when an expected event occurs, then await that Promise.

Applied to files:

src/GraphQL/loaders/filesMutations.js

🧬 Code graph analysis (1)

benchmark/performance.js (1)

benchmark/MongoLatencyWrapper.js (1)

require (20-20)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)

GitHub Check: PostgreSQL 18, PostGIS 3.6
GitHub Check: PostgreSQL 15, PostGIS 3.5
GitHub Check: PostgreSQL 17, PostGIS 3.5
GitHub Check: Node 20
GitHub Check: MongoDB 6, ReplicaSet
GitHub Check: Node 18
GitHub Check: Node 22
GitHub Check: PostgreSQL 15, PostGIS 3.3
GitHub Check: PostgreSQL 15, PostGIS 3.4
GitHub Check: MongoDB 7, ReplicaSet
GitHub Check: PostgreSQL 16, PostGIS 3.5
GitHub Check: Redis Cache
GitHub Check: MongoDB 8, ReplicaSet
GitHub Check: Docker Build
GitHub Check: Code Analysis (javascript)
GitHub Check: Benchmarks

🔇 Additional comments (30)

ci/nodeEngineCheck.js (1)

89-93: LGTM! Unused error binding correctly removed.

The error variable was not being used in the catch block, so removing the binding is appropriate. The behavior remains identical—the catch block still logs the filename and issues a warning for invalid JSON.

src/GraphQL/loaders/filesMutations.js (1)

41-41: LGTM!

Removing the unused error binding is appropriate since the error value is not referenced in the catch block.

src/Adapters/Analytics/AnalyticsAdapter.js (1)

1-1: LGTM!

The ESLint directive update aligns with the PR objectives to adopt the unused-imports plugin.

src/Adapters/WebSocketServer/WSAdapter.js (1)

1-1: LGTM!

The ESLint directive update is consistent with the PR objectives.

src/Adapters/WebSocketServer/WSSAdapter.js (1)

1-1: LGTM!

The ESLint directive update is consistent with the PR objectives to migrate to the unused-imports plugin.

src/Adapters/Logger/LoggerAdapter.js (1)

1-1: LGTM!

The ESLint directive update aligns with the PR objectives.

src/Adapters/Logger/WinstonLogger.js (2)

45-47: LGTM!

Removing the unused error binding is appropriate for this silent error handler.

89-91: LGTM!

Removing the unused error binding is appropriate for this silent error handler.

src/Adapters/Auth/AuthAdapter.js (1)

1-1: LGTM!

The ESLint directive update is consistent with the PR objectives to adopt the unused-imports plugin.

src/Controllers/index.js (1)

220-222: LGTM!

Removing the unused error binding is appropriate since the error is not referenced in this silent error handler.

src/Adapters/PubSub/PubSubAdapter.js (1)

1-1: Header lint directive switch is consistent and non-functional

Disabling unused-imports/no-unused-vars at file scope for this adapter matches the new lint setup and doesn’t affect runtime behavior. Looks good.

src/batch.js (1)

14-18: Parameterless catch in parseURL preserves previous behavior

Removing the unused error binding (catch (error) → catch {}) keeps the same control flow (invalid URLs still return undefined) while satisfying the new unused-vars lint rule.

src/Adapters/Files/FilesAdapter.js (1)

1-1: Adapter-wide unused-vars rule disable is appropriate here

Disabling unused-imports/no-unused-vars for this abstract adapter/interface file is consistent with the rest of the adapters and has no runtime effect.

src/Utils.js (1)

81-87: fileExists catch change is purely syntactic

Switching from catch (e) to catch {} keeps the same behavior (any access error yields false) while removing an unused binding. Safe and aligned with the PR’s linting goal.

src/password.js (1)

5-13: Native bcrypt feature-detection catch remains behaviorally identical

The move to a parameterless catch in the optional @node-rs/bcrypt require keeps the same fallback semantics (silently using bcryptjs on failure) while addressing the unused error variable. No functional change.

src/request.js (1)

30-36: HTTPResponse.getData still safely ignores JSON parse failures

Replacing catch (e) with catch {} keeps the existing behavior of silently ignoring JSON.parse errors and leaving data undefined, just without an unused error binding.

src/cli/utils/runner.js (1)

21-27: Startup options logging keeps same fallback behavior

The parameterless catch still falls back to using the constructor name when JSON.stringify throws, so logging semantics are unchanged while satisfying the unused-vars rule.

package.json (1)

90-96: eslint-plugin-unused-imports devDependency aligns with lint rule changes

Adding "eslint-plugin-unused-imports": "4.3.0" under devDependencies matches the new unused-imports lint rules and is tooling-only, so it won’t affect runtime. Just ensure eslint.config is updated to register this plugin and its rules, as referenced in the PR description.

src/Adapters/Email/MailAdapter.js (1)

1-1: ESLint directive correctly aligned with unused-imports plugin

Switching the top-of-file disable to unused-imports/no-unused-vars is appropriate here given the abstract interface-style methods that intentionally don't use their parameters.

Please confirm eslint-plugin-unused-imports is registered in eslint.config.js and that unused-imports/no-unused-vars is the active rule name in this codebase.

src/Routers/PagesRouter.js (1)

435-437: Optional catch binding change is behavior‑preserving

Updating these blocks from catch (e) to bare catch keeps behavior identical (the error object was never used) while satisfying the unused‑vars lint rule. Given Parse Server’s Node target, ES2019’s optional catch binding should be supported.

Please confirm that the minimum Node/ECMAScript target for this project supports parameterless catch so that tooling (e.g., TypeScript, Babel, ESLint parser) all parse this syntax correctly.

Also applies to: 477-479, 520-522

src/Options/parsers.js (1)

58-60: moduleOrObjectParser catch cleanup keeps semantics unchanged

Using a bare catch here is fine—the parse error was already intentionally ignored so that opt is returned as‑is, and the unused error binding is removed.

As with other parameterless catch usages in this PR, please ensure the project’s JS/Node tooling stack fully supports optional catch bindings.

src/Adapters/Cache/CacheAdapter.js (1)

1-1: Lint directive matches interface-style usage

Aligning this file’s disable comment with unused-imports/no-unused-vars is appropriate given the abstract methods with intentionally unused parameters.

Please double‑check that this rule name matches the configuration in eslint.config.js so the directive has the intended effect.

src/Adapters/Push/PushAdapter.js (1)

1-1: Local disable of unused vars for abstract adapter is appropriate

This file intentionally has unused parameters (interface-like methods). Switching the directive to target unused-imports/no-unused-vars matches the new lint setup and avoids noisy violations without changing behavior.

src/Adapters/Auth/facebook.js (1)

122-131: Catch binding removal keeps behavior unchanged

Swapping catch (error) for a bare catch is safe here since the caught value was never used. The function still maps any failure to the same OBJECT_NOT_FOUND error and message.

src/Adapters/Auth/apple.js (1)

63-72: Bare catch here is a no-op behaviorally

The catch parameter was unused, so moving to catch { ... } simply satisfies the new lint rule without changing the function’s semantics or error surface.

src/middlewares.js (1)

77-84: Catch parameter removal in header/context parsing is safe

In all three places (X-Parse-Cloud-Context header, body JSON parsing, and _context parsing), the caught error value was never inspected; the code only falls back to malformedContext or invalidRequest. Converting to bare catch {} keeps the same control flow while satisfying the unused-variable lint rules.

Also applies to: 127-131, 171-178

src/Routers/ClassesRouter.js (1)

147-155: JSONFromQuery catch binding removal is behavior‑preserving

For query parameters that aren’t valid JSON, this helper still falls back to the original string value. Since the error object was never used, switching to catch { ... } is purely a lint cleanup.

eslint.config.js (1)

4-5: eslint-plugin-unused-imports is correctly configured for ESLint 9

The plugin version 4.x targets ESLint 9 and the official README confirms the flat config usage pattern plugins: { "unused-imports": unusedImports } exactly as implemented. Requiring the plugin, registering it in the plugins object, enabling its no-unused-imports and no-unused-vars rules, and disabling the core no-unused-vars rule is a coherent setup that avoids duplicate diagnostics and aligns with the PR goal of strict unused detection.

benchmark/performance.js (1)

11-15: Import cleanup and switch to node:perf_hooks is correct

The "node:" protocol for require() is supported by Node.js since v14.18.0, and Parse Server v8+ requires Node >= 18.20.4 / 20.18.0 / 22.12.0, so the change is fully compatible. Removing unused PerformanceObserver is the right cleanup.

src/Controllers/HooksController.js (1)

225-234: Unused error binding removed without changing behavior

Switching from catch (e) to a bare catch here is correct: the caught error wasn’t used, and the logic still produces the same generic err object on JSON parse failure. No behavior change, just cleaner code consistent with the new lint rules.

src/Adapters/WebSocketServer/WSAdapter.js

codecov · 2025-11-22T14:47:43Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.08%. Comparing base (94cee5b) to head (0b36fd1).
⚠️ Report is 2 commits behind head on alpha.

Additional details and impacted files

@@           Coverage Diff           @@
##            alpha    #9940   +/-   ##
=======================================
  Coverage   93.08%   93.08%           
=======================================
  Files         187      187           
  Lines       15275    15275           
  Branches      177      177           
=======================================
  Hits        14219    14219           
  Misses       1044     1044           
  Partials       12       12

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

mtrezza

Nice one! Ready for merge?

coratgerl · 2025-11-22T21:08:15Z

Yes !

feat: Add rules for no unused vars and unused import

5d1ba2d

coderabbitai bot requested changes Nov 22, 2025

View reviewed changes

src/Adapters/WebSocketServer/WSAdapter.js Outdated Show resolved Hide resolved

fix: typo

0b36fd1

coderabbitai bot approved these changes Nov 22, 2025

View reviewed changes

mtrezza reviewed Nov 22, 2025

View reviewed changes

mtrezza changed the title ~~feat: Add rules for no unused vars and unused import~~ refactor: Add lint rules for no unused vars and unused import Nov 22, 2025

mtrezza merged commit 69a9258 into parse-community:alpha Nov 22, 2025
26 of 28 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

refactor: Add lint rules for no unused vars and unused import #9940

refactor: Add lint rules for no unused vars and unused import #9940

coratgerl commented Nov 22, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

parse-github-assistant bot commented Nov 22, 2025 •

edited

Loading

Uh oh!

coderabbitai bot commented Nov 22, 2025 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Uh oh!

parseplatformorg commented Nov 22, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

codecov bot commented Nov 22, 2025 •

edited

Loading

Uh oh!

mtrezza left a comment

Uh oh!

coratgerl commented Nov 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

refactor: Add lint rules for no unused vars and unused import #9940

refactor: Add lint rules for no unused vars and unused import #9940

Conversation

coratgerl commented Nov 22, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request

Summary by CodeRabbit

Uh oh!

parse-github-assistant bot commented Nov 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot commented Nov 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Pre-merge checks and finishing touches

Uh oh!

parseplatformorg commented Nov 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Snyk checks have passed. No issues have been found so far.

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Nov 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mtrezza left a comment

Choose a reason for hiding this comment

Uh oh!

coratgerl commented Nov 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coratgerl commented Nov 22, 2025 •

edited by coderabbitai bot

Loading

parse-github-assistant bot commented Nov 22, 2025 •

edited

Loading

coderabbitai bot commented Nov 22, 2025 •

edited

Loading

parseplatformorg commented Nov 22, 2025 •

edited

Loading

codecov bot commented Nov 22, 2025 •

edited

Loading