thefrontside / javascript

Frontside JavaScript tooling
9 stars 5 forks source link

False positive on esnext 'using' declaration #55

Open erictheswift opened 11 months ago

erictheswift commented 11 months ago

Hi! 👋

Firstly, thanks for your work on this project! 🙂

Today I used patch-package to patch eslint-plugin-prefer-let@3.0.1 for the project I'm working on.

Explicit resource management proposal (using keyword) landed in typescript 5.2. prefer-let plugin false positively reacts on using aiming to replace it with let

Here is the diff that solved my problem:

diff --git a/node_modules/eslint-plugin-prefer-let/lib/rules/prefer-let.js b/node_modules/eslint-plugin-prefer-let/lib/rules/prefer-let.js
index 73d487f..abab361 100644
--- a/node_modules/eslint-plugin-prefer-let/lib/rules/prefer-let.js
+++ b/node_modules/eslint-plugin-prefer-let/lib/rules/prefer-let.js
@@ -55,7 +55,7 @@ module.exports = {
             message: 'prefer `let` over `var` to declare value bindings',
             node
           });
-        } else if (node.kind !== 'let' && !isTopLevelScope(node)) {
+        } else if (node.kind === 'const' && !isTopLevelScope(node)) {
           let constToken = sourceCode.getFirstToken(node);

           context.report({

This issue body was partially generated by patch-package.

cowboyd commented 11 months ago

@erictheswift Thanks for reporting this, and thanks for generating a fix. It's definitely something we should address. Is there a test case you add to the test suite to make sure that we don't experience any regression?

erictheswift commented 9 months ago

Sure:

valid: [
    ...
    {code: "await using disposables = new AsyncDisposableStack()"},
    {code: "using disposables = new DisposableStack()"},
]
cowboyd commented 9 months ago

@erictheswift Thanks! This poses an interesting problem however, since the parser can't actually parse the source when I run the test. Only TypeScript understands this syntax and we aren't set up for running through TS.

For example, if I try to include these examples, it just gives me a syntax error:

AssertionError [ERR_ASSERTION]: A fatal parsing error occurred: Parsing error: Unexpected token disposable

Do you know of a common way to do test eslint plugins against TypeScript source?

erictheswift commented 9 months ago

Got it. Yeah, I'm using it with @typescript-eslint/parser. Tests will have to wait for espree versions to support using statements?

erictheswift commented 9 months ago

Do you know of a common way to do test eslint plugins against TypeScript source?

I guess this would require typescript parser specified as well as typescript as a dependency

cowboyd commented 9 months ago

@erictheswift I can just go ahead and merge this as-is, and then comment it back in when it is supported.

erictheswift commented 9 months ago

@cowboyd sounds reasonable