feat(linter): integrate oxlint and resolve all linting warnings#800
feat(linter): integrate oxlint and resolve all linting warnings#800
Conversation
- Add oxlint as dev dependency and integrate into npm run lint - Create oxlint configuration with warning levels for gradual adoption - Add oxlint CI job to GitHub Actions - Fix regex patterns flagged by oxlint: - Remove unnecessary escape characters in file regex patterns - Fix regex patterns in website validation and PHP test files - Update lint script order: biome -> oxlint -> ts -> secretlint oxlint provides 50-100x faster linting with 500+ rules from ESLint ecosystem. Current warnings are configured as non-blocking to allow gradual improvement.
- Remove unused imports across 67 files (RepomixConfigMerged, QueryCapture, etc.) - Fix unused parameters by prefixing with underscore (_context, _index, etc.) - Remove unused catch parameters using modern JavaScript syntax - Fix require-yield warnings in generator functions - Remove unused variables and interface declarations - Add oxlint configuration to ignore integration test fixtures Resolves 144 linting warnings while preserving all functionality. All 743 tests continue to pass. Code quality significantly improved.
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdds Oxlint to the repo: config file, npm script, and a CI job. Cleans up unused imports/params and adopts optional catch bindings across many files and tests. Adjusts a few regex character classes. Updates packager to inject and call writeOutputToDisk via dependencies. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as CLI/Caller
participant Pack as pack()
participant Deps as defaultDeps
participant Git as getGitDiffs/getGitLogs
participant Proc as processFiles
participant Metrics as calculateMetrics
participant Gen as generateHandlebarOutput
participant Writer as writeOutputToDisk
User->>CLI: invoke pack(...)
CLI->>Pack: pack(config, overrideDeps?)
Note over Pack: Merge defaultDeps with overrideDeps
Pack->>Git: getGitDiffs()/getGitLogs()
Git-->>Pack: diffs/logs
Pack->>Proc: processFiles(...)
Proc-->>Pack: processedFiles
Pack->>Metrics: calculateMetrics(processedFiles, ...)
Metrics-->>Pack: metrics
Pack->>Gen: generateHandlebarOutput(processedFiles, metrics, ...)
Gen-->>Pack: outputText
rect rgba(200,255,200,0.25)
Note right of Writer: New in this change
Pack->>Writer: writeOutputToDisk(outputText, config, ...)
end
Writer-->>Pack: write result
Pack-->>CLI: result (including persisted output info)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @yamadashy, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request significantly enhances the project's code quality and development efficiency by integrating Oxlint, a high-performance linter. The primary goal is to enforce stricter code standards and improve developer experience through faster feedback cycles. Beyond just adding the linter, this PR also addresses and resolves all existing linting warnings, ensuring a cleaner and more robust codebase. This change streamlines the development process by automating code style and error checking, leading to more consistent and reliable code.
Highlights
- New Linter Integration: Oxlint: This PR introduces Oxlint, a high-performance linter, into the project's development workflow. Oxlint is significantly faster than traditional linters like ESLint, promising 50-100x speed improvements.
- Code Quality Improvement: Warning Resolution: A comprehensive cleanup was performed to resolve 144 existing linting warnings. This includes removing unused imports, parameters, and variables, fixing generator functions, and correcting regex patterns. This effort substantially improves the overall code quality and maintainability.
- CI/CD Integration: Oxlint has been integrated into the Continuous Integration (CI) pipeline via GitHub Actions, ensuring that all future code contributions adhere to the new linting standards. The linting script order has been updated to biome → oxlint → ts → secretlint.
- Oxlint Configuration: The integration includes proper configuration for Oxlint through a new
.oxlintrc.jsonfile, allowing for project-specific rules and ignored patterns, such as excluding integration tests from linting.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #800 +/- ##
==========================================
- Coverage 87.44% 87.41% -0.03%
==========================================
Files 113 113
Lines 6491 6493 +2
Branches 1331 1331
==========================================
Hits 5676 5676
- Misses 815 817 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Code Review
This pull request successfully integrates the oxlint linter into the project's tooling and addresses all reported linting warnings across the codebase. The changes are extensive, touching dozens of files, but are consistently high-quality, focusing on removing unused code, fixing minor syntax issues, and adopting modern language features. The introduction of oxlint will undoubtedly contribute to maintaining and improving code quality going forward. The changes are well-executed and I have no further suggestions for improvement. Great work!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/core/treeSitter/parseStrategies/PythonParseStrategy.ts (1)
95-100: Python:async defsignatures are not recognizedThe current regex only matches
defand will missasync deffunctions, which means async functions won’t be emitted when captured. Expand the regex to accept an optionalasyncprefix.Apply this focused change:
private getFunctionSignature(lines: string[], startRow: number): string | null { const line = lines[startRow]; - const match = line.match(/def\s+(\w+)\s*\((.*?)\)(\s*->\s*[^:]+)?:/); + const match = line.match(/(?:async\s+)?def\s+(\w+)\s*\((.*?)\)(\s*->\s*[^:]+)?:/); if (!match) return null; return line.replace(/:\s*$/, ''); }If you want, I can add unit tests that include
async defcoverage to prevent regressions.
🧹 Nitpick comments (33)
tests/core/output/diffsInOutput.test.ts (2)
126-131: Drop unused binding; just await the callYou don't use the return value. Prefer not assigning to a throwaway variable in tests.
- const _output = await generateOutput(rootDirs, mockConfig, processedFiles, ['file1.js'], gitDiffResult, undefined, { + await generateOutput(rootDirs, mockConfig, processedFiles, ['file1.js'], gitDiffResult, undefined, {(Optional: if you want stronger coverage, capture it as
outputand assert the exact string returned by your mock.)
189-195: Same here: remove the unused_outputbindingNo consumer of the variable; awaiting the promise is sufficient.
- const _output = await generateOutput(rootDirs, mockConfig, processedFiles, ['file1.js'], gitDiffResult, undefined, { + await generateOutput(rootDirs, mockConfig, processedFiles, ['file1.js'], gitDiffResult, undefined, {tests/core/treeSitter/parseFile.php.test.ts (2)
55-57: Expectation updated to match real PHP import — looks goodAligning the expectation with
use App\\Greeter;ensures the test validates the correct backslash in the parsed output.If you adopt
String.rawfor fileContent, you may also use it inline here for readability:const expectContents = [ 'namespace App;', String.raw`use App\Greeter;`, // ... ];
7-13: Correctly escaped PHP namespace separator; optional: use String.raw for template literalsYou’ve correctly doubled the backslash in the PHP
usestatement so that\Greeterisn’t swallowed by JS’s escape processing. To future-proof any embedded backticks containing backslashes (e.g. PHP namespaces, Windows paths, regexes), you can convert the template literal to aString.rawtag.Apply this minimal change in
tests/core/treeSitter/parseFile.php.test.ts:- const fileContent = ` + const fileContent = String.raw`None of the other
parseFile*.test.tsfixtures currently include backslashes in their code blocks, so this change is only needed in the PHP test.src/core/file/workers/fileProcessWorker.ts (1)
3-3: SwitchprocessContentto dynamic import to honor early log-level init.If you adopt the approach above, drop the static import here to avoid duplicate bindings.
-import { processContent } from '../fileProcessContent.js';tests/mcp/tools/mcpToolRuntime.test.ts (2)
83-91: Mockcrypto.randomBytescloser to real Buffer behaviorMinor robustness tweak:
generateOutputIdlikely calls.toString('hex'). Your mock ignores the encoding arg, which is fine at runtime, but returning a realBufferreduces type friction and future-proofs the test.Apply:
- vi.mocked(crypto.randomBytes).mockImplementation(() => ({ - toString: () => 'abcdef1234567890', - })); + vi.mocked(crypto.randomBytes).mockImplementation(() => + Buffer.from('abcdef1234567890', 'utf8'), + );If the production code uses
'hex', prefer:- vi.mocked(crypto.randomBytes).mockImplementation(() => - Buffer.from('abcdef1234567890', 'utf8'), - ); + vi.mocked(crypto.randomBytes).mockImplementation(() => + Buffer.from('abcdef1234567890', 'hex'), + );
55-59: Path join mock: consider OS-agnostic intentYou’re normalizing
path.jointoargs.join('/'), which is fine in a controlled test but can accidentally mask logic that depends onpath.sep. If the implementation under test appends a trailing separator formkdtemp, prefer a mock that respects separators.Option:
- vi.mocked(path.join).mockImplementation((...args) => args.join('/')); + vi.mocked(path.join).mockImplementation((...args) => { + const sep = '/'; // enforce POSIX consistently for test determinism + return args.join(sep); + });Alternatively, assert arguments more loosely (e.g.,
toHaveBeenCalledWith(expect.stringContaining('/repomix/mcp-outputs'))) instead of pinning the trailing slash..oxlintrc.json (1)
3-5: Optional: broaden ignores for heavy or generated assetsIf you have large fixtures or generated/site assets, consider ignoring them to keep lint fast and focused.
For example:
"ignorePatterns": [ - "tests/integration-tests/**/*" + "tests/integration-tests/**/*", + "tests/fixtures/**/*", + "website/**/public/**/*", + "dist/**/*", + "coverage/**/*" ]website/client/components/utils/validation.ts (1)
5-20: Optional: tighten URL validation to GitHub-only per docstringDocstring says “Validates a GitHub repository URL or shorthand,” but
new URL(remoteValue)currently accepts any absolute URL (e.g., Google). Consider restricting to GitHub/Gist domains and ensuring owner/repo segments exist. Also, the TODO to share logic with core is a good candidate for centralization.Example adjustment:
- try { - new URL(remoteValue); - return true; - } catch { - return false; - } + try { + const url = new URL(remoteValue); + if (url.protocol !== 'https:') return false; + if (!/^(github\.com|gist\.github\.com)$/i.test(url.hostname)) return false; + const cleaned = url.pathname.replace(/^\/|\.git$/g, ''); + const [owner, repo] = cleaned.split('/'); + return Boolean(owner && repo); + } catch { + return false; + }If you’d like, I can extract a shared validator used by both client and
src/cli/actions/remoteAction.ts.src/core/output/outputSort.ts (1)
53-56: Optional catch binding is fine; add a trace for easier diagnosticsBehavior unchanged and aligns with the PR’s oxlint goals. As a tiny DX win, emit a trace on failure so we know why we fell back to the original order without elevating log levels.
- } catch { - // If git command fails, return original order - return files; - } + } catch { + // If git command fails, return original order + logger.trace('Git change-count lookup failed — returning original order'); + return files; + }benchmarks/memory/src/simple-memory-test.ts (1)
39-41: Use fs.rm with force to avoid try/catch noiseNot required, but this helper can be a one-liner and remain silent if the file doesn’t exist.
- try { - await fs.unlink(path.join(__dirname, '../memory-test-output.txt')); - } catch { - // Ignore if file doesn't exist - } + await fs.rm(path.join(__dirname, '../memory-test-output.txt'), { force: true });src/mcp/tools/readRepomixOutputTool.ts (1)
64-68: Bare catch keeps things simple; consider a trace for visibilityCurrent behavior is fine (tool returns a structured error). If helpful during support, add a trace log before returning to note the missing path; keeps logs low-noise while aiding diagnostics.
- } catch { - return buildMcpToolErrorResponse({ + } catch { + logger.trace(`read_repomix_output: file not found at path: ${filePath}`); + return buildMcpToolErrorResponse({ errorMessage: `Error: Output file does not exist at path: ${filePath}. The temporary file may have been cleaned up.`, }); }website/server/src/schemas/request.ts (1)
6-6: Regex escape removal is correct; behavior unchanged.Unescaping the forward slash inside the character class is safe and keeps
/allowed. No functional changes introduced.If you want to simplify readability, consider putting
-at either start or end of the class to avoid escaping it:/^[a-zA-Z0-9*?/_.,!()\s-]*$/website/client/composables/useFileUpload.ts (1)
225-229: Bare catch is appropriate here; optional: log in dev to aid debugging.Switching to
catch {}avoids the unused-binding lint warning and doesn’t affect UX (you return a friendly error). Optionally, you could add a dev-only console log to aid debugging without reintroducing the unused param.Example:
} catch { if (import.meta.env?.DEV) { // Intentionally generic user message; log details elsewhere if needed // e.g., console.error('handleFolderDrop failed'); } const errorMsg = 'Failed to process the folder. Please try again or use the browse button.'; errorMessage.value = errorMsg; return { success: false, error: errorMsg }; }src/core/git/gitRepositoryHandle.ts (1)
33-38: Consider restoring trace logging inisGitRepositorycatch for diagnosability.Other functions in this module trace the error on failure; this one now silently returns
false. Adding a trace keeps behavior consistent and aids debugging without affecting callers.try { await deps.execGitRevParse(directory); return true; - } catch { - return false; + } catch (error) { + logger.trace('Directory is not a git repository:', (error as Error).message); + return false; }tests/core/file/fileStdin.test.ts (2)
256-258: Yield a filtered-out line instead of a valid token to avoid side effects before the throw.Using
'dummy'creates a valid line that could be processed before the error is thrown. Yielding a comment or whitespace keeps semantics minimal while satisfying require-yield.- // This generator needs to throw an error for testing, but must yield first to satisfy require-yield - yield 'dummy'; + // This generator needs to throw an error for testing, but must yield first to satisfy require-yield. + // Yield a comment so it gets filtered out by filterValidLines. + yield '# comment'; throw new Error('Generic error');
293-295: Same here: yield a filtered-out line to keep the pre-error emission inert.Prevents
'dummy'from being treated as a candidate path before the throw.- // This generator needs to throw an error for testing, but must yield first to satisfy require-yield - yield 'dummy'; + // This generator needs to throw an error for testing, but must yield first to satisfy require-yield. + // Yield a comment so it gets filtered out by filterValidLines. + yield '# comment'; throw 'string error';src/core/file/fileProcess.ts (1)
29-34: Optional: drop the unused_indexparameter entirely.Since the index isn’t used, removing it simplifies the callback without changing behavior.
- const tasks = rawFiles.map( - (rawFile, _index) => - ({ - rawFile, - config, - }) satisfies FileProcessTask, - ); + const tasks = rawFiles.map( + (rawFile) => + ({ + rawFile, + config, + }) satisfies FileProcessTask, + );src/core/treeSitter/parseStrategies/PythonParseStrategy.ts (1)
89-93: Redundant conditional; simplify and align name with behavior
matchis computed but unused, and both branches return the same expression. Consider simplifying and, if desired, renaming the method to reflect that it returns the class definition line without the trailing colon (not “inheritance” specifically).Apply this minimal simplification:
private getClassInheritance(lines: string[], startRow: number): string | null { const line = lines[startRow]; - const match = line.match(/class\s+\w+\s*\((.*?)\):/); - return match ? line.replace(/:\s*$/, '') : line.replace(/:\s*$/, ''); + // Return the class definition line without a trailing colon. + return line.replace(/:\s*$/, ''); }src/core/treeSitter/parseStrategies/GoParseStrategy.ts (2)
120-133: Clean up: unused parameter and misleading name infindClosingToken
openTokenis never used and the implementation returns the first line containingcloseToken. The method name suggests bracket matching, but the logic doesn’t do that. Consider renaming and simplifying the function and its call sites for clarity.Here’s a minimal, behavior-preserving refactor:
- private findClosingToken( - lines: string[], - startRow: number, - endRow: number, - openToken: string, - closeToken: string, - ): number { - for (let i = startRow; i <= endRow; i++) { - if (lines[i].includes(closeToken)) { - return i; - } - } - return startRow; - } + // Returns the first row in [startRow, endRow] that contains `token`, or startRow if none found. + private findFirstLineIncluding(lines: string[], startRow: number, endRow: number, token: string): number { + for (let i = startRow; i <= endRow; i++) { + if (lines[i].includes(token)) return i; + } + return startRow; + }And adjust the call sites accordingly:
- ? this.findClosingToken(lines, startRow, endRow, '(', ')') + ? this.findFirstLineIncluding(lines, startRow, endRow, ')') : endRow;- const signatureEndRow = this.findClosingToken(lines, startRow, endRow, '{', '{'); + const signatureEndRow = this.findFirstLineIncluding(lines, startRow, endRow, '{');- ? this.findClosingToken(lines, startRow, endRow, '{', '}') + ? this.findFirstLineIncluding(lines, startRow, endRow, '}') : endRow;Optionally, if you truly need balanced matching in the future, implement a small brace counter.
99-107: Go generics and receivers: names may be missed (FYI, non-blocking)The current regexes won’t extract names when generics appear immediately after the function name (e.g.,
func Foo[T any](...)) or with more complex receivers. You already fall back to emitting the signature snippet even whennameis not found, so behavior is acceptable. Consider expanding the patterns later if deduping by name is important.If you want to confirm coverage, I can add unit snippets containing:
func Foo[T any](x T) {}func (r *Receiver[T]) Bar(x T) {}Also applies to: 110-118
src/core/file/fileManipulate.ts (2)
95-99: Confirm triple-quote counting across mixed-quote edge casesThe updated counting uses negative lookbehinds against the opposite quote type (e.g.,
(?<!["])around'''). This works for the added tests, but mixed constructs like"""'''"""on one line can still be tricky. Suggest ensuring tests cover:
- Same-line open/close with surrounding opposite quotes
- Embedded triple-quotes preceded by an identifier (e.g.,
u"""...""",r'''...'''), which should not be treated as docstringsI can add targeted test cases under
tests/core/file/fileManipulate.test.tsto lock these behaviors in. Do you want me to open a follow-up PR?
78-80: Nit: simplify negative lookbehinds for readabilityThe inner capturing groups inside the negative lookbehinds aren’t necessary and slightly obscure the intent. You can simplify
( ?<!("""))to( ?<!""")(and similarly for(''')) without changing behavior.Apply this minor cleanup:
- const doubleQuoteRegex = /^\s*(?<!\\)(?:""")\s*(?:\n)?[\s\S]*?(?<!("""))(?<!\\)(?:""")/gm; - const singleQuoteRegex = /^\s*(?<!\\)(?:''')\s*(?:\n)?[\s\S]*?(?<!('''))(?<!\\)(?:''')/gm; + const doubleQuoteRegex = /^\s*(?<!\\)(?:""")\s*(?:\n)?[\s\S]*?(?<!""")(?<!\\)(?:""")/gm; + const singleQuoteRegex = /^\s*(?<!\\)(?:''')\s*(?:\n)?[\s\S]*?(?<!''')(?<!\\)(?:''')/gm;tests/core/file/fileManipulate.test.ts (1)
431-447: Optional: broaden Python fixtures for robustnessYou already cover many tricky cases. To harden against regressions, consider adding tests for:
async deffunctions with docstrings- Raw/byte/Unicode prefixes before triple quotes (
r"""...""",u'''...''') that are not docstrings- Same-line triple-quote docstrings with content:
def f(): """one line"""Happy to contribute these cases as follow-ups.
Also applies to: 449-473, 475-493
tests/core/metrics/calculateOutputMetrics.test.ts (2)
9-18: Tighten generics to avoid casts in the happy-path mock.Small typing tweak lets you drop the
as OutputMetricsTaskcast and clarifies intent.Apply this diff:
-const mockInitTaskRunner = <T, R>(_numOfTasks: number, _workerPath: string) => { +const mockInitTaskRunner = <T extends OutputMetricsTask, R extends number>(_numOfTasks: number, _workerPath: string) => { return { - run: async (task: T) => { - return (await outputMetricsWorker(task as OutputMetricsTask)) as R; + run: async (task: T) => { + return (await outputMetricsWorker(task)) as R; }, cleanup: async () => { // Mock cleanup - no-op for tests }, }; };
168-168: Remove dead local_expectedChunkSize.The variable is computed but unused; the following assertions already validate chunk sizing.
- const _expectedChunkSize = Math.ceil(content.length / 1000); // CHUNK_SIZE is 1000src/core/treeSitter/parseStrategies/VueParseStrategy.ts (1)
26-26: Drop unused_normalizedChunkto avoid needless work.This local is computed but never used; safe to remove.
- const _normalizedChunk = chunk.trim();package.json (1)
24-27: Split “check” vs “fix” for oxlint to avoid mutating files in CI.Running
--fixin CI can silently modify the workspace. Recommend using a no-fix script in CI and a separate:fixscript for local use; keeplintchaining the no-fix variant."scripts": { @@ - "lint": "node --run lint-biome && node --run lint-oxlint && node --run lint-ts && node --run lint-secretlint", + "lint": "node --run lint-biome && node --run lint-oxlint && node --run lint-ts && node --run lint-secretlint", "lint-biome": "biome check --write", - "lint-oxlint": "oxlint --fix", + "lint-oxlint": "oxlint", + "lint-oxlint:fix": "oxlint --fix",Optional: when you’re ready to gate on warnings in CI, consider
oxlint --deny-warnings(or the equivalent) in the CI job.src/core/packager.ts (1)
34-46: Dependency injection now includes writeOutputToDisk — solid design; consider exporting a Dep typeAdding writeOutputToDisk to defaultDeps is the right move for testability and DI consistency. As a small improvement, consider exporting a type alias (e.g.,
export type PackDeps = typeof defaultDeps) so tests and consumers can reference the shape without importing values or relying ontypeof. This keeps signatures stable and discoverable.-const defaultDeps = { +export const defaultDeps = { searchFiles, collectFiles, processFiles, generateOutput, validateFileSafety, writeOutputToDisk, copyToClipboardIfEnabled, calculateMetrics, sortPaths, getGitDiffs, getGitLogs, }; +export type PackDeps = typeof defaultDeps;tests/integration-tests/packager.test.ts (1)
170-170: Avoid unnecessary I/O: remove unused expected output read
_expectedOutputis read but never used. Dropping the read reduces test time and flakiness surface without affecting assertions.- const _expectedOutput = await fs.readFile(expectedOutputPath, 'utf-8'); + // No need to read expected output; assertions below check structure/content.src/core/git/gitRemoteParse.ts (1)
67-69: Bare catch is fine; optionally log context for diagnosticsSwitching to a parameterless
catchsilences the unused binding. If you want more observability without binding the error, consider tracing theremoteValuebefore throwing, or attach the original error as a cause when supported byRepomixError.- } catch { - throw new RepomixError('Invalid remote repository URL or repository shorthand (owner/repo)'); + } catch { + // Optionally: logger.trace('Invalid remote value:', remoteValue); + throw new RepomixError('Invalid remote repository URL or repository shorthand (owner/repo)'); }tests/core/security/securityCheck.test.ts (1)
47-51: Optional: assert cleanup is invoked to harden the contractSince your mock exposes a
cleanupmethod, consider verifying it gets called whenrunSecurityCheckcompletes (success and error paths). This guards against resource leaks if the default runner evolves.Example test addition (outside this hunk):
it('should call taskRunner.cleanup after processing', async () => { const cleanup = vi.fn().mockResolvedValue(undefined); const taskRunner = { run: vi.fn().mockImplementation(async (task: SecurityCheckTask) => securityCheckWorker(task)), cleanup, }; const initTaskRunner = vi.fn(() => taskRunner); await runSecurityCheck(mockFiles, () => {}, undefined, undefined, { initTaskRunner }); expect(cleanup).toHaveBeenCalledTimes(1); });src/core/git/gitCommand.ts (1)
185-193: Optional: tighten validation for git+ssh form (git@host:org/repo)
git@URLs bypass structural validation beyond the dangerous-flag check. Consider a light-weight shape check to reject malformed inputs early (e.g.,git@host:owner/repo(.git)?).Sketch:
if (url.startsWith('git@')) { const isValidGitSsh = /^git@[A-Za-z0-9._-]+:[A-Za-z0-9._-]+\/[A-Za-z0-9._-]+(?:\.git)?$/.test(url); if (!isValidGitSsh) { throw new RepomixError(`Invalid git+ssh URL format: ${url.replace(/:.+@/, ':***@')}`); } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (74)
.github/workflows/ci.yml(1 hunks).oxlintrc.json(1 hunks)benchmarks/memory/src/simple-memory-test.ts(1 hunks)browser/scripts/generate-icons.ts(0 hunks)package.json(2 hunks)src/cli/actions/initAction.ts(0 hunks)src/cli/cliRun.ts(1 hunks)src/core/file/fileManipulate.ts(1 hunks)src/core/file/fileProcess.ts(1 hunks)src/core/file/workers/fileProcessWorker.ts(1 hunks)src/core/git/gitCommand.ts(1 hunks)src/core/git/gitHubArchive.ts(1 hunks)src/core/git/gitRemoteParse.ts(2 hunks)src/core/git/gitRepositoryHandle.ts(1 hunks)src/core/output/outputSort.ts(1 hunks)src/core/output/outputStyleDecorate.ts(1 hunks)src/core/packager.ts(1 hunks)src/core/security/validateFileSafety.ts(1 hunks)src/core/treeSitter/loadLanguage.ts(0 hunks)src/core/treeSitter/parseFile.ts(0 hunks)src/core/treeSitter/parseStrategies/CssParseStrategy.ts(1 hunks)src/core/treeSitter/parseStrategies/DefaultParseStrategy.ts(1 hunks)src/core/treeSitter/parseStrategies/GoParseStrategy.ts(1 hunks)src/core/treeSitter/parseStrategies/PythonParseStrategy.ts(1 hunks)src/core/treeSitter/parseStrategies/TypeScriptParseStrategy.ts(1 hunks)src/core/treeSitter/parseStrategies/VueParseStrategy.ts(2 hunks)src/mcp/tools/attachPackedOutputTool.ts(0 hunks)src/mcp/tools/fileSystemReadFileTool.ts(0 hunks)src/mcp/tools/grepRepomixOutputTool.ts(1 hunks)src/mcp/tools/readRepomixOutputTool.ts(1 hunks)tests/cli/actions/defaultAction.buildCliConfig.test.ts(0 hunks)tests/cli/actions/defaultAction.test.ts(0 hunks)tests/cli/actions/diffsFlag.test.ts(1 hunks)tests/cli/cliRun.test.ts(0 hunks)tests/config/configLoad.test.ts(1 hunks)tests/core/file/fileCollect.test.ts(1 hunks)tests/core/file/fileManipulate.test.ts(2 hunks)tests/core/file/fileProcess.test.ts(2 hunks)tests/core/file/fileSearch.test.ts(0 hunks)tests/core/file/fileStdin.test.ts(2 hunks)tests/core/metrics/calculateMetrics.test.ts(0 hunks)tests/core/metrics/calculateOutputMetrics.test.ts(6 hunks)tests/core/metrics/calculateSelectiveFileMetrics.test.ts(1 hunks)tests/core/output/diffsInOutput.test.ts(2 hunks)tests/core/output/outputGenerateDiffs.test.ts(4 hunks)tests/core/output/outputSort.test.ts(0 hunks)tests/core/output/outputStyles/markdownStyle.test.ts(0 hunks)tests/core/security/securityCheck.test.ts(1 hunks)tests/core/treeSitter/parseFile.c.test.ts(0 hunks)tests/core/treeSitter/parseFile.comments.test.ts(0 hunks)tests/core/treeSitter/parseFile.cpp.test.ts(0 hunks)tests/core/treeSitter/parseFile.csharp.test.ts(0 hunks)tests/core/treeSitter/parseFile.css.test.ts(0 hunks)tests/core/treeSitter/parseFile.go.test.ts(0 hunks)tests/core/treeSitter/parseFile.java.test.ts(0 hunks)tests/core/treeSitter/parseFile.javascript.test.ts(0 hunks)tests/core/treeSitter/parseFile.php.test.ts(2 hunks)tests/core/treeSitter/parseFile.python.test.ts(1 hunks)tests/core/treeSitter/parseFile.ruby.test.ts(0 hunks)tests/core/treeSitter/parseFile.rust.test.ts(0 hunks)tests/core/treeSitter/parseFile.swift.test.ts(0 hunks)tests/core/treeSitter/parseFile.test.ts(0 hunks)tests/core/treeSitter/parseFile.typescript.test.ts(3 hunks)tests/core/treeSitter/parseFile.vue.test.ts(1 hunks)tests/integration-tests/packager.test.ts(4 hunks)tests/mcp/tools/fileSystemReadDirectoryTool.test.ts(0 hunks)tests/mcp/tools/fileSystemReadFileTool.test.ts(0 hunks)tests/mcp/tools/grepRepomixOutputTool.test.ts(0 hunks)tests/mcp/tools/mcpToolRuntime.test.ts(1 hunks)website/client/components/utils/resultViewer.ts(1 hunks)website/client/components/utils/validation.ts(1 hunks)website/client/composables/useFileUpload.ts(1 hunks)website/server/src/index.ts(1 hunks)website/server/src/schemas/request.ts(1 hunks)
💤 Files with no reviewable changes (28)
- tests/core/treeSitter/parseFile.go.test.ts
- tests/core/treeSitter/parseFile.cpp.test.ts
- src/cli/actions/initAction.ts
- tests/core/output/outputSort.test.ts
- src/core/treeSitter/loadLanguage.ts
- tests/core/treeSitter/parseFile.swift.test.ts
- src/mcp/tools/attachPackedOutputTool.ts
- tests/core/treeSitter/parseFile.ruby.test.ts
- tests/core/output/outputStyles/markdownStyle.test.ts
- tests/core/treeSitter/parseFile.csharp.test.ts
- tests/core/treeSitter/parseFile.test.ts
- tests/core/treeSitter/parseFile.c.test.ts
- tests/cli/actions/defaultAction.test.ts
- tests/core/treeSitter/parseFile.rust.test.ts
- tests/cli/actions/defaultAction.buildCliConfig.test.ts
- tests/core/treeSitter/parseFile.css.test.ts
- tests/mcp/tools/fileSystemReadFileTool.test.ts
- src/core/treeSitter/parseFile.ts
- tests/core/file/fileSearch.test.ts
- tests/core/treeSitter/parseFile.javascript.test.ts
- src/mcp/tools/fileSystemReadFileTool.ts
- tests/mcp/tools/fileSystemReadDirectoryTool.test.ts
- tests/core/treeSitter/parseFile.comments.test.ts
- tests/core/treeSitter/parseFile.java.test.ts
- tests/mcp/tools/grepRepomixOutputTool.test.ts
- tests/cli/cliRun.test.ts
- browser/scripts/generate-icons.ts
- tests/core/metrics/calculateMetrics.test.ts
🧰 Additional context used
🧬 Code graph analysis (9)
src/core/output/outputStyleDecorate.ts (1)
src/config/configSchema.ts (1)
RepomixConfigMerged(164-164)
tests/core/output/outputGenerateDiffs.test.ts (1)
src/core/output/outputGeneratorTypes.ts (1)
RenderContext(16-37)
tests/core/output/diffsInOutput.test.ts (1)
src/core/output/outputGenerate.ts (1)
generateOutput(166-202)
src/core/treeSitter/parseStrategies/GoParseStrategy.ts (1)
src/core/treeSitter/parseStrategies/ParseStrategy.ts (1)
ParseContext(11-17)
src/core/treeSitter/parseStrategies/PythonParseStrategy.ts (1)
src/core/treeSitter/parseStrategies/ParseStrategy.ts (1)
ParseContext(11-17)
src/core/treeSitter/parseStrategies/CssParseStrategy.ts (1)
src/core/treeSitter/parseStrategies/ParseStrategy.ts (1)
ParseContext(11-17)
src/core/treeSitter/parseStrategies/TypeScriptParseStrategy.ts (1)
src/core/treeSitter/parseStrategies/ParseStrategy.ts (1)
ParseContext(11-17)
src/core/treeSitter/parseStrategies/DefaultParseStrategy.ts (1)
src/core/treeSitter/parseStrategies/ParseStrategy.ts (1)
ParseContext(11-17)
src/core/treeSitter/parseStrategies/VueParseStrategy.ts (1)
src/core/treeSitter/parseStrategies/ParseStrategy.ts (1)
ParseContext(11-17)
⏰ 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). (4)
- GitHub Check: Build and run (windows-latest, 23.x)
- GitHub Check: Build and run (macos-latest, 21.x)
- GitHub Check: Build and run (windows-latest, 22.x)
- GitHub Check: Test with Bun (windows-latest, latest)
🔇 Additional comments (46)
tests/core/output/outputGenerateDiffs.test.ts (1)
73-79: LGTM: underscore-prefixed unused params align with lint rulesRenaming the third mock parameter to
_processedFilesis consistent with the repo’s linting convention and keeps signatures readable. Optional: if TS types allow in your setup, you could omit the unused parameter entirely in the mock implementation.Also applies to: 141-148, 176-182, 219-226
src/core/file/workers/fileProcessWorker.ts (2)
2-2: Unused import cleanup: good call.Dropping the unused logger import tightens the module and aligns with the PR’s oxlint objectives.
6-9: No import-time logging detected; existing log-level init is sufficientI’ve inspected
src/core/file/fileProcessContent.ts(and its dependencies insrc/core/file), and found nologger.*orconsole.*calls at the module top level—every logging call lives inside functions or catch blocks. That means no logging occurs during static imports, so callingsetLogLevelByWorkerData()at module load time already precedes any runtime logging. The proposed dynamic‐import refactor isn’t needed.tests/cli/actions/diffsFlag.test.ts (1)
1-1: Trimmed unused Vitest import — good cleanupRemoving the unused
vibinding keeps the test lean and aligned with the new linting rules. No behavior change.tests/mcp/tools/mcpToolRuntime.test.ts (1)
5-5: Unused Vitest binding removedDropping the unused
afterEachimport is consistent with the repo-wide lint cleanup. The remaining imports reflect actual usage..oxlintrc.json (1)
1-6: Initial oxlint config looks goodThe schema hint aids editor UX, and the integration-tests ignore is sensible to keep signal high.
website/client/components/utils/validation.ts (1)
17-17: Adopts bare catch — matches intent and avoids unused bindingGood change; behavior unchanged while satisfying lint rules.
website/client/components/utils/resultViewer.ts (1)
37-37: Regex simplification is correct
/[\/\\]/g→/[/\\]/gis equivalent and clearer. No behavior change in repository name normalization.src/core/output/outputSort.ts (1)
53-56: Optional catch binding is fully supported by the existing toolchainAll configured runtimes and compilers handle
catch { … }without issue:
- Node.js is pinned to
>=20.0.0, which natively supports optional catch bindings.- TypeScript ^5.8.3 (in devDependencies) introduced and fully understands the
catch {}syntax.- Compiler targets:
- Root tsconfig (
es2016) and any unspecified build target (defaulting to ES5) will compilecatch {}down to a standardcatch (e) {}form.- Other configs targeting
ES2022/ESNextemit the native syntax directly.No changes are needed here.
src/cli/cliRun.ts (1)
2-2: Removed unused Command import — LGTMImport list is now minimal and consistent with actual usage (Option, program). No behavioral change.
tests/core/treeSitter/parseFile.python.test.ts (1)
1-1: Vitest import cleanup — LGTMRemoves unused imports and keeps the test lean. No impact on assertions.
website/server/src/index.ts (1)
14-14: Safe to removecalculateLatencyimport inindex.ts; no dangling references
- The import removal in
website/server/src/index.tsis correct—calculateLatencywasn’t used in this file and remains intact inutils/time.ts.calculateLatencyis still exported (time.ts:17) and consumed byutils/logger.ts(lines 94, 126), so there are no broken or stale references.formatLatencyForDisplay(startTime)is defined to take a start timestamp (number) inutils/time.ts:28, matching its usage here.tests/core/file/fileCollect.test.ts (1)
23-32: Underscore-prefixed unused params remove lint noise without changing behavior.The
_numOfTasksand_workerPathrename is a clean way to satisfy lint rules while preserving the generic signature used by the test harness.tests/core/treeSitter/parseFile.vue.test.ts (1)
2-2: Trimmed import set to only what’s used.Importing only
parseFileremoves dead code from the test and aligns with the lint goals of this PR.tests/core/file/fileProcess.test.ts (2)
1-1: LGTM: pruned unusedviimport from vitest.Import list now matches actual usage in this file.
22-31: LGTM: underscore-prefix for intentionally unused params.Consistent with repo-wide lint strategy. No functional impact on the mock runner.
tests/core/metrics/calculateSelectiveFileMetrics.test.ts (1)
12-21: LGTM: underscore-prefixed unused params in mock runner.Matches the PR’s linting strategy; no behavior change.
src/mcp/tools/grepRepomixOutputTool.ts (1)
118-128: Good adoption of optional catch binding.The catch clause has been updated to use an empty catch block
catch { }instead of binding the error parameter. This is a clean implementation that aligns with the modern JavaScript/TypeScript pattern when the error is not being used within the catch block.src/core/treeSitter/parseStrategies/CssParseStrategy.ts (1)
5-10: Appropriate use of underscore prefix for unused parameter.The
contextparameter has been correctly renamed to_contextto indicate it's intentionally unused. This follows TypeScript conventions and helps with linting tools that flag unused parameters.src/core/git/gitHubArchive.ts (1)
291-291: Minor regex escaping fix for improved clarity.The character class in the regex has been updated from
[\/\\]to[/\\]to remove unnecessary escaping of the forward slash within the character class. Both patterns are functionally equivalent, but the updated version is cleaner and aligns with modern regex best practices.tests/config/configLoad.test.ts (2)
9-9: Clean import simplification.The import has been correctly simplified to only include
RepomixConfigValidationError, removing the unusedRepomixErrorimport. This aligns with the codebase cleanup objective of the PR.
13-22: Well-structured test mocks for logger and global directory.The mock setup provides appropriate stubs for the logger methods (
trace,note,log) and thegetGlobalDirectoryfunction. This ensures tests can run in isolation without external dependencies.src/core/output/outputStyleDecorate.ts (1)
49-49: Proper handling of unused parameter.The
generationDateparameter has been correctly prefixed with underscore to indicate it's intentionally unused. The function signature remains intact for API compatibility while clearly marking the parameter as unused.src/core/treeSitter/parseStrategies/PythonParseStrategy.ts (1)
22-23: Good: underscore-prefix for unused param matches lint intentRenaming
contextto_contextis consistent with the repo-wide convention and silences unused-parameter warnings without changing behavior.src/core/treeSitter/parseStrategies/GoParseStrategy.ts (1)
28-29: Good: underscore-prefix for unused paramRenaming to
_contextis consistent with the convention adopted across parse strategies and avoids unused-parameter warnings.src/core/file/fileManipulate.ts (1)
86-87: Regex tweak looks good and matches intentThe simplified character classes in
indexSingle/indexDoubleimprove readability without changing semantics. This aligns with the tests you updated.tests/core/file/fileManipulate.test.ts (2)
498-504: Test update matches new behavior—goodSwitching to a literal
#inside the string better reflects Python semantics and the updated hash-removal logic. Looks correct.
567-575: Test clarifies mixed-quote edge caseAdjusting to
x = '""""'makes the intent explicit and avoids accidental triple-quote detection. This pairs well with the manipulator changes.src/core/treeSitter/parseStrategies/DefaultParseStrategy.ts (1)
9-9: Underscore-prefixed context correctly signals an intentionally unused parameter.No functional impact; aligns with the repo-wide linting convention.
src/core/treeSitter/parseStrategies/TypeScriptParseStrategy.ts (1)
28-28: Consistent unused-parameter convention.Renaming
contextto_contextis non-functional and keeps implementations uniform across strategies.tests/core/metrics/calculateOutputMetrics.test.ts (4)
49-58: Error-path mock reads clearly and follows the underscore convention.Looks good; the
_taskplaceholder keeps lints quiet while emphasizing the throw path.
98-111: Parallel mock is straightforward and deterministic.Good use of
_taskand a fixed return to make assertions simple.
125-134: Second error-path mock mirrors the first.Consistent and clear.
150-161: Chunk-tracking mock uses the task value as intended.No issues; this is the right place not to underscore the parameter.
src/core/treeSitter/parseStrategies/VueParseStrategy.ts (1)
9-9: Unused context rename is correct.Keeps the method signature consistent with the interface while satisfying lint rules.
package.json (1)
112-112: Version choice looks reasonable.Pinning to
^1.12.0is fine; keep an eye on release notes for rule changes that could introduce new warnings.src/core/packager.ts (2)
8-10: Import narrowing: good cleanup with no loss of type inferenceSwitching to function imports for git helpers and importing only ProcessedFile keeps types inferred at call sites and reduces unused types. Looks good.
124-126: Confirm writeOutputToDisk’s behavior for “no file path”/clipboard-only scenariosNow that writing is unconditional here, please confirm
writeOutputToDisk(output, config)is a no-op (or fails fast with a clear error) whenconfig.output.filePathis absent/disabled. If it’s already handled internally, we’re good; otherwise we may want to guard the call.tests/integration-tests/packager.test.ts (3)
33-42: Underscore-prefixed unused params: aligns with oxlint and keeps intent clearRenaming to
_numOfTasks,_workerPathis consistent with the repo-wide cleanup and avoids lint noise without changing behavior.
114-121: processFiles mock: underscore for unused callback is appropriateGood lint-friendly rename for
_progressCallback. The worker-backed processing remains faithful to production behavior for this test.
135-141: calculateMetrics signature: underscore unused params keep the contract while silencing lintsNice alignment with the injected deps API; no logic changes.
tests/core/treeSitter/parseFile.typescript.test.ts (2)
1-1: Removed unused vi import: tidy and correctThe file doesn’t use
vi; importing only the used vitest symbols is the right cleanup.
160-171: Underscore-prefixed unused parameters in mocks: matches oxlint guidanceRenaming the unused parameters in TreeCursor/Tree mocks avoids lint warnings and keeps the mock surface true to the API.
Also applies to: 176-181, 189-189
src/core/git/gitRemoteParse.ts (1)
72-79: Parameterless catch in isValidRemoteValue: LGTMFor a boolean probe, swallowing the error is expected; returning false on parse failure is correct.
src/core/security/validateFileSafety.ts (1)
4-4: Removed unused ProcessedFile import: correct and low-riskThe function only deals with
RawFile[]; trimming the unused type import is the right fix.tests/core/security/securityCheck.test.ts (1)
42-51: Underscore-prefixed unused params are correct and keep the DI signature intact — LGTMRenaming to
_numOfTasksand_workerPathsilences lint while preserving the expectedinitTaskRunnershape used byrunSecurityCheck. No behavior change.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Deploying repomix with
|
| Latest commit: |
15fab4f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f6ce3646.repomix.pages.dev |
| Branch Preview URL: | https://chore-oxlint.repomix.pages.dev |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
- Add oxlint to the linting pipeline description - Include .oxlintrc.json in configuration references - Update verification steps to mention oxlint checks - Clarify that auto-fixes come from both biome and oxlint
This PR integrates oxlint into the linting pipeline and resolves all linting warnings to significantly improve code quality.
Summary
Changes
Linter Integration
npm run lint.oxlintrc.jsonconfiguration fileCode Quality Fixes (67 files modified)
_context,_index)require-yieldwarnings in test filesPerformance Benefits
Test Results
Checklist
npm run testnpm run lint