From fcfb66914ad98c6b1d09cfc232bb42729e85848f Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Thu, 6 Apr 2023 19:14:08 -0400 Subject: [PATCH] [Babel] Skip files that contain one or more disables of React eslint rules To unblock internal experimentation, for now let's just skip over compiling any file that contains one or more disables of React's eslint rules, and log that. This is a little coarse in the sense that we could skip over just functions that contain the comments, but Babel doesn't provide an easy way to traverse comments afaict so this is the simplest solution. I did check our internal repo and noted that there was only one disable of exhaustive-hooks in that entire directory in one file, so this should be fine. Notably we are not throwing any errors if we detect these violations as we don't want to fail the build, we just want to skip them for now. --- compiler/forget/src/Babel/BabelPlugin.ts | 71 +++++++++++++++++-- .../error.sketchy-code-use-forget.expect.md | 22 ++++++ .../compiler/error.sketchy-code-use-forget.js | 7 ++ .../sketchy-code-exhaustive-deps.expect.md | 35 +++++++++ .../compiler/sketchy-code-exhaustive-deps.js | 11 +++ .../sketchy-code-rules-of-hooks.expect.md | 25 +++++++ .../compiler/sketchy-code-rules-of-hooks.js | 6 ++ 7 files changed, 173 insertions(+), 4 deletions(-) create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/error.sketchy-code-use-forget.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/error.sketchy-code-use-forget.js create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/sketchy-code-exhaustive-deps.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/sketchy-code-exhaustive-deps.js create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/sketchy-code-rules-of-hooks.expect.md create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/sketchy-code-rules-of-hooks.js diff --git a/compiler/forget/src/Babel/BabelPlugin.ts b/compiler/forget/src/Babel/BabelPlugin.ts index f98d1a9969..180cde0fc3 100644 --- a/compiler/forget/src/Babel/BabelPlugin.ts +++ b/compiler/forget/src/Babel/BabelPlugin.ts @@ -10,7 +10,11 @@ import type * as BabelCore from "@babel/core"; import jsx from "@babel/plugin-syntax-jsx"; import * as t from "@babel/types"; -import { CompilerError } from "../CompilerError"; +import { + CompilerError, + CompilerErrorDetail, + ErrorSeverity, +} from "../CompilerError"; import { compile } from "../CompilerPipeline"; import { GeneratedSource } from "../HIR"; import { @@ -23,9 +27,13 @@ type BabelPluginPass = { opts: PluginOptions; }; -function hasUseForgetDirective(directives: t.Directive[]): boolean { +function hasUseForgetDirective(directive: t.Directive): boolean { + return directive.value.value === "use forget"; +} + +function hasAnyUseForgetDirectives(directives: t.Directive[]): boolean { for (const directive of directives) { - if (directive.value.value === "use forget") { + if (hasUseForgetDirective(directive)) { return true; } } @@ -122,6 +130,61 @@ export default function ReactForgetBabelPlugin( // want Forget to run true to source as possible. Program(path, pass): void { const options = parsePluginOptions(pass.opts); + + const violations = []; + const fileComments = pass.file.ast.comments; + let fileHasUseForgetDirective = false; + if (Array.isArray(fileComments)) { + for (const comment of fileComments) { + if ( + /eslint-disable(-next-line)? react-hooks\/(exhaustive-deps|rules-of-hooks)/.test( + comment.value + ) + ) { + violations.push(comment); + } + } + } + + if (violations.length > 0) { + path.traverse({ + Directive(path) { + if (hasUseForgetDirective(path.node)) { + fileHasUseForgetDirective = true; + path.stop(); + } + }, + }); + + const reason = `Skipped compilation as it disables one or more React eslint rules`; + const error = new CompilerError(); + for (const violation of violations) { + if (options.logger != null) { + options.logger.logEvent("err", { + reason, + filename: pass.filename, + violation, + }); + } + + error.pushErrorDetail( + new CompilerErrorDetail({ + reason, + description: violation.value.trim(), + severity: ErrorSeverity.InvalidInput, + codeframe: null, + loc: violation.loc ?? null, + }) + ); + } + + if (fileHasUseForgetDirective) { + throw error; + } + + return; + } + try { path.traverse(visitor, { ...pass, @@ -154,7 +217,7 @@ function shouldCompile( if (!body.isBlockStatement()) { return false; } - if (!hasUseForgetDirective(body.node.directives)) { + if (!hasAnyUseForgetDirectives(body.node.directives)) { return false; } } diff --git a/compiler/forget/src/__tests__/fixtures/compiler/error.sketchy-code-use-forget.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/error.sketchy-code-use-forget.expect.md new file mode 100644 index 0000000000..ed9b3e6c7a --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/error.sketchy-code-use-forget.expect.md @@ -0,0 +1,22 @@ + +## Input + +```javascript +/* eslint-disable react-hooks/rules-of-hooks */ +function lowercasecomponent() { + "use forget"; + const x = []; + return
{x}
; +} +/* eslint-enable react-hooks/rules-of-hooks */ + +``` + + +## Error + +``` +[ReactForget] InvalidInput: Skipped compilation as it disables one or more React eslint rules. eslint-disable react-hooks/rules-of-hooks (1:1) +``` + + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/compiler/error.sketchy-code-use-forget.js b/compiler/forget/src/__tests__/fixtures/compiler/error.sketchy-code-use-forget.js new file mode 100644 index 0000000000..08f080632c --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/error.sketchy-code-use-forget.js @@ -0,0 +1,7 @@ +/* eslint-disable react-hooks/rules-of-hooks */ +function lowercasecomponent() { + "use forget"; + const x = []; + return
{x}
; +} +/* eslint-enable react-hooks/rules-of-hooks */ diff --git a/compiler/forget/src/__tests__/fixtures/compiler/sketchy-code-exhaustive-deps.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/sketchy-code-exhaustive-deps.expect.md new file mode 100644 index 0000000000..bb10d0f524 --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/sketchy-code-exhaustive-deps.expect.md @@ -0,0 +1,35 @@ + +## Input + +```javascript +function Component() { + const item = []; + const foo = useCallback( + () => { + item.push(1); + }, // eslint-disable-next-line react-hooks/exhaustive-deps + [] + ); + + return