-
Notifications
You must be signed in to change notification settings - Fork 14
Disposable object linting #48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
19f89bd
feat(eslint): warn on manual disposable cleanup patterns
cursoragent 050600f
chore(eslint): fix import ordering for custom rule
cursoragent 1c007fd
Fix finally detection across function boundaries
cursoragent 0d68407
docs(eslint): add no-manual-dispose rule documentation
cursoragent 08895db
docs(eslint): move rule docs into eslint-rules directory
cursoragent 5840594
feat(eslint): make no-manual-dispose oxlint-optimized
cursoragent 92190cb
refactor(lint): rename eslint-rules directory to lint-rules
cursoragent File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| import { eslintCompatPlugin } from '@oxlint/plugins' | ||
| import noManualDispose from './no-manual-dispose.js' | ||
|
|
||
| const plugin = eslintCompatPlugin({ | ||
| meta: { | ||
| name: 'epic-web', | ||
| }, | ||
| rules: { | ||
| 'no-manual-dispose': noManualDispose, | ||
| }, | ||
| }) | ||
|
|
||
| export default plugin |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| # Lint rules | ||
|
|
||
| Custom lint rules for this package live in this directory. | ||
|
|
||
| Each rule should have: | ||
|
|
||
| - implementation: `*.js` | ||
| - tests: `*.test.js` | ||
| - documentation: `*.md` | ||
|
|
||
| Rules are registered through [`epic-web-plugin.js`](./epic-web-plugin.js), which | ||
| uses `eslintCompatPlugin(...)` so rules can use Oxlint's `createOnce` API while | ||
| remaining ESLint-compatible. | ||
|
|
||
| ## Rules | ||
|
|
||
| - [`epic-web/no-manual-dispose`](./no-manual-dispose.md) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,120 @@ | ||
| const SYMBOL_DISPOSE_PROPERTY_NAMES = new Set([ | ||
| 'dispose', | ||
| 'asyncDispose', | ||
| 'disposeAsync', | ||
| ]) | ||
|
|
||
| function unwrapChainExpression(node) { | ||
| if (node?.type === 'ChainExpression') { | ||
| return node.expression | ||
| } | ||
| return node | ||
| } | ||
|
|
||
| function isSymbolDisposeProperty(node) { | ||
| const candidate = unwrapChainExpression(node) | ||
|
|
||
| if (candidate?.type !== 'MemberExpression') return false | ||
| if (candidate.computed || candidate.optional) return false | ||
| if (candidate.object.type !== 'Identifier') return false | ||
| if (candidate.object.name !== 'Symbol') return false | ||
| if (candidate.property.type !== 'Identifier') return false | ||
|
|
||
| return SYMBOL_DISPOSE_PROPERTY_NAMES.has(candidate.property.name) | ||
| } | ||
|
|
||
| function getManualDisposeCallKind(node) { | ||
| const callee = unwrapChainExpression(node.callee) | ||
|
|
||
| if (callee?.type !== 'MemberExpression') return null | ||
| if (isSymbolDisposeProperty(callee.property)) return 'symbol' | ||
|
|
||
| if ( | ||
| !callee.computed && | ||
| callee.property.type === 'Identifier' && | ||
| callee.property.name === 'dispose' | ||
| ) { | ||
| return 'method' | ||
| } | ||
|
|
||
| if ( | ||
| callee.computed && | ||
| callee.property.type === 'Literal' && | ||
| callee.property.value === 'dispose' | ||
| ) { | ||
| return 'method' | ||
| } | ||
|
|
||
| return null | ||
| } | ||
|
|
||
| function isFunctionBoundary(node) { | ||
| return ( | ||
| node?.type === 'FunctionDeclaration' || | ||
| node?.type === 'FunctionExpression' || | ||
| node?.type === 'ArrowFunctionExpression' | ||
| ) | ||
| } | ||
|
|
||
| function isInFinallyBlock(node) { | ||
| let current = node | ||
|
|
||
| while (current?.parent) { | ||
| if (isFunctionBoundary(current.parent)) { | ||
| return false | ||
| } | ||
|
|
||
| if ( | ||
| current.parent.type === 'TryStatement' && | ||
| current.parent.finalizer === current | ||
| ) { | ||
| return true | ||
| } | ||
|
|
||
| current = current.parent | ||
| } | ||
|
|
||
| return false | ||
| } | ||
|
|
||
| const rule = { | ||
| meta: { | ||
| type: 'suggestion', | ||
| docs: { | ||
| description: | ||
| 'Prefer `using`/`await using` over manual disposable cleanup patterns', | ||
| }, | ||
| schema: [], | ||
| messages: { | ||
| preferUsingInFinally: | ||
| 'Avoid manual disposal in `finally`; prefer `using` or `await using`.', | ||
| avoidManualSymbolDispose: | ||
| 'Do not call `[Symbol.dispose]`/`[Symbol.asyncDispose]` directly; prefer `using` or `await using`.', | ||
| }, | ||
| }, | ||
| createOnce(context) { | ||
| return { | ||
| CallExpression(node) { | ||
| const callKind = getManualDisposeCallKind(node) | ||
| if (callKind === null) return | ||
|
|
||
| if (callKind === 'symbol') { | ||
| context.report({ | ||
| node, | ||
| messageId: 'avoidManualSymbolDispose', | ||
| }) | ||
| return | ||
| } | ||
|
|
||
| if (isInFinallyBlock(node)) { | ||
| context.report({ | ||
| node, | ||
| messageId: 'preferUsingInFinally', | ||
| }) | ||
| } | ||
| }, | ||
| } | ||
| }, | ||
| } | ||
|
|
||
| export default rule | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| # `epic-web/no-manual-dispose` | ||
|
|
||
| Warns when disposable resources are cleaned up manually in patterns that should | ||
| use `using` or `await using`. | ||
|
|
||
| ## Why | ||
|
|
||
| Manual cleanup with `try/finally` and disposal calls is easier to get wrong and | ||
| less readable than language-level disposables. | ||
|
|
||
| This rule is implemented with Oxlint's `createOnce` API and wrapped with | ||
| `eslintCompatPlugin(...)`, so it is optimized for Oxlint and still works in | ||
| ESLint. | ||
|
|
||
| ## What it warns on | ||
|
|
||
| - direct calls to `[Symbol.dispose]` | ||
| - direct calls to `[Symbol.asyncDispose]` | ||
| - direct calls to `[Symbol.disposeAsync]` | ||
| - `.dispose()` and `['dispose']()` calls inside `finally` blocks | ||
|
|
||
| ## Examples | ||
|
|
||
| ### Invalid | ||
|
|
||
| ```js | ||
| let tempFile | ||
| try { | ||
| tempFile = createTempFile() | ||
| } finally { | ||
| tempFile?.[Symbol.dispose]() | ||
| } | ||
| ``` | ||
|
|
||
| ```js | ||
| let tempFile | ||
| try { | ||
| tempFile = createTempFile() | ||
| } finally { | ||
| tempFile?.dispose() | ||
| } | ||
| ``` | ||
|
|
||
| ### Valid | ||
|
|
||
| ```js | ||
| using tempFile = createTempFile() | ||
| ``` | ||
|
|
||
| ```js | ||
| await using db = await createDisposableDatabase() | ||
| ``` | ||
|
|
||
| ```js | ||
| function cleanup(resource) { | ||
| resource.dispose() | ||
| } | ||
| ``` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| import { RuleTester } from 'eslint' | ||
| import plugin from './epic-web-plugin.js' | ||
|
|
||
| const rule = plugin.rules['no-manual-dispose'] | ||
|
|
||
| const tester = new RuleTester({ | ||
| languageOptions: { | ||
| ecmaVersion: 'latest', | ||
| sourceType: 'module', | ||
| }, | ||
| }) | ||
|
|
||
| tester.run('no-manual-dispose', rule, { | ||
| valid: [ | ||
| ` | ||
| test('reads a temp file', () => { | ||
| using tempFile = createTempFile() | ||
| return Bun.file(tempFile.path).text() | ||
| }) | ||
| `, | ||
| ` | ||
| async function setup() { | ||
| await using db = await createDisposableDatabase() | ||
| return db | ||
| } | ||
| `, | ||
| ` | ||
| function cleanup(resource) { | ||
| resource.dispose() | ||
| } | ||
| `, | ||
| ` | ||
| class TempFile { | ||
| [Symbol.dispose]() { | ||
| closeFileHandle() | ||
| } | ||
| async [Symbol.asyncDispose]() { | ||
| await closeFileHandle() | ||
| } | ||
| } | ||
| `, | ||
| ` | ||
| let tempFile | ||
| try { | ||
| tempFile = createTempFile() | ||
| } finally { | ||
| logCleanup(tempFile) | ||
| } | ||
| `, | ||
| ], | ||
| invalid: [ | ||
| { | ||
| code: ` | ||
| let tempFile | ||
| try { | ||
| tempFile = createTempFile() | ||
| } finally { | ||
| tempFile?.[Symbol.dispose]() | ||
| } | ||
| `, | ||
| errors: [{ messageId: 'avoidManualSymbolDispose' }], | ||
| }, | ||
| { | ||
| code: ` | ||
| let tempFile | ||
| try { | ||
| tempFile = createTempFile() | ||
| } finally { | ||
| if (tempFile) { | ||
| tempFile['dispose']() | ||
| } | ||
| } | ||
| `, | ||
| errors: [{ messageId: 'preferUsingInFinally' }], | ||
| }, | ||
| { | ||
| code: ` | ||
| let tempFile | ||
| try { | ||
| tempFile = createTempFile() | ||
| } finally { | ||
| tempFile?.dispose() | ||
| } | ||
| `, | ||
| errors: [{ messageId: 'preferUsingInFinally' }], | ||
| }, | ||
| { | ||
| code: ` | ||
| async function closeResource() { | ||
| let tempFile | ||
| try { | ||
| tempFile = await createTempFile() | ||
| } finally { | ||
| await tempFile?.[Symbol.asyncDispose]() | ||
| } | ||
| } | ||
| `, | ||
| errors: [{ messageId: 'avoidManualSymbolDispose' }], | ||
| }, | ||
| { | ||
| code: ` | ||
| const tempFile = createTempFile() | ||
| tempFile[Symbol.dispose]() | ||
| `, | ||
| errors: [{ messageId: 'avoidManualSymbolDispose' }], | ||
| }, | ||
| { | ||
| code: ` | ||
| const tempFile = createTempFile() | ||
| tempFile?.[Symbol.disposeAsync]() | ||
| `, | ||
| errors: [{ messageId: 'avoidManualSymbolDispose' }], | ||
| }, | ||
| ], | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.