Skip to content
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

no-empty-file always false positives in @typescript-eslint/parser@6 #2175

Closed
Tracked by #665
JoshuaKGoldberg opened this issue Jul 11, 2023 · 5 comments · Fixed by #2180
Closed
Tracked by #665

no-empty-file always false positives in @typescript-eslint/parser@6 #2175

JoshuaKGoldberg opened this issue Jul 11, 2023 · 5 comments · Fixed by #2180

Comments

@JoshuaKGoldberg
Copy link

Right now, no-empty-file assumes that if an AST node includes a directive property, then that's enough to indicate it's a directive:

const isDirective = node => node.type === 'ExpressionStatement' && 'directive' in node;

That assumption no longer holds true in @typescript-eslint/parser's v6.0.0 due to typescript-eslint/typescript-eslint#6274. We changed the AST shape roughly like:

- directive?: string;
+ directive: string | undefined;

https://github.com/typescript-eslint/typescript-eslint/blob/df131e258c93e5714c88c0373cfeb2e1e75afc75/packages/ast-spec/src/statement/ExpressionStatement/spec.ts#L8

The PR explains why we did that (tl;dr: normalizing AST node shapes). So eslint-plugin-unicorn will need to update with a change like:

- const isDirective = node => node.type === 'ExpressionStatement' && 'directive' in node;
+ const isDirective = node => node.type === 'ExpressionStatement' && node.directive;

Right now, no-empty-line is flagging totally valid files such as https://github.com/jakebailey/pyright-action/pull/19/files#diff-1e058ca1442e46581b13571fb8d261f9e1f5657e26c96634d4c1072f0f0347f1. Thanks @jakebailey for reporting!

@muuvmuuv
Copy link

Seeing this in all my test files where just a describe is in

@muuvmuuv
Copy link

Tested @JoshuaKGoldberg suggested changes and we no longer get these error, here's a patch for [email protected] (used pnpm):

diff --git a/rules/no-empty-file.js b/rules/no-empty-file.js
index 858e0d008f773d5237f0fb919e82d048aefffbdb..9fa6699d74f62eba66e98c9fb0809164d046f97e 100644
--- a/rules/no-empty-file.js
+++ b/rules/no-empty-file.js
@@ -6,7 +6,7 @@ const messages = {
 	[MESSAGE_ID]: 'Empty files are not allowed.',
 };
 
-const isDirective = node => node.type === 'ExpressionStatement' && 'directive' in node;
+const isDirective = node => node.type === 'ExpressionStatement' && node.directive;
 const isEmpty = node => isEmptyNode(node, isDirective);
 
 const isTripleSlashDirective = node =>

@yakov116
Copy link
Contributor

Why don't you submit a PR here?

@yoyo837
Copy link

yoyo837 commented Jul 23, 2023

Same here.

@mcous
Copy link
Contributor

mcous commented Jul 23, 2023

I opened #2180 to resolve this issue! If you're working around this issue with the patch above in the meantime, please be aware that it is not quite right, as the directive field may be defined but falsy, e.g. an empty string

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants